Merge pull request #75157 from aaron-prindle/version-check-apply

Added version check between patch and live object in server side apply

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD

@@ -7,14 +7,17 @@ go_library(
     importpath = "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager",
     visibility = ["//visibility:public"],
     deps = [
+        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal:go_default_library",
         "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
         "//vendor/sigs.k8s.io/structured-merge-diff/fieldpath:go_default_library",
         "//vendor/sigs.k8s.io/structured-merge-diff/merge:go_default_library",
+        "//vendor/sigs.k8s.io/yaml:go_default_library",
     ],
 )
 
@@ -41,6 +44,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go

@@ -20,14 +20,17 @@ import (
 	"fmt"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	"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"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
 	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
 	"sigs.k8s.io/structured-merge-diff/fieldpath"
 	"sigs.k8s.io/structured-merge-diff/merge"
+	"sigs.k8s.io/yaml"
 )
 
 // FieldManager updates the managed fields and merge applied
@@ -149,9 +152,20 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager
 	if err != nil {
 		return nil, fmt.Errorf("failed to decode managed fields: %v", err)
 	}
-	// We can assume that patchObj is already on the proper version:
+	// Check that the patch object has the same version as the live object
-	// it shouldn't have to be converted so that it's not defaulted.
+	patchObj := &unstructured.Unstructured{Object: map[string]interface{}{}}
-	// TODO (jennybuckley): Explicitly checkt that patchObj is in the proper version.
+
+	if err := yaml.Unmarshal(patch, &patchObj.Object); err != nil {
+		return nil, fmt.Errorf("error decoding YAML: %v", err)
+	}
+	if patchObj.GetAPIVersion() != f.groupVersion.String() {
+		return nil,
+			errors.NewBadRequest(
+				fmt.Sprintf("Incorrect version specified in apply patch. "+
+					"Specified patch version: %s, expected: %s",
+					patchObj.GetAPIVersion(), f.groupVersion.String()))
+	}
+
 	liveObjVersioned, err := f.toVersioned(liveObj)
 	if err != nil {
 		return nil, fmt.Errorf("failed to convert live object to proper version: %v", err)

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go

@@ -18,9 +18,11 @@ package fieldmanager_test
 
 import (
 	"errors"
+	"net/http"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -72,7 +74,7 @@ func TestApplyStripsFields(t *testing.T) {
 	obj := &corev1.Pod{}
 
 	newObj, err := f.Apply(obj, []byte(`{
-		"apiVersion": "v1",
+		"apiVersion": "apps/v1",
 		"kind": "Deployment",
 		"metadata": {
 			"name": "b",
@@ -85,7 +87,7 @@ func TestApplyStripsFields(t *testing.T) {
 			"managedFields": [{
 					"manager": "apply",
 					"operation": "Apply",
-					"apiVersion": "v1",
+					"apiVersion": "apps/v1",
 					"fields": {
 						"f:metadata": {
 							"f:labels": {
@@ -111,13 +113,46 @@ func TestApplyStripsFields(t *testing.T) {
 	}
 }
 
+func TestVersionCheck(t *testing.T) {
+	f := NewTestFieldManager(t)
+
+	obj := &corev1.Pod{}
+
+	// patch has 'apiVersion: apps/v1' and live version is apps/v1 -> no errors
+	_, err := f.Apply(obj, []byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "Deployment",
+	}`), "fieldmanager_test", false)
+	if err != nil {
+		t.Fatalf("failed to apply object: %v", err)
+	}
+
+	// patch has 'apiVersion: apps/v2' but live version is apps/v1 -> error
+	_, err = f.Apply(obj, []byte(`{
+		"apiVersion": "apps/v2",
+		"kind": "Deployment",
+	}`), "fieldmanager_test", false)
+	if err == nil {
+		t.Fatalf("expected an error from mismatched patch and live versions")
+	}
+	switch typ := err.(type) {
+	default:
+		t.Fatalf("expected error to be of type %T was %T", apierrors.StatusError{}, typ)
+	case apierrors.APIStatus:
+		if typ.Status().Code != http.StatusBadRequest {
+			t.Fatalf("expected status code to be %d but was %d",
+				http.StatusBadRequest, typ.Status().Code)
+		}
+	}
+}
+
 func TestApplyDoesNotStripLabels(t *testing.T) {
 	f := NewTestFieldManager(t)
 
 	obj := &corev1.Pod{}
 
 	newObj, err := f.Apply(obj, []byte(`{
-		"apiVersion": "v1",
+		"apiVersion": "apps/v1",
 		"kind": "Pod",
 		"metadata": {
 			"labels": {