Deserialization Error During Server-Side Apply (SSA) with Protobuf Content Type

[filipcirtog]

Hello team!

Problem Statement

We're migrating from client.Patch(Apply) to client.Apply(). While making this transition, we've encountered the following issue.

Observed Behavior

When using client.Apply together with a configuration where config.ContentType = "application/vnd.kubernetes.protobuf", an error occurs during the deserialization process.

The protobuf decoder fails to parse the configuration object and throws the following error: object %v does not implement the protobuf marshalling interface and cannot be encoded to a *client.applyconfigurationRuntimeObject protobuf message

Steps to Reproduce

To reproduce the issue, you can add cfg.ContentType = "application/vnd.kubernetes.protobuf" in the client_suite_test.go file and execute the tests. This triggers the error during the deserialization stage.

Please let us know if we are doing anything wrong or this is indeed a bug. Thank you!

[alvaroaleman]

is protobuf supported for SSA? To my knowledge, only yaml and cbor are: https://github.com/kubernetes/kubernetes/blob/ddc2c5d192275207b7ebb43b066fd0b0e94fc011/staging/src/k8s.io/client-go/util/apply/apply.go#L35

Why are you setting the content-type explicitly in the config?

[filipcirtog]

I explicitly set the Content-Type in the test to ensure the error can be consistently reproduced. While we don't explicitly set the Content-Type in our operator's code, we've observed situations in the controller code where the API server sometimes returns protobuf responses, especially when apply is creating a new resource.

[s-urbaniak]

Thank you @alvaroaleman! Indeed we oversaw the fact that SSA doesn't support protobuf, and you're right, NewRequest enforces the YAML content-type so we really need to find out why we in our case a application/vnd.kubernetes.protobuf content-type is being constructed for the Apply, this is indeed strange.

@filipcirtog let's continue debugging, @alvaroaleman's hint was already a great help 👍

I suggest we leave this open here as we use a standard controller-runtime Client taken from the underlying Manager#GetClient instance so we need to figure out if it is a config error or something else.

[filipcirtog]

Thank you @alvaroaleman! In our specific operator setup, we noticed situations where the API server defaulted to Protobuf responses (AFAIK because of this) during certain resource creation scenarios, which caused Apply operations to fail. Explicitly setting the Content-Type to JSON resolved this issue.

[alvaroaleman]

Explicitly setting the Content-Type to JSON resolved this issue

That doesn't seem correct either, because that means json is always used which is very inefficient. We have tests with in-tree resources for SSA so I don't quite understand what is different that made things fail for you. We have code that will if the content type is unset at runtime decide what to use and use protobuf for in-tree resources unless the request is for SSA or unstructured.

in other words: This should just work correctly by default without requiring any content type setting. Did you previously explicitly set the content type to protobuf?

[filipcirtog]

Thank you @alvaroaleman for pointing the methods out. We are not explicitly setting the content type to protobuf.

I believe that because of the first creation method, the GroupVersionKind (GVK) is identified here. Consequently, execution terminates prematurely here, preventing the client from being created with the forceDisableProtoBuf option in here. I believe this allows the client to generate protobuf requests, as there is no longer any restriction preventing it.

I was also able to recreate the error in your tests using the following code.

			secretObject := &corev1.Secret{
					TypeMeta: metav1.TypeMeta{
						Kind:       "Secret",
						APIVersion: "v1",
					},
					ObjectMeta: metav1.ObjectMeta{
						Name:      "secret-one",
						Namespace: "default",
					},
				}

				secretApplyConfiguration := corev1applyconfigurations.
					Secret("secret-two", "default").
					WithAPIVersion("v1").
					WithKind("Secret")

				err = cl.Create(ctx, secretObject)
				Expect(err).NotTo(HaveOccurred())

				err = cl.Apply(ctx, secretApplyConfiguration, &client.ApplyOptions{FieldManager: "test-manager"})
				Expect(err).NotTo(HaveOccurred())

[filipcirtog]

@alvaroaleman To follow up on this, I believe this could be the solution. If you agree I could open a PR and solve this issue and also add a test for that. Please let me know your thoughts!
if known && !forceDisableProtoBuf { return r, nil }

[alvaroaleman]

Fixed in 🐛 Allow SSA after normal resource creation #3346 and fix released in 0.22.3