diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index a45618a14..1b43ea0ce 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -35,6 +35,7 @@ import ( const ( couldNotMarshalErrMsg = "Could not unmarshal to object of type %s: %v" AnnotationLastAppliedConfig = "kubectl.kubernetes.io/last-applied-configuration" + replacement = "++++++++" ) // Holds diffing result of two resources @@ -969,13 +970,13 @@ func CreateTwoWayMergePatch(orig, new, dataStruct interface{}) ([]byte, bool, er return patch, string(patch) != "{}", nil } -// HideSecretData replaces secret data values in specified target, live secrets and in last applied configuration of live secret with stars. Also preserves differences between -// target, live and last applied config values. E.g. if all three are equal the values would be replaced with same number of stars. If all the are different then number of stars +// HideSecretData replaces secret data & optional annotations values in specified target, live secrets and in last applied configuration of live secret with plus(+). Also preserves differences between +// target, live and last applied config values. E.g. if all three are equal the values would be replaced with same number of plus(+). If all are different then number of plus(+) // in replacement should be different. -func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstructured) (*unstructured.Unstructured, *unstructured.Unstructured, error) { - var orig *unstructured.Unstructured +func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstructured, hideAnnotations map[string]bool) (*unstructured.Unstructured, *unstructured.Unstructured, error) { + var liveLastAppliedAnnotation *unstructured.Unstructured if live != nil { - orig, _ = GetLastAppliedConfigAnnotation(live) + liveLastAppliedAnnotation, _ = GetLastAppliedConfigAnnotation(live) live = live.DeepCopy() } if target != nil { @@ -983,7 +984,7 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru } keys := map[string]bool{} - for _, obj := range []*unstructured.Unstructured{target, live, orig} { + for _, obj := range []*unstructured.Unstructured{target, live, liveLastAppliedAnnotation} { if obj == nil { continue } @@ -995,25 +996,57 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru } } + var err error + target, live, liveLastAppliedAnnotation, err = hide(target, live, liveLastAppliedAnnotation, keys, "data") + if err != nil { + return nil, nil, err + } + + target, live, liveLastAppliedAnnotation, err = hide(target, live, liveLastAppliedAnnotation, hideAnnotations, "metadata", "annotations") + if err != nil { + return nil, nil, err + } + + if live != nil && liveLastAppliedAnnotation != nil { + annotations := live.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + // special case: hide "kubectl.kubernetes.io/last-applied-configuration" annotation + if _, ok := hideAnnotations[corev1.LastAppliedConfigAnnotation]; ok { + annotations[corev1.LastAppliedConfigAnnotation] = replacement + } else { + lastAppliedData, err := json.Marshal(liveLastAppliedAnnotation) + if err != nil { + return nil, nil, fmt.Errorf("error marshaling json: %s", err) + } + annotations[corev1.LastAppliedConfigAnnotation] = string(lastAppliedData) + } + live.SetAnnotations(annotations) + } + return target, live, nil +} + +func hide(target, live, liveLastAppliedAnnotation *unstructured.Unstructured, keys map[string]bool, fields ...string) (*unstructured.Unstructured, *unstructured.Unstructured, *unstructured.Unstructured, error) { for k := range keys { // we use "+" rather than the more common "*" - nextReplacement := "++++++++" + nextReplacement := replacement valToReplacement := make(map[string]string) - for _, obj := range []*unstructured.Unstructured{target, live, orig} { + for _, obj := range []*unstructured.Unstructured{target, live, liveLastAppliedAnnotation} { var data map[string]interface{} if obj != nil { // handles an edge case when secret data has nil value // https://github.com/argoproj/argo-cd/issues/5584 - dataValue, ok := obj.Object["data"] + dataValue, ok, _ := unstructured.NestedFieldCopy(obj.Object, fields...) if ok { if dataValue == nil { continue } } var err error - data, _, err = unstructured.NestedMap(obj.Object, "data") + data, _, err = unstructured.NestedMap(obj.Object, fields...) if err != nil { - return nil, nil, fmt.Errorf("unstructured.NestedMap error: %s", err) + return nil, nil, nil, fmt.Errorf("unstructured.NestedMap error: %s", err) } } if data == nil { @@ -1031,25 +1064,13 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru valToReplacement[val] = replacement } data[k] = replacement - err := unstructured.SetNestedField(obj.Object, data, "data") + err := unstructured.SetNestedField(obj.Object, data, fields...) if err != nil { - return nil, nil, fmt.Errorf("unstructured.SetNestedField error: %s", err) + return nil, nil, nil, fmt.Errorf("unstructured.SetNestedField error: %s", err) } } } - if live != nil && orig != nil { - annotations := live.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - lastAppliedData, err := json.Marshal(orig) - if err != nil { - return nil, nil, fmt.Errorf("error marshaling json: %s", err) - } - annotations[corev1.LastAppliedConfigAnnotation] = string(lastAppliedData) - live.SetAnnotations(annotations) - } - return target, live, nil + return target, live, liveLastAppliedAnnotation, nil } func toString(val interface{}) string { diff --git a/pkg/diff/diff_test.go b/pkg/diff/diff_test.go index fd2fd754e..806919a1c 100644 --- a/pkg/diff/diff_test.go +++ b/pkg/diff/diff_test.go @@ -986,7 +986,9 @@ var ( func TestHideSecretDataSameKeysDifferentValues(t *testing.T) { target, live, err := HideSecretData( createSecret(map[string]string{"key1": "test", "key2": "test"}), - createSecret(map[string]string{"key1": "test-1", "key2": "test-1"})) + createSecret(map[string]string{"key1": "test-1", "key2": "test-1"}), + nil, + ) require.NoError(t, err) assert.Equal(t, map[string]interface{}{"key1": replacement1, "key2": replacement1}, secretData(target)) @@ -996,7 +998,9 @@ func TestHideSecretDataSameKeysDifferentValues(t *testing.T) { func TestHideSecretDataSameKeysSameValues(t *testing.T) { target, live, err := HideSecretData( createSecret(map[string]string{"key1": "test", "key2": "test"}), - createSecret(map[string]string{"key1": "test", "key2": "test"})) + createSecret(map[string]string{"key1": "test", "key2": "test"}), + nil, + ) require.NoError(t, err) assert.Equal(t, map[string]interface{}{"key1": replacement1, "key2": replacement1}, secretData(target)) @@ -1006,13 +1010,155 @@ func TestHideSecretDataSameKeysSameValues(t *testing.T) { func TestHideSecretDataDifferentKeysDifferentValues(t *testing.T) { target, live, err := HideSecretData( createSecret(map[string]string{"key1": "test", "key2": "test"}), - createSecret(map[string]string{"key2": "test-1", "key3": "test-1"})) + createSecret(map[string]string{"key2": "test-1", "key3": "test-1"}), + nil, + ) require.NoError(t, err) assert.Equal(t, map[string]interface{}{"key1": replacement1, "key2": replacement1}, secretData(target)) assert.Equal(t, map[string]interface{}{"key2": replacement2, "key3": replacement1}, secretData(live)) } +func TestHideSecretAnnotations(t *testing.T) { + tests := []struct { + name string + hideAnnots map[string]bool + annots map[string]interface{} + expectedAnnots map[string]interface{} + targetNil bool + }{ + { + name: "no hidden annotations", + hideAnnots: nil, + annots: map[string]interface{}{"token/value": "secret", "key": "secret-key", "app": "test"}, + expectedAnnots: map[string]interface{}{"token/value": "secret", "key": "secret-key", "app": "test"}, + }, + { + name: "hide annotations", + hideAnnots: map[string]bool{"token/value": true, "key": true}, + annots: map[string]interface{}{"token/value": "secret", "key": "secret-key", "app": "test"}, + expectedAnnots: map[string]interface{}{"token/value": replacement1, "key": replacement1, "app": "test"}, + }, + { + name: "hide annotations in last-applied-config", + hideAnnots: map[string]bool{"token/value": true, "key": true}, + annots: map[string]interface{}{ + "token/value": "secret", + "app": "test", + "kubectl.kubernetes.io/last-applied-configuration": `{"apiVersion":"v1","kind":"Secret","metadata":{"annotations":{"app":"test","token/value":"secret","key":"secret-key"},"labels":{"app.kubernetes.io/instance":"test"},"name":"my-secret","namespace":"default"},"type":"Opaque"}`, + }, + expectedAnnots: map[string]interface{}{ + "token/value": replacement1, + "app": "test", + "kubectl.kubernetes.io/last-applied-configuration": `{"apiVersion":"v1","kind":"Secret","metadata":{"annotations":{"app":"test","key":"++++++++","token/value":"++++++++"},"labels":{"app.kubernetes.io/instance":"test"},"name":"my-secret","namespace":"default"},"type":"Opaque"}`, + }, + targetNil: true, + }, + { + name: "special case: hide last-applied-config annotation", + hideAnnots: map[string]bool{"kubectl.kubernetes.io/last-applied-configuration": true}, + annots: map[string]interface{}{ + "token/value": replacement1, + "app": "test", + "kubectl.kubernetes.io/last-applied-configuration": `{"apiVersion":"v1","kind":"Secret","metadata":{"annotations":{"app":"test","token/value":"secret","key":"secret-key"},"labels":{"app.kubernetes.io/instance":"test"},"name":"my-secret","namespace":"default"},"type":"Opaque"}`, + }, + expectedAnnots: map[string]interface{}{ + "app": "test", + "kubectl.kubernetes.io/last-applied-configuration": replacement1, + }, + targetNil: true, + }, + { + name: "hide annotations for malformed annotations", + hideAnnots: map[string]bool{"token/value": true, "key": true}, + annots: map[string]interface{}{"token/value": 0, "key": "secret", "app": true}, + expectedAnnots: map[string]interface{}{"token/value": replacement1, "key": replacement1, "app": true}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + unSecret := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "annotations": tt.annots, + }, + "type": "Opaque", + }, + } + + liveUn := remarshal(unSecret, applyOptions(diffOptionsForTest())) + targetUn := remarshal(unSecret, applyOptions(diffOptionsForTest())) + + if tt.targetNil { + targetUn = nil + } + + target, live, err := HideSecretData(targetUn, liveUn, tt.hideAnnots) + require.NoError(t, err) + + // verify configured annotations are hidden + for _, obj := range []*unstructured.Unstructured{target, live} { + if obj != nil { + annots, _, _ := unstructured.NestedMap(obj.Object, "metadata", "annotations") + for ek, ev := range tt.expectedAnnots { + v, found := annots[ek] + assert.True(t, found) + assert.Equal(t, ev, v) + } + } + } + }) + } +} + +func TestHideSecretAnnotationsPreserveDifference(t *testing.T) { + hideAnnots := map[string]bool{"token/value": true} + + liveUn := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "annotations": map[string]interface{}{"token/value": "secret", "app": "test"}, + }, + "type": "Opaque", + }, + } + targetUn := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "annotations": map[string]interface{}{"token/value": "new-secret", "app": "test"}, + }, + "type": "Opaque", + }, + } + + liveUn = remarshal(liveUn, applyOptions(diffOptionsForTest())) + targetUn = remarshal(targetUn, applyOptions(diffOptionsForTest())) + + target, live, err := HideSecretData(targetUn, liveUn, hideAnnots) + require.NoError(t, err) + + liveAnnots := live.GetAnnotations() + v, found := liveAnnots["token/value"] + assert.True(t, found) + assert.Equal(t, replacement2, v) + + targetAnnots := target.GetAnnotations() + v, found = targetAnnots["token/value"] + assert.True(t, found) + assert.Equal(t, replacement1, v) +} + func getTargetSecretJsonBytes() []byte { return []byte(` { @@ -1078,7 +1224,7 @@ func TestHideSecretDataHandleEmptySecret(t *testing.T) { liveSecret := bytesToUnstructured(t, getLiveSecretJsonBytes()) // when - target, live, err := HideSecretData(targetSecret, liveSecret) + target, live, err := HideSecretData(targetSecret, liveSecret, nil) // then assert.NoError(t, err) @@ -1096,7 +1242,7 @@ func TestHideSecretDataLastAppliedConfig(t *testing.T) { require.NoError(t, err) liveSecret.SetAnnotations(map[string]string{corev1.LastAppliedConfigAnnotation: string(lastAppliedStr)}) - target, live, err := HideSecretData(targetSecret, liveSecret) + target, live, err := HideSecretData(targetSecret, liveSecret, nil) require.NoError(t, err) err = json.Unmarshal([]byte(live.GetAnnotations()[corev1.LastAppliedConfigAnnotation]), &lastAppliedSecret) require.NoError(t, err) diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 00d8a05fa..a7a0c419d 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -80,7 +80,7 @@ func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj if err != nil { return "", err } - redacted, _, err := diff.HideSecretData(&obj, nil) + redacted, _, err := diff.HideSecretData(&obj, nil, nil) if err != nil { return "", err }