diff --git a/build.gradle b/build.gradle index 8449c354..62aa1220 100644 --- a/build.gradle +++ b/build.gradle @@ -5,7 +5,7 @@ plugins { id 'org.sonarqube' version '6.0.1.5171' id "com.diffplug.spotless" version "7.0.2" id "com.google.cloud.tools.jib" version "3.4.4" - id 'org.cyclonedx.bom' version '2.0.0' + id 'org.cyclonedx.bom' version '2.1.0' } cyclonedxBom { @@ -51,8 +51,8 @@ jib { project.ext { mongoDbDriverVersion = "5.3.1" slf4jVersion = "2.0.16" - operatorFrameworkVersion = "4.9.7" - kubernetesServerMockVersion = "6.13.4" // align with transitive dependency of operator framework + operatorFrameworkVersion = "5.0.1" + kubernetesServerMockVersion = "7.1.0" // align with transitive dependency of operator framework mockitoVersion = "5.2.0" jacksonVersion = "2.18.2" logbackContribVersion = "0.1.5" @@ -105,7 +105,7 @@ dependencies { exclude group: "ch.qos.logback", module: "logback-core" } - implementation 'io.micrometer:micrometer-registry-prometheus:1.14.3' + implementation 'io.micrometer:micrometer-registry-prometheus:1.14.4' // test testImplementation enforcedPlatform("org.junit:junit-bom:5.11.4") @@ -144,6 +144,9 @@ dependencies { // CVE-2020-15250 in 4.12 // -> pulled transitively from OkHttp3 mockwebserver used by kubernetes-server-mock testImplementation 'junit:junit:4.13.2' + testImplementation 'com.squareup.okhttp3:okhttp:3.12.12', { + exclude group: 'com.squareup.okio', module: 'okio' + } } dependencyLocking { lockAllConfigurations() } diff --git a/docs/deployment.md b/docs/deployment.md index f1f9d71b..fb0dd7a5 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -85,14 +85,17 @@ The MongoDB Operator requires a `ServiceAccount` with some privileges for the Ku * `list` * `get` * `update` + * `patch` * For the resource `mongodbs/status` the following verbs are required: * `watch` * `list` * `get` * `update` + * `patch` * For the resource `secrets` the following verbs are required: * `create` * `update` + * `patch` * For the resource `customresourcedefinitions` with the resource name `mongodbs.persistence.sda-se.com` the following verbs are required: * `get` @@ -104,7 +107,7 @@ The [CRD `mongodbs`](https://github.com/SDA-SE/mongodb-operator/tree/master/kust must be applied. -### Database +### Database A MongoDB database instance or an AWS DocumentDB cluster must be set up separately from the deployment of the MongoDB Operator. diff --git a/kustomize/bases/operator/mongodb-operator-cr.yaml b/kustomize/bases/operator/mongodb-operator-cr.yaml index 8e982dda..604a09b9 100644 --- a/kustomize/bases/operator/mongodb-operator-cr.yaml +++ b/kustomize/bases/operator/mongodb-operator-cr.yaml @@ -5,13 +5,13 @@ metadata: rules: - apiGroups: [""] resources: ["secrets"] - verbs: ["create", "update"] + verbs: ["create", "update", "patch"] - apiGroups: ["persistence.sda-se.com"] resources: ["mongodbs"] - verbs: ["watch", "list", "get", "update"] + verbs: ["watch", "list", "get", "update", "patch"] - apiGroups: ["persistence.sda-se.com"] resources: ["mongodbs/status"] - verbs: ["watch", "list", "get", "update"] + verbs: ["watch", "list", "get", "update", "patch"] - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] resourceNames: diff --git a/src/main/java/com/sdase/k8s/operator/mongodb/controller/MongoDbResourceConditions.java b/src/main/java/com/sdase/k8s/operator/mongodb/controller/MongoDbResourceConditions.java index e1ac2877..2d5d37d5 100644 --- a/src/main/java/com/sdase/k8s/operator/mongodb/controller/MongoDbResourceConditions.java +++ b/src/main/java/com/sdase/k8s/operator/mongodb/controller/MongoDbResourceConditions.java @@ -97,9 +97,9 @@ UpdateControl createStatusUpdate(MongoDbCustomResource re createSecretSuccessful, resource.getStatus().getAttempts()); if (allConditionsTrue(resource)) { - return UpdateControl.updateStatus(resource); + return UpdateControl.patchStatus(resource); } else { - return UpdateControl.updateStatus(resource) + return UpdateControl.patchStatus(resource) .rescheduleAfter( RECONCILE_REQUEST_MIN_DURATION_ON_ERROR_SECONDS * resource.getStatus().getAttempts(), TimeUnit.SECONDS); diff --git a/src/test/java/com/sdase/k8s/operator/mongodb/controller/MongoDbControllerTest.java b/src/test/java/com/sdase/k8s/operator/mongodb/controller/MongoDbControllerTest.java index 9e79b6df..aa2104aa 100644 --- a/src/test/java/com/sdase/k8s/operator/mongodb/controller/MongoDbControllerTest.java +++ b/src/test/java/com/sdase/k8s/operator/mongodb/controller/MongoDbControllerTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.tuple; import static org.assertj.core.api.SoftAssertions.assertSoftly; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -24,6 +23,7 @@ import com.sdase.k8s.operator.mongodb.model.v1beta1.DatabaseSpec; import com.sdase.k8s.operator.mongodb.model.v1beta1.MongoDbCustomResource; import com.sdase.k8s.operator.mongodb.model.v1beta1.MongoDbSpec; +import com.sdase.k8s.operator.mongodb.model.v1beta1.MongoDbStatus; import com.sdase.k8s.operator.mongodb.model.v1beta1.SecretSpec; import io.fabric8.kubernetes.api.model.Condition; import io.fabric8.kubernetes.api.model.ObjectMeta; @@ -35,13 +35,16 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; import io.javaoperatorsdk.operator.api.reconciler.IndexedResourceCache; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -92,7 +95,7 @@ void shouldPerformDefaultDelete() { var actual = mongoDbController.cleanup(given, new MongoDbCustomResourceContext()); - // By default owned resources (like the created secret) will be deleted as well. + // By default, owned resources (like the created secret) will be deleted as well. assertThat(actual).usingRecursiveComparison().isEqualTo(DeleteControl.defaultDelete()); verify(mongoDbServiceMock).dropDatabaseUser("the-namespace_the-name", "the-namespace_the-name"); verifyNoMoreInteractions(mongoDbServiceMock); @@ -118,7 +121,7 @@ void shouldSkipDeleteDatabaseAfterLastAttempt() { var actual = mongoDbController.cleanup(given, contextMock); - // By default owned resources (like the created secret) will be deleted as well. + // By default, owned resources (like the created secret) will be deleted as well. assertThat(actual).usingRecursiveComparison().isEqualTo(DeleteControl.defaultDelete()); verify(mongoDbServiceMock).dropDatabaseUser("the-namespace_the-name", "the-namespace_the-name"); verifyNoMoreInteractions(mongoDbServiceMock); @@ -140,7 +143,7 @@ void shouldFailWhenUserCantBeDeleted() { assertThatExceptionOfType(IllegalStateException.class) .isThrownBy(() -> mongoDbController.cleanup(given, givenContext)); - // By default owned resources (like the created secret) will be deleted as well. + // By default, owned resources (like the created secret) will be deleted as well. verify(mongoDbServiceMock).dropDatabaseUser("the-namespace_the-name", "the-namespace_the-name"); } @@ -159,7 +162,7 @@ void shouldPerformDeleteWithPruneDb() { var actual = mongoDbController.cleanup(given, new MongoDbCustomResourceContext()); - // By default owned resources (like the created secret) will be deleted as well. + // By default, owned resources (like the created secret) will be deleted as well. assertThat(actual).usingRecursiveComparison().isEqualTo(DeleteControl.defaultDelete()); verify(mongoDbServiceMock).dropDatabaseUser("the-namespace_the-name", "the-namespace_the-name"); verify(mongoDbServiceMock).dropDatabase("the-namespace_the-name"); @@ -186,7 +189,7 @@ void shouldShouldFailWhenPruneDbNotPossible() { assertThatExceptionOfType(IllegalStateException.class) .isThrownBy(() -> mongoDbController.cleanup(given, contextMock)); - // By default owned resources (like the created secret) will be deleted as well. + // By default, owned resources (like the created secret) will be deleted as well. verify(mongoDbServiceMock).dropDatabaseUser("the-namespace_the-name", "the-namespace_the-name"); verify(mongoDbServiceMock).dropDatabase("the-namespace_the-name"); } @@ -261,14 +264,10 @@ void shouldCallAllNecessaryServicesForSuccess() { // For the moment no updates to the original resource, this may change in the future if // needed. - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isTrue(); - softly - .assertThat(actual.getResource().getStatus().getConditions()) - .isNotEmpty() - .extracting(Condition::getStatus) - .containsOnly("True"); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isTrue(); + softly.assertThat(areAllStatusConditionsTrue(actual)).isTrue(); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isFalse(); }); } @@ -301,16 +300,14 @@ void shouldFailWithBadConditionsForTooLongName() { softly -> { // For the moment no updates to the original resource, this may change in the future if // needed. - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isTrue(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isTrue(); softly - .assertThat(actual.getResource().getStatus().getConditions()) - .extracting(Condition::getType, Condition::getStatus) - .containsExactlyInAnyOrder( - tuple("CreateUsername", "False"), - tuple("CreateDatabase", "Unknown"), - tuple("CreateSecret", "Unknown")); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + .assertThat(extractStatusConditions(actual)) + .containsEntry("CreateUsername", "False") + .containsEntry("CreateDatabase", "Unknown") + .containsEntry("CreateSecret", "Unknown"); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isFalse(); }); } @@ -353,16 +350,14 @@ void shouldFailWhenSecretCantBeCreated() { .extracting(Secret::getData) .extracting("u", "p") .containsExactly("dGhlLW5hbWVzcGFjZV90aGUtbmFtZQ==", "c3RhdGljLXRlc3QtcGFzc3dvcmQ="); - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isTrue(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isTrue(); softly - .assertThat(actual.getResource().getStatus().getConditions()) - .extracting(Condition::getType, Condition::getStatus) - .containsExactlyInAnyOrder( - tuple("CreateUsername", "True"), - tuple("CreateDatabase", "True"), - tuple("CreateSecret", "False")); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + .assertThat(extractStatusConditions(actual)) + .containsEntry("CreateUsername", "True") + .containsEntry("CreateDatabase", "True") + .containsEntry("CreateSecret", "False"); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isFalse(); }); } @@ -388,16 +383,14 @@ void shouldNotCreateSecretButFailWhenNoDatabaseHasBeenCreated() { verifyNoInteractions(kubernetesClientAdapterMock); assertSoftly( softly -> { - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isTrue(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isTrue(); softly - .assertThat(actual.getResource().getStatus().getConditions()) - .extracting(Condition::getType, Condition::getStatus) - .containsExactlyInAnyOrder( - tuple("CreateUsername", "True"), - tuple("CreateDatabase", "False"), - tuple("CreateSecret", "Unknown")); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + .assertThat(extractStatusConditions(actual)) + .containsEntry("CreateUsername", "True") + .containsEntry("CreateDatabase", "False") + .containsEntry("CreateSecret", "Unknown"); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isFalse(); }); } @@ -423,16 +416,14 @@ void shouldUpdateStatusOfExistingResourcesWithoutStatus() { verifyNoInteractions(kubernetesClientAdapterMock); assertSoftly( softly -> { - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isTrue(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isTrue(); softly - .assertThat(actual.getResource().getStatus().getConditions()) - .extracting(Condition::getType, Condition::getStatus) - .containsExactlyInAnyOrder( - tuple("CreateUsername", "True"), - tuple("CreateDatabase", "True"), - tuple("CreateSecret", "True")); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + .assertThat(extractStatusConditions(actual)) + .containsEntry("CreateUsername", "True") + .containsEntry("CreateDatabase", "True") + .containsEntry("CreateSecret", "True"); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isFalse(); }); } @@ -444,27 +435,48 @@ void shouldDoNothingWhenStatusIsComplete() { new ObjectMetaBuilder().withNamespace("the-namespace").withName("the-name").build()); givenMongoDbCr.setSpec( new MongoDbSpec().setSecret(new SecretSpec().setUsernameKey("u").setPasswordKey("p"))); + //noinspection OptionalGetWithoutIsPresent givenMongoDbCr = new MongoDbResourceConditions() .applyUsernameCreated("the-namespace_the-name") .applyDatabaseCreated() .applySecretCreated() .createStatusUpdate(givenMongoDbCr) - .getResource(); + .getResource() + .get(); var givenContext = new MongoDbCustomResourceContext(); var actual = mongoDbController.reconcile(givenMongoDbCr, givenContext); assertSoftly( softly -> { - softly.assertThat(actual.isUpdateResource()).isFalse(); - softly.assertThat(actual.isUpdateStatus()).isFalse(); - softly.assertThat(actual.isUpdateResourceAndStatus()).isFalse(); + softly.assertThat(actual.isPatchResource()).isFalse(); + softly.assertThat(actual.isPatchStatus()).isFalse(); + softly.assertThat(actual.isPatchResourceAndStatus()).isFalse(); softly.assertThat(actual.isNoUpdate()).isTrue(); }); verifyNoInteractions(taskFactorySpy, mongoDbServiceMock, kubernetesClientAdapterMock); } + private boolean areAllStatusConditionsTrue(UpdateControl resource) { + Map conditionsByName = extractStatusConditions(resource); + return !conditionsByName.isEmpty() + && conditionsByName.values().stream().allMatch("True"::equals); + } + + private Map extractStatusConditions( + UpdateControl resource) { + return resource + .getResource() + .map(MongoDbCustomResource::getStatus) + .map(MongoDbStatus::getConditions) + .map( + conditions -> + conditions.stream() + .collect(Collectors.toMap(Condition::getType, Condition::getStatus))) + .orElse(new HashMap<>()); + } + private class MongoDbCustomResourceContext implements Context { @Override @@ -478,24 +490,17 @@ public Set getSecondaryResources(Class expectedType) { } @Override - @SuppressWarnings("removal") public Optional getSecondaryResource(Class expectedType, String eventSourceName) { return Optional.empty(); } - @Override - public Optional getSecondaryResource( - Class expectedType, ResourceDiscriminator discriminator) { - return Optional.empty(); - } - @Override public ControllerConfiguration getControllerConfiguration() { return null; } @Override - public ManagedDependentResourceContext managedDependentResourceContext() { + public ManagedWorkflowAndDependentResourceContext managedWorkflowAndDependentResourceContext() { return null; } @@ -518,5 +523,10 @@ public ExecutorService getWorkflowExecutorService() { public IndexedResourceCache getPrimaryCache() { return null; } + + @Override + public boolean isNextReconciliationImminent() { + return false; + } } }