diff --git a/cm.yaml b/cm.yaml new file mode 100644 index 0000000000..9e9bab6716 --- /dev/null +++ b/cm.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 + namespace: default + ownerReferences: + - apiVersion: v1 + kind: ConfigMap + name: kube-root-ca.crt + uid: 1ef74cb4-dbbd-45ef-9caf-aa76186594ea +data: + key1: "val1" +# key2: "val2" + diff --git a/cm2.yaml b/cm2.yaml new file mode 100644 index 0000000000..3c7c14c111 --- /dev/null +++ b/cm2.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 + namespace: default +data: + key3: "val3" + + diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index bd939d4b32..d92294d048 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -33,6 +33,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; public class BaseConfigurationService extends AbstractConfigurationService { @@ -135,6 +136,10 @@ protected

ControllerConfiguration

configFor(Reconcile timeUnit = reconciliationInterval.timeUnit(); } + final var dependentFieldManager = + annotation.fieldManager().equals(CONTROLLER_NAME_AS_FIELD_MANAGER) ? name + : annotation.fieldManager(); + final var config = new ResolvedControllerConfiguration

( resourceClass, name, generationAware, associatedReconcilerClass, retry, rateLimiter, @@ -152,7 +157,8 @@ protected

ControllerConfiguration

configFor(Reconcile io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector, Constants.NO_VALUE_SET), null, - Utils.instantiate(annotation.itemStore(), ItemStore.class, context), this); + Utils.instantiate(annotation.itemStore(), ItemStore.class, context), dependentFieldManager, + this); ResourceEventFilter

