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

svghadi · 2024-05-14T15:57:06Z

Closes #15693
Implements hide-annotations.md proposal.
Depends on argoproj/gitops-engine#577

This PR adds ability to define a list of annotations to hide on secret resources. A new hide.secret.annotations key is introduced in argocd-cm configmap which can be used to configure a list of annotation keys to hide on secret resources.

hidden-annotations.mov

Checklist:

codecov · 2024-06-20T07:00:48Z

Codecov Report

Attention: Patch coverage is 68.18182% with 7 lines in your changes missing coverage. Please review.

Project coverage is 55.84%. Comparing base (0710ff9) to head (5922883).

Files with missing lines	Patch %	Lines
server/application/application.go	42.85%	4 Missing ⚠️
util/settings/settings.go	78.57%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #18216      +/-   ##
==========================================
+ Coverage   55.81%   55.84%   +0.02%     
==========================================
  Files         320      320              
  Lines       44431    44445      +14     
==========================================
+ Hits        24800    24821      +21     
+ Misses      17066    17054      -12     
- Partials     2565     2570       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pasha-codefresh · 2024-06-30T15:31:27Z

@CodiumAI-Agent /review

CodiumAI-Agent · 2024-06-30T15:32:04Z

PR Reviewer Guide 🔍

(Review updated until commit `1bc8297`)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Key issues to review Code Quality The method `GetHideSecretAnnotations` in `util/settings/settings.go` uses regular expressions to process annotation keys. This could be simplified or better documented to improve maintainability. Error Handling In `controller/appcontroller.go`, the error from `HideSecretData` is checked, but there's no handling for potential nil objects which might be returned in case of errors. This could lead to runtime panics.

danielstankw · 2024-09-16T07:43:45Z

What is the update on this feature? - we need it

CodiumAI-Agent · 2024-09-16T07:44:23Z

Persistent review updated to latest commit 1bc8297

svghadi · 2024-09-16T09:04:34Z

Hi @danielstankw, it is awaiting review on dependent PR argoproj/gitops-engine#577.

util/settings/settings.go

agaudreault · 2024-09-23T16:12:51Z

docs/operator-manual/argocd-cm.yaml

@@ -429,3 +429,8 @@ data:

  # application.sync.impersonation.enabled indicates whether the application sync can be decoupled from control plane service account using impersonation.
  application.sync.impersonation.enabled: "false"
+
+  # An optional list of annotation keys to hide in UI on secret resources
+  hide.secret.annotations: |


I am not a big fan of the property key. I think it should be something like <group>.<scope>.<feature>. Something like resource.sensitive.mask.annotations. Why?

It acts on resource, so it goes in the same category as customization, compare options, etc.

The secret is not configurable, it is hardcoded. So I think we can generalize it to "sensitive" resources.

We don't hide the annotations, we mask them. I think the configuration should reflect that to the user. (even if the internal functions names use hide)

Incorporated this suggestion.

agaudreault · 2024-09-23T16:13:33Z

go.mod

@@ -2,6 +2,8 @@ module github.com/argoproj/argo-cd/v2

 go 1.22.0

+replace github.com/argoproj/gitops-engine => github.com/svghadi/gitops-engine v0.0.0-20240919034454-2dd3ac24b0a9


Reminder to revert before merge

agaudreault · 2024-09-23T16:16:01Z

util/settings/settings.go

