Skip to content

feat(secrets): allow diffing stringData sections of secrets as well as data sections #393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

philomory
Copy link

Fixes #302.

If the same key appears in both the data and stringData sections of the same secret, the one in the stringData section is the one considered, because that is how Kubernetes itself behaves.

@philomory philomory marked this pull request as draft June 19, 2022 20:26
@philomory philomory force-pushed the feature/diff-secret-string-data branch from 651c1af to b9a563f Compare June 19, 2022 20:43
@philomory philomory marked this pull request as ready for review June 19, 2022 20:43
if newSecret.Data == nil {
newSecret.Data = make(map[string][]byte, len(newSecret.StringData))
}
for k, v := range newSecret.StringData {
Copy link
Collaborator

@mumoshu mumoshu Jun 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be confused, but shouldn't we reset the whole newSecret.Data before merging in StringData entries? Otherwise, it might get redundant keys when you have different set of keys for data and stringData respectively?

In other words, what if the input was like:

data:
  foo: <BASE64 encoded value "FOO">
  bar: <BASE64 encoded value "BAR1">
stringData:
  bar: BAR2
  baz: BAZ

Should this function output:

stringData:
  foo: FOO
  bar: BAR2
  baz: BAZ

or

stringData:
  bar: BAR2
  baz: BAZ

?

Copy link
Author

@philomory philomory Jun 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to handle the case where the incoming YAML manifest looks as follows:

apiVersion: v1
kind: Secret
metadata:
  name: example
data:
  foo: <encoded:FOO>
  bar: <encoded:BAR>
stringData:
  bar: BAR2
  baz: BAZ

In such a case, per the Kubernetes documentation, the effective value of such a manifest is as follows:

apiVersion: v1
kind: Secret
metadata:
  name: example
data:
  foo: <encoded:FOO>
  bar: <encoded:BAR2>
  baz: <encoded:BAZ>

... or, equivalently:

apiVersion: v1
kind: Secret
metadata:
  name: example
stringData:
  foo: FOO
  bar: BAR2
  baz: BAZ

This is a pretty edge-case scenario: it's perfectly legal to write, but pretty uncommon because it accomplishes so very little other than causing confusion (especially since, once the API server persists the object, the distinction between data and stringData is lost entirely).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mumoshu Specifically, reference the following from the Kubernetes documentation:

All key-value pairs in the stringData field are internally merged into the data field. If a key appears in both the data and the stringData field, the value specified in the stringData field takes precedence.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mumoshu Were there any other questions you had about this?

@databus23
Copy link
Owner

Superseded by #407

@databus23 databus23 closed this Sep 23, 2022
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.

Input stringData gets stripped completely
3 participants