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.

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

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

Merged
merged 10 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions docs/operator-manual/argocd-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 8 additions & 0 deletions docs/operator-manual/declarative-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
14 changes: 7 additions & 7 deletions server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
58 changes: 58 additions & 0 deletions test/e2e/mask_secret_values_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
Empty file.
24 changes: 24 additions & 0 deletions util/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 34 additions & 0 deletions util/settings/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}