Skip to content

Commit

Permalink
apply recommandations
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
@mmorel-35
mmorel-35 committed Jan 6, 2025
1 parent 143402d commit 61b2da7
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 52 deletions.
6 changes: 3 additions & 3 deletions controller/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
}
gvk := obj.GroupVersionKind()

isSelfReferencedObj := m.isSelfReferencedObj(liveObj, targetObj, app.GetName(), appLabelKey, trackingMethod, installationID)
isSelfReferencedObj := m.isSelfReferencedObj(liveObj, targetObj, app.GetName(), trackingMethod, installationID)

resState := v1alpha1.ResourceStatus{
Namespace: obj.GetNamespace(),
Expand Down Expand Up @@ -1108,7 +1108,7 @@ func NewAppStateManager(
// group and kind) match the properties of the live object, or if the tracking method
// used does not provide the required properties for matching.
// Reference: https://github.com/argoproj/argo-cd/issues/8683
func (m *appStateManager) isSelfReferencedObj(live, config *unstructured.Unstructured, appName, appLabelKey string, trackingMethod v1alpha1.TrackingMethod, installationID string) bool {
func (m *appStateManager) isSelfReferencedObj(live, config *unstructured.Unstructured, appName string, trackingMethod v1alpha1.TrackingMethod, installationID string) bool {
if live == nil {
return true
}
Expand Down Expand Up @@ -1141,7 +1141,7 @@ func (m *appStateManager) isSelfReferencedObj(live, config *unstructured.Unstruc
// to match the properties from the live object. Cluster scoped objects
// carry the app's destination namespace in the tracking annotation,
// but are unique in GVK + name combination.
appInstance := m.resourceTracking.GetAppInstance(live, appLabelKey, trackingMethod, installationID)
appInstance := m.resourceTracking.GetAppInstance(live, trackingMethod, installationID)
if appInstance != nil {
return isSelfReferencedObj(live, *appInstance)
}
Expand Down
26 changes: 13 additions & 13 deletions controller/state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1410,52 +1410,52 @@ func TestIsLiveResourceManaged(t *testing.T) {
configObj := managedObj.DeepCopy()

// then
assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, argo.TrackingMethodLabel, ""))
assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, argo.TrackingMethodAnnotation, ""))
})
t.Run("will return true if tracked with label", func(t *testing.T) {
// given
t.Parallel()
configObj := managedObjWithLabel.DeepCopy()

// then
assert.True(t, manager.isSelfReferencedObj(managedObjWithLabel, configObj, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.True(t, manager.isSelfReferencedObj(managedObjWithLabel, configObj, appName, argo.TrackingMethodLabel, ""))
})
t.Run("will handle if trackingId has wrong resource name and config is nil", func(t *testing.T) {
// given
t.Parallel()

// then
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, argo.TrackingMethodAnnotation, ""))
})
t.Run("will handle if trackingId has wrong resource group and config is nil", func(t *testing.T) {
// given
t.Parallel()

// then
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, argo.TrackingMethodAnnotation, ""))
})
t.Run("will handle if trackingId has wrong kind and config is nil", func(t *testing.T) {
// given
t.Parallel()

// then
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, argo.TrackingMethodAnnotation, ""))
})
t.Run("will handle if trackingId has wrong namespace and config is nil", func(t *testing.T) {
// given
t.Parallel()

// then
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotationAndLabel, ""))
assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, argo.TrackingMethodLabel, ""))
assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, argo.TrackingMethodAnnotationAndLabel, ""))
})
t.Run("will return true if live is nil", func(t *testing.T) {
t.Parallel()
assert.True(t, manager.isSelfReferencedObj(nil, nil, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(nil, nil, appName, argo.TrackingMethodAnnotation, ""))
})

t.Run("will handle upgrade in desired state APIGroup", func(t *testing.T) {
Expand All @@ -1465,7 +1465,7 @@ func TestIsLiveResourceManaged(t *testing.T) {
delete(config.GetAnnotations(), common.AnnotationKeyAppInstance)

// then
assert.True(t, manager.isSelfReferencedObj(managedWrongAPIGroup, config, appName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation, ""))
assert.True(t, manager.isSelfReferencedObj(managedWrongAPIGroup, config, appName, argo.TrackingMethodAnnotation, ""))
})
}

Expand Down
7 changes: 1 addition & 6 deletions controller/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,6 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
reconciliationResult.Target = patchedTargets
}

