Merge pull request #134216 from Goend/master

fixed the CRD statusResetFields init issue

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -1039,30 +1039,6 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 
 		scaleScopes[v.Name] = &scaleScope
 
-		// override status subresource values
-		// shallow copy
-		statusScope := *requestScopes[v.Name]
-		statusScope.Subresource = "status"
-		statusScope.Namer = handlers.ContextBasedNaming{
-			Namer:         meta.NewAccessor(),
-			ClusterScoped: clusterScoped,
-		}
-
-		if subresources != nil && subresources.Status != nil {
-			resetFields := storages[v.Name].Status.GetResetFields()
-			statusScope, err = scopeWithFieldManager(
-				typeConverter,
-				statusScope,
-				resetFields,
-				"status",
-			)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		statusScopes[v.Name] = &statusScope
-
 		if v.Deprecated {
 			deprecated[v.Name] = true
 			if v.DeprecationWarning != nil {
@@ -1073,6 +1049,12 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 		}
 	}
 
+	statusResetFields := getStatusResetFields(crd, storages)
+	statusScopes, err = setStatusScope(crd, requestScopes, typeConverter, statusResetFields, statusScopes)
+	if err != nil {
+		return nil, err
+	}
+
 	ret := &crdInfo{
 		spec:                &crd.Spec,
 		acceptedNames:       &crd.Status.AcceptedNames,
@@ -1096,6 +1078,57 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 	return ret, nil
 }
 
+// getStatusResetFields returns the reset fields which include all versions for the status subresource of the CRD.
+func getStatusResetFields(crd *apiextensionsv1.CustomResourceDefinition, storages map[string]customresource.CustomResourceStorage) map[fieldpath.APIVersion]*fieldpath.Set {
+	var statusResetFields = make(map[fieldpath.APIVersion]*fieldpath.Set)
+	for _, value := range crd.Spec.Versions {
+		if storages[value.Name].Status == nil {
+			continue
+		}
+		resetField := storages[value.Name].Status.GetResetFields()
+		for apiVersion, set := range resetField {
+			statusResetFields[apiVersion] = set
+		}
+	}
+	return statusResetFields
+}
+
+// setStatusScope sets the status scope for each version of the CRD.
+func setStatusScope(crd *apiextensionsv1.CustomResourceDefinition, requestScopes map[string]*handlers.RequestScope, typeConverter managedfields.TypeConverter, resetFields map[fieldpath.APIVersion]*fieldpath.Set, statusScopes map[string]*handlers.RequestScope) (map[string]*handlers.RequestScope, error) {
+	for _, v := range crd.Spec.Versions {
+		clusterScoped := crd.Spec.Scope == apiextensionsv1.ClusterScoped
+		if requestScopes[v.Name] == nil {
+			continue
+		}
+		statusScope := *requestScopes[v.Name]
+		statusScope.Subresource = "status"
+		statusScope.Namer = handlers.ContextBasedNaming{
+			Namer:         meta.NewAccessor(),
+			ClusterScoped: clusterScoped,
+		}
+
+		subresources, err := apiextensionshelpers.GetSubresourcesForVersion(crd, v.Name)
+		if err != nil {
+			utilruntime.HandleError(err)
+			return nil, fmt.Errorf("the server could not properly serve the CR subresources")
+		}
+		if subresources != nil && subresources.Status != nil {
+			statusScope, err = scopeWithFieldManager(
+				typeConverter,
+				statusScope,
+				resetFields,
+				"status",
+			)
+			if err != nil {
+				return nil, err
+			}
+		}
+		statusScopes[v.Name] = &statusScope
+	}
+	return statusScopes, nil
+
+}
+
 func scopeWithFieldManager(typeConverter managedfields.TypeConverter, reqScope handlers.RequestScope, resetFields map[fieldpath.APIVersion]*fieldpath.Set, subresource string) (handlers.RequestScope, error) {
 	fieldManager, err := managedfields.NewDefaultCRDFieldManager(
 		typeConverter,

test/integration/apiserver/apply/apply_crd_beta_test.go

@@ -227,3 +227,82 @@ func nearlyRemovedBetaMultipleVersionNoxuCRD(scope apiextensionsv1beta1.Resource
 		},
 	}
 }
+
+func nearlyRemovedBetaMultipleVersionNoxuCRDWithStatus(scope apiextensionsv1beta1.ResourceScope) *apiextensionsv1beta1.CustomResourceDefinition {
+	return &apiextensionsv1beta1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "myresources.example.com"},
+		Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
+			Group:   "example.com",
+			Version: "v1beta1",
+			Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
+				Plural:     "myresources",
+				Singular:   "myresource",
+				Kind:       "MyResource",
+				ShortNames: []string{"mr"},
+				ListKind:   "MyResourceList",
+			},
+			Scope: scope,
+			Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{
+				{
+					Name:    "v1beta1",
+					Served:  true,
+					Storage: false,
+					Schema: &apiextensionsv1beta1.CustomResourceValidation{
+						OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+								"spec": {
+									Type: "object",
+									Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+										"a": {Type: "string"},
+										"b": {Type: "string"},
+									},
+								},
+								"status": {
+									Type: "object",
+									Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+										"a": {Type: "string"},
+									},
+								},
+							},
+						},
+					},
+					Subresources: &apiextensionsv1beta1.CustomResourceSubresources{
+						Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
+					},
+				},
+				{
+					Name:    "v1",
+					Served:  true,
+					Storage: true,
+					Schema: &apiextensionsv1beta1.CustomResourceValidation{
+						OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+								"spec": {
+									Type: "object",
+									Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+										"a": {Type: "string"},
+										"b": {Type: "string"},
+									},
+								},
+								"status": {
+									Type: "object",
+									Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
+										"a": {Type: "string"},
+									},
+								},
+							},
+						},
+					},
+					Subresources: &apiextensionsv1beta1.CustomResourceSubresources{
+						Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
+					},
+				},
+			},
+			Subresources: &apiextensionsv1beta1.CustomResourceSubresources{
+				Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
+			},
+		},
+	}
+}

