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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b9a563f
feat(secrets): allow diffing stringData sections of secrets as well a…
philomory Jun 19, 2022
90c8a27
Change database23 to philomory to allow installation of fork to succeed
philomory Jun 19, 2022
5cf6c45
Merge branch 'master' into feature/diff-secret-string-data
philomory Jun 19, 2022
af950d8
revert change made for local testing
philomory Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions diff/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package diff

import (
"bytes"
"encoding/base64"
"fmt"
"io"
"math"
Expand Down Expand Up @@ -163,11 +164,29 @@ func redactSecrets(old, new *manifest.MappingResult) {
if err := yaml.NewYAMLToJSONDecoder(bytes.NewBufferString(old.Content)).Decode(&oldSecret); err != nil {
old.Content = fmt.Sprintf("Error parsing old secret: %s", err)
}
if oldSecret.Data == nil {
oldSecret.Data = make(map[string][]byte, len(oldSecret.StringData))
}
for k, v := range oldSecret.StringData {
data := []byte(v)
dst := make([]byte, base64.StdEncoding.EncodedLen(len(data)))
base64.StdEncoding.Encode(dst, data)
oldSecret.Data[k] = dst
}
}
if new != nil {
if err := yaml.NewYAMLToJSONDecoder(bytes.NewBufferString(new.Content)).Decode(&newSecret); err != nil {
new.Content = fmt.Sprintf("Error parsing new secret: %s", err)
}
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 •
edited
Loading

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 •
edited
Loading

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?

data := []byte(v)
dst := make([]byte, base64.StdEncoding.EncodedLen(len(data)))
base64.StdEncoding.Encode(dst, data)
newSecret.Data[k] = dst
}
}
if old != nil {
oldSecret.StringData = make(map[string]string, len(oldSecret.Data))
Expand Down