Skip to content

Commit

Permalink
feat(server): make deep copies of objects returned by informers (#22173
Browse files Browse the repository at this point in the history
…) (#22179)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
  • Loading branch information
rumstead authored Mar 10, 2025
1 parent 922d080 commit e3bd569
Show file tree
Hide file tree
Showing 6 changed files with 689 additions and 17 deletions.
5 changes: 4 additions & 1 deletion .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,7 @@ packages:
SessionServiceClient:
github.com/argoproj/argo-cd/v3/pkg/apiclient/cluster:
interfaces:
ClusterServiceServer:
ClusterServiceServer:
github.com/argoproj/argo-cd/v3/pkg/client/clientset/versioned/typed/application/v1alpha1:
interfaces:
AppProjectInterface:

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 6 additions & 13 deletions server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import (
"github.com/argoproj/argo-cd/v3/reposerver/apiclient"
servercache "github.com/argoproj/argo-cd/v3/server/cache"
"github.com/argoproj/argo-cd/v3/server/deeplinks"
"github.com/argoproj/argo-cd/v3/util"
"github.com/argoproj/argo-cd/v3/util/argo"
"github.com/argoproj/argo-cd/v3/util/collections"
"github.com/argoproj/argo-cd/v3/util/db"
Expand Down Expand Up @@ -127,8 +126,8 @@ func NewServer(
}
s := &Server{
ns: namespace,
appclientset: appclientset,
appLister: appLister,
appclientset: &deepCopyAppClientset{appclientset},
appLister: &deepCopyApplicationLister{appLister},
appInformer: appInformer,
appBroadcaster: appBroadcaster,
kubeclientset: kubeclientset,
Expand Down Expand Up @@ -261,9 +260,7 @@ func (s *Server) getApplicationEnforceRBACClient(ctx context.Context, action, pr
if err != nil {
return nil, err
}
// Objects returned by the lister must be treated as read-only.
// To allow us to modify the app later, make a copy
return app.DeepCopy(), nil
return app, nil
})
}

Expand Down Expand Up @@ -380,9 +377,6 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to check existing application details (%s): %v", appNs, err)
}
// Objects returned from listers have to be treated as read-only
// Take a deep copy so we can edit it below
existing = existing.DeepCopy()

if _, err := argo.GetDestinationCluster(ctx, existing.Spec.Destination, s.db); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "application destination spec for %s is invalid: %s", existing.Name, err.Error())
Expand Down Expand Up @@ -491,7 +485,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan
}

sources := make([]v1alpha1.ApplicationSource, 0)
appSpec := a.Spec.DeepCopy()
appSpec := a.Spec
if a.Spec.HasMultipleSources() {
numOfSources := int64(len(a.Spec.GetSources()))
for i, pos := range q.SourcePositions {
Expand Down Expand Up @@ -888,7 +882,7 @@ func (s *Server) ListResourceEvents(ctx context.Context, q *application.Applicat
if err != nil {
return nil, fmt.Errorf("error listing resource events: %w", err)
}
return list, nil
return list.DeepCopy(), nil
}

// validateAndUpdateApp validates and updates the application. currentProject is the name of the project the app
Expand Down Expand Up @@ -1226,7 +1220,6 @@ func (s *Server) Watch(q *application.ApplicationQuery, ws application.Applicati
if err != nil {
return fmt.Errorf("error listing apps with selector: %w", err)
}
apps = util.SliceCopy(apps)
sort.Slice(apps, func(i, j int) bool {
return apps[i].QualifiedName() < apps[j].QualifiedName()
})
Expand Down Expand Up @@ -2330,7 +2323,7 @@ func (s *Server) TerminateOperation(ctx context.Context, termOpReq *application.
}
log.Warnf("failed to set operation for app %q due to update conflict. retrying again...", *termOpReq.Name)
time.Sleep(100 * time.Millisecond)
a, err = s.appclientset.ArgoprojV1alpha1().Applications(appNs).Get(ctx, appName, metav1.GetOptions{})
_, err = s.appclientset.ArgoprojV1alpha1().Applications(appNs).Get(ctx, appName, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("error getting application by name: %w", err)
}
Expand Down
Loading

0 comments on commit e3bd569

Please sign in to comment.