feat(provider/kubernetes): string annotations w/o quotes (#2595)

spinnaker · May 3, 2018 · 4851ede · 4851ede
1 parent 817f82f
commit 4851ede
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/...spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesManifestAnnotater.java b/...spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesManifestAnnotater.java
@@ -21,9 +21,11 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.frigga.Names;
+import com.netflix.spinnaker.cats.cache.CacheFilter;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import com.netflix.spinnaker.moniker.Moniker;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 
 import java.util.List;
 import java.util.Map;
@@ -63,8 +65,14 @@ private static void storeAnnotation(Map<String, String> annotations, String key,
       return;
     }
 
+
     try {
-      annotations.put(key, objectMapper.writeValueAsString(value));
+      if (value instanceof String) {
+        // The "write value as string" method will attach quotes which are ugly to read
+        annotations.put(key, (String) value);
+      } else {
+        annotations.put(key, objectMapper.writeValueAsString(value));
+      }
     } catch (JsonProcessingException e) {
       throw new IllegalArgumentException("Illegal annotation value for '" + key + "': " + e);
     }
@@ -74,14 +82,38 @@ private static <T> T getAnnotation(Map<String, String> annotations, String key,
     return getAnnotation(annotations, key, typeReference, null);
   }
 
+  private static boolean stringTypeReference(TypeReference typeReference) {
+    if (typeReference.getType() == null || typeReference.getType().getTypeName() == null) {
+      log.warn("Malformed type reference {}", typeReference);
+      return false;
+    }
+
+    return typeReference.getType().getTypeName().equals(String.class.getName());
+  }
+
+  // This is to read values that were annotated with the ObjectMapper with quotes, before we started ignoring the quotes
+  private static boolean looksLikeSerializedString(String value) {
+    if (StringUtils.isEmpty(value) || value.length() == 1) {
+      return false;
+    }
+
+    return value.charAt(0) == '"' && value.charAt(value.length() - 1) == '"';
+  }
+
   private static <T> T getAnnotation(Map<String, String> annotations, String key, TypeReference<T> typeReference, T defaultValue) {
     String value = annotations.get(key);
     if (value == null) {
       return defaultValue;
     }
 
     try {
-      return objectMapper.readValue(value, typeReference);
+      boolean wantsString = stringTypeReference(typeReference);
+
+      if (wantsString && !looksLikeSerializedString(value)) {
+        return (T) value;
+      } else {
+        return objectMapper.readValue(value, typeReference);
+      }
     } catch (Exception e) {
       log.warn("Illegally annotated resource for '" + key + "': " + e);
       return null;

diff --git a/...ix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesManifestAnnotatorSpec.groovy b/...ix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesManifestAnnotatorSpec.groovy
@@ -74,8 +74,8 @@ class KubernetesManifestAnnotatorSpec extends Specification {
       .build()
 
     KubernetesManifestAnnotater.annotateManifest(manifest, moniker)
-    manifest.getAnnotations().get(clusterKey) == '"' + cluster + '"'
-    manifest.getAnnotations().get(applicationKey) == '"' + application + '"'
+    manifest.getAnnotations().get(clusterKey) == cluster
+    manifest.getAnnotations().get(applicationKey) == application
 
     where:
     cluster | application