appLabelKey, err := m.settingsMgr.GetAppInstanceLabelKey()
if err != nil {
log.Errorf("Could not get appInstanceLabelKey: %v", err)
return
}
installationID, err := m.settingsMgr.GetInstallationID()
if err != nil {
log.Errorf("Could not get installation ID: %v", err)
Expand Down Expand Up @@ -368,7 +363,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
return (len(syncOp.Resources) == 0 ||
isPostDeleteHook(target) ||
argo.ContainsSyncResource(key.Name, key.Namespace, schema.GroupVersionKind{Kind: key.Kind, Group: key.Group}, syncOp.Resources)) &&
m.isSelfReferencedObj(live, target, app.GetName(), appLabelKey, trackingMethod, installationID)
m.isSelfReferencedObj(live, target, app.GetName(), trackingMethod, installationID)
}),
sync.WithManifestValidation(!syncOp.SyncOptions.HasOption(common.SyncOptionsDisableValidation)),
sync.WithSyncWaveHook(delayBetweenSyncWaves),
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/fixture/cluster/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (a *Actions) DoNotIgnoreErrors() *Actions {
return a
}

func (a *Actions) Create(_ ...string) *Actions {
func (a *Actions) Create() *Actions {
_, clusterClient, _ := fixture.ArgoCDClientset.NewClusterClient()

_, err := clusterClient.Create(context.Background(), &clusterpkg.ClusterCreateRequest{
Expand Down Expand Up @@ -67,7 +67,7 @@ func (a *Actions) Create(_ ...string) *Actions {
return a
}

func (a *Actions) CreateWithRBAC(_ ...string) *Actions {
func (a *Actions) CreateWithRBAC() *Actions {
pathOpts := clientcmd.NewDefaultPathOptions()
config, err := pathOpts.GetStartingConfig()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion test/testdata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func NewFakeConfigMap() *corev1.ConfigMap {
return &cm
}

func NewFakeSecret(_ ...string) *corev1.Secret {
func NewFakeSecret() *corev1.Secret {
secret := corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
Expand Down
4 changes: 2 additions & 2 deletions util/argo/argo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ func GetAppProjectWithScopedResources(name string, projLister applicationsv1.App
if err != nil {
return nil, nil, nil, fmt.Errorf("error getting project clusters: %w", err)
}
repos, err := db.GetProjectRepositories(ctx, name)
repos, err := db.GetProjectRepositories(name)
if err != nil {
return nil, nil, nil, fmt.Errorf("error getting project repos: %w", err)
}
Expand All @@ -721,7 +721,7 @@ func GetAppProjectByName(name string, projLister applicationsv1.AppProjectLister
return nil, fmt.Errorf("error getting app project %q: %w", name, err)
}
project := projOrig.DeepCopy()
repos, err := db.GetProjectRepositories(ctx, name)
repos, err := db.GetProjectRepositories(name)
if err != nil {
return nil, fmt.Errorf("error getting project repositories: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions util/argo/resource_tracking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ var (
// ResourceTracking defines methods which allow setup and retrieve tracking information to resource
type ResourceTracking interface {
GetAppName(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod, installationID string) string
GetAppInstance(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod, installationID string) *AppInstanceValue
GetAppInstance(un *unstructured.Unstructured, trackingMethod v1alpha1.TrackingMethod, installationID string) *AppInstanceValue
SetAppInstance(un *unstructured.Unstructured, key, val, namespace string, trackingMethod v1alpha1.TrackingMethod, instanceID string) error
BuildAppInstanceValue(value AppInstanceValue) string
ParseAppInstanceValue(value string) (*AppInstanceValue, error)
Expand Down Expand Up @@ -111,7 +111,7 @@ func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string
// GetAppInstance returns the representation of the app instance annotation.
// If the tracking method does not support metadata, or the annotation could
// not be parsed, it returns nil.
func (rt *resourceTracking) GetAppInstance(un *unstructured.Unstructured, _ string, trackingMethod v1alpha1.TrackingMethod, instanceID string) *AppInstanceValue {
func (rt *resourceTracking) GetAppInstance(un *unstructured.Unstructured, trackingMethod v1alpha1.TrackingMethod, instanceID string) *AppInstanceValue {
switch trackingMethod {
case TrackingMethodAnnotation, TrackingMethodAnnotationAndLabel:
return rt.getAppInstanceValue(un, instanceID)
Expand Down
4 changes: 2 additions & 2 deletions util/db/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type ArgoDB interface {
// GetRepository returns a repository by URL
GetRepository(ctx context.Context, url, project string) (*appv1.Repository, error)
// GetProjectRepositories returns project scoped repositories by given project name
GetProjectRepositories(ctx context.Context, project string) ([]*appv1.Repository, error)
GetProjectRepositories(project string) ([]*appv1.Repository, error)
// RepositoryExists returns whether a repository is configured for the given URL
RepositoryExists(ctx context.Context, repoURL, project string) (bool, error)
// UpdateRepository updates a repository
Expand All @@ -68,7 +68,7 @@ type ArgoDB interface {
// GetWriteRepository returns a repository by URL with write credentials
GetWriteRepository(ctx context.Context, url, project string) (*appv1.Repository, error)
// GetProjectWriteRepositories returns project scoped repositories from write credentials by given project name
GetProjectWriteRepositories(ctx context.Context, project string) ([]*appv1.Repository, error)
GetProjectWriteRepositories(project string) ([]*appv1.Repository, error)
// WriteRepositoryExists returns whether a repository is configured for the given URL with write credentials
WriteRepositoryExists(ctx context.Context, repoURL, project string) (bool, error)
// UpdateWriteRepository updates a repository with write credentials
Expand Down
36 changes: 18 additions & 18 deletions util/db/mocks/ArgoDB.go
View file

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

4 changes: 2 additions & 2 deletions util/db/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ func (db *db) GetWriteRepository(ctx context.Context, repoURL, project string) (
return repository, err
}

func (db *db) GetProjectRepositories(_ context.Context, project string) ([]*v1alpha1.Repository, error) {
func (db *db) GetProjectRepositories(project string) ([]*v1alpha1.Repository, error) {
return db.getRepositories(settings.ByProjectRepoIndexer, project)
}

func (db *db) GetProjectWriteRepositories(_ context.Context, project string) ([]*v1alpha1.Repository, error) {
func (db *db) GetProjectWriteRepositories(project string) ([]*v1alpha1.Repository, error) {
return db.getRepositories(settings.ByProjectRepoWriteIndexer, project)
}

Expand Down
2 changes: 1 addition & 1 deletion util/db/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func Test_GetProjectRepositories(t *testing.T) {
clientset := getClientset(map[string]string{}, repoSecretWithProject, repoSecretWithoutProject)
argoDB := NewDB(testNamespace, settings.NewSettingsManager(context.TODO(), clientset, testNamespace), clientset)

repos, err := argoDB.GetProjectRepositories(context.TODO(), "some-project")
repos, err := argoDB.GetProjectRepositories("some-project")
require.NoError(t, err)
assert.Len(t, repos, 1)
assert.Equal(t, "[email protected]:argoproj/argo-cd", repos[0].Repo)
Expand Down

0 comments on commit 61b2da7

Please sign in to comment.