Merge pull request #83552 from liggitt/unstructured-metadata-decoder

Verify metadata schema when decoding unstructured objects in resource builder

staging/src/k8s.io/cli-runtime/pkg/resource/BUILD

@@ -10,6 +10,7 @@ go_library(
         "helper.go",
         "interfaces.go",
         "mapper.go",
+        "metadata_decoder.go",
         "result.go",
         "scheme.go",
         "selector.go",
@@ -30,6 +31,7 @@ go_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/apimachinery/pkg/runtime/serializer:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",

staging/src/k8s.io/cli-runtime/pkg/resource/builder.go

@@ -265,7 +265,7 @@ func (b *Builder) Unstructured() *Builder {
 		localFn:      b.isLocal,
 		restMapperFn: b.restMapperFn,
 		clientFn:     b.getClient,
-		decoder:      unstructured.UnstructuredJSONScheme,
+		decoder:      &metadataValidatingDecoder{unstructured.UnstructuredJSONScheme},
 	}
 
 	return b

staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go

@@ -225,6 +225,35 @@ var aPod string = `
     }
 }
 `
+var aPodBadAnnotations string = `
+{
+    "kind": "Pod",
+    "apiVersion": "` + corev1GV.String() + `",
+    "metadata": {
+        "name": "busybox{id}",
+        "labels": {
+            "name": "busybox{id}"
+        },
+        "annotations": {
+            "name": 0
+        }
+    },
+    "spec": {
+        "containers": [
+            {
+                "name": "busybox",
+                "image": "busybox",
+                "command": [
+                    "sleep",
+                    "3600"
+                ],
+                "imagePullPolicy": "IfNotPresent"
+            }
+        ],
+        "restartPolicy": "Always"
+    }
+}
+`
 
 var aRC string = `
 {
@@ -280,6 +309,22 @@ func newDefaultBuilderWith(fakeClientFn FakeClientFunc) *Builder {
 		WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...)
 }
 
+func newUnstructuredDefaultBuilder() *Builder {
+	return newUnstructuredDefaultBuilderWith(fakeClient())
+}
+
+func newUnstructuredDefaultBuilderWith(fakeClientFn FakeClientFunc) *Builder {
+	return NewFakeBuilder(
+		fakeClientFn,
+		func() (meta.RESTMapper, error) {
+			return testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme), nil
+		},
+		func() (restmapper.CategoryExpander, error) {
+			return FakeCategoryExpander, nil
+		}).
+		Unstructured()
+}
+
 type errorRestMapper struct {
 	meta.RESTMapper
 	err error
@@ -1679,3 +1724,61 @@ func TestHasNames(t *testing.T) {
 		})
 	}
 }
+
+func TestUnstructured(t *testing.T) {
+	// create test dirs
+	tmpDir, err := utiltesting.MkTmpdir("unstructured_test")
+	if err != nil {
+		t.Fatalf("error creating temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// create test files
+	writeTestFile(t, fmt.Sprintf("%s/pod.json", tmpDir), aPod)
+	writeTestFile(t, fmt.Sprintf("%s/badpod.json", tmpDir), aPodBadAnnotations)
+
+	tests := []struct {
+		name          string
+		file          string
+		expectedError string
+	}{
+		{
+			name:          "pod",
+			file:          "pod.json",
+			expectedError: "",
+		},
+		{
+			name:          "badpod",
+			file:          "badpod.json",
+			expectedError: "v1.ObjectMeta.Annotations",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := newUnstructuredDefaultBuilder().
+				ContinueOnError().
+				FilenameParam(false, &FilenameOptions{Recursive: false, Filenames: []string{fmt.Sprintf("%s/%s", tmpDir, tc.file)}}).
+				Flatten().
+				Do()
+
+			err := result.Err()
+			if err == nil {
+				_, err = result.Infos()
+			}
+
+			if len(tc.expectedError) == 0 {
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("expected error, got none")
+				} else if !strings.Contains(err.Error(), tc.expectedError) {
+					t.Errorf("expected error with '%s', got: %v", tc.expectedError, err)
+				}
+			}
+
+		})
+	}
+}

staging/src/k8s.io/cli-runtime/pkg/resource/metadata_decoder.go

@@ -0,0 +1,59 @@
+/*
+Copyright 2019 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 resource
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/runtime/serializer/json"
+)
+
+// hold a single instance of the case-sensitive decoder
+var caseSensitiveJsonIterator = json.CaseSensitiveJsonIterator()
+
+// metadataValidatingDecoder wraps a decoder and additionally ensures metadata schema fields decode before returning an unstructured object
+type metadataValidatingDecoder struct {
+	decoder runtime.Decoder
+}
+
+func (m *metadataValidatingDecoder) Decode(data []byte, defaults *schema.GroupVersionKind, into runtime.Object) (runtime.Object, *schema.GroupVersionKind, error) {
+	obj, gvk, err := m.decoder.Decode(data, defaults, into)
+
+	// if we already errored, return
+	if err != nil {
+		return obj, gvk, err
+	}
+
+	// if we're not unstructured, return
+	if _, isUnstructured := obj.(runtime.Unstructured); !isUnstructured {
+		return obj, gvk, err
+	}
+
+	// make sure the data can decode into ObjectMeta before we return,
+	// so we don't silently truncate schema errors in metadata later with accesser get/set calls
+	v := &metadataOnlyObject{}
+	if typedErr := caseSensitiveJsonIterator.Unmarshal(data, v); typedErr != nil {
+		return obj, gvk, typedErr
+	}
+	return obj, gvk, err
+}
+
+type metadataOnlyObject struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+}