ObjectMeta breaking change in 6.2.0

[gyfora]

Describe the bug

In 711482c there was a breaking change to the ObjectMeta object.

The clusterName field was removed.

This also makes CRD-s generated using the generator that contain any ObjectMetadata field to be non backward compatible.

Fabric8 Kubernetes Client version

6.2.0

Steps to reproduce

Check code / generated CRD

Expected behavior

backward compatibility for objectmeta

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.25.3@latest

Environment

macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

[manusa]

This also makes CRD-s generated using the generator that contain any ObjectMetadata field to be non backward compatible.

I'm not sure I'm following this point here. How do your generated CRDs end up having a clusterName entry with a value?

[gyfora]

We have a spec object that contains a Pod field to define a podTemplate. You can find it here:
https://github.com/apache/flink-kubernetes-operator/blob/main/flink-kubernetes-operator-api/src/main/java/org/apache/flink/kubernetes/operator/api/spec/FlinkDeploymentSpec.java#L60

Now when we generate the CRD from the spec class the resulting CRD is incompatible after 6.2.0

[manusa]

Could you please share the differences between the CRD generated before 6.2.0 and the one generated now. I still don't understand how your CRD ends up with a:

# parent CRD fields...
metadata: 
  clusterName: whatever

[gyfora]

Well the point is that it ends up without the clusterName. You generate the CRD with 6.1.0, you have clusterName there, with 6.2.0 you dont have it.

Removing a field is a breaking change.

[gyfora]

[gyfora]

this is the diff compared to the CRD that was generated with fabric8 before 6.2.0

[shawkins]

@gyfora cluster name was deprecated in kubernetes 1.24, and removed in 1.25. Prior to 1.24 the docs seemed to imply it was unused - https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#objectmeta-v1-meta

Can you describe how you were using this field?

[gyfora]

I am personally not using the field but we are publishing the generated CRD.
Now if we generate it with the new fabric8 version, that field is removed, so if we update the CRD based on this, it leads to a breaking change. (you cannot simply remove a field)

This means that every CRD generated with fabric8 that contains a field that has metadata results in a breaking change after 6.2.0.

Users might have CR-s that define the clusterName field...

[gyfora]

@gyfora cluster name was deprecated in kubernetes 1.24, and removed in 1.25. Prior to 1.24 the docs seemed to imply it was unused - https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#objectmeta-v1-meta

Can you describe how you were using this field?

It seems like kubernetes itself broke the objectmetadata api? They kept version v1 but removed a field?

[manusa]

this is the diff compared to the CRD that was generated with fabric8 before 6.2.0

OK, I understand now 😟

Can you describe how you were using this field?

They aren't using this field, but the field is described in their custom resource definitions.

Users might have CR-s that define the clusterName field...

They shouldn't do that.

I'm not really sure how we should fix this one. My best guess is that the CR deserializer should be tweaked to ignore this field.
----> ObjectMeta should ignore unknown properties, or at least clusterName

[manusa]

It seems like kubernetes itself broke the objectmetadata api? They kept version v1 but removed a field?

That field has been marked as unused for a while

Relates to kubernetes/kubernetes#65885

[gyfora]

The problem here is even though that meta field for the Pod object doesn't do anything, it is still possible to have it in the CR.

We cannot simply drop it from the CRD because it can theoretically break the upgrades and cause some serious issues. I think best would be to re-add it in fabric8 and maybe disregard it during serialization.

[gyfora]

I understand that this is a weird situation but it's already pretty strange that they would just remove a field from a v1 object...

[manusa]

The main issue is that ObjectMeta is a generated class.

The field (as advertised by K8s) is completely ignored by the Kube API server.
My assumption is that your code breaks whenever a user deserializes a CR using our client (probably from Flink?). I still think the best option is to simply ignore this field (just like Kubernetes does) from the client side by tweaking the deserializer. Although any other thoughts (considering the generated fashion of the class) are welcome :)

I understand that this is a weird situation but it's already pretty strange that they would just remove a field from a v1 object...

🤣 if you were to keep track of the "stable" version diffs across releases you'd be surprised :)

[shawkins]

I'm not really sure how we should fix this one. My best guess is that the CR deserializer should be tweaked to ignore this field.

It already does - since it's ultimately using io.fabric8.kubernetes.api.model.ObjectMeta correct?

I'm not sure how this change could be breaking - or at least it shouldn't be under normal circumstances. If you are using explicitly setting the clusterName field for create / update - which the docs tell you will be ignored - then with this updated api that direction option will be removed.

You could set the clusterName via additionalProperties, again documented as ignored / dropped by the api server. Only if you have enabled some kind of field validation, which we won't generally support until #4136 could you see a problem.

[gyfora]

Thanks I will test this locally :)

[gyfora]

Thank you! It is indeed not a breaking change in practice it seems.