Repository: kubernetes-sigs/controller-runtime

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

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!

<img width="1644" height="524" alt="Image" src="https://github.com/user-attachments/assets/c8f0f329-faa9-4900-8efd-fa53c5f7a1fc" />
<img width="1638" height="530" alt="Image" src="https://github.com/user-attachments/assets/ae9f1c22-9356-4568-be88-d9e62c1b021f" />
<img width="1658" height="458" alt="Image" src="https://github.com/user-attachments/assets/cd2a66e5-f0bf-46d8-b00d-a46b753f686a" />

Discussion:
@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](https://github.com/kubernetes-sigs/controller-runtime/blob/f48fe1c564dc8e418f302d3ab0086d3a7403233a/alias.go#L110-L114)) 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](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/client_test.go#L931) 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](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/apiutil/apimachinery.go#L200-L201) unless the request is for [SSA or unstructured.](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/client_rest_resources.go#L115)

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](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/client_rest_resources.go#L120-L125). Consequently, execution terminates prematurely [here](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/client_rest_resources.go#L127), preventing the client from being created with the forceDisableProtoBuf option [in here](https://github.com/kubernetes-sigs/controller-runtime/blob/655fb2ceb9f26640b0281fd5c3b66fcd58967ea6/pkg/client/client_rest_resources.go#L139). 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](https://github.com/kubernetes-sigs/controller-runtime/pull/3346)  and fix released in 0.22.3

Solution patch:
diff --git a/pkg/client/client.go b/pkg/client/client.go
index 7e381422..39050de4 100644
--- a/pkg/client/client.go
+++ b/pkg/client/client.go
@@ -151,8 +151,7 @@ func newClient(config *rest.Config, options Options) (*client, error) {
 		mapper:     options.Mapper,
 		codecs:     serializer.NewCodecFactory(options.Scheme),
 
-		structuredResourceByType:   make(map[schema.GroupVersionKind]*resourceMeta),
-		unstructuredResourceByType: make(map[schema.GroupVersionKind]*resourceMeta),
+		resourceByType: make(map[cacheKey]*resourceMeta),
 	}
 
 	rawMetaClient, err := metadata.NewForConfigAndClient(metadata.ConfigFor(config), options.HTTPClient)
diff --git a/pkg/client/client_rest_resources.go b/pkg/client/client_rest_resources.go
index acff7a46..d75d685c 100644
--- a/pkg/client/client_rest_resources.go
+++ b/pkg/client/client_rest_resources.go
@@ -48,11 +48,15 @@ type clientRestResources struct {
 	// codecs are used to create a REST client for a gvk
 	codecs serializer.CodecFactory
 
-	// structuredResourceByType stores structured type metadata
-	structuredResourceByType map[schema.GroupVersionKind]*resourceMeta
-	// unstructuredResourceByType stores unstructured type metadata
-	unstructuredResourceByType map[schema.GroupVersionKind]*resourceMeta
-	mu                         sync.RWMutex
+	// resourceByType stores type metadata
+	resourceByType map[cacheKey]*resourceMeta
+
+	mu sync.RWMutex
+}
+
+type cacheKey struct {
+	gvk                  schema.GroupVersionKind
+	forceDisableProtoBuf bool
 }
 
 // newResource maps obj to a Kubernetes Resource and constructs a client for that Resource.
@@ -117,11 +121,11 @@ func (c *clientRestResources) getResource(obj any) (*resourceMeta, error) {
 	// It's better to do creation work twice than to not let multiple
 	// people make requests at once
 	c.mu.RLock()
-	resourceByType := c.structuredResourceByType
-	if isUnstructured {
-		resourceByType = c.unstructuredResourceByType
-	}
-	r, known := resourceByType[gvk]
+
+	cacheKey := cacheKey{gvk: gvk, forceDisableProtoBuf: forceDisableProtoBuf}
+
+	r, known := c.resourceByType[cacheKey]
+
 	c.mu.RUnlock()
 
 	if known {
@@ -140,7 +144,7 @@ func (c *clientRestResources) getResource(obj any) (*resourceMeta, error) {
 	if err != nil {
 		return nil, err
 	}
-	resourceByType[gvk] = r
+	c.resourceByType[cacheKey] = r
 	return r, err
 }
 



Test patch:
diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go
index 42e14771..021fbeb0 100644
--- a/pkg/client/client_test.go
+++ b/pkg/client/client_test.go
@@ -953,6 +953,52 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				Expect(cm.Data).To(BeComparableTo(data))
 				Expect(cm.Data).To(BeComparableTo(obj.Data))
 			})
+
+			It("should create a secret without SSA and later create update a secret using SSA", func(ctx SpecContext) {
+				cl, err := client.New(cfg, client.Options{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cl).NotTo(BeNil())
+				data := map[string][]byte{
+					"some-key": []byte("some-value"),
+				}
+				secretObject := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "secret-one",
+						Namespace: "default",
+					},
+					Data: data,
+				}
+
+				secretApplyConfiguration := corev1applyconfigurations.
+					Secret("secret-two", "default").
+					WithData(data)
+
+				err = cl.Create(ctx, secretObject)
+				Expect(err).NotTo(HaveOccurred())
+
+				err = cl.Apply(ctx, secretApplyConfiguration, &client.ApplyOptions{FieldManager: "test-manager"})
+				Expect(err).NotTo(HaveOccurred())
+
+				secret, err := clientset.CoreV1().Secrets(ptr.Deref(secretApplyConfiguration.GetNamespace(), "")).Get(ctx, ptr.Deref(secretApplyConfiguration.GetName(), ""), metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(secret.Data).To(BeComparableTo(data))
+				Expect(secret.Data).To(BeComparableTo(secretApplyConfiguration.Data))
+
+				data = map[string][]byte{
+					"some-key": []byte("some-new-value"),
+				}
+				secretApplyConfiguration.Data = data
+
+				err = cl.Apply(ctx, secretApplyConfiguration, &client.ApplyOptions{FieldManager: "test-manager"})
+				Expect(err).NotTo(HaveOccurred())
+
+				secret, err = clientset.CoreV1().Secrets(ptr.Deref(secretApplyConfiguration.GetNamespace(), "")).Get(ctx, ptr.Deref(secretApplyConfiguration.GetName(), ""), metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(secret.Data).To(BeComparableTo(data))
+				Expect(secret.Data).To(BeComparableTo(secretApplyConfiguration.Data))
+			})
 		})
 	})