Expose plaintext template.data values in template rendering context#1940
Open
hamza-younas94 wants to merge 2 commits intobitnami-labs:mainfrom
Open
Expose plaintext template.data values in template rendering context#1940hamza-younas94 wants to merge 2 commits intobitnami-labs:mainfrom
hamza-younas94 wants to merge 2 commits intobitnami-labs:mainfrom
Conversation
When a SealedSecret defines plaintext keys alongside encrypted keys
inside `spec.template.data`, sibling templates in the same map could
not reference those plaintext keys as `{{ .key }}` variables and the
controller would render `<no value>` instead of the configured
plaintext value.
The template execution context was being built only from the
decrypted entries of `spec.encryptedData`, so any reference to a
plaintext key defined in `spec.template.data` was treated as a
missing field by `text/template`.
Pre-populate the template context with the raw values from
`spec.template.data` before rendering. Encrypted values take
precedence on key collision so a plaintext key can never silently
shadow a real secret value.
Add three regression tests covering:
- pure plaintext template.data with cross-key references
- mixed encrypted + plaintext template.data
- encryptedData precedence over a colliding template.data key
Closes bitnami-labs#1607
Signed-off-by: Hamza Younas <hamza.younas94@gmail.com>
There was a problem hiding this comment.
Pull request overview
Fixes template rendering so plaintext keys in spec.template.data are available in the Go text/template execution context (alongside decrypted spec.encryptedData keys), addressing <no value> substitutions when sibling templates reference plaintext entries.
Changes:
- Pre-populate the template execution context with raw
spec.template.datavalues (without overriding decrypted values). - Add regression tests covering plaintext-only, mixed encrypted+plaintext templating, and collision precedence expectations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/apis/sealedsecrets/v1alpha1/sealedsecret_expansion.go | Populates the template rendering context with plaintext template.data entries before executing templates. |
| pkg/apis/sealedsecrets/v1alpha1/sealedsecret_test.go | Adds regression tests for plaintext references, mixed encrypted/plaintext references, and precedence behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Author
|
Hi @agarcia-oss — following up on this PR. DCO is signed, the fix is additive (14 lines), and tests cover the 3 cases from the original issue. Happy to incorporate any feedback. Thanks for your time! |
Previously, spec.template.data entries were written to secret.Data unconditionally, silently overwriting decrypted encryptedData values on key collision. Guard the assignment behind a check against s.Spec.EncryptedData so encrypted values win in both the template rendering context and the output Secret. Add assertion to TestTemplateDataEncryptedTakesPrecedenceOverPlaintext covering the output Secret's colliding key. Signed-off-by: Hamza Younas <hamza.younas94@gmail.com>
164b997 to
fabb119
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a
SealedSecretdefines plaintext keys alongside encrypted keys insidespec.template.data, sibling templates in the same map could not reference those plaintext keys as{{ .key }}variables — the controller rendered<no value>instead of the configured plaintext value.This PR fixes that by pre-populating the template execution context with the raw plaintext values from
spec.template.databefore rendering, so templates can reference them just like decryptedencryptedDatakeys.Closes #1607
Reproducer (matches the issue)
Before this PR, the rendered
settings.xmlis:After this PR, it is:
Root cause
In
pkg/apis/sealedsecrets/v1alpha1/sealedsecret_expansion.go,Unseal()builds adatamap that is used as thetext/templateexecution context. That map was being populated only from decrypted entries ofspec.encryptedData. The subsequent loop that renders eachspec.template.dataentry then ran with a context that did not contain any of the sibling plaintext values, so{{ .username }}resolved to a missing field and Go'stext/templatepackage emitted the literal string<no value>.Fix
Add a small additional loop before the rendering loop that copies each
spec.template.dataentry into the context as a raw string, but only if the key does not already exist in the context. This means:spec.template.dataare now visible to all sibling templates as{{ .key }}variables.TestSealRoundTripTemplateData) is unchanged.Tests
Three new tests in
pkg/apis/sealedsecrets/v1alpha1/sealedsecret_test.go:TestTemplateDataPlaintextReference— pure plaintexttemplate.datawith cross-key references. Reproduces the exact symptom in the issue.TestTemplateDataMixedEncryptedAndPlaintext— mixed encrypted + plaintexttemplate.data, with a template that references both at once.TestTemplateDataEncryptedTakesPrecedenceOverPlaintext— guards the precedence rule so future refactors cannot accidentally let a plaintext key shadow a decrypted one.I followed TDD: tests 1 and 2 reproduce the bug and fail on
mainwith the exact<no value>output users were seeing. Test 3 already passes onmainand exists as a forward-looking guard.Verification performed locally
go test ./pkg/apis/sealedsecrets/v1alpha1/...— all 11 tests in the package pass (3 new + 8 pre-existing)go test ./pkg/...— every package green (controller,crypto,kubeseal,multidocyaml,pflagenv,flagenv)go vet ./pkg/apis/sealedsecrets/v1alpha1/...— cleangofmt -lon touched files — cleanTest plan for reviewers
Secret'ssettings.xmlcontains bothmyUsernameand the decrypted password.