Skip to content

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

Open
jcogilvie wants to merge 1 commit into
crossplane:mainfrom
jcogilvie:jco-render-credentials-kind-check
Open

jcogilvie wants to merge 1 commit into
crossplane:mainfrom
jcogilvie:jco-render-credentials-kind-check

Conversation

@jcogilvie

@jcogilvie jcogilvie commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Description of your changes

render.LoadCredentials reads every document in --function-credentials straight into a corev1.Secret without checking its kind. A document that isn't a Secret causes one of two problems:

  • ConfigMap with plain-text data: the whole load fails with cannot parse YAML secret manifest: illegal base64 data .... The error names a Secret, even though the bad document isn't one.
  • Document without data (for example a Deployment): it's loaded as an empty Secret with that name and passed to functions as a credential. Nothing warns about it.

Function credentials are always Secrets (a step's credentials[].source: Secret). So this change reads each document's apiVersion/kind first, and rejects anything that isn't v1/Secret with an error that names it, such as ConfigMap/foo is not a v1 Secret. That also means a document with no apiVersion/kind at all is now rejected.

This affects render xr and render op. #436 switches project simulate to the same loader. TestLoadCredentials is new, with three cases: Secrets only, a Secret plus a ConfigMap, and a Deployment.

Related: #436, #452.

I have:

  • Read and followed Crossplane's contribution process.
  • Run ./nix.sh flake check to ensure this PR is ready for review. (./nix.sh run .#test and ./nix.sh run .#lint pass.)
  • Added or updated unit tests.
  • Linked a PR or a docs tracking issue to document this change. (bug fix, no docs change)
  • Added backport release-x.y labels to auto-backport this PR.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie
jcogilvie marked this pull request as ready for review October 27, 2026 20:03
@jcogilvie
jcogilvie requested review from a team and tampakrap as code owners October 27, 2026 20:04
@jcogilvie
jcogilvie requested review from negz and removed request for a team October 27, 2026 20:04
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 58 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 2ca8fd0 and cc0b661.

Files ignored due to path filters: 3
  • cmd/crossplane/render/testdata/credentials-configmap.yaml is excluded by !**/testdata/** and included by **/*.yaml
  • cmd/crossplane/render/testdata/credentials-deployment.yaml is excluded by !**/testdata/** and included by **/*.yaml
  • cmd/crossplane/render/testdata/credentials.yaml is excluded by !**/testdata/** and included by **/*.yaml
Files selected for processing: 2
  • cmd/crossplane/render/load.go
  • cmd/crossplane/render/load_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant