Read a secret's stringData as well as its data - #464
Open
arpitjain099 wants to merge 1 commit into
Open
arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
The Secret struct only has Data, so a secret written with stringData unmarshals with an empty Data and the credentials are dropped. Nothing reports it: every reader looks the key up in Data, finds nothing, and carries on without auth, which surfaces later as a 401 from the registry rather than as a problem with the config. stringData is how a secret gets written by hand, since the values are not base64, so this is the shape most people hitting it will have used. Fold StringData into Data after unmarshalling, with StringData winning on a duplicate key, which is what Kubernetes does. Fixes carvel-dev#460 Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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
2 participants
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.
Fixes #460.
config.Secretonly declaresData map[string][]byte, sostringDatahas nothing to unmarshal into and is dropped on the floor. Every reader then doessecret.Data[ctlconf.SecretK8sCorev1BasicAuthUsernameKey], gets the zero value and carries on without credentials, which is why this shows up as a 401 from the registry instead of as a problem with the config.stringDatais the form you get when a secret is written by hand rather than generated, since the values are not base64, so it is likely the common shape for people hitting this.This adds
StringDatato the struct and folds it intoDataright after the unmarshal inNewConfigFromFiles, so nothing downstream has to know about it. On a key present in both,stringDatawins, matching Kubernetes.Three subtests through
NewConfigFromFilesusing thehelmChart.repository.secretRefconfig from the issue:stringDataalone,dataalone, and both with an overlapping key. Againstdevelop:The middle one passing either way is the point of including it, since the change touches the path that already worked.
go build ./...,go vet ./pkg/vendir/config/andgo test ./pkg/...are clean. I did not run./test/e2e, it wants ahelm3binary and a local registry that I do not have here.