fix(kubernetes): address crd delete error (#5334)

* fix(kubernetes): address crd delete error * fix(kubernetes): address crd delete error - 2 * fix(kubernetes): address crd delete error - 3 Co-authored-by: Roberto Badillo <rbadillo@ext.uber.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Apr 14, 2021 · 12abd4d · 12abd4d
1 parent 91769dc
commit 12abd4d
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 17 deletions.
diff --git a/.../netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java b/.../netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java
@@ -34,6 +34,8 @@
 @ParametersAreNonnullByDefault
 public class GlobalResourcePropertyRegistry implements ResourcePropertyRegistry {
   private final ImmutableMap<KubernetesKind, KubernetesResourceProperties> globalProperties;
+  private ImmutableMap<KubernetesKind, KubernetesResourceProperties> crdProperties =
+      ImmutableMap.of();
   private final KubernetesResourceProperties defaultProperties;
 
   @Autowired
@@ -50,12 +52,26 @@ public GlobalResourcePropertyRegistry(
         new KubernetesResourceProperties(defaultHandler, defaultHandler.versioned());
   }
 
+  public void updateCrdProperties(List<KubernetesHandler> handlers) {
+    this.crdProperties =
+        handlers.stream()
+            .collect(
+                toImmutableMap(
+                    KubernetesHandler::kind,
+                    h -> new KubernetesResourceProperties(h, h.versioned())));
+  }
+
   @Override
   @Nonnull
   public KubernetesResourceProperties get(KubernetesKind kind) {
-    KubernetesResourceProperties globalResult = globalProperties.get(kind);
-    if (globalResult != null) {
-      return globalResult;
+    KubernetesResourceProperties result = globalProperties.get(kind);
+    if (result != null) {
+      return result;
+    }
+
+    result = crdProperties.get(kind);
+    if (result != null) {
+      return result;
     }
 
     return defaultProperties;

diff --git a/.../netflix/spinnaker/clouddriver/kubernetes/op/handler/KubernetesCustomResourceHandler.java b/.../netflix/spinnaker/clouddriver/kubernetes/op/handler/KubernetesCustomResourceHandler.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2021 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package com.netflix.spinnaker.clouddriver.kubernetes.op.handler;
+
+import static com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler.DeployPriority.LOWEST_PRIORITY;
+
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesCachingAgentFactory;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesUnregisteredCustomResourceCachingAgent;
+import com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind;
+import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind;
+import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
+import com.netflix.spinnaker.clouddriver.kubernetes.model.Manifest;
+import javax.annotation.Nonnull;
+
+public class KubernetesCustomResourceHandler extends KubernetesHandler implements CanDelete {
+
+  private final KubernetesKind kind;
+
+  public KubernetesCustomResourceHandler(KubernetesKind kind) {
+    this.kind = kind;
+  }
+
+  @Override
+  public int deployPriority() {
+    return LOWEST_PRIORITY.getValue();
+  }
+
+  @Nonnull
+  @Override
+  public KubernetesKind kind() {
+    return this.kind;
+  }
+
+  @Override
+  public boolean versioned() {
+    return false;
+  }
+
+  @Nonnull
+  @Override
+  public SpinnakerKind spinnakerKind() {
+    return SpinnakerKind.UNCLASSIFIED;
+  }
+
+  @Override
+  public Manifest.Status status(KubernetesManifest manifest) {
+    return Manifest.Status.defaultStatus();
+  }
+
+  @Override
+  protected KubernetesCachingAgentFactory cachingAgentFactory() {
+    return KubernetesUnregisteredCustomResourceCachingAgent::new;
+  }
+}
diff --git a/...ain/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java b/...ain/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java
@@ -40,6 +40,7 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.config.LinkedDockerRegistryConfiguration;
 import com.netflix.spinnaker.clouddriver.kubernetes.config.RawResourcesEndpointConfig;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.AccountResourcePropertyRegistry;
+import com.netflix.spinnaker.clouddriver.kubernetes.description.GlobalResourcePropertyRegistry;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.JsonPatch;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesCoordinates;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesPatchOptions;
@@ -51,6 +52,8 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKindProperties;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
 import com.netflix.spinnaker.clouddriver.kubernetes.names.KubernetesNamerRegistry;
+import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesCustomResourceHandler;
+import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.job.KubectlJobExecutor;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.job.KubectlJobExecutor.KubectlException;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.job.KubectlJobExecutor.KubectlNotFoundException;
@@ -83,6 +86,7 @@ public class KubernetesCredentials {
   private final Registry registry;
   private final Clock clock;
   private final KubectlJobExecutor jobExecutor;
+  private final GlobalResourcePropertyRegistry globalResourcePropertyRegistry;
 
   @Include @Getter @Nonnull private final String accountName;
 
@@ -145,7 +149,8 @@ private KubernetesCredentials(
       KubernetesKindRegistry.Factory kindRegistryFactory,
       KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap,
       String kubeconfigFile,
-      Namer<KubernetesManifest> manifestNamer) {
+      Namer<KubernetesManifest> manifestNamer,
+      GlobalResourcePropertyRegistry globalResourcePropertyRegistry) {
     this.registry = registry;
     this.clock = registry.clock();
     this.jobExecutor = jobExecutor;
@@ -200,6 +205,7 @@ private KubernetesCredentials(
     this.namer = manifestNamer;
     this.cacheAllApplicationRelationships = managedAccount.isCacheAllApplicationRelationships();
     this.rawResourcesEndpointConfig = managedAccount.getRawResourcesEndpointConfig();
+    this.globalResourcePropertyRegistry = globalResourcePropertyRegistry;
   }
 
   /**
@@ -313,14 +319,23 @@ private ImmutableMap<KubernetesKind, KubernetesKindProperties> crdSupplier() {
       return ImmutableMap.of();
     }
     try {
-      return list(KubernetesKind.CUSTOM_RESOURCE_DEFINITION, "").stream()
-          .map(
-              manifest ->
-                  KubernetesCacheDataConverter.getResource(
-                      manifest, V1beta1CustomResourceDefinition.class))
-          .map(KubernetesKindProperties::fromCustomResourceDefinition)
-          .collect(
-              toImmutableMap(KubernetesKindProperties::getKubernetesKind, Function.identity()));
+      ImmutableMap<KubernetesKind, KubernetesKindProperties> crds =
+          list(KubernetesKind.CUSTOM_RESOURCE_DEFINITION, "").stream()
+              .map(
+                  manifest ->
+                      KubernetesCacheDataConverter.getResource(
+                          manifest, V1beta1CustomResourceDefinition.class))
+              .map(KubernetesKindProperties::fromCustomResourceDefinition)
+              .collect(
+                  toImmutableMap(KubernetesKindProperties::getKubernetesKind, Function.identity()));
+
+      List<KubernetesHandler> crdHandlers =
+          crds.keySet().stream()
+              .map(KubernetesCustomResourceHandler::new)
+              .collect(toImmutableList());
+      this.globalResourcePropertyRegistry.updateCrdProperties(crdHandlers);
+
+      return crds;
     } catch (KubectlException e) {
       // not logging here -- it will generate a lot of noise in cases where crds aren't
       // available/registered in the first place
@@ -739,6 +754,7 @@ public static class Factory {
     private final AccountResourcePropertyRegistry.Factory resourcePropertyRegistryFactory;
     private final KubernetesKindRegistry.Factory kindRegistryFactory;
     private final KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap;
+    private final GlobalResourcePropertyRegistry globalResourcePropertyRegistry;
 
     public KubernetesCredentials build(
         KubernetesConfigurationProperties.ManagedAccount managedAccount) {
@@ -752,7 +768,8 @@ public KubernetesCredentials build(
           kindRegistryFactory,
           kubernetesSpinnakerKindMap,
           getKubeconfigFile(configFileService, managedAccount),
-          manifestNamer);
+          manifestNamer,
+          globalResourcePropertyRegistry);
     }
 
     private String getKubeconfigFile(

diff --git a/...vy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsSpec.groovy b/...vy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsSpec.groovy
@@ -22,12 +22,14 @@ import com.netflix.spectator.api.NoopRegistry
 import com.netflix.spectator.api.Registry
 import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesConfigurationProperties
 import com.netflix.spinnaker.clouddriver.kubernetes.description.AccountResourcePropertyRegistry
+import com.netflix.spinnaker.clouddriver.kubernetes.description.GlobalResourcePropertyRegistry
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesApiGroup
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKindProperties
 import com.netflix.spinnaker.clouddriver.kubernetes.names.KubernetesManifestNamer
 import com.netflix.spinnaker.clouddriver.kubernetes.names.KubernetesNamerRegistry
+import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesUnregisteredCustomResourceHandler
 import com.netflix.spinnaker.clouddriver.kubernetes.op.job.KubectlJobExecutor
 import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials.KubernetesKindStatus
 import com.netflix.spinnaker.kork.configserver.ConfigFileService
@@ -44,6 +46,7 @@ class KubernetesCredentialsSpec extends Specification {
   KubernetesNamerRegistry namerRegistry = new KubernetesNamerRegistry([new KubernetesManifestNamer()])
   ConfigFileService configFileService = new ConfigFileService()
   KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap = new KubernetesSpinnakerKindMap(ImmutableList.of())
+  GlobalResourcePropertyRegistry globalResourcePropertyRegistry = new GlobalResourcePropertyRegistry(ImmutableList.of(), new KubernetesUnregisteredCustomResourceHandler())
 
   KubernetesCredentials.Factory credentialFactory = new KubernetesCredentials.Factory(
     new NoopRegistry(),
@@ -52,7 +55,8 @@ class KubernetesCredentialsSpec extends Specification {
     configFileService,
     resourcePropertyRegistryFactory,
     kindRegistryFactory,
-    kubernetesSpinnakerKindMap
+    kubernetesSpinnakerKindMap,
+    globalResourcePropertyRegistry
   )
 
 

diff --git a/...ix/spinnaker/clouddriver/kubernetes/security/KubernetesNamedAccountCredentialsSpec.groovy b/...ix/spinnaker/clouddriver/kubernetes/security/KubernetesNamedAccountCredentialsSpec.groovy
@@ -21,9 +21,11 @@ import com.google.common.collect.ImmutableList
 import com.netflix.spectator.api.NoopRegistry
 import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesConfigurationProperties
 import com.netflix.spinnaker.clouddriver.kubernetes.description.AccountResourcePropertyRegistry
+import com.netflix.spinnaker.clouddriver.kubernetes.description.GlobalResourcePropertyRegistry
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap
 import com.netflix.spinnaker.clouddriver.kubernetes.names.KubernetesManifestNamer
 import com.netflix.spinnaker.clouddriver.kubernetes.names.KubernetesNamerRegistry
+import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesUnregisteredCustomResourceHandler
 import com.netflix.spinnaker.clouddriver.kubernetes.op.job.KubectlJobExecutor
 import com.netflix.spinnaker.fiat.model.Authorization
 import com.netflix.spinnaker.kork.configserver.ConfigFileService
@@ -37,14 +39,16 @@ class KubernetesNamedAccountCredentialsSpec extends Specification {
   AccountResourcePropertyRegistry.Factory resourcePropertyRegistryFactory = Mock(AccountResourcePropertyRegistry.Factory)
   KubernetesKindRegistry.Factory kindRegistryFactory = Mock(KubernetesKindRegistry.Factory)
   KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap = new KubernetesSpinnakerKindMap(ImmutableList.of())
+  GlobalResourcePropertyRegistry globalResourcePropertyRegistry = new GlobalResourcePropertyRegistry(ImmutableList.of(), new KubernetesUnregisteredCustomResourceHandler())
   KubernetesCredentials.Factory credentialFactory = new KubernetesCredentials.Factory(
     new NoopRegistry(),
     namerRegistry,
     Mock(KubectlJobExecutor),
     configFileService,
     resourcePropertyRegistryFactory,
     kindRegistryFactory,
-    kubernetesSpinnakerKindMap
+    kubernetesSpinnakerKindMap,
+    globalResourcePropertyRegistry
   )
 
 

diff --git a/...r/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java b/...r/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java
@@ -610,7 +610,9 @@ private static KubernetesNamedAccountCredentials getNamedAccountCredentials() {
             new ConfigFileService(new CloudConfigResourceService()),
             new AccountResourcePropertyRegistry.Factory(resourcePropertyRegistry),
             new KubernetesKindRegistry.Factory(new GlobalKubernetesKindRegistry()),
-            kindMap);
+            kindMap,
+            new GlobalResourcePropertyRegistry(
+                ImmutableList.of(), new KubernetesUnregisteredCustomResourceHandler()));
     return new KubernetesNamedAccountCredentials(managedAccount, credentialFactory);
   }
 

diff --git a/...java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsTest.java b/...java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsTest.java
@@ -70,7 +70,9 @@ private KubernetesCredentials getCredentials(Registry registry, KubectlJobExecut
                     ImmutableList.of(), new KubernetesUnregisteredCustomResourceHandler())),
             new KubernetesKindRegistry.Factory(
                 new GlobalKubernetesKindRegistry(ImmutableList.of())),
-            new KubernetesSpinnakerKindMap(ImmutableList.of()));
+            new KubernetesSpinnakerKindMap(ImmutableList.of()),
+            new GlobalResourcePropertyRegistry(
+                ImmutableList.of(), new KubernetesUnregisteredCustomResourceHandler()));
     ManagedAccount managedAccount = new ManagedAccount();
     managedAccount.setName("my-account");
     return factory.build(managedAccount);