Merge pull request #63385 from CaoShuFeng/customresource_status

Automatic merge from submit-queue (batch tested with PRs 63537, 63385). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

[CustomResourceSubresources] fix status subresource

This change make the codes consistent with the document.
Fixes: https://github.com/kubernetes/kubernetes/issues/63359



**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:
/assign @nikhita 
**Release note**:

```release-note
When updating /status subresource of a custom resource, only the value at the `.status` subpath for the update is considered.
```

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/BUILD

@@ -87,3 +87,10 @@ go_test(
         "//vendor/k8s.io/client-go/discovery:go_default_library",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["status_strategy_test.go"],
+    embed = [":go_default_library"],
+    deps = ["//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library"],
+)

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.go

@@ -33,28 +33,24 @@ func NewStatusStrategy(strategy customResourceStrategy) statusStrategy {
 }
 
 func (a statusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+	// update is only allowed to set status
 	newCustomResourceObject := obj.(*unstructured.Unstructured)
-	oldCustomResourceObject := old.(*unstructured.Unstructured)
-
 	newCustomResource := newCustomResourceObject.UnstructuredContent()
-	oldCustomResource := oldCustomResourceObject.UnstructuredContent()
+	status, ok := newCustomResource["status"]
 
-	// update is not allowed to set spec and metadata
-	_, ok1 := newCustomResource["spec"]
-	_, ok2 := oldCustomResource["spec"]
-	switch {
-	case ok2:
-		newCustomResource["spec"] = oldCustomResource["spec"]
-	case ok1:
-		delete(newCustomResource, "spec")
+	// copy old object into new object
+	oldCustomResourceObject := old.(*unstructured.Unstructured)
+	// overridding the resourceVersion in metadata is safe here, we have already checked that
+	// new object and old object have the same resourceVersion.
+	*newCustomResourceObject = *oldCustomResourceObject.DeepCopy()
+
+	// set status
+	newCustomResource = newCustomResourceObject.UnstructuredContent()
+	if ok {
+		newCustomResource["status"] = status
+	} else {
+		delete(newCustomResource, "status")
 	}
-
-	newCustomResourceObject.SetAnnotations(oldCustomResourceObject.GetAnnotations())
-	newCustomResourceObject.SetFinalizers(oldCustomResourceObject.GetFinalizers())
-	newCustomResourceObject.SetGeneration(oldCustomResourceObject.GetGeneration())
-	newCustomResourceObject.SetLabels(oldCustomResourceObject.GetLabels())
-	newCustomResourceObject.SetOwnerReferences(oldCustomResourceObject.GetOwnerReferences())
-	newCustomResourceObject.SetSelfLink(oldCustomResourceObject.GetSelfLink())
 }
 
 // ValidateUpdate is the default update validation for an end user updating status.

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy_test.go

@@ -0,0 +1,138 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package customresource
+
+import (
+	"context"
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func TestPrepareForUpdate(t *testing.T) {
+	strategy := statusStrategy{}
+	tcs := []struct {
+		old      *unstructured.Unstructured
+		obj      *unstructured.Unstructured
+		expected *unstructured.Unstructured
+	}{
+		{
+			// changes to spec are ignored
+			old: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "new",
+				},
+			},
+			expected: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+		},
+		{
+			// changes to other places are also ignored
+			old: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"new": "new",
+				},
+			},
+			expected: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+		},
+		{
+			// delete status
+			old: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "old",
+					"status": "old",
+				},
+			},
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+			expected: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": "old",
+				},
+			},
+		},
+		{
+			// update status
+			old: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "old",
+					"status": "old",
+				},
+			},
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"status": "new",
+				},
+			},
+			expected: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "old",
+					"status": "new",
+				},
+			},
+		},
+		{
+			// update status and other parts
+			old: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "old",
+					"status": "old",
+				},
+			},
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "new",
+					"new":    "new",
+					"status": "new",
+				},
+			},
+			expected: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec":   "old",
+					"status": "new",
+				},
+			},
+		},
+	}
+	for index, tc := range tcs {
+		strategy.PrepareForUpdate(context.TODO(), tc.obj, tc.old)
+		if !reflect.DeepEqual(tc.obj, tc.expected) {
+			t.Errorf("test %d failed: expected: %v, got %v", index, tc.expected, tc.obj)
+		}
+	}
+}