answer = deprecatedEventFilter(annotation); config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 52d78fb1e9..1ab358155e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -264,4 +264,28 @@ static ConfigurationService newOverriddenConfigurationService( default ExecutorServiceManager getExecutorServiceManager() { return new ExecutorServiceManager(this); } + + /** + * Allows to revert to the 4.3 behavior when it comes to creating or updating Kubernetes Dependent + * Resources when set to {@code false}. The default approach how these resources are + * created/updated was change to use + * Server-Side + * Apply (SSA) by default. Note that the legacy approach, and this setting, might be removed + * in the future. + */ + default boolean ssaBasedCreateUpdateForDependentResources() { + return true; + } + + /** + * Allows to revert to the 4.3 generic matching algorithm for Kubernetes Dependent Resources when + * set to {@code false}. Version 4.4 introduced a new generic matching algorithm for Kubernetes + * Dependent Resources which is quite complex. As a consequence, we introduced this setting to + * allow folks to revert to the previous matching algorithm if needed. Note, however, that the + * legacy algorithm, and this setting, might be removed in the future. + */ + default boolean ssaBasedDefaultMatchingForDependentResources() { + return true; + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 8d9f4200bc..b2b4b93b12 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -32,6 +32,8 @@ public class ConfigurationServiceOverrider { private Boolean stopOnInformerErrorDuringStartup; private Duration cacheSyncTimeout; private ResourceClassResolver resourceClassResolver; + private Boolean ssaBasedCreateUpdateForDependentResources; + private Boolean ssaBasedDefaultMatchingForDependentResources; ConfigurationServiceOverrider(ConfigurationService original) { this.original = original; @@ -139,6 +141,18 @@ public ConfigurationServiceOverrider withResourceClassResolver( return this; } + public ConfigurationServiceOverrider withSSABasedCreateUpdateForDependentResources( + boolean value) { + this.ssaBasedCreateUpdateForDependentResources = value; + return this; + } + + public ConfigurationServiceOverrider withSSABasedDefaultMatchingForDependentResources( + boolean value) { + this.ssaBasedDefaultMatchingForDependentResources = value; + return this; + } + public ConfigurationService build() { return new BaseConfigurationService(original.getVersion(), cloner, objectMapper) { @Override @@ -248,6 +262,20 @@ public ResourceClassResolver getResourceClassResolver() { return resourceClassResolver != null ? resourceClassResolver : super.getResourceClassResolver(); } + + @Override + public boolean ssaBasedCreateUpdateForDependentResources() { + return ssaBasedCreateUpdateForDependentResources != null + ? ssaBasedCreateUpdateForDependentResources + : super.ssaBasedCreateUpdateForDependentResources(); + } + + @Override + public boolean ssaBasedDefaultMatchingForDependentResources() { + return ssaBasedDefaultMatchingForDependentResources != null + ? ssaBasedDefaultMatchingForDependentResources + : super.ssaBasedDefaultMatchingForDependentResources(); + } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 60a053d0a8..13ddd995ad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -22,6 +22,10 @@ public interface ControllerConfiguration

extends Resource @SuppressWarnings("rawtypes") RateLimiter DEFAULT_RATE_LIMITER = LinearRateLimiter.deactivatedRateLimiter(); + /** + * Will use the controller name as fieldManager if set. + */ + String CONTROLLER_NAME_AS_FIELD_MANAGER = "use_controller_name"; default String getName() { return ensureValidName(null, getAssociatedReconcilerClassName()); @@ -124,4 +128,16 @@ default Class

getResourceClass() { default Set getEffectiveNamespaces() { return ResourceConfiguration.super.getEffectiveNamespaces(getConfigurationService()); } + + /** + * Retrieves the name used to assign as field manager for + * Server-Side + * Apply (SSA) operations. If unset, the sanitized controller name will be used. + * + * @return the name used as field manager for SSA operations + */ + default String fieldManager() { + return getName(); + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 5d96c2abf6..3b2dcd8289 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -39,6 +39,7 @@ public class ControllerConfigurationOverrider { private Map configurations; private ItemStore itemStore; private String name; + private String fieldManager; private ControllerConfigurationOverrider(ControllerConfiguration original) { this.finalizer = original.getFinalizerName(); @@ -54,6 +55,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration original) { this.original = original; this.rateLimiter = original.getRateLimiter(); this.name = original.getName(); + this.fieldManager = original.fieldManager(); } public ControllerConfigurationOverrider withFinalizer(String finalizer) { @@ -168,6 +170,12 @@ public ControllerConfigurationOverrider withName(String name) { return this; } + public ControllerConfigurationOverrider withFieldManager( + String dependentFieldManager) { + this.fieldManager = dependentFieldManager; + return this; + } + public ControllerConfigurationOverrider replacingNamedDependentResourceConfig(String name, Object dependentResourceConfig) { @@ -190,7 +198,7 @@ public ControllerConfiguration build() { generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter, reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, original.getDependentResources(), - namespaces, finalizer, labelSelector, configurations, itemStore, + namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager, original.getConfigurationService()); overridden.setEventFilter(customResourcePredicate); return overridden; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java index f91e9c506a..0b8b7306f8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java @@ -32,6 +32,7 @@ public class ResolvedControllerConfiguration

private final Map configurations; private final ItemStore

itemStore; private final ConfigurationService configurationService; + private final String fieldManager; private ResourceEventFilter

eventFilter; private List dependentResources; @@ -44,7 +45,8 @@ public ResolvedControllerConfiguration(Class

resourceClass, ControllerConfigu other.genericFilter().orElse(null), other.getDependentResources(), other.getNamespaces(), other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap(), - other.getItemStore().orElse(null), other.getConfigurationService()); + other.getItemStore().orElse(null), other.fieldManager(), + other.getConfigurationService()); } public static Duration getMaxReconciliationInterval(long interval, TimeUnit timeUnit) { @@ -72,10 +74,12 @@ public ResolvedControllerConfiguration(Class

resourceClass, String name, List dependentResources, Set namespaces, String finalizer, String labelSelector, Map configurations, ItemStore

itemStore, + String fieldManager, ConfigurationService configurationService) { this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter, maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, - namespaces, finalizer, labelSelector, configurations, itemStore, configurationService); + namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager, + configurationService); setDependentResources(dependentResources); } @@ -85,6 +89,7 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, OnAddFilter

onAddFilter, OnUpdateFilter

onUpdateFilter, GenericFilter

genericFilter, Set namespaces, String finalizer, String labelSelector, Map configurations, ItemStore

itemStore, + String fieldManager, ConfigurationService configurationService) { super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, itemStore); @@ -99,13 +104,14 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, this.itemStore = itemStore; this.finalizer = ControllerConfiguration.ensureValidFinalizerName(finalizer, getResourceTypeName()); + this.fieldManager = fieldManager; } protected ResolvedControllerConfiguration(Class

resourceClass, String name, Class reconcilerClas, ConfigurationService configurationService) { this(resourceClass, name, false, getAssociatedReconcilerClassName(reconcilerClas), null, null, null, null, null, null, null, - null, null, null, null, configurationService); + null, null, null, null, null, configurationService); } @Override @@ -182,4 +188,9 @@ public Object getConfigurationFor(DependentResourceSpec spec) { public Optional> getItemStore() { return Optional.ofNullable(itemStore); } + + @Override + public String fieldManager() { + return fieldManager; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index e2c0de896b..dcd5484b0a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -17,6 +17,8 @@ import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; +import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER; + @Inherited @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) @@ -47,7 +49,7 @@ * Specified which namespaces this Controller monitors for custom resources events. If no * namespace is specified then the controller will monitor all namespaces by default. * - * @return the list of namespaces this controller monitors + * @return the array of namespaces this controller monitors */ String[] namespaces() default Constants.WATCH_ALL_NAMESPACES; @@ -108,7 +110,7 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation * Optional list of {@link Dependent} configurations which associate a resource type to a * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} implementation * - * @return the list of {@link Dependent} configurations + * @return the array of {@link Dependent} configurations */ Dependent[] dependents() default {}; @@ -129,4 +131,13 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation Class rateLimiter() default LinearRateLimiter.class; Class itemStore() default ItemStore.class; + + /** + * Retrieves the name used to assign as field manager for + * Server-Side + * Apply (SSA) operations. If unset, the sanitized controller name will be used. + * + * @return the name used as field manager for SSA operations + */ + String fieldManager() default CONTROLLER_NAME_AS_FIELD_MANAGER; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 0cee7c2413..f0ca32bd98 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -19,8 +19,8 @@ public class GenericKubernetesResourceMatcher kubernetesDependentResourceConfig; + @SuppressWarnings("unchecked") public KubernetesDependentResource(Class resourceType) { super(resourceType); @@ -128,16 +129,41 @@ protected R handleUpdate(R actual, R desired, P primary, Context

context) { @SuppressWarnings("unused") public R create(R target, P primary, Context

context) { - return prepare(target, primary, "Creating").create(); + if (!context.getControllerConfiguration().getConfigurationService() + .ssaBasedCreateUpdateForDependentResources()) { + return prepare(target, primary, "Creating").create(); + } else { + return prepare(target, primary, "Creating") + .fieldManager(context.getControllerConfiguration().fieldManager()) + .forceConflicts() + .serverSideApply(); + } } public R update(R actual, R target, P primary, Context

context) { - var updatedActual = processor.replaceSpecOnActual(actual, target, context); - return prepare(updatedActual, primary, "Updating").replace(); + if (!context.getControllerConfiguration().getConfigurationService() + .ssaBasedCreateUpdateForDependentResources()) { + var updatedActual = processor.replaceSpecOnActual(actual, target, context); + return prepare(updatedActual, primary, "Updating").replace(); + } else { + target.getMetadata().setResourceVersion(actual.getMetadata().getResourceVersion()); + return prepare(target, primary, "Updating") + .fieldManager(context.getControllerConfiguration().fieldManager()) + .forceConflicts().serverSideApply(); + } } public Result match(R actualResource, P primary, Context

context) { - return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false); + if (!context.getControllerConfiguration().getConfigurationService() + .ssaBasedDefaultMatchingForDependentResources()) { + return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false); + } else { + final var desired = desired(primary, context); + addReferenceHandlingMetadata(desired, primary); + var matches = SSABasedGenericKubernetesResourceMatcher.getInstance().matches(actualResource, + desired, context); + return Result.computed(matches, desired); + } } @SuppressWarnings("unused") @@ -164,11 +190,7 @@ protected Resource prepare(R desired, P primary, String actionName) { desired.getClass(), ResourceID.fromResource(desired)); - if (addOwnerReference()) { - desired.addOwnerReference(primary); - } else if (useDefaultAnnotationsToIdentifyPrimary()) { - addDefaultSecondaryToPrimaryMapperAnnotations(desired, primary); - } + addReferenceHandlingMetadata(desired, primary); if (desired instanceof Namespaced) { return client.resource(desired).inNamespace(desired.getMetadata().getNamespace()); @@ -177,6 +199,14 @@ protected Resource prepare(R desired, P primary, String actionName) { } } + protected void addReferenceHandlingMetadata(R desired, P primary) { + if (addOwnerReference()) { + desired.addOwnerReference(primary); + } else if (useDefaultAnnotationsToIdentifyPrimary()) { + addDefaultSecondaryToPrimaryMapperAnnotations(desired, primary); + } + } + @Override @SuppressWarnings("unchecked") protected InformerEventSource createEventSource(EventSourceContext

context) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java new file mode 100644 index 0000000000..50158cad18 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java @@ -0,0 +1,359 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ManagedFieldsEntry; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * Matches the actual state on the server vs the desired state. Based on the managedFields of SSA. + * + *

+ * The basis of algorithm is to extract the fields managed we convert resources to Map/List + * composition. The actual resource (from the server) is pruned, all the fields which are not + * mentioed in managedFields of the target manager is removed. Some irrelevant fields are also + * removed from desired. And the two resulted Maps are compared for equality. The implementation is + * a bit nasty since have to deal with some specific cases of managedFields format. + *

+ * + * @param matched resource type + */ +// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#fieldsv1-v1-meta +// https://github.com/kubernetes-sigs/structured-merge-diff +// https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-field-management.html +// see also: https://kubernetes.slack.com/archives/C0123CNN8F3/p1686141087220719 +public class SSABasedGenericKubernetesResourceMatcher { + + @SuppressWarnings("rawtypes") + private static final SSABasedGenericKubernetesResourceMatcher INSTANCE = + new SSABasedGenericKubernetesResourceMatcher<>(); + public static final String APPLY_OPERATION = "Apply"; + public static final String DOT_KEY = "."; + + @SuppressWarnings("unchecked") + public static SSABasedGenericKubernetesResourceMatcher getInstance() { + return INSTANCE; + } + + private static final TypeReference> typeRef = new TypeReference<>() {}; + + private static final String F_PREFIX = "f:"; + private static final String K_PREFIX = "k:"; + private static final String V_PREFIX = "v:"; + private static final String METADATA_KEY = "metadata"; + private static final String NAME_KEY = "name"; + private static final String NAMESPACE_KEY = "namespace"; + private static final String KIND_KEY = "kind"; + private static final String API_VERSION_KEY = "apiVersion"; + + private static final Logger log = + LoggerFactory.getLogger(SSABasedGenericKubernetesResourceMatcher.class); + + + public boolean matches(R actual, R desired, Context context) { + try { + var optionalManagedFieldsEntry = + checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager()); + // If no field is managed by our controller, that means the controller hasn't touched the + // resource yet and the resource probably doesn't match the desired state. Not matching here + // means that the resource will need to be updated and since this will be done using SSA, the + // fields our controller cares about will become managed by it + if (optionalManagedFieldsEntry.isEmpty()) { + return false; + } + + var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow(); + + var objectMapper = + context.getControllerConfiguration().getConfigurationService().getObjectMapper(); + + var actualMap = objectMapper.convertValue(actual, typeRef); + var desiredMap = objectMapper.convertValue(desired, typeRef); + + log.trace("Original actual: \n {} \n original desired: \n {} ", actual, desiredMap); + + var prunedActual = new HashMap(actualMap.size()); + keepOnlyManagedFields(prunedActual, actualMap, + managedFieldsEntry.getFieldsV1().getAdditionalProperties(), objectMapper); + + removeIrrelevantValues(desiredMap); + + log.debug("Pruned actual: \n {} \n desired: \n {} ", prunedActual, desiredMap); + + return prunedActual.equals(desiredMap); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } + + @SuppressWarnings("unchecked") + private static void removeIrrelevantValues(Map desiredMap) { + var metadata = (Map) desiredMap.get(METADATA_KEY); + metadata.remove(NAME_KEY); + metadata.remove(NAMESPACE_KEY); + if (metadata.isEmpty()) { + desiredMap.remove(METADATA_KEY); + } + desiredMap.remove(KIND_KEY); + desiredMap.remove(API_VERSION_KEY); + } + + @SuppressWarnings("unchecked") + private static void keepOnlyManagedFields(Map result, + Map actualMap, + Map managedFields, ObjectMapper objectMapper) throws JsonProcessingException { + + if (managedFields.isEmpty()) { + result.putAll(actualMap); + return; + } + for (Map.Entry entry : managedFields.entrySet()) { + String key = entry.getKey(); + if (key.startsWith(F_PREFIX)) { + String keyInActual = keyWithoutPrefix(key); + var managedFieldValue = (Map) entry.getValue(); + if (isNestedValue(managedFieldValue)) { + var managedEntrySet = managedFieldValue.entrySet(); + + // two special cases "k:" and "v:" prefixes + if (isListKeyEntrySet(managedEntrySet)) { + handleListKeyEntrySet(result, actualMap, objectMapper, keyInActual, managedEntrySet); + } else if (isSetValueField(managedEntrySet)) { + handleSetValues(result, actualMap, objectMapper, keyInActual, managedEntrySet); + } else { + // basically if we should traverse further + fillResultsAndTraverseFurther(result, actualMap, managedFields, objectMapper, key, + keyInActual, managedFieldValue); + } + } else { + // this should handle the case when the value is complex in the actual map (not just a + // simple value). + result.put(keyInActual, actualMap.get(keyInActual)); + } + } else { + // .:{} is ignored, other should not be present + if (!DOT_KEY.equals(key)) { + throw new IllegalStateException("Key: " + key + " has no prefix: " + F_PREFIX); + } + } + } + } + + @SuppressWarnings("unchecked") + private static void fillResultsAndTraverseFurther(Map result, + Map actualMap, Map managedFields, ObjectMapper objectMapper, + String key, String keyInActual, Object managedFieldValue) throws JsonProcessingException { + var emptyMapValue = new HashMap(); + result.put(keyInActual, emptyMapValue); + var actualMapValue = actualMap.get(keyInActual); + log.debug("key: {} actual map value: {} managedFieldValue: {}", keyInActual, + actualMapValue, managedFieldValue); + + keepOnlyManagedFields(emptyMapValue, (Map) actualMapValue, + (Map) managedFields.get(key), objectMapper); + } + + private static boolean isNestedValue(Map managedFieldValue) { + return !managedFieldValue.isEmpty(); + } + + /** + * List entries referenced by key, or when "k:" prefix is used. It works in a way that it selects + * the target element based on the field(s) in "k:" for example when there is a list of element of + * owner references, the uid can serve as a key for a list element: + * "k:{"uid":"1ef74cb4-dbbd-45ef-9caf-aa76186594ea"}". It selects the element and recursively + * processes it. Note that in these lists the order matters and seems that if there are more keys + * ("k:"), the ordering of those in the managed fields are not the same as the value order. So + * this also explicitly orders the result based on the value order in the resource not the key + * order in managed field. + */ + @SuppressWarnings("unchecked") + private static void handleListKeyEntrySet(Map result, + Map actualMap, + ObjectMapper objectMapper, String keyInActual, + Set> managedEntrySet) { + var valueList = new ArrayList<>(); + result.put(keyInActual, valueList); + var actualValueList = (List>) actualMap.get(keyInActual); + + SortedMap> targetValuesByIndex = new TreeMap<>(); + Map> managedEntryByIndex = new HashMap<>(); + + for (Map.Entry listEntry : managedEntrySet) { + if (DOT_KEY.equals(listEntry.getKey())) { + continue; + } + var actualListEntry = selectListEntryBasedOnKey(keyWithoutPrefix(listEntry.getKey()), + actualValueList, objectMapper); + targetValuesByIndex.put(actualListEntry.getKey(), actualListEntry.getValue()); + managedEntryByIndex.put(actualListEntry.getKey(), (Map) listEntry.getValue()); + } + + targetValuesByIndex.forEach((key, value) -> { + var emptyResMapValue = new HashMap(); + valueList.add(emptyResMapValue); + try { + keepOnlyManagedFields(emptyResMapValue, value, managedEntryByIndex.get(key), objectMapper); + } catch (JsonProcessingException ex) { + throw new IllegalStateException(ex); + } + }); + } + + /** + * Set values, the "v:" prefix. Form in managed fields: "f:some-set":{"v:1":{}},"v:2":{},"v:3":{}} + * Note that this should be just used in very rare cases, actually was not able to produce a + * sample. Kubernetes developers who worked on this feature were not able to provide one either + * when prompted. Basically this method just adds the values from {@code "v:"} to the + * result. + */ + @SuppressWarnings("rawtypes") + private static void handleSetValues(Map result, Map actualMap, + ObjectMapper objectMapper, String keyInActual, + Set> managedEntrySet) { + var valueList = new ArrayList<>(); + result.put(keyInActual, valueList); + for (Map.Entry valueEntry : managedEntrySet) { + // not clear if this can happen + if (DOT_KEY.equals(valueEntry.getKey())) { + continue; + } + Class targetClass = null; + List values = (List) actualMap.get(keyInActual); + if (!(values.get(0) instanceof Map)) { + targetClass = values.get(0).getClass(); + } + + var value = + parseKeyValue(keyWithoutPrefix(valueEntry.getKey()), targetClass, objectMapper); + valueList.add(value); + } + } + + public static Object parseKeyValue(String stringValue, Class targetClass, + ObjectMapper objectMapper) { + try { + stringValue = stringValue.trim(); + if (targetClass != null) { + return objectMapper.readValue(stringValue, targetClass); + } else { + return objectMapper.readValue(stringValue, typeRef); + } + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } + + private static boolean isSetValueField(Set> managedEntrySet) { + return isKeyPrefixedSkippingDotKey(managedEntrySet, V_PREFIX); + } + + private static boolean isListKeyEntrySet(Set> managedEntrySet) { + return isKeyPrefixedSkippingDotKey(managedEntrySet, K_PREFIX); + } + + /** + * Sometimes (not always) the first subfield of a managed field ("f:") is ".:{}", it looks that + * those are added when there are more subfields of a referenced field. See test samples. Does not + * seem to provide additional functionality, so can be just skipped for now. + */ + private static boolean isKeyPrefixedSkippingDotKey(Set> managedEntrySet, + String prefix) { + var iterator = managedEntrySet.iterator(); + var managedFieldEntry = iterator.next(); + if (managedFieldEntry.getKey().equals(DOT_KEY)) { + managedFieldEntry = iterator.next(); + } + return managedFieldEntry.getKey().startsWith(prefix); + } + + private static java.util.Map.Entry> selectListEntryBasedOnKey( + String key, + List> values, + ObjectMapper objectMapper) { + try { + Map ids = objectMapper.readValue(key, typeRef); + List> possibleTargets = new ArrayList<>(1); + int index = -1; + for (int i = 0; i < values.size(); i++) { + var v = values.get(i); + if (v.entrySet().containsAll(ids.entrySet())) { + possibleTargets.add(v); + index = i; + } + } + if (possibleTargets.isEmpty()) { + throw new IllegalStateException( + "Cannot find list element for key:" + key + ", in map: " + values); + } + if (possibleTargets.size() > 1) { + throw new IllegalStateException( + "More targets found in list element for key:" + key + ", in map: " + values); + } + final var finalIndex = index; + return new Map.Entry<>() { + @Override + public Integer getKey() { + return finalIndex; + } + + @Override + public Map getValue() { + return possibleTargets.get(0); + } + + @Override + public Map setValue(Map stringObjectMap) { + throw new IllegalStateException("should not be called"); + } + }; + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } + + + private Optional checkIfFieldManagerExists(R actual, String fieldManager) { + var targetManagedFields = actual.getMetadata().getManagedFields().stream() + // Only the apply operations are interesting for us since those were created properly be SSA + // Patch. An update can be present with same fieldManager when migrating and having the same + // field manager name. + .filter( + f -> f.getManager().equals(fieldManager) && f.getOperation().equals(APPLY_OPERATION)) + .collect(Collectors.toList()); + if (targetManagedFields.isEmpty()) { + log.debug("No field manager exists for resource {} with name: {} and operation Apply ", + actual, actual.getMetadata().getName()); + return Optional.empty(); + } + // this should not happen in theory + if (targetManagedFields.size() > 1) { + throw new OperatorException( + "More than one field manager exists with name: " + fieldManager + "in resource: " + + actual + " with name: " + actual.getMetadata().getName()); + } + return Optional.of(targetManagedFields.get(0)); + } + + private static String keyWithoutPrefix(String key) { + return key.substring(2); + } + +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java new file mode 100644 index 0000000000..3864ad1c43 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java @@ -0,0 +1,120 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class SSABasedGenericKubernetesResourceMatcherTest { + + Context mockedContext = mock(Context.class); + + SSABasedGenericKubernetesResourceMatcher matcher = + new SSABasedGenericKubernetesResourceMatcher<>(); + + @BeforeEach + @SuppressWarnings("unchecked") + void setup() { + var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.fieldManager()).thenReturn("controller"); + var configurationService = mock(ConfigurationService.class); + when(configurationService.getObjectMapper()).thenCallRealMethod(); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(mockedContext.getControllerConfiguration()).thenReturn(controllerConfiguration); + } + + @Test + void checksIfAddsNotAddedByController() { + var desired = loadResource("nginx-deployment.yaml", Deployment.class); + var actual = + loadResource("deployment-with-managed-fields-additional-controller.yaml", Deployment.class); + + assertThat(matcher.matches(actual, desired, mockedContext)).isTrue(); + } + + // In the example the owner reference in a list is referenced by "k:", while all the fields are + // managed but not listed + @Test + void emptyListElementMatchesAllFields() { + var desiredConfigMap = loadResource("configmap.empty-owner-reference-desired.yaml", + ConfigMap.class); + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", + ConfigMap.class); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue(); + } + + // the whole "rules:" part is just implicitly managed + @Test + void wholeComplexFieldManaged() { + var desiredConfigMap = loadResource("sample-whole-complex-part-managed-desired.yaml", + ConfigMap.class); + var actualConfigMap = loadResource("sample-whole-complex-part-managed.yaml", + ConfigMap.class); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue(); + } + + @Test + void multiItemList() { + var desiredConfigMap = loadResource("multi-container-pod-desired.yaml", + ConfigMap.class); + var actualConfigMap = loadResource("multi-container-pod.yaml", + ConfigMap.class); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue(); + } + + @Test + void changeValueInDesiredMakesMatchFail() { + var desiredConfigMap = loadResource("configmap.empty-owner-reference-desired.yaml", + ConfigMap.class); + desiredConfigMap.getData().put("key1", "different value"); + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", + ConfigMap.class); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse(); + } + + @Test + void changeValueActualMakesMatchFail() { + var desiredConfigMap = loadResource("configmap.empty-owner-reference-desired.yaml", + ConfigMap.class); + + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", + ConfigMap.class); + actualConfigMap.getData().put("key1", "different value"); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse(); + } + + @Test + void addedLabelInDesiredMakesMatchFail() { + var desiredConfigMap = loadResource("configmap.empty-owner-reference-desired.yaml", + ConfigMap.class); + desiredConfigMap.getMetadata().setLabels(Map.of("newlabel", "val")); + + var actualConfigMap = loadResource("configmap.empty-owner-reference.yaml", + ConfigMap.class); + + assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isFalse(); + } + + private R loadResource(String fileName, Class clazz) { + return ReconcilerUtils.loadYaml(clazz, SSABasedGenericKubernetesResourceMatcherTest.class, + fileName); + } + +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java index 9e58a93ddf..003b623b5d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java @@ -145,7 +145,7 @@ public ControllerConfig(String finalizer, boolean generationAware, null, null, null, - null, null, null, finalizer, null, null, new BaseConfigurationService()); + null, null, null, finalizer, null, null, null, new BaseConfigurationService()); setEventFilter(eventFilter); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java index a22d2f85b6..6ee3d19d6c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java @@ -198,7 +198,7 @@ public TestConfiguration(boolean generationAware, OnAddFilter /data/index.html"] \ No newline at end of file diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/multi-container-pod.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/multi-container-pod.yaml new file mode 100644 index 0000000000..e1334117b6 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/multi-container-pod.yaml @@ -0,0 +1,214 @@ +apiVersion: v1 +kind: Pod +metadata: + creationTimestamp: "2023-06-08T11:50:59Z" + managedFields: + - apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:containers: + k:{"name":"debian-container"}: + .: {} + f:args: {} + f:command: {} + f:image: {} + f:name: {} + f:volumeMounts: + k:{"mountPath":"/data"}: + .: {} + f:mountPath: {} + f:name: {} + k:{"name":"nginx-container"}: + .: {} + f:image: {} + f:name: {} + f:volumeMounts: + k:{"mountPath":"/usr/share/nginx/html"}: + .: {} + f:mountPath: {} + f:name: {} + f:volumes: + k:{"name":"shared-data"}: + .: {} + f:emptyDir: {} + f:name: {} + manager: controller + operation: Apply + time: "2023-06-08T11:50:59Z" + - apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:status: + f:conditions: + k:{"type":"ContainersReady"}: + .: {} + f:lastProbeTime: {} + f:lastTransitionTime: {} + f:message: {} + f:reason: {} + f:status: {} + f:type: {} + k:{"type":"Initialized"}: + .: {} + f:lastProbeTime: {} + f:lastTransitionTime: {} + f:status: {} + f:type: {} + k:{"type":"Ready"}: + .: {} + f:lastProbeTime: {} + f:lastTransitionTime: {} + f:message: {} + f:reason: {} + f:status: {} + f:type: {} + f:containerStatuses: {} + f:hostIP: {} + f:phase: {} + f:podIP: {} + f:podIPs: + .: {} + k:{"ip":"10.244.0.3"}: + .: {} + f:ip: {} + f:startTime: {} + manager: kubelet + operation: Update + subresource: status + time: "2023-06-08T11:51:21Z" + name: shared-storage + namespace: default + resourceVersion: "1950" + uid: 0c916935-8198-4d62-980e-193f3c3ec877 +spec: + containers: + - image: nginx + imagePullPolicy: Always + name: nginx-container + resources: {} + terminationMessagePath: /dev/termination-log + terminationMessagePolicy: File + volumeMounts: + - mountPath: /usr/share/nginx/html + name: shared-data + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + name: kube-api-access-gxpbz + readOnly: true + - args: + - -c + - echo Level Up Blue Team! > /data/index.html + command: + - /bin/sh + image: debian + imagePullPolicy: Always + name: debian-container + resources: {} + terminationMessagePath: /dev/termination-log + terminationMessagePolicy: File + volumeMounts: + - mountPath: /data + name: shared-data + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + name: kube-api-access-gxpbz + readOnly: true + dnsPolicy: ClusterFirst + enableServiceLinks: true + nodeName: minikube + preemptionPolicy: PreemptLowerPriority + priority: 0 + restartPolicy: Always + schedulerName: default-scheduler + securityContext: {} + serviceAccount: default + serviceAccountName: default + terminationGracePeriodSeconds: 30 + tolerations: + - effect: NoExecute + key: node.kubernetes.io/not-ready + operator: Exists + tolerationSeconds: 300 + - effect: NoExecute + key: node.kubernetes.io/unreachable + operator: Exists + tolerationSeconds: 300 + volumes: + - emptyDir: {} + name: shared-data + - name: kube-api-access-gxpbz + projected: + defaultMode: 420 + sources: + - serviceAccountToken: + expirationSeconds: 3607 + path: token + - configMap: + items: + - key: ca.crt + path: ca.crt + name: kube-root-ca.crt + - downwardAPI: + items: + - fieldRef: + apiVersion: v1 + fieldPath: metadata.namespace + path: namespace +status: + conditions: + - lastProbeTime: null + lastTransitionTime: "2023-06-08T11:50:59Z" + status: "True" + type: Initialized + - lastProbeTime: null + lastTransitionTime: "2023-06-08T11:50:59Z" + message: 'containers with unready status: [debian-container]' + reason: ContainersNotReady + status: "False" + type: Ready + - lastProbeTime: null + lastTransitionTime: "2023-06-08T11:50:59Z" + message: 'containers with unready status: [debian-container]' + reason: ContainersNotReady + status: "False" + type: ContainersReady + - lastProbeTime: null + lastTransitionTime: "2023-06-08T11:50:59Z" + status: "True" + type: PodScheduled + containerStatuses: + - containerID: docker://ead1d3e4beaaa9176daca99e55673a2176e0da51d9953d6a11d5786b730178ee + image: debian:latest + imageID: docker-pullable://debian@sha256:432f545c6ba13b79e2681f4cc4858788b0ab099fc1cca799cc0fae4687c69070 + lastState: + terminated: + containerID: docker://ead1d3e4beaaa9176daca99e55673a2176e0da51d9953d6a11d5786b730178ee + exitCode: 0 + finishedAt: "2023-06-08T11:51:19Z" + reason: Completed + startedAt: "2023-06-08T11:51:19Z" + name: debian-container + ready: false + restartCount: 1 + started: false + state: + waiting: + message: back-off 10s restarting failed container=debian-container pod=shared-storage_default(0c916935-8198-4d62-980e-193f3c3ec877) + reason: CrashLoopBackOff + - containerID: docker://afd6260e41afa0b149ebfd904162fb2f22bb037c18904eed599eb9ac1ce4faf0 + image: nginx:latest + imageID: docker-pullable://nginx@sha256:af296b188c7b7df99ba960ca614439c99cb7cf252ed7bbc23e90cfda59092305 + lastState: {} + name: nginx-container + ready: true + restartCount: 0 + started: true + state: + running: + startedAt: "2023-06-08T11:51:09Z" + hostIP: 192.168.49.2 + phase: Running + podIP: 10.244.0.3 + podIPs: + - ip: 10.244.0.3 + qosClass: BestEffort + startTime: "2023-06-08T11:50:59Z" \ No newline at end of file diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed-desired.yaml new file mode 100644 index 0000000000..3baff88db0 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed-desired.yaml @@ -0,0 +1,24 @@ +kind: FlowSchema +metadata: + annotations: + apf.kubernetes.io/autoupdate-spec: "true" + name: probes +spec: + matchingPrecedence: 2 + priorityLevelConfiguration: + name: exempt + rules: + - nonResourceRules: + - nonResourceURLs: + - /healthz + - /readyz + - /livez + verbs: + - get + subjects: + - group: + name: system:unauthenticated + kind: Group + - group: + name: system:authenticated + kind: Group diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed.yaml new file mode 100644 index 0000000000..9d9e20efbb --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-whole-complex-part-managed.yaml @@ -0,0 +1,44 @@ +kind: FlowSchema +metadata: + annotations: + apf.kubernetes.io/autoupdate-spec: "true" + creationTimestamp: "2023-06-08T11:18:25Z" + generation: 1 + managedFields: + - apiVersion: flowcontrol.apiserver.k8s.io/v1beta3 + fieldsType: FieldsV1 + fieldsV1: + f:metadata: + f:annotations: + .: {} + f:apf.kubernetes.io/autoupdate-spec: {} + f:spec: + f:matchingPrecedence: {} + f:priorityLevelConfiguration: + f:name: {} + f:rules: {} + manager: controller + operation: Apply + time: "2023-06-08T11:18:25Z" + name: probes + resourceVersion: "68" + uid: 50913e35-e855-469f-bec6-3e8cd2607ab4 +spec: + matchingPrecedence: 2 + priorityLevelConfiguration: + name: exempt + rules: + - nonResourceRules: + - nonResourceURLs: + - /healthz + - /readyz + - /livez + verbs: + - get + subjects: + - group: + name: system:unauthenticated + kind: Group + - group: + name: system:authenticated + kind: Group diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java new file mode 100644 index 0000000000..0249b44927 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java @@ -0,0 +1,99 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec; +import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource; +import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class DependentSSAMatchingIT { + + public static final String TEST_RESOURCE_NAME = "test1"; + public static final String INITIAL_VALUE = "INITIAL_VALUE"; + public static final String CHANGED_VALUE = "CHANGED_VALUE"; + + public static final String CUSTOM_FIELD_MANAGER_NAME = "customFieldManagerName"; + public static final String OTHER_FIELD_MANAGER = "otherFieldManager"; + public static final String ADDITIONAL_KEY = "key2"; + public static final String ADDITIONAL_VALUE = "Additional Value"; + + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new DependentSSAReconciler(), + o -> o.withFieldManager(CUSTOM_FIELD_MANAGER_NAME)) + .build(); + + @Test + void testMatchingAndUpdate() { + SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0); + var resource = extension.create(testResource()); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(cm).isNotNull(); + assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, INITIAL_VALUE); + assertThat(cm.getMetadata().getManagedFields().stream() + .filter(fm -> fm.getManager().equals(CUSTOM_FIELD_MANAGER_NAME))).isNotEmpty(); + assertThat(SSAConfigMapDependent.NUMBER_OF_UPDATES.get()).isZero(); + }); + + ConfigMap cmPatch = new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .withNamespace(resource.getMetadata().getNamespace()) + .build()) + .withData(Map.of(ADDITIONAL_KEY, ADDITIONAL_VALUE)) + .build(); + + extension.getKubernetesClient().configMaps().resource(cmPatch).patch(new PatchContext.Builder() + .withFieldManager(OTHER_FIELD_MANAGER) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + + await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(cm.getData()).hasSize(2); + assertThat(SSAConfigMapDependent.NUMBER_OF_UPDATES.get()).isZero(); + assertThat(cm.getMetadata().getManagedFields()).hasSize(2); + }); + + resource.getSpec().setValue(CHANGED_VALUE); + extension.replace(resource); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(cm.getData()).hasSize(2); + assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, CHANGED_VALUE); + assertThat(cm.getData()).containsEntry(ADDITIONAL_KEY, ADDITIONAL_VALUE); + assertThat(cm.getMetadata().getManagedFields()).hasSize(2); + assertThat(SSAConfigMapDependent.NUMBER_OF_UPDATES.get()).isEqualTo(1); + }); + } + + public DependnetSSACustomResource testResource() { + DependnetSSACustomResource resource = new DependnetSSACustomResource(); + resource.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()); + resource.setSpec(new DependentSSASpec()); + resource.getSpec().setValue(INITIAL_VALUE); + return resource; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java new file mode 100644 index 0000000000..7bedde86fe --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java @@ -0,0 +1,171 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec; +import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource; +import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class DependentSSAMigrationIT { + + public static final String FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER = "fabric8-kubernetes-client"; + public static final String TEST_RESOURCE_NAME = "test1"; + public static final String INITIAL_VALUE = "INITIAL_VALUE"; + public static final String CHANGED_VALUE = "CHANGED_VALUE"; + + private String namespace; + private final KubernetesClient client = new KubernetesClientBuilder().build(); + + @BeforeEach + void setup(TestInfo testInfo) { + SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0); + LocallyRunOperatorExtension.applyCrd(DependnetSSACustomResource.class, client); + testInfo.getTestMethod().ifPresent(method -> { + namespace = KubernetesResourceUtil.sanitizeName(method.getName()); + cleanup(); + client.namespaces().resource(new NamespaceBuilder().withMetadata(new ObjectMetaBuilder() + .withName(namespace) + .build()).build()).create(); + }); + } + + @AfterEach + void cleanup() { + client.namespaces().resource(new NamespaceBuilder().withMetadata(new ObjectMetaBuilder() + .withName(namespace) + .build()).build()).delete(); + } + + @Test + void migratesFromLegacyToWorksAndBack() { + var legacyOperator = createOperator(client, true, null); + DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); + + var operator = createOperator(client, false, null); + testResource = reconcileWithNewApproach(testResource, operator); + var cm = getDependentConfigMap(); + assertThat(cm.getMetadata().getManagedFields()).hasSize(2); + + reconcileAgainWithLegacy(legacyOperator, testResource); + } + + @Test + void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() { + var legacyOperator = createOperator(client, true, null); + DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); + + var operator = createOperator(client, false, + FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER); + reconcileWithNewApproach(testResource, operator); + + var cm = getDependentConfigMap(); + + assertThat(cm.getMetadata().getManagedFields()).hasSize(2); + assertThat(cm.getMetadata().getManagedFields()) + // Jetty seems to be a bug in fabric8 client, it is only the default fieldManager if Jetty + // is used as http client + .allMatch(fm -> fm.getManager().equals(FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER) + || fm.getManager().equals("Jetty")); + } + + private void reconcileAgainWithLegacy(Operator legacyOperator, + DependnetSSACustomResource testResource) { + legacyOperator.start(); + + testResource.getSpec().setValue(INITIAL_VALUE); + testResource.getMetadata().setResourceVersion(null); + client.resource(testResource).update(); + + await().untilAsserted(() -> { + var cm = getDependentConfigMap(); + assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, INITIAL_VALUE); + }); + + legacyOperator.stop(); + } + + private DependnetSSACustomResource reconcileWithNewApproach( + DependnetSSACustomResource testResource, Operator operator) { + operator.start(); + + await().untilAsserted(() -> { + var cm = getDependentConfigMap(); + assertThat(cm).isNotNull(); + assertThat(cm.getData()).hasSize(1); + }); + + testResource.getSpec().setValue(CHANGED_VALUE); + testResource.getMetadata().setResourceVersion(null); + testResource = client.resource(testResource).update(); + + await().untilAsserted(() -> { + var cm = getDependentConfigMap(); + assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, CHANGED_VALUE); + }); + operator.stop(); + return testResource; + } + + private ConfigMap getDependentConfigMap() { + return client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get(); + } + + private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) { + legacyOperator.start(); + + var testResource = client.resource(testResource()).create(); + + await().untilAsserted(() -> { + var cm = getDependentConfigMap(); + assertThat(cm).isNotNull(); + assertThat(cm.getMetadata().getManagedFields()).hasSize(1); + assertThat(cm.getData()).hasSize(1); + }); + + legacyOperator.stop(); + return testResource; + } + + + private Operator createOperator(KubernetesClient client, boolean legacyDependentHandling, + String fieldManager) { + Operator operator = new Operator(client, + o -> o.withSSABasedCreateUpdateForDependentResources(!legacyDependentHandling) + .withSSABasedDefaultMatchingForDependentResources(!legacyDependentHandling) + .withCloseClientOnStop(false)); + operator.register(new DependentSSAReconciler(), o -> { + o.settingNamespace(namespace); + if (fieldManager != null) { + o.withFieldManager(fieldManager); + } + }); + return operator; + } + + + public DependnetSSACustomResource testResource() { + DependnetSSACustomResource resource = new DependnetSSACustomResource(); + resource.setMetadata(new ObjectMetaBuilder() + .withNamespace(namespace) + .withName(TEST_RESOURCE_NAME) + .build()); + resource.setSpec(new DependentSSASpec()); + resource.getSpec().setValue(INITIAL_VALUE); + return resource; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java new file mode 100644 index 0000000000..d5c48256f7 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java @@ -0,0 +1,27 @@ +package io.javaoperatorsdk.operator.sample.dependentssa; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration(dependents = {@Dependent(type = SSAConfigMapDependent.class)}) +public class DependentSSAReconciler + implements Reconciler, TestExecutionInfoProvider { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + DependnetSSACustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSASpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSASpec.java new file mode 100644 index 0000000000..c7b4dd0053 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSASpec.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.dependentssa; + +public class DependentSSASpec { + + private String value; + + public String getValue() { + return value; + } + + public DependentSSASpec setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java new file mode 100644 index 0000000000..06834fe211 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.dependentssa; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dssa") +public class DependnetSSACustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java new file mode 100644 index 0000000000..70f0ddcf15 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java @@ -0,0 +1,42 @@ +package io.javaoperatorsdk.operator.sample.dependentssa; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class SSAConfigMapDependent extends + CRUDKubernetesDependentResource { + + public static AtomicInteger NUMBER_OF_UPDATES = new AtomicInteger(0); + + public static final String DATA_KEY = "key1"; + + public SSAConfigMapDependent() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(DependnetSSACustomResource primary, + Context context) { + return new ConfigMapBuilder() + .withMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()) + .withData(Map.of(DATA_KEY, primary.getSpec().getValue())) + .build(); + } + + @Override + public ConfigMap update(ConfigMap actual, ConfigMap target, + DependnetSSACustomResource primary, + Context context) { + NUMBER_OF_UPDATES.incrementAndGet(); + return super.update(actual, target, primary, context); + } +} diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/leader-elector-stop-role-noaccess.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/leader-elector-stop-role-noaccess.yaml index b08bbb5c5a..7d0b451bc9 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/leader-elector-stop-role-noaccess.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/leader-elector-stop-role-noaccess.yaml @@ -6,4 +6,4 @@ metadata: rules: - apiGroups: [ "" ] resources: [ "configmaps" ] - verbs: [ "get", "watch", "list","post", "delete", "create" ] \ No newline at end of file + verbs: [ "get", "watch", "list","post", "delete", "create","patch" ] \ No newline at end of file diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-full-access-role.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-full-access-role.yaml index 024f298462..d5fe6b5862 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-full-access-role.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-full-access-role.yaml @@ -9,6 +9,6 @@ rules: verbs: [ "get", "watch", "list","post", "delete" ] - apiGroups: [ "" ] resources: [ "configmaps" ] - verbs: [ "get", "watch", "list","post", "delete", "create" ] + verbs: [ "get", "watch", "list","post", "delete", "create","patch" ] diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-configmap-access.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-configmap-access.yaml index bac8c1149f..2afe7e2fdc 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-configmap-access.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-configmap-access.yaml @@ -6,6 +6,6 @@ metadata: rules: - apiGroups: [ "sample.javaoperatorsdk" ] resources: [ "informerrelatedbehaviortestcustomresources" ] - verbs: [ "get", "watch", "list","post", "delete" ] + verbs: [ "get", "watch", "list","post", "delete","patch" ] diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-cr-access.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-cr-access.yaml index 8c6ae85aac..63e9951fda 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-cr-access.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-no-cr-access.yaml @@ -6,5 +6,5 @@ metadata: rules: - apiGroups: [""] resources: [ "configmaps" ] - verbs: [ "get", "watch", "list","post", "delete","create" ] + verbs: [ "get", "watch", "list","post", "delete","create","patch"] diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml index a780209c2d..776acfb258 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml @@ -8,4 +8,4 @@ rules: verbs: [ "get", "watch", "list","post", "delete" ] - apiGroups: [ "" ] resources: [ "configmaps" ] - verbs: [ "get", "watch", "list","post", "delete", "create" ] \ No newline at end of file + verbs: [ "get", "watch", "list","post", "delete", "create","patch" ] \ No newline at end of file diff --git a/testi-multi-container.yaml b/testi-multi-container.yaml new file mode 100644 index 0000000000..92ece6df00 --- /dev/null +++ b/testi-multi-container.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: Pod +metadata: + name: shared-storage +spec: + volumes: + - name: shared-data + emptyDir: {} + containers: + - name: nginx-container + image: nginx + volumeMounts: + - name: shared-data + mountPath: /usr/share/nginx/html + - name: debian-container + image: debian + volumeMounts: + - name: shared-data + mountPath: /data + command: ["/bin/sh"] + args: ["-c", "echo Level Up Blue Team! > /data/index.html"] \ No newline at end of file