+	if value, ok := argoCDCM.Data[hideSecretAnnotations]; ok {
+		annots := strings.Split(value, "\n")
+		for _, a := range annots {
+			a = regexp.MustCompile(`^\s*-`).ReplaceAllString(a, "")


Is there a kubernetes library function we can use to make sure the annotations follow the Kubernetes guidelines instead?

The regex was used to clean up the input annotations from configmap and not validate them. I have removed the regex by updated the code to take in comma-seperated list of annotation keys from configmap instead of a list.

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

svghadi · 2024-09-24T13:38:56Z

Hi @jsoref , @agaudreault,
Thanks for the inputs. I have incorporated your feedback. Please take a look whenever you get time. I am working on updating doc and adding an e2e test.

jsoref · 2024-09-24T13:46:58Z

docs/operator-manual/argocd-cm.yaml

@@ -428,4 +431,4 @@ data:
  webhook.maxPayloadSizeMB: 1024

  # application.sync.impersonation.enabled indicates whether the application sync can be decoupled from control plane service account using impersonation.
-  application.sync.impersonation.enabled: "false"
+  application.sync.impersonation.enabled: "false"


I doubt you want to remove the end of line from the end of the file

Oops! My bad. Must have removed it while cleaning up the code. Will fix it.

svghadi changed the title ~~feat: Add option to hide certain annotations on secret resources in the Argo CD Web UI~~ feat: Add option to hide certain annotations on secret resources May 14, 2024

svghadi changed the title ~~feat: Add option to hide certain annotations on secret resources~~ feat: Add ability to hide certain annotations on secret resources May 14, 2024

svghadi mentioned this pull request May 14, 2024

feat: Add ability to hide certain annotations on secret resources argoproj/gitops-engine#577

Open

svghadi force-pushed the hide-secret-annots branch from e699991 to 9d72639 Compare June 20, 2024 06:08

svghadi mentioned this pull request Jun 20, 2024

Provide ability to hide defined annotations in ArgoCD web UI #15693

Open

svghadi force-pushed the hide-secret-annots branch from 9d72639 to 1bc8297 Compare July 8, 2024 12:45

svghadi mentioned this pull request Aug 6, 2024

[SIG Security]: Add Siddhesh Ghadi to SIG Security argoproj/argoproj#315

Merged

svghadi force-pushed the hide-secret-annots branch from 1bc8297 to ba1ceae Compare September 16, 2024 09:22

svghadi marked this pull request as ready for review September 16, 2024 09:22

svghadi requested review from a team as code owners September 16, 2024 09:22

svghadi force-pushed the hide-secret-annots branch from ba1ceae to b8b4896 Compare September 19, 2024 03:42

jsoref reviewed Sep 19, 2024

View reviewed changes

util/settings/settings.go Outdated Show resolved Hide resolved

agaudreault requested changes Sep 23, 2024

View reviewed changes

svghadi added 6 commits September 24, 2024 19:01

Add option to hide annotations on secrets

795a223

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

Retrigger CI

ac7b019

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

Return map instead of slice for annotations

83c67d0

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

Update gitops-engine

999fada

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

Use mask instead of hide

f63e2f4

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

Update gitops-commit

5922883

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>

svghadi force-pushed the hide-secret-annots branch from 6197a8f to 5922883 Compare September 24, 2024 13:32

jsoref reviewed Sep 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

svghadi commented May 14, 2024 •

edited

Loading

codecov bot commented Jun 20, 2024 •

edited

Loading

pasha-codefresh commented Jun 30, 2024

CodiumAI-Agent commented Jun 30, 2024 •

edited

Loading

danielstankw commented Sep 16, 2024

CodiumAI-Agent commented Sep 16, 2024

svghadi commented Sep 16, 2024

agaudreault Sep 23, 2024

svghadi Sep 24, 2024

agaudreault Sep 23, 2024

agaudreault Sep 23, 2024

svghadi Sep 24, 2024

svghadi commented Sep 24, 2024

jsoref Sep 24, 2024

svghadi Sep 24, 2024

		@@ -2,6 +2,8 @@ module github.com/argoproj/argo-cd/v2

		go 1.22.0

		replace github.com/argoproj/gitops-engine => github.com/svghadi/gitops-engine v0.0.0-20240919034454-2dd3ac24b0a9

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

Are you sure you want to change the base?

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

Conversation

svghadi commented May 14, 2024 • edited Loading

codecov bot commented Jun 20, 2024 • edited Loading

Codecov Report

pasha-codefresh commented Jun 30, 2024

CodiumAI-Agent commented Jun 30, 2024 • edited Loading

PR Reviewer Guide 🔍

(Review updated until commit 1bc8297)

danielstankw commented Sep 16, 2024

CodiumAI-Agent commented Sep 16, 2024

svghadi commented Sep 16, 2024

agaudreault Sep 23, 2024

Choose a reason for hiding this comment

svghadi Sep 24, 2024

Choose a reason for hiding this comment

agaudreault Sep 23, 2024

Choose a reason for hiding this comment

agaudreault Sep 23, 2024

Choose a reason for hiding this comment

svghadi Sep 24, 2024

Choose a reason for hiding this comment

svghadi commented Sep 24, 2024

jsoref Sep 24, 2024

Choose a reason for hiding this comment

svghadi Sep 24, 2024

Choose a reason for hiding this comment

svghadi commented May 14, 2024 •

edited

Loading

codecov bot commented Jun 20, 2024 •

edited

Loading

CodiumAI-Agent commented Jun 30, 2024 •

edited

Loading

(Review updated until commit `1bc8297`)