test/integration/apiserver/apply/reset_fields_test.go

@@ -25,11 +25,14 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
+	"k8s.io/apiextensions-apiserver/test/integration/fixtures"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
 	apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
@@ -321,6 +324,116 @@ func TestApplyResetFields(t *testing.T) {
 	}
 }
 
+// TestUpdateStatusWithOldVersion tests that apply with resetFields works correctly when updating
+// a custom resource's status subresource using an older API version while maintaining field ownership.
+func TestUpdateStatusWithOldVersion(t *testing.T) {
+	server, err := apiservertesting.StartTestServer(t, apiservertesting.NewDefaultTestServerOptions(), []string{"--disable-admission-plugins", "ServiceAccount,TaintNodesByCondition"}, framework.SharedEtcd())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer server.TearDownFn()
+
+	client, err := kubernetes.NewForConfig(server.ClientConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	apiExtensionClient, err := apiextensionsclientset.NewForConfig(server.ClientConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+	dynamicClient, err := dynamic.NewForConfig(server.ClientConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	noxuBetaDefinition := nearlyRemovedBetaMultipleVersionNoxuCRDWithStatus(apiextensionsv1beta1.NamespaceScoped)
+
+	noxuDefinition, err := fixtures.CreateCRDUsingRemovedAPI(server.EtcdClient, server.EtcdStoragePrefix, noxuBetaDefinition, apiExtensionClient, dynamicClient)
+	if err != nil {
+		t.Fatal(err)
+	}
+	kind := noxuDefinition.Spec.Names.Kind
+	apiVersion := noxuDefinition.Spec.Group + "/" + noxuDefinition.Spec.Versions[1].Name
+	name := "mytest"
+
+	rest := apiExtensionClient.Discovery().RESTClient()
+	// create namespace ns test
+	if _, err := client.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: resetFieldsNamespace}}, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create the resource using the v1 CRD API.
+	yamlBody := []byte(fmt.Sprintf(`
+apiVersion: %s
+kind: %s
+metadata:
+ name: %s
+ namespace: %s
+spec:
+ a: value-for-a
+ b: value-for-b`, apiVersion, kind, name, resetFieldsNamespace))
+	result, err := rest.Patch(types.ApplyPatchType).
+		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Versions[1].Name, "/namespaces", resetFieldsNamespace, noxuDefinition.Spec.Names.Plural).
+		Name(name).
+		Param("fieldManager", "apply_test").
+		Body(yamlBody).
+		DoRaw(context.TODO())
+	if err != nil {
+		t.Fatalf("failed to create custom resource with apply: %v:\n%v", err, string(result))
+	}
+	t.Logf("result: %s", string(result))
+	oldManagedFields, err := getManagedFields(result)
+	if err != nil {
+		t.Fatalf("failed to get managed fields: %v", err)
+	}
+	// When updating the status subresource via the v1beta1 CRD API,
+	// we assign a value to the spec field for testing purposes.
+	// However, in this case, the operation should NOT trigger any field manager updates
+	// related to server-side apply tracking.
+	updateStatusBytes := []byte(`{
+  "spec": { "a": "value-for-a-update" },
+  "status": {
+    "a": "status-for-a"
+  }
+}`)
+	result, err = rest.Patch(types.MergePatchType).
+		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Versions[0].Name, "/namespaces", resetFieldsNamespace, noxuDefinition.Spec.Names.Plural).
+		Name(name).
+		SubResource("status").
+		Param("fieldManager", "subresource_test").
+		Body(updateStatusBytes).
+		DoRaw(context.TODO())
+	if err != nil {
+		t.Fatalf("Error updating subresource: %v ", err)
+	}
+	t.Logf("result: %s", string(result))
+	newManagedFields, err := getManagedFields(result)
+	if err != nil {
+		t.Fatalf("failed to get managed fields: %v", err)
+	}
+	// newManagedFields should include oldManagedFields
+	var applyManagerFound, subresourceManagerFound bool
+	for i, field := range newManagedFields {
+		if field.Manager == "apply_test" {
+			if !reflect.DeepEqual(newManagedFields[i], oldManagedFields[0]) {
+				t.Fatalf("Expected managed fields to not have changed when trying manually setting them via subresoures.\n\nExpected: %#v\n\nGot: %#v", oldManagedFields[0], newManagedFields[i])
+			}
+			applyManagerFound = true
+		}
+		if field.Manager == "subresource_test" {
+			subresourceManagerFound = true
+		}
+	}
+	if !applyManagerFound {
+		t.Errorf("expected field manager 'apply_test' to be present in newManagedFields")
+	}
+	if !subresourceManagerFound {
+		t.Errorf("expected field manager 'subresource_test' to be present in newManagedFields")
+	}
+
+}
+
 func expectConflict(objRet *unstructured.Unstructured, err error, dynamicClient dynamic.Interface, resource schema.GroupVersionResource, namespace, name string) error {
 	if err != nil && strings.Contains(err.Error(), "conflict") {
 		return nil