From f514053631f8a83b007182b9beb2e06ef0ebc595 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 22 Feb 2023 10:55:42 +0100 Subject: [PATCH 1/2] fix: starup on multiple namespace watch --- .../operator/InformerRelatedBehaviorITS.java | 32 +++++++++++++++++-- ...back-test-only-main-ns-access-binding.yaml | 12 +++++++ .../rback-test-only-main-ns-access.yaml | 11 +++++++ 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access-binding.yaml create mode 100644 operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 314d2199ce..fffd3572b1 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -40,6 +40,7 @@ class InformerRelatedBehaviorITS { public static final String TEST_RESOURCE_NAME = "test1"; + public static final String ADDITIONAL_NAMESPACE_NAME = "additionalns"; KubernetesClient adminClient = new KubernetesClientBuilder().build(); InformerRelatedBehaviorTestReconciler reconciler; @@ -101,6 +102,22 @@ void startsUpWhenNoPermissionToSecondaryResource() { assertReconciled(); } +// @Test + void startsUpIfNoPermissionToOneOfTwoNamespaces() { + var otherNamespace = adminClient.namespaces().resource(namespace(ADDITIONAL_NAMESPACE_NAME)).create(); + try { + + + + } finally { + adminClient.resource(otherNamespace).delete(); + await().untilAsserted(()->{ + var ns = adminClient.namespaces().resource(otherNamespace).get(); + assertThat(ns).isNull(); + }); + } + } + @Test void resilientForLoosingPermissionForCustomResource() throws InterruptedException { setFullResourcesAccess(); @@ -240,7 +257,7 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup) { return startOperator(stopOnInformerErrorDuringStartup, true); } - Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler) { + Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler, String ... namespaces) { ConfigurationServiceProvider.reset(); reconciler = new InformerRelatedBehaviorTestReconciler(); @@ -253,6 +270,11 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStop } }); operator.register(reconciler); +// operator.register(reconciler,o-> { +// if (namespaces.length > 0) { +// o.settingNamespaces(namespaces); +// } +// }); operator.start(); return operator; } @@ -285,10 +307,14 @@ private void applyClusterRole(String filename) { } private Namespace namespace() { + return namespace(actualNamespace); + } + + private Namespace namespace(String name) { Namespace n = new Namespace(); n.setMetadata(new ObjectMetaBuilder() - .withName(actualNamespace) - .build()); + .withName(name) + .build()); return n; } } diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access-binding.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access-binding.yaml new file mode 100644 index 0000000000..ded058531b --- /dev/null +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access-binding.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: read-namespace-access +subjects: + - kind: User + name: rbac-test-user + apiGroup: rbac.authorization.k8s.io +roleRef: + kind: Role + name: rbac-behavior + apiGroup: rbac.authorization.k8s.io \ No newline at end of file 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 new file mode 100644 index 0000000000..a780209c2d --- /dev/null +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-only-main-ns-access.yaml @@ -0,0 +1,11 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: rbac-behavior +rules: + - apiGroups: [ "sample.javaoperatorsdk" ] + resources: [ "informerrelatedbehaviortestcustomresources" ] + verbs: [ "get", "watch", "list","post", "delete" ] + - apiGroups: [ "" ] + resources: [ "configmaps" ] + verbs: [ "get", "watch", "list","post", "delete", "create" ] \ No newline at end of file From 0005ac9a32aa10837f54fa949837bfce822cd0e6 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 22 Feb 2023 12:41:24 +0100 Subject: [PATCH 2/2] wip --- .../operator/InformerRelatedBehaviorITS.java | 79 +++++++++++++++---- .../operator/rback-test-role-binding.yaml | 2 +- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index fffd3572b1..7bc96912b4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -8,13 +8,17 @@ import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.rbac.ClusterRole; import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; +import io.fabric8.kubernetes.api.model.rbac.Role; +import io.fabric8.kubernetes.api.model.rbac.RoleBinding; import io.fabric8.kubernetes.client.ConfigBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; +import io.javaoperatorsdk.operator.sample.informerrelatedbehavior.ConfigMapDependentResource; import io.javaoperatorsdk.operator.sample.informerrelatedbehavior.InformerRelatedBehaviorTestCustomResource; import io.javaoperatorsdk.operator.sample.informerrelatedbehavior.InformerRelatedBehaviorTestReconciler; @@ -55,6 +59,8 @@ void beforeEach(TestInfo testInfo) { actualNamespace = KubernetesResourceUtil.sanitizeName(method.getName()); adminClient.resource(namespace()).createOrReplace(); }); + // cleans up binding before test, not all test cases use cluster role + removeClusterRoleBinding(); } @AfterEach @@ -102,22 +108,51 @@ void startsUpWhenNoPermissionToSecondaryResource() { assertReconciled(); } -// @Test + @Test void startsUpIfNoPermissionToOneOfTwoNamespaces() { - var otherNamespace = adminClient.namespaces().resource(namespace(ADDITIONAL_NAMESPACE_NAME)).create(); + var otherNamespace = + adminClient.resource(namespace(ADDITIONAL_NAMESPACE_NAME)).createOrReplace(); try { + addRoleBindingsToTestNamespaces(); + var operator = startOperator(false, false, actualNamespace, ADDITIONAL_NAMESPACE_NAME); + assertInformerNotWatchingForAdditionalNamespace(operator); - + adminClient.resource(testCustomResource()).createOrReplace(); + waitForWatchReconnect(); + assertReconciled(); } finally { adminClient.resource(otherNamespace).delete(); - await().untilAsserted(()->{ - var ns = adminClient.namespaces().resource(otherNamespace).get(); + await().untilAsserted(() -> { + var ns = adminClient.namespaces().resource(otherNamespace).fromServer().get(); assertThat(ns).isNull(); }); } } + private void assertInformerNotWatchingForAdditionalNamespace(Operator operator) { + assertThat(operator.getRuntimeInfo().allEventSourcesAreHealthy()).isFalse(); + var unhealthyEventSources = + operator.getRuntimeInfo().unhealthyInformerWrappingEventSourceHealthIndicator() + .get(INFORMER_RELATED_BEHAVIOR_TEST_RECONCILER); + + InformerHealthIndicator controllerHealthIndicator = + (InformerHealthIndicator) unhealthyEventSources + .get(ControllerResourceEventSource.class.getSimpleName()) + .informerHealthIndicators().get(ADDITIONAL_NAMESPACE_NAME); + assertThat(controllerHealthIndicator).isNotNull(); + assertThat(controllerHealthIndicator.getTargetNamespace()).isEqualTo(ADDITIONAL_NAMESPACE_NAME); + assertThat(controllerHealthIndicator.isWatching()).isFalse(); + + InformerHealthIndicator configMapHealthIndicator = + (InformerHealthIndicator) unhealthyEventSources + .get(ConfigMapDependentResource.class.getSimpleName()) + .informerHealthIndicators().get(ADDITIONAL_NAMESPACE_NAME); + assertThat(configMapHealthIndicator).isNotNull(); + assertThat(configMapHealthIndicator.getTargetNamespace()).isEqualTo(ADDITIONAL_NAMESPACE_NAME); + assertThat(configMapHealthIndicator.isWatching()).isFalse(); + } + @Test void resilientForLoosingPermissionForCustomResource() throws InterruptedException { setFullResourcesAccess(); @@ -257,7 +292,8 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup) { return startOperator(stopOnInformerErrorDuringStartup, true); } - Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler, String ... namespaces) { + Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler, + String... namespaces) { ConfigurationServiceProvider.reset(); reconciler = new InformerRelatedBehaviorTestReconciler(); @@ -269,12 +305,11 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStop co.withInformerStoppedHandler((informer, ex) -> replacementStopHandlerCalled = true); } }); - operator.register(reconciler); -// operator.register(reconciler,o-> { -// if (namespaces.length > 0) { -// o.settingNamespaces(namespaces); -// } -// }); + operator.register(reconciler, o -> { + if (namespaces.length > 0) { + o.settingNamespaces(namespaces); + } + }); operator.start(); return operator; } @@ -294,6 +329,16 @@ private void setFullResourcesAccess() { applyClusterRoleBinding(); } + private void addRoleBindingsToTestNamespaces() { + var role = ReconcilerUtils + .loadYaml(Role.class, this.getClass(), "rback-test-only-main-ns-access.yaml"); + adminClient.resource(role).inNamespace(actualNamespace).createOrReplace(); + var roleBinding = ReconcilerUtils + .loadYaml(RoleBinding.class, this.getClass(), + "rback-test-only-main-ns-access-binding.yaml"); + adminClient.resource(roleBinding).inNamespace(actualNamespace).createOrReplace(); + } + private void applyClusterRoleBinding() { var clusterRoleBinding = ReconcilerUtils .loadYaml(ClusterRoleBinding.class, this.getClass(), "rback-test-role-binding.yaml"); @@ -313,8 +358,14 @@ private Namespace namespace() { private Namespace namespace(String name) { Namespace n = new Namespace(); n.setMetadata(new ObjectMetaBuilder() - .withName(name) - .build()); + .withName(name) + .build()); return n; } + + private void removeClusterRoleBinding() { + var clusterRoleBinding = ReconcilerUtils + .loadYaml(ClusterRoleBinding.class, this.getClass(), "rback-test-role-binding.yaml"); + adminClient.resource(clusterRoleBinding).delete(); + } } diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-role-binding.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-role-binding.yaml index 5b9c9ee2d9..ec83726617 100644 --- a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-role-binding.yaml +++ b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/rback-test-role-binding.yaml @@ -2,7 +2,7 @@ apiVersion: rbac.authorization.k8s.io/v1 # This cluster role binding allows anyone in the "manager" group to read secrets in any namespace. kind: ClusterRoleBinding metadata: - name: read-secrets-global + name: informer-rbac-startup-global subjects: - kind: User name: rbac-test-user