feat: Additional audit log information for updating argocd applications (resolves #23130) #23131

TomHellier · 2025-05-23T12:26:44Z

When users update applications in the argocd ui, the audit log should be clear about what was changed in the application/applicationset. This change allows auditors to understand the difference in an update.

For example, if a user sets a helm parameter override, or points the application at a branch in the webui. Auditing should reflect that.

This information will not get included in the kubernetes event log, you can include very large differences in a spec.source.helm.values field, and it would clutter the kubernetes event log.

This change also adds logFields to the various calls to logAppEvent to allow future developers to add arbitrary contents to the audit logs.

Log Output (changed)

Kubernetes Event Output (unchanged)

Closes [ISSUE #23130]

Checklist:

bunnyshell · 2025-05-23T12:26:48Z

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

🔵 /bns:start to start the environment
🚀 /bns:deploy to redeploy the environment
❌ /bns:delete to remove the environment

codecov · 2025-05-23T13:35:49Z

Codecov Report

Attention: Patch coverage is 58.10811% with 62 lines in your changes missing coverage. Please review.

Project coverage is 60.04%. Comparing base (18ce5c2) to head (5e51f31).

Files with missing lines	Patch %	Lines
server/application/application.go	59.37%	19 Missing and 7 partials ⚠️
server/applicationset/applicationset.go	52.27%	14 Missing and 7 partials ⚠️
util/argo/audit_logger.go	50.00%	14 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #23131      +/-   ##
==========================================
+ Coverage   59.98%   60.04%   +0.05%     
==========================================
  Files         343      343              
  Lines       57890    57992     +102     
==========================================
+ Hits        34727    34819      +92     
- Misses      20381    20384       +3     
- Partials     2782     2789       +7

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

crenshaw-dev · 2025-05-23T13:46:04Z

server/application/application.go

@@ -987,6 +991,23 @@ func (s *Server) updateApp(ctx context.Context, app *v1alpha1.Application, newAp
 	return nil, status.Errorf(codes.Internal, "Failed to update application. Too many conflicts")
 }

+func diffBetweenApplicationSpecs(a *v1alpha1.ApplicationSpec, b *v1alpha1.ApplicationSpec) (string, error) {


A couple suggestions:

I'd unmarshal the patch into a map[any]any and log it as its own field, so it's parseable by log tools

I'd exclude the patch from the event: some people put huge stuff in their app spec

Hi @crenshaw-dev - thanks for the review.

I've pushed changes based on my understanding of your feedback, if you wouldn't mind another look.

I've pushed logFields right up to the top of logAppEvent so future audit context can be added more easily.

TomHellier · 2025-05-23T15:19:10Z

server/application/application.go

@@ -2028,16 +2053,16 @@ func (s *Server) Sync(ctx context.Context, syncReq *application.ApplicationSyncR
 	if len(syncReq.Resources) > 0 {
 		partial = "partial "
 	}
-	var reason string
+	var action string


this is an "action", not a "reason".

argo.EventReasonOperationStarted is the reason, so makes sense to update this.

TomHellier · 2025-05-23T15:23:16Z

util/argo/audit_logger.go

@@ -68,7 +68,7 @@ func (l *AuditLogger) logEvent(objMeta ObjectRef, gvk schema.GroupVersionKind, i
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        fmt.Sprintf("%v.%x", objMeta.Name, t.UnixNano()),
 			Labels:      eventLabels,
-			Annotations: logFields,
+			Annotations: eventAnnotations,


If we want to start adding information into logFields which won't make their way into the event, we need to split this parameter into two.

This is why the PR has exploded in size.

crenshaw-dev · 2025-05-23T15:59:26Z

server/application/application.go

@@ -956,6 +956,10 @@ func (s *Server) waitSync(app *v1alpha1.Application) {
 }

 func (s *Server) updateApp(ctx context.Context, app *v1alpha1.Application, newApp *v1alpha1.Application, merge bool) (*v1alpha1.Application, error) {
+	applicationPatch, err := diffBetweenApplicationSpecs(&app.Spec, &newApp.Spec)
+	if err != nil {
+		return nil, err


Let's wrap this error for easier debugging.

crenshaw-dev · 2025-05-23T15:59:40Z

server/application/application.go

@@ -956,6 +956,10 @@ func (s *Server) waitSync(app *v1alpha1.Application) {
 }

 func (s *Server) updateApp(ctx context.Context, app *v1alpha1.Application, newApp *v1alpha1.Application, merge bool) (*v1alpha1.Application, error) {
+	applicationPatch, err := diffBetweenApplicationSpecs(&app.Spec, &newApp.Spec)


Is the spec enough? Should we also get metadata?

I've added metadata, I didn't really care about it for my purposes but I think adding metadata is helpful as annotations might be set on the application which affects behaviour.

crenshaw-dev · 2025-05-23T15:59:57Z

server/application/application.go

+			logFields := make(map[string]string)
+			logFields["patch"] = applicationPatch


Can probably initialize the map inline. Not a big deal, might just look nicer. :-)

crenshaw-dev · 2025-05-23T16:00:47Z

server/application/application.go

+	if err != nil {
+		return "", fmt.Errorf("failed to create json diff between applications: %w", err)
+	}
+	return string(patch), nil


Would it be possible to unmarshal the patch into a map[string]any so log tools aren't forced to parse a JSON string?

I've made this change

…as logFields Addressing code review comments as part of argoproj#23131, also added the error to the logFields for appcontroller to stop unparam lint. It makes sense to keep the logFields in that function call despite nothing using it at the moment, so may as well pass in the error string. Converted the json string into a map[string]any as that is what sirupsen/logrus expects for the WithField api. Signed-off-by: Tom Hellier <[email protected]>

…as logFields Addressing code review comments as part of argoproj#23131, also added the error to the logFields for appcontroller to stop unparam lint. It makes sense to keep the logFields in that function call despite nothing using it at the moment, so may as well pass in the error string. Converted the json string into a map[string]any as that is what sirupsen/logrus expects for the WithField api. Also support the diff on updates to ApplicationSpecs. Signed-off-by: Tom Hellier <[email protected]>

TomHellier · 2025-05-23T23:31:33Z

I've marked as a draft as I haven't had a chance to test the latest changes with a real Argo CD deployment, with some logging tool (like loki), and the kubernetes event log. I'll probably be able to do this early next week and upload some screenshots.

I've also extended the diff logging to include ApplicationSets. I've copied the jsonDiff function into the applicationset package, as I don't know where the best place for that function to live.

…as logFields Addressing code review comments as part of argoproj#23131, also added the error to the logFields for appcontroller to stop unparam lint. It makes sense to keep the logFields in that function call despite nothing using it at the moment, so may as well pass in the error string. Converted the json string into a map[string]any as that is what sirupsen/logrus expects for the WithField api. Also support the diff on updates to ApplicationSpecs. Signed-off-by: Tom Hellier <[email protected]>

TomHellier · 2025-05-26T16:34:27Z

Hey @crenshaw-dev, please could I get another review at your convenience, thanks.

docs/operator-manual/security.md

… be clear about what was changed in the application spec. This change allows auditors to understand the difference in an update. For example, if a user sets a helm parameter override, or points the application at a branch in the webui. Auditing should reflect that. This information will also get included in the kubernetes event log, but I think that is ok as I wouldn't expect very large differences between applications. Signed-off-by: Tom Hellier <[email protected]>

…what are annotations vs logs It looks like logFields were previously being dual used as annotations on the event, so splitting logFields into logFields and eventAnnotations, as we want to start using logFields to log additional information in the audit log. This could be expanded in future PRs to add more context to the audit logs. Signed-off-by: Tom Hellier <[email protected]>

…as logFields Addressing code review comments as part of argoproj#23131, also added the error to the logFields for appcontroller to stop unparam lint. It makes sense to keep the logFields in that function call despite nothing using it at the moment, so may as well pass in the error string. Converted the json string into a map[string]any as that is what sirupsen/logrus expects for the WithField api. Also support the diff on updates to ApplicationSpecs. Signed-off-by: Tom Hellier <[email protected]>

The logfield for patch has changed from a json style to a go `map[string]interface{}`. Signed-off-by: Tom Hellier <[email protected]>

TomHellier requested a review from a team as a code owner May 23, 2025 12:26

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch 2 times, most recently from a9b7183 to f3b95b8 Compare May 23, 2025 12:28

TomHellier changed the title ~~#23130 - Additional audit log information for updating argocd applications~~ feat: Additional audit log information for updating argocd applications May 23, 2025

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch 2 times, most recently from 8736a80 to 6b89a71 Compare May 23, 2025 12:53

crenshaw-dev requested changes May 23, 2025

View reviewed changes

TomHellier commented May 23, 2025

View reviewed changes

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 6ed819b to 600ffc3 Compare May 23, 2025 15:21

TomHellier commented May 23, 2025

View reviewed changes

crenshaw-dev requested changes May 23, 2025

View reviewed changes

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 268938a to 88859e0 Compare May 23, 2025 22:06

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 88859e0 to 6a44c05 Compare May 23, 2025 22:08

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 6a44c05 to 7ccfff1 Compare May 23, 2025 22:21

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 7ccfff1 to 7e4ec0b Compare May 23, 2025 22:29

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 7e4ec0b to 7e406e0 Compare May 23, 2025 22:38

TomHellier marked this pull request as draft May 23, 2025 22:41

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 7e406e0 to fc255a6 Compare May 23, 2025 23:07

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from fc255a6 to f1c4d4a Compare May 23, 2025 23:15

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from f1c4d4a to 32e4e65 Compare May 23, 2025 23:44

TomHellier changed the title ~~feat: Additional audit log information for updating argocd applications~~ feat: Additional audit log information for updating argocd applications (fixes #23130) May 23, 2025

TomHellier changed the title ~~feat: Additional audit log information for updating argocd applications (fixes #23130)~~ feat: Additional audit log information for updating argocd applications (resolves #23130) May 23, 2025

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from 32e4e65 to 2341772 Compare May 26, 2025 11:57

TomHellier marked this pull request as ready for review May 26, 2025 12:26

TomHellier requested a review from a team as a code owner May 26, 2025 12:26

TomHellier commented May 26, 2025

View reviewed changes

docs/operator-manual/security.md Outdated Show resolved Hide resolved

TomHellier added 4 commits May 28, 2025 07:54

Update Documentation for map style instead of json style patch logField

5e51f31

The logfield for patch has changed from a json style to a go `map[string]interface{}`. Signed-off-by: Tom Hellier <[email protected]>

TomHellier force-pushed the 23130-add-additional-information-into-audit-log branch from d3363b2 to 5e51f31 Compare May 28, 2025 06:54

TomHellier requested a review from crenshaw-dev May 28, 2025 07:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Additional audit log information for updating argocd applications (resolves #23130) #23131

feat: Additional audit log information for updating argocd applications (resolves #23130) #23131

TomHellier commented May 23, 2025 •

edited

Loading

Uh oh!

bunnyshell bot commented May 23, 2025 •

edited

Loading

Uh oh!

codecov bot commented May 23, 2025 •

edited

Loading

Uh oh!

crenshaw-dev May 23, 2025

Uh oh!

TomHellier May 23, 2025 •

edited

Loading

Uh oh!

TomHellier May 23, 2025

Uh oh!

TomHellier May 23, 2025

Uh oh!

crenshaw-dev May 23, 2025

Uh oh!

crenshaw-dev May 23, 2025

Uh oh!

TomHellier May 23, 2025

Uh oh!

crenshaw-dev May 23, 2025

Uh oh!

crenshaw-dev May 23, 2025

Uh oh!

TomHellier May 23, 2025

Uh oh!

TomHellier commented May 23, 2025 •

edited

Loading

Uh oh!

TomHellier commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

		logFields := make(map[string]string)
		logFields["patch"] = applicationPatch

feat: Additional audit log information for updating argocd applications (resolves #23130) #23131

Are you sure you want to change the base?

feat: Additional audit log information for updating argocd applications (resolves #23130) #23131

Conversation

TomHellier commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bunnyshell bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔴 Preview Environment stopped on Bunnyshell

Uh oh!

codecov bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomHellier May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomHellier commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomHellier commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

TomHellier commented May 23, 2025 •

edited

Loading

bunnyshell bot commented May 23, 2025 •

edited

Loading

codecov bot commented May 23, 2025 •

edited

Loading

TomHellier May 23, 2025 •

edited

Loading

TomHellier commented May 23, 2025 •

edited

Loading