fix(crd-generator): allow defaults to be full json or yaml (6903)

fix: allow defaults to be full json or yaml

closes: #6908

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
---
Merge branch 'main' into iss6900
---
not requiring the JsonProperty annotation to be valid

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

Author: shawkins

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 #### Bugs
 * Fix #6892: rolling().restart() doesn't remove preexistent pod template annotations
+* Fix #6908: The Default annotation and JsonProperty default value should accept JSON values
 * Fix #6906: Knative VolatileTime should be serialized as String
 * Fix #6930: Add support for Boolean enums in the java-generator
 * Fix #6886: Remove invalid JUnit 4 references

crd-generator/api-v2/src/main/java/io/fabric8/crdv2/generator/AbstractJsonSchema.java

@@ -85,7 +85,6 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static io.fabric8.crdv2.generator.CRDUtils.toTargetType;
 import static java.util.Optional.ofNullable;
 
 /**
@@ -232,7 +231,7 @@ class PropertyMetadata {
 
     private boolean required;
     private final String description;
-    private final Object defaultValue;
+    private final JsonNode defaultValue;
     private Double min;
     private Boolean exclusiveMinimum;
     private Double max;
@@ -314,14 +313,27 @@ public PropertyMetadata(JsonSchema value, BeanProperty beanProperty) {
 
       // TODO: should the following be deprecated?
       required = beanProperty.getAnnotation(Required.class) != null;
+      defaultValue = toDefault(beanProperty);
+    }
+
+    JsonNode toDefault(BeanProperty beanProperty) {
+      Optional<String> defaultAnnotationValue = ofNullable(beanProperty.getAnnotation(Default.class)).map(Default::value);
+      String value = defaultAnnotationValue.orElse(beanProperty.getMetadata().getDefaultValue());
 
-      if (beanProperty.getMetadata().getDefaultValue() != null) {
-        defaultValue = toTargetType(beanProperty.getType(), beanProperty.getMetadata().getDefaultValue());
-      } else if (ofNullable(beanProperty.getAnnotation(Default.class)).map(Default::value).isPresent()) {
-        defaultValue = toTargetType(beanProperty.getType(),
-            ofNullable(beanProperty.getAnnotation(Default.class)).map(Default::value).get());
-      } else {
-        defaultValue = null;
+      if (value == null) {
+        return null;
+      }
+      Optional<Class<?>> rawType = Optional.ofNullable(beanProperty.getType()).map(JavaType::getRawClass);
+      try {
+        Object typedValue = resolvingContext.kubernetesSerialization.unmarshal(value, rawType.orElse(Object.class));
+        return resolvingContext.kubernetesSerialization.convertValue(typedValue, JsonNode.class);
+      } catch (Exception e) {
+        if (defaultAnnotationValue.isEmpty()) {
+          LOGGER.warn("Cannot parse default value: '" + value
+              + "' from JsonProperty annotation as valid YAML or JSON, no default value will be used.");
+          return null;
+        }
+        throw new IllegalArgumentException("Cannot parse default value: '" + value + "' as valid YAML or JSON.", e);
       }
     }
 
@@ -355,14 +367,7 @@ private void setMinMax(BeanProperty beanProperty,
 
     public void updateSchema(T schema) {
       schema.setDescription(description);
-
-      if (defaultValue != null) {
-        try {
-          schema.setDefault(resolvingContext.kubernetesSerialization.convertValue(defaultValue, JsonNode.class));
-        } catch (IllegalArgumentException e) {
-          throw new IllegalArgumentException("Cannot parse default value: '" + defaultValue + "' as valid YAML.", e);
-        }
-      }
+      schema.setDefault(defaultValue);
       if (nullable) {
         schema.setNullable(true);
       }

crd-generator/api-v2/src/main/java/io/fabric8/crdv2/generator/CRDUtils.java

@@ -16,12 +16,10 @@
 package io.fabric8.crdv2.generator;
 
 import com.fasterxml.jackson.databind.BeanDescription;
-import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationConfig;
 import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
 
-import java.text.NumberFormat;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -94,23 +92,4 @@ public static Map<String, String> toMap(String[] arr) {
     return res;
   }
 
-  static Object toTargetType(JavaType type, String value) {
-    if (type == null || value == null) {
-      return null;
-    }
-    try {
-      if (Number.class.isAssignableFrom(type.getRawClass()) || int.class.isAssignableFrom(type.getRawClass())
-          || long.class.isAssignableFrom(type.getRawClass()) || float.class.isAssignableFrom(type.getRawClass())
-          || double.class.isAssignableFrom(type.getRawClass())) {
-        return NumberFormat.getInstance().parse(value);
-      }
-      if (Boolean.class.isAssignableFrom(type.getRawClass()) || boolean.class.isAssignableFrom(type.getRawClass())) {
-        return Boolean.valueOf(value);
-      }
-    } catch (Exception ex) {
-      // NO OP
-    }
-    return value;
-
-  }
 }

crd-generator/api-v2/src/test/java/io/fabric8/crdv2/generator/v1/JsonSchemaDefaultValueTest.java

@@ -17,8 +17,11 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.BooleanNode;
 import com.fasterxml.jackson.databind.node.DoubleNode;
+import com.fasterxml.jackson.databind.node.FloatNode;
+import com.fasterxml.jackson.databind.node.IntNode;
 import com.fasterxml.jackson.databind.node.LongNode;
 import io.fabric8.generator.annotation.Default;
 import org.assertj.core.api.InstanceOfAssertFactories;
@@ -55,24 +58,31 @@ private static final class ClassInTest {
     @JsonProperty(defaultValue = "1337")
     int defaultValueForInt;
 
-    @JsonProperty(defaultValue = "1337L")
+    @JsonProperty(defaultValue = "1337")
     long defaultValueForLong;
 
     @JsonProperty(defaultValue = "13.37")
     float defaultValueForFloat;
 
-    @JsonProperty(defaultValue = "13.37d")
+    @JsonProperty(defaultValue = "13.37")
     double defaultValueForDouble;
+
+    @JsonProperty(defaultValue = "invalid")
+    double invalidDefaultValueForDouble;
+
+    @JsonProperty
+    @Default("[]")
+    double[] defaultValueForDoubleArray;
   }
 
   @Test
-  @DisplayName("JsonProperty default value should take precedence over Default annotation")
+  @DisplayName("Default annotation should take precedence over JsonProperty")
   void precedence() {
     assertThat(JsonSchema.from(ClassInTest.class).getProperties())
         .extracting("precedence._default")
         .asInstanceOf(InstanceOfAssertFactories.type(JsonNode.class))
         .extracting(JsonNode::asText)
-        .isEqualTo("precedence-from-json-property");
+        .isEqualTo("precedence-from-default-annotation");
   }
 
   @Test
@@ -135,7 +145,7 @@ void intFromJsonPropertyAnnotation() {
     assertThat(JsonSchema.from(ClassInTest.class).getProperties())
         .extracting("defaultValueForInt._default")
         .asInstanceOf(InstanceOfAssertFactories.type(JsonNode.class))
-        .isInstanceOf(LongNode.class)
+        .isInstanceOf(IntNode.class)
         .extracting(JsonNode::asInt)
         .isEqualTo(1337);
   }
@@ -157,9 +167,9 @@ void floatFromJsonPropertyAnnotation() {
     assertThat(JsonSchema.from(ClassInTest.class).getProperties())
         .extracting("defaultValueForFloat._default")
         .asInstanceOf(InstanceOfAssertFactories.type(JsonNode.class))
-        .isInstanceOf(DoubleNode.class)
-        .extracting(JsonNode::asDouble)
-        .isEqualTo(13.37);
+        .isInstanceOf(FloatNode.class)
+        .extracting(JsonNode::asText)
+        .isEqualTo("13.37");
   }
 
   @Test
@@ -172,4 +182,23 @@ void doubleFromJsonPropertyAnnotation() {
         .extracting(JsonNode::asDouble)
         .isEqualTo(13.37);
   }
+
+  @Test
+  @DisplayName("JsonProperty defaultValue annotation for double array")
+  void doubleArrayFromJsonPropertyAnnotation() {
+    assertThat(JsonSchema.from(ClassInTest.class).getProperties())
+        .extracting("defaultValueForDoubleArray._default")
+        .asInstanceOf(InstanceOfAssertFactories.type(JsonNode.class))
+        .isInstanceOf(ArrayNode.class)
+        .extracting(JsonNode::toPrettyString)
+        .isEqualTo("[ ]");
+  }
+
+  @Test
+  @DisplayName("JsonProperty invalid defaultValue annotation not used")
+  void invalidDefaultFromJsonPropertyAnnotationNotUsed() {
+    assertThat(JsonSchema.from(ClassInTest.class).getProperties())
+        .extracting("invalidDefaultValueForDouble._default")
+        .isNull();
+  }
 }

doc/CRD-generator-migration-v2.md

@@ -100,14 +100,7 @@ myMap:
   type: "object"
 ```
 
-## Default values for CRD fields can be numeric or boolean
-
-Previously default values defined by `@Default` could only be used on string fields.
-With CRD Generator v2 defaults can be set on numeric and boolean fields, too.
-In the same way is `@JsonProperty(defaultValue)` now working.
-
 ## Post-processing CRDs before they are written out to disk
 
 It is now possible to provide a `CRDPostProcessor` implementation when generating CRDs via the
 `CRDGenerator.detailedGenerate` method. This allows to process generated CRDs before they are written out to the disk.
-

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/KubernetesSerialization.java

@@ -369,7 +369,7 @@ public Type getType() {
    * Create a copy of the resource via serialization.
    *
    * @return a deep clone of the resource
-   * @throws IllegalArgumentException if the cloning cannot be performed
+   * @throws IllegalStateException if the cloning cannot be performed
    */
   public <T> T clone(T resource) {
     // if full serialization seems too expensive, there is also