From 129dcb6e784edb7c7a646e1e00f088fd37c5ddd1 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 25 May 2025 07:09:20 +0800 Subject: [PATCH 1/3] refactor(diff): extract secret handling to preHandleSecrets Signed-off-by: yxxhero --- diff/diff.go | 57 +++++++++++++++++++---------------------------- diff/diff_test.go | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 34 deletions(-) diff --git a/diff/diff.go b/diff/diff.go index 0934e8eb..bb17f5f8 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -239,16 +239,9 @@ func doDiff(report *Report, key string, oldContent *manifest.MappingResult, newC } } -// redactSecrets redacts secrets from the diff output. -func redactSecrets(old, new *manifest.MappingResult) { +func preHandleSecrets(old, new *manifest.MappingResult) (v1.Secret, v1.Secret, error, error) { var oldSecretDecodeErr, newSecretDecodeErr error - if (old != nil && old.Kind != "Secret") || (new != nil && new.Kind != "Secret") { - return - } - serializer := json.NewYAMLSerializer(json.DefaultMetaFactory, scheme.Scheme, - scheme.Scheme) var oldSecret, newSecret v1.Secret - if old != nil { oldSecretDecodeErr = yaml.NewYAMLToJSONDecoder(bytes.NewBufferString(old.Content)).Decode(&oldSecret) if oldSecretDecodeErr != nil { @@ -279,6 +272,17 @@ func redactSecrets(old, new *manifest.MappingResult) { } } } + return oldSecret, newSecret, oldSecretDecodeErr, newSecretDecodeErr +} + +// redactSecrets redacts secrets from the diff output. +func redactSecrets(old, new *manifest.MappingResult) { + if (old != nil && old.Kind != "Secret") || (new != nil && new.Kind != "Secret") { + return + } + serializer := json.NewYAMLSerializer(json.DefaultMetaFactory, scheme.Scheme, scheme.Scheme) + + oldSecret, newSecret, oldSecretDecodeErr, newSecretDecodeErr := preHandleSecrets(old, new) if old != nil && oldSecretDecodeErr == nil { oldSecret.StringData = make(map[string]string, len(oldSecret.Data)) @@ -324,38 +328,23 @@ func redactSecrets(old, new *manifest.MappingResult) { // decodeSecrets decodes secrets from the diff output. func decodeSecrets(old, new *manifest.MappingResult) { - var oldSecretDecodeErr, newSecretDecodeErr error if (old != nil && old.Kind != "Secret") || (new != nil && new.Kind != "Secret") { return } - serializer := json.NewYAMLSerializer(json.DefaultMetaFactory, scheme.Scheme, - scheme.Scheme) - var oldSecret, newSecret v1.Secret + serializer := json.NewYAMLSerializer(json.DefaultMetaFactory, scheme.Scheme, scheme.Scheme) - if old != nil { - oldSecretDecodeErr = yaml.NewYAMLToJSONDecoder(bytes.NewBufferString(old.Content)).Decode(&oldSecret) - if oldSecretDecodeErr != nil { - old.Content = fmt.Sprintf("Error parsing old secret: %s", oldSecretDecodeErr) - } else { - if len(oldSecret.Data) > 0 && oldSecret.StringData == nil { - oldSecret.StringData = make(map[string]string, len(oldSecret.Data)) - } - for k, v := range oldSecret.Data { - oldSecret.StringData[k] = string(v) - } + oldSecret, newSecret, oldSecretDecodeErr, newSecretDecodeErr := preHandleSecrets(old, new) + + if old != nil && oldSecretDecodeErr == nil { + oldSecret.StringData = make(map[string]string, len(oldSecret.Data)) + for k, v := range oldSecret.Data { + oldSecret.StringData[k] = string(v) } } - if new != nil { - newSecretDecodeErr = yaml.NewYAMLToJSONDecoder(bytes.NewBufferString(new.Content)).Decode(&newSecret) - if newSecretDecodeErr != nil { - new.Content = fmt.Sprintf("Error parsing new secret: %s", newSecretDecodeErr) - } else { - if len(newSecret.Data) > 0 && newSecret.StringData == nil { - newSecret.StringData = make(map[string]string, len(newSecret.StringData)) - } - for k, v := range newSecret.Data { - newSecret.StringData[k] = string(v) - } + if new != nil && newSecretDecodeErr == nil { + newSecret.StringData = make(map[string]string, len(newSecret.Data)) + for k, v := range newSecret.Data { + newSecret.StringData[k] = string(v) } } diff --git a/diff/diff_test.go b/diff/diff_test.go index bbafa8a2..5bcb91d6 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -943,6 +943,48 @@ stringData: require.Contains(t, new.Content, "key1: value1changed") require.Contains(t, new.Content, "key2: value2") }) + t.Run("decodeSecrets with stringData and Data that stringData precedes data on Secrets", func(t *testing.T) { + old := &manifest.MappingResult{ + Name: "default, foo, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foo +type: Opaque +stringData: + key1: value1 + key2: value2 +data: + key2: dmFsdWUyaW5kYXRh + key3: dmFsdWUz +`, + } + new := &manifest.MappingResult{ + Name: "default, foo, Secret (v1)", + Kind: "Secret", + Content: ` +apiVersion: v1 +kind: Secret +metadata: + name: foo +type: Opaque +stringData: + key1: value1changed + key2: value2 +data: + key3: dmFsdWUzaW5kYXRh +`, + } + decodeSecrets(old, new) + require.Contains(t, old.Content, "key1: value1") + require.Contains(t, old.Content, "key2: value2") + require.Contains(t, old.Content, "key3: value3") + require.Contains(t, new.Content, "key1: value1changed") + require.Contains(t, new.Content, "key2: value2") + require.Contains(t, new.Content, "key3: value3indata") + }) t.Run("decodeSecrets with invalid base64", func(t *testing.T) { old := &manifest.MappingResult{ From f3e073d97c60213cdbdda5f6f15d81bbb102246f Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Sun, 25 May 2025 10:59:30 +0800 Subject: [PATCH 2/3] Update diff/diff_test.go Co-authored-by: Lucas Fernando Cardoso Nunes --- diff/diff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff/diff_test.go b/diff/diff_test.go index 5bcb91d6..e8382989 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -943,7 +943,7 @@ stringData: require.Contains(t, new.Content, "key1: value1changed") require.Contains(t, new.Content, "key2: value2") }) - t.Run("decodeSecrets with stringData and Data that stringData precedes data on Secrets", func(t *testing.T) { + t.Run("decodeSecrets with stringData and data ensuring that stringData always precedes/overrides data on Secrets", func(t *testing.T) { old := &manifest.MappingResult{ Name: "default, foo, Secret (v1)", Kind: "Secret", From 5602233036047e0606181e2c9e0b70775b586302 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 25 May 2025 11:13:05 +0800 Subject: [PATCH 3/3] fix more test Signed-off-by: yxxhero --- diff/diff_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/diff/diff_test.go b/diff/diff_test.go index e8382989..3bdb4ec0 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -954,11 +954,11 @@ metadata: name: foo type: Opaque stringData: - key1: value1 - key2: value2 + key1: value1.stringdata + key2: value2.stringdata data: - key2: dmFsdWUyaW5kYXRh - key3: dmFsdWUz + key2: dmFsdWUyLmRhdGE= + key3: dmFsdWUzLmRhdGE= `, } new := &manifest.MappingResult{ @@ -971,19 +971,19 @@ metadata: name: foo type: Opaque stringData: - key1: value1changed - key2: value2 + key1: value1changed.stringdata + key2: value2.stringdata data: - key3: dmFsdWUzaW5kYXRh + key3: dmFsdWUzLmRhdGE= `, } decodeSecrets(old, new) - require.Contains(t, old.Content, "key1: value1") - require.Contains(t, old.Content, "key2: value2") - require.Contains(t, old.Content, "key3: value3") - require.Contains(t, new.Content, "key1: value1changed") - require.Contains(t, new.Content, "key2: value2") - require.Contains(t, new.Content, "key3: value3indata") + require.Contains(t, old.Content, "key1: value1.stringdata") + require.Contains(t, old.Content, "key2: value2.stringdata") + require.Contains(t, old.Content, "key3: value3.data") + require.Contains(t, new.Content, "key1: value1changed.stringdata") + require.Contains(t, new.Content, "key2: value2.stringdata") + require.Contains(t, new.Content, "key3: value3.data") }) t.Run("decodeSecrets with invalid base64", func(t *testing.T) {