From 12abd4d778d22829707137673717466e5334511a Mon Sep 17 00:00:00 2001 From: Roberto Badillo Date: Wed, 14 Apr 2021 15:49:32 -0400 Subject: [PATCH] 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 Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../GlobalResourcePropertyRegistry.java | 22 +++++- .../KubernetesCustomResourceHandler.java | 69 +++++++++++++++++++ .../security/KubernetesCredentials.java | 37 +++++++--- .../security/KubernetesCredentialsSpec.groovy | 6 +- ...bernetesNamedAccountCredentialsSpec.groovy | 6 +- ...KubernetesDataProviderIntegrationTest.java | 4 +- .../security/KubernetesCredentialsTest.java | 4 +- 7 files changed, 131 insertions(+), 17 deletions(-) create mode 100644 clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/KubernetesCustomResourceHandler.java diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java index d2a3db71484..ffa87d9676c 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/GlobalResourcePropertyRegistry.java @@ -34,6 +34,8 @@ @ParametersAreNonnullByDefault public class GlobalResourcePropertyRegistry implements ResourcePropertyRegistry { private final ImmutableMap globalProperties; + private ImmutableMap crdProperties = + ImmutableMap.of(); private final KubernetesResourceProperties defaultProperties; @Autowired @@ -50,12 +52,26 @@ public GlobalResourcePropertyRegistry( new KubernetesResourceProperties(defaultHandler, defaultHandler.versioned()); } + public void updateCrdProperties(List 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/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/KubernetesCustomResourceHandler.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/KubernetesCustomResourceHandler.java new file mode 100644 index 00000000000..cfcdda21786 --- /dev/null +++ b/clouddriver-kubernetes/src/main/java/com/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/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java index db3067d9beb..70a97bc8995 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java +++ b/clouddriver-kubernetes/src/main/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 manifestNamer) { + Namer 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 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 crds = + list(KubernetesKind.CUSTOM_RESOURCE_DEFINITION, "").stream() + .map( + manifest -> + KubernetesCacheDataConverter.getResource( + manifest, V1beta1CustomResourceDefinition.class)) + .map(KubernetesKindProperties::fromCustomResourceDefinition) + .collect( + toImmutableMap(KubernetesKindProperties::getKubernetesKind, Function.identity())); + + List 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/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsSpec.groovy b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsSpec.groovy index 42420388abf..1fa036c846b 100644 --- a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsSpec.groovy +++ b/clouddriver-kubernetes/src/test/groovy/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/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesNamedAccountCredentialsSpec.groovy b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesNamedAccountCredentialsSpec.groovy index ed2676f2a07..8c0ab9eab13 100644 --- a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesNamedAccountCredentialsSpec.groovy +++ b/clouddriver-kubernetes/src/test/groovy/com/netflix/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,6 +39,7 @@ 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, @@ -44,7 +47,8 @@ class KubernetesNamedAccountCredentialsSpec extends Specification { configFileService, resourcePropertyRegistryFactory, kindRegistryFactory, - kubernetesSpinnakerKindMap + kubernetesSpinnakerKindMap, + globalResourcePropertyRegistry ) diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java index e5c9df95b3b..fbfe5dfed77 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/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/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsTest.java index 19be1668a9e..aaf8b400d86 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentialsTest.java +++ b/clouddriver-kubernetes/src/test/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);