CustomResourceFluentImpl produces a shallow copy

[shawkins]

In the CustomResourceFluentImpl, there are methods such as:

    public A withMetadata(ObjectMeta metadata) {
        this.metadata=metadata; return (A) this;
    }

which are invoked when a reference object is passed to the builder constructor.

In core classes the implementation looks like:

    public A withMetadata(ObjectMeta metadata) {
        _visitables.get("metadata").remove(this.metadata);
        if (metadata!=null){ this.metadata= new ObjectMetaBuilder(metadata); _visitables.get("metadata").add(this.metadata);} return (A) this;
    }

which does produce a deep copy.

[shawkins]

cc @iocanel

[iocanel]

It seems that ObjectMeta is not recognized as bildable.

Indeed it seems that there should be a @BuildableReference(ObjectMeta.class) defined as is the case with all model types:

https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/CustomResource.java#L69

[manusa]

CustomResource should be annotated like any other Resource (from modules different to kubernetes-model-core):

@Buildable(editableEnabled = false, validationEnabled = false, generateBuilderPackage = false, lazyCollectionInitEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder", refs = {
    @BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),
    @BuildableReference(LabelSelector.class),
    @BuildableReference(Container.class),
    @BuildableReference(PodTemplateSpec.class),
    @BuildableReference(ResourceRequirements.class),
    @BuildableReference(IntOrString.class),
    @BuildableReference(ObjectReference.class),
    @BuildableReference(LocalObjectReference.class),
    @BuildableReference(PersistentVolumeClaim.class)
})

The list should match:

kubernetes-client/kubernetes-model-generator/kubernetes-model-jsonschema2pojo/src/main/java/io/fabric8/kubernetes/jsonschema2pojo/KubernetesTypeAnnotator.java

Lines 41 to 50 in 4eee56e

	JAnnotationArrayMember arrayMember = buildable.paramArray("refs");
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.ObjectMeta"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.LabelSelector"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.Container"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.PodTemplateSpec"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.ResourceRequirements"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.IntOrString"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.ObjectReference"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.LocalObjectReference"));
	arrayMember.annotate(BuildableReference.class).param(BUILDABLE_REFERENCE_VALUE, new JCodeModel()._class("io.fabric8.kubernetes.api.model.PersistentVolumeClaim"));

[rohanKanojia]

@shawkins : Are you interested in submitting a fix for this issue?

[shawkins]

Are you interested in submitting a fix for this issue?

I am not yet familiar with this part of the code. It would be better if I just followed what fix was applied.

[rohanKanojia]

@shawkins : I think Ioannis and Marc are saying to add explicit BuildableReference annotation for ObjectMeta:

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/CustomResource.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/CustomResource.java
index 5f078a0fe..1bb42c26a 100644
--- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/CustomResource.java
+++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/CustomResource.java
@@ -34,6 +34,8 @@ import io.fabric8.kubernetes.model.annotation.ShortNames;
 import io.fabric8.kubernetes.model.annotation.Version;
 import io.sundr.builder.annotations.Buildable;
 import java.util.Optional;
+
+import io.sundr.builder.annotations.BuildableReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -66,8 +68,10 @@ import org.slf4j.LoggerFactory;
   "spec",
   "status"
 })
-@Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", editableEnabled = false)
 @JsonInclude(Include.NON_NULL)
+@Buildable(editableEnabled = false, validationEnabled = false, generateBuilderPackage = false, lazyCollectionInitEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder", refs = {
+  @BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),
+})
 public abstract class CustomResource<S, T> implements HasMetadata {
   private static final Logger LOG = LoggerFactory.getLogger(CustomResource.class);

[shawkins]

Does this also mean that anyone expecting this cloning behavior will need to add refs for spec and status classes for whatever extends CustomResource as well?

[rohanKanojia]

Are you using sundrio for CustomResource builder generation? I think this @BuildableReference is only used to refer to those classes which are not available at compile time. If you see all core Kubernetes resource types (Pod, Secret etc) don't have this since ObjectMeta is also compiled in kubernetes-model-core

[shawkins]

Are you using sundrio for CustomResource builder generation?

Yes

We have custom resource:

@Buildable(
        builderPackage = "io.fabric8.kubernetes.api.builder",
        refs = @BuildableReference(CustomResource.class),
        editableEnabled = false
)
@Group("managedkafka.bf2.org")
@Version("v1alpha1")
public class MyResource extends CustomResource<MyResourceSpec, MyResourceStatus> implements Namespaced {

Which generates a MyResourceFluentImpl:

public MyResourceFluentImpl(MyResource instance) {
        this.withMetadata(instance.getMetadata());
        
        this.withSpec(instance.getSpec());
        
        this.withStatus(instance.getStatus());
        
        this.withKind(instance.getKind());
        
        this.withApiVersion(instance.getApiVersion());
    }

All of the with methods are implemented in CustomResourceFluentImpl. This change will address the metadata - but I'm not quite seeing how spec and status will work. Even if I add BuildableReference for those on MyResource, the MyResourceFluentImpl is still calling the CustomResourceFluentImpl.

[metacosm]

Relates to #2807.

[shawkins]

Captured upstream as sundrio/sundrio#248

After the pr based upon @rohanKanojia suggested change above is committed, then we can close this issue.

[manusa]

#3232, closing issue as the rest should be addressed upstream (if possible).