Skip to content
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

Jump to bottom

feat: Add ability to hide certain annotations on secret resources #577

Merged
merged 7 commits into from
Oct 29, 2024
Merged

feat: Add ability to hide certain annotations on secret resources #577

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f3c54db
Add option to hide annotations on secrets
svghadi May 14, 2024
acd76f2
Handle err
svghadi May 14, 2024
6395c89
Move hide logic to a generic func
svghadi Jul 8, 2024
2dd3ac2
Remove test code
svghadi Jul 8, 2024
e3e911a
Address review comments
svghadi Sep 20, 2024
916f64e
Handle lastAppliedConfig special case
svghadi Sep 24, 2024
5c87732
Fix if logic and remove comments
svghadi Oct 23, 2024
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
66 changes: 41 additions & 25 deletions pkg/diff/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,10 +969,10 @@ 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) {
func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstructured, hideAnnotations map[string]bool) (*unstructured.Unstructured, *unstructured.Unstructured, error) {
var orig *unstructured.Unstructured
if live != nil {
orig, _ = GetLastAppliedConfigAnnotation(live)
Expand All @@ -995,6 +995,36 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru
}
}

var err error
// hide data
target, live, orig, err = hide(target, live, orig, keys, "data")
if err != nil {
return nil, nil, err
}

// hide annotations
target, live, orig, err = hide(target, live, orig, hideAnnotations, "metadata", "annotations")
if err != nil {
return nil, nil, err
}

// hide last-applied-config annotation
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
}

func hide(target, live, orig *unstructured.Unstructured, keys map[string]bool, fields ...string) (*unstructured.Unstructured, *unstructured.Unstructured, *unstructured.Unstructured, error) {
svghadi marked this conversation as resolved.
Show resolved Hide resolved
for k := range keys {
// we use "+" rather than the more common "*"
nextReplacement := "++++++++"
Expand All @@ -1004,16 +1034,14 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru
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"]
if ok {
if dataValue == nil {
continue
}
dataValue, ok, _ := unstructured.NestedFieldCopy(obj.Object, fields...)
if !ok || dataValue == nil {
svghadi marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -1031,25 +1059,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, orig, nil
}

func toString(val interface{}) string {
Expand Down
142 changes: 137 additions & 5 deletions pkg/diff/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -1006,13 +1010,141 @@ 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"},
agaudreault marked this conversation as resolved.
Show resolved Hide resolved
},
{
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: "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(`
{
Expand Down Expand Up @@ -1078,7 +1210,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)
Expand All @@ -1096,7 +1228,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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/kube/resource_ops.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading