From cc0b661498673954914cfc846ee0039ad2504987 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Tue, 27 Oct 2026 11:11:43 -0400 Subject: [PATCH] fix(render): reject non-Secret documents in function credentials LoadCredentials decoded every YAML document straight into a corev1.Secret, so a ConfigMap with plain-text data failed with a confusing base64 error and documents without data were silently loaded as empty Secrets. Check each document's apiVersion and kind first and return an error naming it. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Jonathan Ogilvie --- cmd/crossplane/render/load.go | 14 +++++ cmd/crossplane/render/load_test.go | 63 +++++++++++++++++++ .../testdata/credentials-configmap.yaml | 14 +++++ .../testdata/credentials-deployment.yaml | 5 ++ .../render/testdata/credentials.yaml | 14 +++++ 5 files changed, 110 insertions(+) create mode 100644 cmd/crossplane/render/testdata/credentials-configmap.yaml create mode 100644 cmd/crossplane/render/testdata/credentials-deployment.yaml create mode 100644 cmd/crossplane/render/testdata/credentials.yaml diff --git a/cmd/crossplane/render/load.go b/cmd/crossplane/render/load.go index fa5971b0..536cfcd4 100644 --- a/cmd/crossplane/render/load.go +++ b/cmd/crossplane/render/load.go @@ -23,6 +23,7 @@ import ( "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/util/yaml" @@ -121,6 +122,19 @@ func LoadCredentials(fs afero.Fs, file string) ([]corev1.Secret, error) { secrets := make([]corev1.Secret, 0, len(stream)) for _, y := range stream { + // Check the kind before decoding into a Secret, so that a non-Secret + // document (e.g. a ConfigMap with plain-text data) gets a clear error + // rather than a confusing base64 decoding failure, or being silently + // treated as an empty Secret. + m := &metav1.PartialObjectMetadata{} + if err := yaml.Unmarshal(y, m); err != nil { + return nil, errors.Wrap(err, "cannot parse YAML secret manifest") + } + + if gvk := m.GroupVersionKind(); gvk != corev1.SchemeGroupVersion.WithKind("Secret") { + return nil, errors.Errorf("%s/%s is not a v1 Secret", gvk.Kind, m.GetName()) + } + s := &corev1.Secret{} if err := yaml.Unmarshal(y, s); err != nil { return nil, errors.Wrap(err, "cannot parse YAML secret manifest") diff --git a/cmd/crossplane/render/load_test.go b/cmd/crossplane/render/load_test.go index 9c226de0..962f0dda 100644 --- a/cmd/crossplane/render/load_test.go +++ b/cmd/crossplane/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,68 @@ 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: "ConfigMap/not-a-secret is not a v1 Secret", + }, + }, + "DeploymentWithNoData": { + file: "testdata/credentials-deployment.yaml", + want: want{ + err: "Deployment/not-a-secret is not a v1 Secret", + }, + }, + } + + 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} diff --git a/cmd/crossplane/render/testdata/credentials-configmap.yaml b/cmd/crossplane/render/testdata/credentials-configmap.yaml new file mode 100644 index 00000000..71fd8bde --- /dev/null +++ b/cmd/crossplane/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! diff --git a/cmd/crossplane/render/testdata/credentials-deployment.yaml b/cmd/crossplane/render/testdata/credentials-deployment.yaml new file mode 100644 index 00000000..54cd81f5 --- /dev/null +++ b/cmd/crossplane/render/testdata/credentials-deployment.yaml @@ -0,0 +1,5 @@ +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: not-a-secret diff --git a/cmd/crossplane/render/testdata/credentials.yaml b/cmd/crossplane/render/testdata/credentials.yaml new file mode 100644 index 00000000..abb55688 --- /dev/null +++ b/cmd/crossplane/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