fix(render): reject non-Secret documents in function credentials

pkg/render/load.go

@@ -18,12 +18,15 @@ package render
 
 import (
 	"bufio"
+	"fmt"
 	"io"
 	"path/filepath"
 
 	"github.com/spf13/afero"
 	corev1 "k8s.io/api/core/v1"
+	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/util/yaml"
 
 	"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
@@ -123,15 +126,39 @@ func LoadCredentials(fs afero.Fs, file string) ([]corev1.Secret, error) {
 	for _, y := range stream {
 		s := &corev1.Secret{}
 		if err := yaml.Unmarshal(y, s); err != nil {
+			// Decoding a non-Secret document (e.g. a ConfigMap with
+			// plain-text data) into a Secret can fail with a confusing
+			// base64 error before we ever see its kind. Only on failure,
+			// decode just the metadata to report the wrong kind clearly.
+			m := &metav1.PartialObjectMetadata{}
+			if merr := yaml.Unmarshal(y, m); merr == nil && m.GroupVersionKind() != corev1.SchemeGroupVersion.WithKind("Secret") {
+				return nil, errNotSecret(m.GroupVersionKind(), m.GetName())
+			}
 			return nil, errors.Wrap(err, "cannot parse YAML secret manifest")
 		}
 
+		// A non-Secret document without Secret-incompatible fields decodes
+		// cleanly, so check the kind too rather than silently treating it
+		// as an empty Secret.
+		if gvk := s.GroupVersionKind(); gvk != corev1.SchemeGroupVersion.WithKind("Secret") {
+			return nil, errNotSecret(gvk, s.GetName())
+		}
+
 		secrets = append(secrets, *s)
 	}
 
 	return secrets, nil
 }
 
+// errNotSecret reports a credentials document whose type is not a v1 Secret.
+func errNotSecret(gvk schema.GroupVersionKind, name string) error {
+	doc := "unnamed credentials document"
+	if name != "" {
+		doc = fmt.Sprintf("credentials document %q", name)
+	}
+	return errors.Errorf("%s has apiVersion %q and kind %q; set apiVersion: v1 and kind: Secret", doc, gvk.GroupVersion().String(), gvk.Kind)
+}
+
 // LoadRequiredResources from a stream of YAML manifests.
 func LoadRequiredResources(fs afero.Fs, file string) ([]unstructured.Unstructured, error) {
 	stream, err := LoadYAMLStream(fs, file)

pkg/render/load_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/afero"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -329,6 +330,98 @@ func TestLoadFunctions(t *testing.T) {
 	}
 }
 
+func TestLoadCredentials(t *testing.T) {
+	fs := afero.FromIOFS{FS: testdatafs}
+
+	type want struct {
+		secrets []corev1.Secret
+		err     string
+	}
+
+	cases := map[string]struct {
+		file string
+		want want
+	}{
+		"Success": {
+			file: "testdata/credentials.yaml",
+			want: want{
+				secrets: []corev1.Secret{
+					{
+						TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Secret"},
+						ObjectMeta: metav1.ObjectMeta{Name: "creds-a"},
+						Data:       map[string][]byte{"token": []byte("secret")},
+					},
+					{
+						TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Secret"},
+						ObjectMeta: metav1.ObjectMeta{Name: "creds-b"},
+						StringData: map[string]string{"password": "hunter2"},
+					},
+				},
+			},
+		},
+		"ConfigMapWithPlainTextData": {
+			file: "testdata/credentials-configmap.yaml",
+			want: want{
+				err: `credentials document "not-a-secret" has apiVersion "v1" and kind "ConfigMap"; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"DeploymentWithNoData": {
+			file: "testdata/credentials-deployment.yaml",
+			want: want{
+				err: `credentials document "not-a-secret" has apiVersion "apps/v1" and kind "Deployment"; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"MissingKind": {
+			file: "testdata/credentials-missing-kind.yaml",
+			want: want{
+				err: `credentials document "creds" has apiVersion "v1" and kind ""; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"MissingAPIVersion": {
+			file: "testdata/credentials-missing-apiversion.yaml",
+			want: want{
+				err: `credentials document "creds" has apiVersion "" and kind "Secret"; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"WrongAPIVersion": {
+			file: "testdata/credentials-wrong-apiversion.yaml",
+			want: want{
+				err: `credentials document "creds" has apiVersion "v2" and kind "Secret"; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"Unnamed": {
+			file: "testdata/credentials-unnamed.yaml",
+			want: want{
+				err: `unnamed credentials document has apiVersion "v1" and kind "ConfigMap"; set apiVersion: v1 and kind: Secret`,
+			},
+		},
+		"SecretWithInvalidBase64Data": {
+			file: "testdata/credentials-invalid-base64.yaml",
+			want: want{
+				err: "cannot parse YAML secret manifest: error unmarshaling JSON: while decoding JSON: illegal base64 data at input byte 3",
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			secrets, err := LoadCredentials(fs, tc.file)
+
+			if diff := cmp.Diff(tc.want.secrets, secrets); diff != "" {
+				t.Errorf("LoadCredentials(..), -want, +got:\n%s", diff)
+			}
+
+			gotErr := ""
+			if err != nil {
+				gotErr = err.Error()
+			}
+			if diff := cmp.Diff(tc.want.err, gotErr); diff != "" {
+				t.Errorf("LoadCredentials(..), -want error, +got error:\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestLoadObservedResources(t *testing.T) {
 	fs := afero.FromIOFS{FS: testdatafs}
 

pkg/render/testdata/credentials-configmap.yaml

@@ -0,0 +1,14 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: creds-a
+data:
+  token: c2VjcmV0
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: not-a-secret
+data:
+  setting: plain text, not base64!

pkg/render/testdata/credentials-deployment.yaml

@@ -0,0 +1,5 @@
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: not-a-secret

pkg/render/testdata/credentials-invalid-base64.yaml

@@ -0,0 +1,7 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: bad-data
+data:
+  token: not-base64!

pkg/render/testdata/credentials-missing-apiversion.yaml

@@ -0,0 +1,3 @@
+kind: Secret
+metadata:
+  name: creds

pkg/render/testdata/credentials-missing-kind.yaml

@@ -0,0 +1,3 @@
+apiVersion: v1
+metadata:
+  name: creds

pkg/render/testdata/credentials-unnamed.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

pkg/render/testdata/credentials-wrong-apiversion.yaml

@@ -0,0 +1,4 @@
+apiVersion: v2
+kind: Secret
+metadata:
+  name: creds

pkg/render/testdata/credentials.yaml

@@ -0,0 +1,14 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: creds-a
+data:
+  token: c2VjcmV0
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: creds-b
+stringData:
+  password: hunter2