diff --git a/diff/diff.go b/diff/diff.go index 9ed30bbb..42bb5d21 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -163,12 +163,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 we have a Secret containing `stringData`, apply the same + //transformation that the apiserver would do with it (this protects + //stringData keys from being overwritten down below) + if len(oldSecret.StringData) > 0 && oldSecret.Data == nil { + oldSecret.Data = make(map[string][]byte, len(oldSecret.StringData)) + } + for k, v := range oldSecret.StringData { + oldSecret.Data[k] = []byte(v) + } } 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) } + //same as above + if len(newSecret.StringData) > 0 && newSecret.Data == nil { + newSecret.Data = make(map[string][]byte, len(newSecret.StringData)) + } + for k, v := range newSecret.StringData { + newSecret.Data[k] = []byte(v) + } } + if old != nil { oldSecret.StringData = make(map[string]string, len(oldSecret.Data)) for k, v := range oldSecret.Data { @@ -189,6 +206,7 @@ func redactSecrets(old, new *manifest.MappingResult) { } } } + // remove Data field now that we are using StringData for serialization var buf bytes.Buffer if old != nil { diff --git a/diff/diff_test.go b/diff/diff_test.go index 667e4a4d..8df93823 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -169,6 +169,8 @@ func assertDiff(t *testing.T, before, after string, context int, stripTrailingCR } func TestManifests(t *testing.T) { + ansi.DisableColors(true) + specBeta := map[string]*manifest.MappingResult{ "default, nginx, Deployment (apps)": { @@ -461,3 +463,124 @@ Plan: 0 to add, 1 to change, 0 to destroy. require.Equal(t, "Resource name: nginx\n", buf1.String()) }) } + +func TestManifestsWithRedactedSecrets(t *testing.T) { + ansi.DisableColors(true) + + specSecretWithByteData := map[string]*manifest.MappingResult{ + "default, foobar, Secret (v1)": { + Name: "default, foobar, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foobar +type: Opaque +data: + key1: dmFsdWUx + key2: dmFsdWUy + key3: dmFsdWUz +`, + }} + + specSecretWithByteDataChanged := map[string]*manifest.MappingResult{ + "default, foobar, Secret (v1)": { + Name: "default, foobar, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foobar +type: Opaque +data: + key1: dmFsdWUxY2hhbmdlZA== + key2: dmFsdWUy + key4: dmFsdWU0 +`, + }} + + specSecretWithStringData := map[string]*manifest.MappingResult{ + "default, foobar, Secret (v1)": { + Name: "default, foobar, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foobar +type: Opaque +stringData: + key1: value1 + key2: value2 + key3: value3 +`, + }} + + specSecretWithStringDataChanged := map[string]*manifest.MappingResult{ + "default, foobar, Secret (v1)": { + Name: "default, foobar, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foobar +type: Opaque +stringData: + key1: value1changed + key2: value2 + key4: value4 +`, + }} + + t.Run("OnChangeSecretWithByteData", func(t *testing.T) { + var buf1 bytes.Buffer + diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false + + if changesSeen := Manifests(specSecretWithByteData, specSecretWithByteDataChanged, &diffOptions, &buf1); !changesSeen { + t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") + } + + //TODO: Why is there no empty line between the header and the start of the diff, like in the other diffs? + require.Equal(t, `default, foobar, Secret (v1) has changed: + apiVersion: v1 + kind: Secret + metadata: + name: foobar + data: +- key1: '-------- # (6 bytes)' ++ key1: '++++++++ # (13 bytes)' + key2: 'REDACTED # (6 bytes)' +- key3: '-------- # (6 bytes)' ++ key4: '++++++++ # (6 bytes)' + type: Opaque + +`, buf1.String()) + }) + + t.Run("OnChangeSecretWithStringData", func(t *testing.T) { + var buf1 bytes.Buffer + diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false + + if changesSeen := Manifests(specSecretWithStringData, specSecretWithStringDataChanged, &diffOptions, &buf1); !changesSeen { + t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") + } + + require.Equal(t, `default, foobar, Secret (v1) has changed: + apiVersion: v1 + kind: Secret + metadata: + name: foobar + data: +- key1: '-------- # (6 bytes)' ++ key1: '++++++++ # (13 bytes)' + key2: 'REDACTED # (6 bytes)' +- key3: '-------- # (6 bytes)' ++ key4: '++++++++ # (6 bytes)' + type: Opaque + +`, buf1.String()) + }) +}