diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 431777dd78972..a3947ca634a72 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -759,7 +759,7 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar resDiff := res.Diff if res.Kind == kube.SecretKind && res.Group == "" { var err error - target, live, err = diff.HideSecretData(res.Target, res.Live) + target, live, err = diff.HideSecretData(res.Target, res.Live, ctrl.settingsMgr.GetSensitiveAnnotations()) if err != nil { return nil, fmt.Errorf("error hiding secret data: %w", err) } diff --git a/docs/operator-manual/argocd-cm.yaml b/docs/operator-manual/argocd-cm.yaml index 59edf5f156bd1..e00c2f420d240 100644 --- a/docs/operator-manual/argocd-cm.yaml +++ b/docs/operator-manual/argocd-cm.yaml @@ -222,6 +222,9 @@ data: clusters: - "*.local" + # An optional comma-separated list of annotation keys to mask in UI/CLI on secrets + resource.sensitive.mask.annotations: openshift.io/token-secret.value,api-key + # An optional comma-separated list of metadata.labels to observe in the UI. resource.customLabels: tier diff --git a/docs/operator-manual/declarative-setup.md b/docs/operator-manual/declarative-setup.md index 134f98cfc5eeb..d3b93d27c1601 100644 --- a/docs/operator-manual/declarative-setup.md +++ b/docs/operator-manual/declarative-setup.md @@ -1225,6 +1225,14 @@ Notes: * Invalid globs result in the whole rule being ignored. * If you add a rule that matches existing resources, these will appear in the interface as `OutOfSync`. +## Mask sensitive Annotations on Secrets + +An optional comma-separated list of `metadata.annotations` keys can be configured with `resource.sensitive.mask.annotations` to mask their values in UI/CLI on Secrets. + +```yaml + resource.sensitive.mask.annotations: openshift.io/token-secret.value, api-key +``` + ## Auto respect RBAC for controller Argocd controller can be restricted from discovering/syncing specific resources using just controller rbac, without having to manually configure resource exclusions. diff --git a/go.mod b/go.mod index cc30b6d9d3429..d697a4c74f427 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d github.com/alicebob/miniredis/v2 v2.33.0 github.com/antonmedv/expr v1.15.1 - github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472 + github.com/argoproj/gitops-engine v0.7.1-0.20241029102952-9ab0b2ecae96 github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 github.com/aws/aws-sdk-go v1.55.5 diff --git a/go.sum b/go.sum index 8f36620fb58d8..aa6cdee5f74f7 100644 --- a/go.sum +++ b/go.sum @@ -84,8 +84,8 @@ github.com/antonmedv/expr v1.15.1/go.mod h1:0E/6TxnOlRNp81GMzX9QfDPAmHo2Phg00y4J github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/appscode/go v0.0.0-20191119085241-0887d8ec2ecc/go.mod h1:OawnOmAL4ZX3YaPdN+8HTNwBveT1jMsqP74moa9XUbE= -github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472 h1:NSUzj5CWkOR6xrbGBT4dhZ7WsHhT/pbud+fsvQuUe7k= -github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472/go.mod h1:b1vuwkyMUszyUK+USUJqC8vJijnQsEPNDpC+sDdDLtM= +github.com/argoproj/gitops-engine v0.7.1-0.20241029102952-9ab0b2ecae96 h1:7Guh0VsAHmccy0c55XfzVMT5Y/t76N3j/O0CXk22/A4= +github.com/argoproj/gitops-engine v0.7.1-0.20241029102952-9ab0b2ecae96/go.mod h1:b1vuwkyMUszyUK+USUJqC8vJijnQsEPNDpC+sDdDLtM= github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd h1:lOVVoK89j9Nd4+JYJiKAaMNYC1402C0jICROOfUPWn0= github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd/go.mod h1:N0A4sEws2soZjEpY4hgZpQS8mRIEw6otzwfkgc3g9uQ= github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 h1:qsHwwOJ21K2Ao0xPju1sNuqphyMnMYkyB3ZLoLtxWpo= diff --git a/server/application/application.go b/server/application/application.go index 055bdca54f2b9..de961c82ea5f7 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -557,7 +557,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan return nil, fmt.Errorf("error unmarshaling manifest into unstructured: %w", err) } if obj.GetKind() == kube.SecretKind && obj.GroupVersionKind().Group == "" { - obj, _, err = diff.HideSecretData(obj, nil) + obj, _, err = diff.HideSecretData(obj, nil, s.settingsMgr.GetSensitiveAnnotations()) if err != nil { return nil, fmt.Errorf("error hiding secret data: %w", err) } @@ -684,7 +684,7 @@ func (s *Server) GetManifestsWithFiles(stream application.ApplicationService_Get return fmt.Errorf("error unmarshaling manifest into unstructured: %w", err) } if obj.GetKind() == kube.SecretKind && obj.GroupVersionKind().Group == "" { - obj, _, err = diff.HideSecretData(obj, nil) + obj, _, err = diff.HideSecretData(obj, nil, s.settingsMgr.GetSensitiveAnnotations()) if err != nil { return fmt.Errorf("error hiding secret data: %w", err) } @@ -1373,7 +1373,7 @@ func (s *Server) GetResource(ctx context.Context, q *application.ApplicationReso if err != nil { return nil, fmt.Errorf("error getting resource: %w", err) } - obj, err = replaceSecretValues(obj) + obj, err = s.replaceSecretValues(obj) if err != nil { return nil, fmt.Errorf("error replacing secret values: %w", err) } @@ -1385,9 +1385,9 @@ func (s *Server) GetResource(ctx context.Context, q *application.ApplicationReso return &application.ApplicationResourceResponse{Manifest: &manifest}, nil } -func replaceSecretValues(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { +func (s *Server) replaceSecretValues(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { if obj.GetKind() == kube.SecretKind && obj.GroupVersionKind().Group == "" { - _, obj, err := diff.HideSecretData(nil, obj) + _, obj, err := diff.HideSecretData(nil, obj, s.settingsMgr.GetSensitiveAnnotations()) if err != nil { return nil, err } @@ -1424,7 +1424,7 @@ func (s *Server) PatchResource(ctx context.Context, q *application.ApplicationRe if manifest == nil { return nil, fmt.Errorf("failed to patch resource: manifest was nil") } - manifest, err = replaceSecretValues(manifest) + manifest, err = s.replaceSecretValues(manifest) if err != nil { return nil, fmt.Errorf("error replacing secret values: %w", err) } @@ -2184,7 +2184,7 @@ func (s *Server) ListResourceLinks(ctx context.Context, req *application.Applica return nil, fmt.Errorf("failed to read application deep links from configmap: %w", err) } - obj, err = replaceSecretValues(obj) + obj, err = s.replaceSecretValues(obj) if err != nil { return nil, fmt.Errorf("error replacing secret values: %w", err) } diff --git a/test/e2e/mask_secret_values_test.go b/test/e2e/mask_secret_values_test.go new file mode 100644 index 0000000000000..691cf2f7ec50e --- /dev/null +++ b/test/e2e/mask_secret_values_test.go @@ -0,0 +1,58 @@ +package e2e + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/argoproj/gitops-engine/pkg/health" + + . "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + . "github.com/argoproj/argo-cd/v2/test/e2e/fixture" + . "github.com/argoproj/argo-cd/v2/test/e2e/fixture/app" +) + +// Values of `.data` & `.stringData“ fields in Secret resources are masked in UI/CLI +// Optionally, values of `.metadata.annotations` can also be masked, if needed. +func TestMaskSecretValues(t *testing.T) { + sensitiveData := regexp.MustCompile(`SECRETVAL|NEWSECRETVAL|U0VDUkVUVkFM`) + + Given(t). + Path("empty-dir"). + When(). + SetParamInSettingConfigMap("resource.sensitive.mask.annotations", "token"). // hide sensitive annotation + AddFile("secrets.yaml", `apiVersion: v1 +kind: Secret +metadata: + name: secret + annotations: + token: SECRETVAL + app: test +stringData: + username: SECRETVAL +data: + password: U0VDUkVUVkFM +`). + CreateApp(). + Sync(). + Then(). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + // sensitive data should be masked in manifests output + And(func(app *Application) { + mnfs, _ := RunCli("app", "manifests", app.Name) + assert.False(t, sensitiveData.MatchString(mnfs)) + }). + When(). + PatchFile("secrets.yaml", `[{"op": "replace", "path": "/stringData/username", "value": "NEWSECRETVAL"}]`). + PatchFile("secrets.yaml", `[{"op": "add", "path": "/metadata/annotations", "value": {"token": "NEWSECRETVAL"}}]`). + Refresh(RefreshTypeHard). + Then(). + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + // sensitive data should be masked in diff output + And(func(app *Application) { + diff, _ := RunCli("app", "diff", app.Name) + assert.False(t, sensitiveData.MatchString(diff)) + }) +} diff --git a/test/e2e/testdata/empty-dir/.gitignore b/test/e2e/testdata/empty-dir/.gitignore new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/util/settings/settings.go b/util/settings/settings.go index 2d1a072505867..516116f0ecd99 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -461,6 +461,8 @@ const ( resourceInclusionsKey = "resource.inclusions" // resourceIgnoreResourceUpdatesEnabledKey is the key to a boolean determining whether the resourceIgnoreUpdates feature is enabled resourceIgnoreResourceUpdatesEnabledKey = "resource.ignoreResourceUpdatesEnabled" + // resourceSensitiveAnnotationsKey is the key to list of annotations to mask in secret resource + resourceSensitiveAnnotationsKey = "resource.sensitive.mask.annotations" // resourceCustomLabelKey is the key to a custom label to show in node info, if present resourceCustomLabelsKey = "resource.customLabels" // resourceIncludeEventLabelKeys is the key to labels to be added onto Application k8s events if present on an Application or it's AppProject. Supports wildcard. @@ -2341,6 +2343,28 @@ func (mgr *SettingsManager) GetExcludeEventLabelKeys() []string { return labelKeys } +func (mgr *SettingsManager) GetSensitiveAnnotations() map[string]bool { + annotationKeys := make(map[string]bool) + + argoCDCM, err := mgr.getConfigMap() + if err != nil { + log.Error(fmt.Errorf("failed getting configmap: %w", err)) + return annotationKeys + } + + value, ok := argoCDCM.Data[resourceSensitiveAnnotationsKey] + if !ok || value == "" { + return annotationKeys + } + + value = strings.ReplaceAll(value, " ", "") + keys := strings.Split(value, ",") + for _, k := range keys { + annotationKeys[k] = true + } + return annotationKeys +} + func (mgr *SettingsManager) GetMaxWebhookPayloadSize() int64 { argoCDCM, err := mgr.getConfigMap() if err != nil { diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go index b8a01e69bed9d..1a220bc2f063a 100644 --- a/util/settings/settings_test.go +++ b/util/settings/settings_test.go @@ -1820,3 +1820,37 @@ func TestIsImpersonationEnabled(t *testing.T) { require.NoError(t, err, "when user enables the flag in argocd-cm config map, IsImpersonationEnabled() must not return any error") } + +func TestSettingsManager_GetHideSecretAnnotations(t *testing.T) { + tests := []struct { + name string + input string + output map[string]bool + }{ + { + name: "Empty input", + input: "", + output: map[string]bool{}, + }, + { + name: "Comma separated data", + input: "example.com/token-secret.value,token,key", + output: map[string]bool{"example.com/token-secret.value": true, "token": true, "key": true}, + }, + { + name: "Comma separated data with space", + input: "example.com/token-secret.value, token, key", + output: map[string]bool{"example.com/token-secret.value": true, "token": true, "key": true}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, settingsManager := fixtures(map[string]string{ + resourceSensitiveAnnotationsKey: tt.input, + }) + keys := settingsManager.GetSensitiveAnnotations() + assert.Equal(t, len(tt.output), len(keys)) + assert.Equal(t, tt.output, keys) + }) + } +}