From 7150d49fe864c6662c8df11b3a3d2786a345a662 Mon Sep 17 00:00:00 2001 From: Eugene Doudine Date: Sun, 28 Sep 2025 14:41:59 +0300 Subject: [PATCH 1/2] feat: backport monorepocontroller annotations to acr controller Signed-off-by: Eugene Doudine --- acr_controller/controller/controller.go | 6 +- acr_controller/server.go | 3 +- acr_controller/service/acr_service.go | 138 +++++++++++++----- acr_controller/service/acr_service_test.go | 6 +- .../application_change_revision_controller.go | 3 + 5 files changed, 116 insertions(+), 40 deletions(-) diff --git a/acr_controller/controller/controller.go b/acr_controller/controller/controller.go index b880b94f01243..dc08173cdd0a9 100644 --- a/acr_controller/controller/controller.go +++ b/acr_controller/controller/controller.go @@ -24,9 +24,10 @@ type ACRController interface { type applicationChangeRevisionController struct { appBroadcaster Broadcaster acrService service.ACRService + useAnnotations bool } -func NewApplicationChangeRevisionController(appInformer cache.SharedIndexInformer, applicationServiceClient appclient.ApplicationClient, applicationClientset appclientset.Interface) ACRController { +func NewApplicationChangeRevisionController(appInformer cache.SharedIndexInformer, applicationServiceClient appclient.ApplicationClient, applicationClientset appclientset.Interface, useAnnotations bool) ACRController { appBroadcaster := NewBroadcaster() _, err := appInformer.AddEventHandler(appBroadcaster) if err != nil { @@ -35,6 +36,7 @@ func NewApplicationChangeRevisionController(appInformer cache.SharedIndexInforme return &applicationChangeRevisionController{ appBroadcaster: appBroadcaster, acrService: service.NewACRService(applicationClientset, applicationServiceClient), + useAnnotations: useAnnotations, } } @@ -46,7 +48,7 @@ func (c *applicationChangeRevisionController) Run(ctx context.Context) { return nil // ignore this event } - return c.acrService.ChangeRevision(ctx, &a) + return c.acrService.ChangeRevision(ctx, &a, c.useAnnotations) } // TODO: move to abstraction diff --git a/acr_controller/server.go b/acr_controller/server.go index 30d5000b116a6..59fe446d2f8b6 100644 --- a/acr_controller/server.go +++ b/acr_controller/server.go @@ -54,6 +54,7 @@ type ACRServerOpts struct { ApplicationNamespaces []string BaseHRef string RootPath string + DisableAnnotations bool } type handlerSwitcher struct { @@ -96,7 +97,7 @@ func (a *ACRServer) Init(ctx context.Context) { } func (a *ACRServer) RunController(ctx context.Context) { - controller := acr_controller.NewApplicationChangeRevisionController(a.appInformer, a.ApplicationServiceClient, a.applicationClientset) + controller := acr_controller.NewApplicationChangeRevisionController(a.appInformer, a.ApplicationServiceClient, a.applicationClientset, !a.DisableAnnotations) go controller.Run(ctx) } diff --git a/acr_controller/service/acr_service.go b/acr_controller/service/acr_service.go index bd4c7b60997f1..81013efa0ab90 100644 --- a/acr_controller/service/acr_service.go +++ b/acr_controller/service/acr_service.go @@ -3,6 +3,7 @@ package service import ( "context" "encoding/json" + "fmt" "sync" log "github.com/sirupsen/logrus" @@ -16,8 +17,15 @@ import ( appclientset "github.com/argoproj/argo-cd/v3/pkg/client/clientset/versioned" ) +const ( + CHANGE_REVISION_ANN = "mrp-controller.argoproj.io/change-revision" + CHANGE_REVISIONS_ANN = "mrp-controller.argoproj.io/change-revisions" + GIT_REVISION_ANN = "mrp-controller.argoproj.io/git-revision" + GIT_REVISIONS_ANN = "mrp-controller.argoproj.io/git-revisions" +) + type ACRService interface { - ChangeRevision(ctx context.Context, application *application.Application) error + ChangeRevision(ctx context.Context, application *application.Application, useAnnotations bool) error } type acrService struct { @@ -55,7 +63,7 @@ func getChangeRevision(app *application.Application) string { return "" } -func (c *acrService) ChangeRevision(ctx context.Context, a *application.Application) error { +func (c *acrService) ChangeRevision(ctx context.Context, a *application.Application, useAnnotations bool) error { c.lock.Lock() defer c.lock.Unlock() @@ -73,36 +81,108 @@ func (c *acrService) ChangeRevision(ctx context.Context, a *application.Applicat return nil } - revision, err := c.calculateRevision(ctx, app) + currentRevision, previousRevision := c.getRevisions(ctx, a) + if currentRevision == "" { + c.logger.Infof("Got empty revision for application %s, is it an unsupported multisource or helm repo based application?", app.Name) + return nil + } + revision, err := c.calculateRevision(ctx, app, currentRevision, previousRevision) if err != nil { return err } + var revisions []string if revision == nil || *revision == "" { c.logger.Infof("Revision for application %s is empty", app.Name) - return nil + } else { + c.logger.Infof("Change revision for application %s is %s", app.Name, *revision) + revisions = []string{*revision} } - c.logger.Infof("Change revision for application %s is %s", app.Name, *revision) - app, err = c.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(ctx, app.Name, metav1.GetOptions{}) if err != nil { return err } - revisions := []string{*revision} + patchMap := make(map[string]any, 2) - if app.Status.OperationState != nil && app.Status.OperationState.Operation.Sync != nil { - c.logger.Infof("Patch operation status for application %s", app.Name) - return c.patchOperationSyncResultWithChangeRevision(ctx, app, revisions) + if len(revisions) > 0 { + if app.Status.OperationState != nil && app.Status.OperationState.Operation.Sync != nil { + c.logger.Infof("Patch operation status for application %s", app.Name) + patchMap = c.patchOperationSyncResultWithChangeRevision(revisions) + } else { + c.logger.Infof("Patch operation for application %s", app.Name) + patchMap = c.patchOperationWithChangeRevision(revisions) + } } + if useAnnotations { + err = c.addAnnotationPatch(patchMap, app, *revision, revisions, currentRevision, []string{currentRevision}) + if err != nil { + return err + } + } + if len(patchMap) > 0 { + c.logger.Infof("patching resource: %v", patchMap) + patch, err := json.Marshal(patchMap) + if err != nil { + return err + } + _, err = c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) + return err + } + c.logger.Infof("no patch needed") + return nil +} - c.logger.Infof("Patch operation for application %s", app.Name) - return c.patchOperationWithChangeRevision(ctx, app, revisions) +func addPatchIfNeeded(annotations map[string]string, currentAnnotations map[string]string, key string, val string) { + currentVal, ok := currentAnnotations[key] + if !ok || currentVal != val { + annotations[key] = val + } } -func (c *acrService) calculateRevision(ctx context.Context, a *application.Application) (*string, error) { - currentRevision, previousRevision := c.getRevisions(ctx, a) +func (c *acrService) addAnnotationPatch(m map[string]any, + a *application.Application, + changeRevision string, + changeRevisions []string, + gitRevision string, + gitRevisions []string, +) error { + c.logger.Infof("annotating application '%s', changeRevision=%s, changeRevisions=%v, gitRevision=%s, gitRevisions=%v", a.Name, changeRevision, changeRevisions, gitRevision, gitRevisions) + annotations := map[string]string{} + currentAnnotations := a.Annotations + + if changeRevision != "" { + addPatchIfNeeded(annotations, currentAnnotations, CHANGE_REVISION_ANN, changeRevision) + } + if len(changeRevisions) > 0 { + changeRevisionsJSON, err := json.Marshal(changeRevisions) + if err != nil { + return fmt.Errorf("failed to marshall changeRevisions %v: %w", changeRevisions, err) + } + addPatchIfNeeded(annotations, currentAnnotations, CHANGE_REVISIONS_ANN, string(changeRevisionsJSON)) + } + if gitRevision != "" { + addPatchIfNeeded(annotations, currentAnnotations, GIT_REVISION_ANN, gitRevision) + } + if len(gitRevisions) > 0 { + gitRevisionsJSON, err := json.Marshal(gitRevisions) + if err != nil { + return fmt.Errorf("failed to marshall gitRevisions %v: %w", gitRevisions, err) + } + addPatchIfNeeded(annotations, currentAnnotations, GIT_REVISIONS_ANN, string(gitRevisionsJSON)) + } + + if len(annotations) == 0 { + c.logger.Info("no need to add annotations") + } else { + c.logger.Infof("added annotations to application %s patch: %v", a.Name, annotations) + m["metadata"] = map[string]any{"annotations": annotations} + } + return nil +} + +func (c *acrService) calculateRevision(ctx context.Context, a *application.Application, currentRevision string, previousRevision string) (*string, error) { c.logger.Infof("Calculate revision for application '%s', current revision '%s', previous revision '%s'", a.Name, currentRevision, previousRevision) changeRevisionResult, err := c.applicationServiceClient.GetChangeRevision(ctx, &appclient.ChangeRevisionRequest{ AppName: ptr.To(a.GetName()), @@ -116,33 +196,28 @@ func (c *acrService) calculateRevision(ctx context.Context, a *application.Appli return changeRevisionResult.Revision, nil } -func (c *acrService) patchOperationWithChangeRevision(ctx context.Context, a *application.Application, revisions []string) error { +func (c *acrService) patchOperationWithChangeRevision(revisions []string) map[string]any { if len(revisions) == 1 { - patch, _ := json.Marshal(map[string]any{ + return map[string]any{ "operation": map[string]any{ "sync": map[string]any{ "changeRevision": revisions[0], }, }, - }) - _, err := c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) - return err + } } - - patch, _ := json.Marshal(map[string]any{ + return map[string]any{ "operation": map[string]any{ "sync": map[string]any{ "changeRevisions": revisions, }, }, - }) - _, err := c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) - return err + } } -func (c *acrService) patchOperationSyncResultWithChangeRevision(ctx context.Context, a *application.Application, revisions []string) error { +func (c *acrService) patchOperationSyncResultWithChangeRevision(revisions []string) map[string]any { if len(revisions) == 1 { - patch, _ := json.Marshal(map[string]any{ + return map[string]any{ "status": map[string]any{ "operationState": map[string]any{ "operation": map[string]any{ @@ -152,12 +227,9 @@ func (c *acrService) patchOperationSyncResultWithChangeRevision(ctx context.Cont }, }, }, - }) - _, err := c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) - return err + } } - - patch, _ := json.Marshal(map[string]any{ + return map[string]any{ "status": map[string]any{ "operationState": map[string]any{ "operation": map[string]any{ @@ -167,9 +239,7 @@ func (c *acrService) patchOperationSyncResultWithChangeRevision(ctx context.Cont }, }, }, - }) - _, err := c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) - return err + } } func getCurrentRevisionFromOperation(a *application.Application) string { diff --git a/acr_controller/service/acr_service_test.go b/acr_controller/service/acr_service_test.go index f1a451a89f995..7b8718b5a8fda 100644 --- a/acr_controller/service/acr_service_test.go +++ b/acr_controller/service/acr_service_test.go @@ -274,7 +274,7 @@ func Test_ChangeRevision(r *testing.T) { acrService := newTestACRService(client) app := createTestApp(syncedAppWithHistory) - err := acrService.ChangeRevision(r.Context(), app) + err := acrService.ChangeRevision(r.Context(), app, false) require.NoError(t, err) app, err = acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) @@ -296,7 +296,7 @@ func Test_ChangeRevision(r *testing.T) { app := createTestApp(syncedAppWithHistory) - err := acrService.ChangeRevision(r.Context(), app) + err := acrService.ChangeRevision(r.Context(), app, false) require.NoError(t, err) app, err = acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) @@ -304,7 +304,7 @@ func Test_ChangeRevision(r *testing.T) { assert.Equal(t, "new-revision", app.Status.OperationState.Operation.Sync.ChangeRevision) - err = acrService.ChangeRevision(r.Context(), app) + err = acrService.ChangeRevision(r.Context(), app, false) require.NoError(t, err) diff --git a/cmd/application-change-revision-controller/commands/application_change_revision_controller.go b/cmd/application-change-revision-controller/commands/application_change_revision_controller.go index 9ee8e67a6f78b..28df2ed3da87a 100644 --- a/cmd/application-change-revision-controller/commands/application_change_revision_controller.go +++ b/cmd/application-change-revision-controller/commands/application_change_revision_controller.go @@ -61,6 +61,7 @@ func NewCommand() *cobra.Command { applicationNamespaces []string argocdToken string rootpath string + disableAnnotations bool ) command := &cobra.Command{ Use: cliName, @@ -114,6 +115,7 @@ func NewCommand() *cobra.Command { RedisClient: redisClient, ApplicationNamespaces: applicationNamespaces, ApplicationServiceClient: getApplicationClient(applicationServerAddress, argocdToken, rootpath), + DisableAnnotations: disableAnnotations, } log.Info("Starting change revision controller server") @@ -145,6 +147,7 @@ func NewCommand() *cobra.Command { command.Flags().IntVar(&listenPort, "port", common.DefaultPortACRServer, "Listen on given port") command.Flags().StringVar(&contentSecurityPolicy, "content-security-policy", env.StringFromEnv("ACR_CONTROLLER_CONTENT_SECURITY_POLICY", "frame-ancestors 'self';"), "Set Content-Security-Policy header in HTTP responses to `value`. To disable, set to \"\".") command.Flags().StringSliceVar(&applicationNamespaces, "application-namespaces", env.StringsFromEnv("ARGOCD_APPLICATION_NAMESPACES", []string{}, ","), "List of additional namespaces where application resources can be managed in") + command.Flags().BoolVar(&disableAnnotations, "disable-annotations", env.ParseBoolFromEnv("ACR_CONTROLLER_DISABLE_AMMOTATIONS", false), "Disable generating the monorepo-controller compatible annotations") cacheSrc = servercache.AddCacheFlagsToCmd(command, cacheutil.Options{ OnClientCreated: func(client *redis.Client) { redisClient = client From 65b0e7e03a82bb1b87ed8135d77ad53c718fc0d1 Mon Sep 17 00:00:00 2001 From: Eugene Doudine Date: Mon, 29 Sep 2025 03:05:42 +0300 Subject: [PATCH 2/2] test: improved acr_service tests Improved to check annotations, annotations feature flag and handle more error/no-op conditions Signed-off-by: Eugene Doudine --- acr_controller/service/acr_service.go | 6 +- acr_controller/service/acr_service_test.go | 112 +++++++++++++++------ 2 files changed, 85 insertions(+), 33 deletions(-) diff --git a/acr_controller/service/acr_service.go b/acr_controller/service/acr_service.go index 81013efa0ab90..5eb2f13fe58ce 100644 --- a/acr_controller/service/acr_service.go +++ b/acr_controller/service/acr_service.go @@ -83,7 +83,7 @@ func (c *acrService) ChangeRevision(ctx context.Context, a *application.Applicat currentRevision, previousRevision := c.getRevisions(ctx, a) if currentRevision == "" { - c.logger.Infof("Got empty revision for application %s, is it an unsupported multisource or helm repo based application?", app.Name) + c.logger.Infof("Got empty current revision for application %s, is it an unsupported multisource or helm repo based application?", app.Name) return nil } revision, err := c.calculateRevision(ctx, app, currentRevision, previousRevision) @@ -122,7 +122,7 @@ func (c *acrService) ChangeRevision(ctx context.Context, a *application.Applicat } } if len(patchMap) > 0 { - c.logger.Infof("patching resource: %v", patchMap) + c.logger.Infof("Patching resource: %v", patchMap) patch, err := json.Marshal(patchMap) if err != nil { return err @@ -130,7 +130,7 @@ func (c *acrService) ChangeRevision(ctx context.Context, a *application.Applicat _, err = c.applicationClientset.ArgoprojV1alpha1().Applications(a.Namespace).Patch(ctx, a.Name, types.MergePatchType, patch, metav1.PatchOptions{}) return err } - c.logger.Infof("no patch needed") + c.logger.Infof("No patch needed") return nil } diff --git a/acr_controller/service/acr_service_test.go b/acr_controller/service/acr_service_test.go index 7b8718b5a8fda..e7d3b67f00333 100644 --- a/acr_controller/service/acr_service_test.go +++ b/acr_controller/service/acr_service_test.go @@ -38,6 +38,26 @@ spec: server: https://cluster-api.example.com ` +const multisourceApp = ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + annotations: + argocd.argoproj.io/manifest-generate-paths: . + name: guestbook + namespace: codefresh +spec: + sources: + - path: some/path + repoURL: https://github.com/argoproj/argocd-example-apps.git + targetRevision: HEAD + ksonnet: + environment: default + destination: + namespace: ` + test.FakeDestNamespace + ` + server: https://cluster-api.example.com +` + const fakeAppWithOperation = ` apiVersion: argoproj.io/v1alpha1 kind: Application @@ -266,53 +286,85 @@ func Test_getRevisions(r *testing.T) { } func Test_ChangeRevision(r *testing.T) { - r.Run("Change revision", func(t *testing.T) { - client := &mocks.ApplicationClient{} - client.On("GetChangeRevision", mock.Anything, mock.Anything).Return(&appclient.ChangeRevisionResponse{ - Revision: ptr.To("new-revision"), - }, nil) - acrService := newTestACRService(client) - app := createTestApp(syncedAppWithHistory) - - err := acrService.ChangeRevision(r.Context(), app, false) - require.NoError(t, err) - - app, err = acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) - require.NoError(t, err) - - assert.Equal(t, "new-revision", app.Status.OperationState.Operation.Sync.ChangeRevision) - }) + newChangeRevision := "new-revision" + gitRevision := "c732f4d2ef24c7eeb900e9211ff98f90bb646505" + useAnnotations := true + appSrc := syncedAppWithHistory - r.Run("Change revision already exists", func(t *testing.T) { + testNewChangeRevision := func(t *testing.T) (*acrService, *test2.Hook, *appsv1.Application) { + t.Helper() client := &mocks.ApplicationClient{} client.On("GetChangeRevision", mock.Anything, mock.Anything).Return(&appclient.ChangeRevisionResponse{ - Revision: ptr.To("new-revision"), + Revision: ptr.To(newChangeRevision), }, nil) - logger, logHook := test2.NewNullLogger() - acrService := newTestACRService(client) acrService.logger = logger + app := createTestApp(appSrc) + orgManifestPath := app.GetAnnotation(appsv1.AnnotationKeyManifestGeneratePaths) + err := acrService.ChangeRevision(r.Context(), app, useAnnotations) + require.NoError(t, err) + assert.Equal(t, orgManifestPath, app.GetAnnotation(appsv1.AnnotationKeyManifestGeneratePaths)) + return acrService, logHook, app + } - app := createTestApp(syncedAppWithHistory) + testLastLogEntry := func(t *testing.T, hook *test2.Hook, expectedMsg string) { + t.Helper() + lastLogEntry := hook.LastEntry() + if lastLogEntry == nil { + t.Fatal("No log entry") + } + require.Equal(t, expectedMsg, lastLogEntry.Message) + } - err := acrService.ChangeRevision(r.Context(), app, false) + r.Run("New change revision with annotations", func(t *testing.T) { + acrService, _, app := testNewChangeRevision(t) + app, err := acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) require.NoError(t, err) + assert.Len(t, app.GetAnnotations(), 5) + assert.Equal(t, newChangeRevision, app.Status.OperationState.Operation.Sync.ChangeRevision) + assert.Equal(t, newChangeRevision, app.GetAnnotation(CHANGE_REVISION_ANN)) + assert.Equal(t, "[\""+"new-revision"+"\"]", app.GetAnnotation(CHANGE_REVISIONS_ANN)) + assert.Equal(t, gitRevision, app.GetAnnotation(GIT_REVISION_ANN)) + assert.Equal(t, "[\""+gitRevision+"\"]", app.GetAnnotation(GIT_REVISIONS_ANN)) + }) - app, err = acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) + r.Run("Change revision already exists with annotations", func(t *testing.T) { + acrService, logHook, app := testNewChangeRevision(t) + app, err := acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) + require.NoError(t, err) + err = acrService.ChangeRevision(r.Context(), app, useAnnotations) require.NoError(t, err) + testLastLogEntry(t, logHook, "Change revision already calculated for application guestbook") + }) - assert.Equal(t, "new-revision", app.Status.OperationState.Operation.Sync.ChangeRevision) + useAnnotations = false - err = acrService.ChangeRevision(r.Context(), app, false) + r.Run("New change revision without annotations", func(t *testing.T) { + acrService, _, app := testNewChangeRevision(t) + app, err := acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) + require.NoError(t, err) + assert.Len(t, app.GetAnnotations(), 1) + assert.Equal(t, newChangeRevision, app.Status.OperationState.Operation.Sync.ChangeRevision) + }) + appSrc = syncedAppWithSingleHistory + newChangeRevision = "" + r.Run("No previous revision", func(t *testing.T) { + acrService, logHook, app := testNewChangeRevision(t) + app, err := acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) require.NoError(t, err) + assert.Len(t, app.GetAnnotations(), 1) + testLastLogEntry(t, logHook, "No patch needed") + }) - lastLogEntry := logHook.LastEntry() - if lastLogEntry == nil { - t.Fatal("No log entry") - } + appSrc = multisourceApp - require.Equal(t, "Change revision already calculated for application guestbook", lastLogEntry.Message) + r.Run("No current revision", func(t *testing.T) { + acrService, logHook, app := testNewChangeRevision(t) + app, err := acrService.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(r.Context(), app.Name, metav1.GetOptions{}) + require.NoError(t, err) + assert.Len(t, app.GetAnnotations(), 1) + testLastLogEntry(t, logHook, "Got empty current revision for application guestbook, is it an unsupported multisource or helm repo based application?") }) }