From 8faf1268b8ab2efc0cf31f4496ccc50ee46dcdef Mon Sep 17 00:00:00 2001 From: Lars Wander Date: Fri, 6 Oct 2017 13:32:16 -0400 Subject: [PATCH] fix(provider/kubernetes): Don't store-unowned artifacts --- .../v2/artifact/KubernetesArtifactConverter.java | 4 ++-- .../v2/caching/agent/KubernetesCacheDataConverter.java | 4 ++++ .../kubernetes/v2/op/KubernetesManifestDeployer.java | 8 ++++---- .../KubernetesUnversionedArtifactConverterSpec.groovy | 2 +- .../KubernetesVersionedArtifactConverterSpec.groovy | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesArtifactConverter.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesArtifactConverter.java index 6ea8b5bab10..d5d704871a8 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesArtifactConverter.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesArtifactConverter.java @@ -34,7 +34,7 @@ public abstract class KubernetesArtifactConverter { protected String getType(KubernetesManifest manifest) { return String.join("/", KubernetesCloudProvider.getID(), - manifest.getApiVersion().toString() + ":" + manifest.getKind().toString() + manifest.getApiVersion().toString() + "|" + manifest.getKind().toString() ); } @@ -48,7 +48,7 @@ private String[] getLatterType(Artifact artifact) { throw new IllegalArgumentException("Not a kubernetes artifact: " + artifact); } - split = String.join("/", Arrays.copyOfRange(split, 1, split.length)).split(":"); + split = String.join("/", Arrays.copyOfRange(split, 1, split.length)).split("\\|"); if (split.length != 2) { throw new IllegalArgumentException("Not a kubernetes artifact: " + artifact); diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java index 661e7e625a2..b4933b23bf5 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java @@ -52,6 +52,10 @@ public class KubernetesCacheDataConverter { public static CacheData convertAsArtifact(String account, ObjectMapper mapper, Object resource) { KubernetesManifest manifest = mapper.convertValue(resource, KubernetesManifest.class); Artifact artifact = KubernetesManifestAnnotater.getArtifact(manifest); + if (artifact.getType() == null) { + log.info("No assigned artifact type for this resource."); + return null; + } Map attributes = new ImmutableMap.Builder() .put("artifact", artifact) diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/KubernetesManifestDeployer.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/KubernetesManifestDeployer.java index 81a66e98f5f..09903c22a6c 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/KubernetesManifestDeployer.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/KubernetesManifestDeployer.java @@ -66,6 +66,10 @@ public DeploymentResult operate(List _unused) { getTask().updateStatus(OP_NAME, "Beginning deployment of manifest..."); KubernetesManifest manifest = description.getManifest(); + if (StringUtils.isEmpty(manifest.getNamespace())) { + manifest.setNamespace(credentials.getDefaultNamespace()); + } + KubernetesResourceProperties properties = findResourceProperties(manifest); KubernetesDeployer deployer = properties.getDeployer(); KubernetesArtifactConverter converter = properties.getConverter(); @@ -75,10 +79,6 @@ public DeploymentResult operate(List _unused) { Moniker moniker = description.getMoniker(); KubernetesManifestSpinnakerRelationships relationships = description.getRelationships(); - if (StringUtils.isEmpty(manifest.getNamespace())) { - manifest.setNamespace(credentials.getDefaultNamespace()); - } - getTask().updateStatus(OP_NAME, "Annotating manifest with artifact, relationships & moniker..."); KubernetesManifestAnnotater.annotateManifest(manifest, artifact); KubernetesManifestAnnotater.annotateManifest(manifest, relationships); diff --git a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesUnversionedArtifactConverterSpec.groovy b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesUnversionedArtifactConverterSpec.groovy index 524a8522079..ff1e71c2327 100644 --- a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesUnversionedArtifactConverterSpec.groovy +++ b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesUnversionedArtifactConverterSpec.groovy @@ -27,7 +27,7 @@ class KubernetesUnversionedArtifactConverterSpec extends Specification { @Unroll def "correctly infer unversioned artifact properties"() { expect: - def type = "kubernetes/$apiVersion:$kind" + def type = "kubernetes/$apiVersion|$kind" def artifact = Artifact.builder() .type(type) diff --git a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesVersionedArtifactConverterSpec.groovy b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesVersionedArtifactConverterSpec.groovy index 3560009116a..e77f2aa9d70 100644 --- a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesVersionedArtifactConverterSpec.groovy +++ b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/artifact/KubernetesVersionedArtifactConverterSpec.groovy @@ -28,7 +28,7 @@ class KubernetesVersionedArtifactConverterSpec extends Specification { @Unroll def "correctly infer versioned artifact properties"() { expect: - def type = "kubernetes/$apiVersion:$kind" + def type = "kubernetes/$apiVersion|$kind" def artifact = Artifact.builder() .type(type)