Skip to content

Commit 030a7be

Browse files
adriananeciadriananeci
authored
fix: Populate destination name when destination server is specified (#21063)
* Populate destination name when destination server is specified Signed-off-by: Adrian Aneci <[email protected]> * Lint nit Signed-off-by: Adrian Aneci <[email protected]> * Trigger CI Signed-off-by: Adrian Aneci <[email protected]> --------- Signed-off-by: Adrian Aneci <[email protected]>
1 parent 8dbddb1 commit 030a7be

File tree

11 files changed

+169
-87
lines changed

11 files changed

+169
-87
lines changed

‎applicationset/controllers/applicationset_controller_test.go

+54-64
Original file line numberDiff line numberDiff line change
@@ -1894,12 +1894,6 @@ func TestValidateGeneratedApplications(t *testing.T) {
18941894
err := v1alpha1.AddToScheme(scheme)
18951895
require.NoError(t, err)
18961896

1897-
// Valid cluster
1898-
myCluster := v1alpha1.Cluster{
1899-
Server: "https://kubernetes.default.svc",
1900-
Name: "my-cluster",
1901-
}
1902-
19031897
// Valid project
19041898
myProject := &v1alpha1.AppProject{
19051899
ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "namespace"},
@@ -2066,18 +2060,12 @@ func TestValidateGeneratedApplications(t *testing.T) {
20662060
objects := append([]runtime.Object{}, secret)
20672061
kubeclientset := kubefake.NewSimpleClientset(objects...)
20682062

2069-
argoDBMock := dbmocks.ArgoDB{}
2070-
argoDBMock.On("GetCluster", mock.Anything, "https://kubernetes.default.svc").Return(&myCluster, nil)
2071-
argoDBMock.On("ListClusters", mock.Anything).Return(&v1alpha1.ClusterList{Items: []v1alpha1.Cluster{
2072-
myCluster,
2073-
}}, nil)
2074-
20752063
r := ApplicationSetReconciler{
20762064
Client: client,
20772065
Scheme: scheme,
20782066
Recorder: record.NewFakeRecorder(1),
20792067
Generators: map[string]generators.Generator{},
2080-
ArgoDB: &argoDBMock,
2068+
ArgoDB: &dbmocks.ArgoDB{},
20812069
ArgoCDNamespace: "namespace",
20822070
KubeClientset: kubeclientset,
20832071
Metrics: metrics,
@@ -2159,17 +2147,9 @@ func TestReconcilerValidationProjectErrorBehaviour(t *testing.T) {
21592147
}
21602148

21612149
kubeclientset := kubefake.NewSimpleClientset()
2162-
argoDBMock := dbmocks.ArgoDB{}
21632150

21642151
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet, &project).WithStatusSubresource(&appSet).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).Build()
21652152
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
2166-
goodCluster := v1alpha1.Cluster{Server: "https://good-cluster", Name: "good-cluster"}
2167-
badCluster := v1alpha1.Cluster{Server: "https://bad-cluster", Name: "bad-cluster"}
2168-
argoDBMock.On("GetCluster", mock.Anything, "https://good-cluster").Return(&goodCluster, nil)
2169-
argoDBMock.On("GetCluster", mock.Anything, "https://bad-cluster").Return(&badCluster, nil)
2170-
argoDBMock.On("ListClusters", mock.Anything).Return(&v1alpha1.ClusterList{Items: []v1alpha1.Cluster{
2171-
goodCluster,
2172-
}}, nil)
21732153

21742154
r := ApplicationSetReconciler{
21752155
Client: client,
@@ -2179,7 +2159,7 @@ func TestReconcilerValidationProjectErrorBehaviour(t *testing.T) {
21792159
Generators: map[string]generators.Generator{
21802160
"List": generators.NewListGenerator(),
21812161
},
2182-
ArgoDB: &argoDBMock,
2162+
ArgoDB: &dbmocks.ArgoDB{},
21832163
KubeClientset: kubeclientset,
21842164
Policy: v1alpha1.ApplicationsSyncPolicySync,
21852165
ArgoCDNamespace: "argocd",
@@ -2363,7 +2343,6 @@ func TestSetApplicationSetStatusCondition(t *testing.T) {
23632343
}
23642344

23652345
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
2366-
argoDBMock := dbmocks.ArgoDB{}
23672346

23682347
for _, testCase := range testCases {
23692348
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&testCase.appset).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).WithStatusSubresource(&testCase.appset).Build()
@@ -2377,7 +2356,7 @@ func TestSetApplicationSetStatusCondition(t *testing.T) {
23772356
Generators: map[string]generators.Generator{
23782357
"List": generators.NewListGenerator(),
23792358
},
2380-
ArgoDB: &argoDBMock,
2359+
ArgoDB: &dbmocks.ArgoDB{},
23812360
KubeClientset: kubeclientset,
23822361
Metrics: metrics,
23832362
}
@@ -2433,16 +2412,28 @@ func applicationsUpdateSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
24332412
},
24342413
}
24352414

2436-
kubeclientset := kubefake.NewSimpleClientset()
2437-
argoDBMock := dbmocks.ArgoDB{}
2415+
secret := &corev1.Secret{
2416+
ObjectMeta: metav1.ObjectMeta{
2417+
Name: "my-cluster",
2418+
Namespace: "argocd",
2419+
Labels: map[string]string{
2420+
argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster,
2421+
},
2422+
},
2423+
Data: map[string][]byte{
2424+
// Since this test requires the cluster to be an invalid destination, we
2425+
// always return a cluster named 'my-cluster2' (different from app 'my-cluster', above)
2426+
"name": []byte("good-cluster"),
2427+
"server": []byte("https://good-cluster"),
2428+
"config": []byte("{\"username\":\"foo\",\"password\":\"foo\"}"),
2429+
},
2430+
}
2431+
2432+
objects := append([]runtime.Object{}, secret)
2433+
kubeclientset := kubefake.NewSimpleClientset(objects...)
24382434

24392435
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet, &defaultProject).WithStatusSubresource(&appSet).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).Build()
24402436
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
2441-
goodCluster := v1alpha1.Cluster{Server: "https://good-cluster", Name: "good-cluster"}
2442-
argoDBMock.On("GetCluster", mock.Anything, "https://good-cluster").Return(&goodCluster, nil)
2443-
argoDBMock.On("ListClusters", mock.Anything).Return(&v1alpha1.ClusterList{Items: []v1alpha1.Cluster{
2444-
goodCluster,
2445-
}}, nil)
24462437

24472438
r := ApplicationSetReconciler{
24482439
Client: client,
@@ -2452,7 +2443,7 @@ func applicationsUpdateSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
24522443
Generators: map[string]generators.Generator{
24532444
"List": generators.NewListGenerator(),
24542445
},
2455-
ArgoDB: &argoDBMock,
2446+
ArgoDB: &dbmocks.ArgoDB{},
24562447
ArgoCDNamespace: "argocd",
24572448
KubeClientset: kubeclientset,
24582449
Policy: v1alpha1.ApplicationsSyncPolicySync,
@@ -2595,16 +2586,28 @@ func applicationsDeleteSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
25952586
},
25962587
}
25972588

2598-
kubeclientset := kubefake.NewSimpleClientset()
2599-
argoDBMock := dbmocks.ArgoDB{}
2589+
secret := &corev1.Secret{
2590+
ObjectMeta: metav1.ObjectMeta{
2591+
Name: "my-cluster",
2592+
Namespace: "argocd",
2593+
Labels: map[string]string{
2594+
argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster,
2595+
},
2596+
},
2597+
Data: map[string][]byte{
2598+
// Since this test requires the cluster to be an invalid destination, we
2599+
// always return a cluster named 'my-cluster2' (different from app 'my-cluster', above)
2600+
"name": []byte("good-cluster"),
2601+
"server": []byte("https://good-cluster"),
2602+
"config": []byte("{\"username\":\"foo\",\"password\":\"foo\"}"),
2603+
},
2604+
}
2605+
2606+
objects := append([]runtime.Object{}, secret)
2607+
kubeclientset := kubefake.NewSimpleClientset(objects...)
26002608

26012609
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet, &defaultProject).WithStatusSubresource(&appSet).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).Build()
26022610
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
2603-
goodCluster := v1alpha1.Cluster{Server: "https://good-cluster", Name: "good-cluster"}
2604-
argoDBMock.On("GetCluster", mock.Anything, "https://good-cluster").Return(&goodCluster, nil)
2605-
argoDBMock.On("ListClusters", mock.Anything).Return(&v1alpha1.ClusterList{Items: []v1alpha1.Cluster{
2606-
goodCluster,
2607-
}}, nil)
26082611

26092612
r := ApplicationSetReconciler{
26102613
Client: client,
@@ -2614,7 +2617,7 @@ func applicationsDeleteSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
26142617
Generators: map[string]generators.Generator{
26152618
"List": generators.NewListGenerator(),
26162619
},
2617-
ArgoDB: &argoDBMock,
2620+
ArgoDB: &dbmocks.ArgoDB{},
26182621
ArgoCDNamespace: "argocd",
26192622
KubeClientset: kubeclientset,
26202623
Policy: v1alpha1.ApplicationsSyncPolicySync,
@@ -2689,23 +2692,23 @@ func TestDeletePerformedWithSyncPolicyCreateDelete(t *testing.T) {
26892692

26902693
apps := applicationsDeleteSyncPolicyTest(t, applicationsSyncPolicy, 3, true)
26912694

2692-
assert.Empty(t, apps.Items)
2695+
assert.NotNil(t, apps.Items[0].DeletionTimestamp)
26932696
}
26942697

26952698
func TestDeletePerformedWithSyncPolicySync(t *testing.T) {
26962699
applicationsSyncPolicy := v1alpha1.ApplicationsSyncPolicySync
26972700

26982701
apps := applicationsDeleteSyncPolicyTest(t, applicationsSyncPolicy, 3, true)
26992702

2700-
assert.Empty(t, apps.Items)
2703+
assert.NotNil(t, apps.Items[0].DeletionTimestamp)
27012704
}
27022705

27032706
func TestDeletePerformedWithSyncPolicyCreateOnlyAndAllowPolicyOverrideFalse(t *testing.T) {
27042707
applicationsSyncPolicy := v1alpha1.ApplicationsSyncPolicyCreateOnly
27052708

27062709
apps := applicationsDeleteSyncPolicyTest(t, applicationsSyncPolicy, 3, false)
27072710

2708-
assert.Empty(t, apps.Items)
2711+
assert.NotNil(t, apps.Items[0].DeletionTimestamp)
27092712
}
27102713

27112714
func TestPolicies(t *testing.T) {
@@ -2717,14 +2720,8 @@ func TestPolicies(t *testing.T) {
27172720
ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "argocd"},
27182721
Spec: v1alpha1.AppProjectSpec{SourceRepos: []string{"*"}, Destinations: []v1alpha1.ApplicationDestination{{Namespace: "*", Server: "https://kubernetes.default.svc"}}},
27192722
}
2720-
myCluster := v1alpha1.Cluster{
2721-
Server: "https://kubernetes.default.svc",
2722-
Name: "my-cluster",
2723-
}
27242723

27252724
kubeclientset := kubefake.NewSimpleClientset()
2726-
argoDBMock := dbmocks.ArgoDB{}
2727-
argoDBMock.On("GetCluster", mock.Anything, "https://kubernetes.default.svc").Return(&myCluster, nil)
27282725

27292726
for _, c := range []struct {
27302727
name string
@@ -2807,7 +2804,7 @@ func TestPolicies(t *testing.T) {
28072804
Generators: map[string]generators.Generator{
28082805
"List": generators.NewListGenerator(),
28092806
},
2810-
ArgoDB: &argoDBMock,
2807+
ArgoDB: &dbmocks.ArgoDB{},
28112808
ArgoCDNamespace: "argocd",
28122809
KubeClientset: kubeclientset,
28132810
Policy: policy,
@@ -2881,7 +2878,6 @@ func TestSetApplicationSetApplicationStatus(t *testing.T) {
28812878
require.NoError(t, err)
28822879

28832880
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
2884-
argoDBMock := dbmocks.ArgoDB{}
28852881

28862882
for _, cc := range []struct {
28872883
name string
@@ -2965,7 +2961,7 @@ func TestSetApplicationSetApplicationStatus(t *testing.T) {
29652961
Generators: map[string]generators.Generator{
29662962
"List": generators.NewListGenerator(),
29672963
},
2968-
ArgoDB: &argoDBMock,
2964+
ArgoDB: &dbmocks.ArgoDB{},
29692965
KubeClientset: kubeclientset,
29702966
Metrics: metrics,
29712967
}
@@ -3714,14 +3710,13 @@ func TestBuildAppDependencyList(t *testing.T) {
37143710
} {
37153711
t.Run(cc.name, func(t *testing.T) {
37163712
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
3717-
argoDBMock := dbmocks.ArgoDB{}
37183713

37193714
r := ApplicationSetReconciler{
37203715
Client: client,
37213716
Scheme: scheme,
37223717
Recorder: record.NewFakeRecorder(1),
37233718
Generators: map[string]generators.Generator{},
3724-
ArgoDB: &argoDBMock,
3719+
ArgoDB: &dbmocks.ArgoDB{},
37253720
KubeClientset: kubeclientset,
37263721
Metrics: metrics,
37273722
}
@@ -4381,14 +4376,13 @@ func TestBuildAppSyncMap(t *testing.T) {
43814376
} {
43824377
t.Run(cc.name, func(t *testing.T) {
43834378
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
4384-
argoDBMock := dbmocks.ArgoDB{}
43854379

43864380
r := ApplicationSetReconciler{
43874381
Client: client,
43884382
Scheme: scheme,
43894383
Recorder: record.NewFakeRecorder(1),
43904384
Generators: map[string]generators.Generator{},
4391-
ArgoDB: &argoDBMock,
4385+
ArgoDB: &dbmocks.ArgoDB{},
43924386
KubeClientset: kubeclientset,
43934387
Metrics: metrics,
43944388
}
@@ -5326,7 +5320,6 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
53265320
} {
53275321
t.Run(cc.name, func(t *testing.T) {
53285322
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
5329-
argoDBMock := dbmocks.ArgoDB{}
53305323

53315324
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cc.appSet).WithStatusSubresource(&cc.appSet).Build()
53325325
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
@@ -5336,7 +5329,7 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
53365329
Scheme: scheme,
53375330
Recorder: record.NewFakeRecorder(1),
53385331
Generators: map[string]generators.Generator{},
5339-
ArgoDB: &argoDBMock,
5332+
ArgoDB: &dbmocks.ArgoDB{},
53405333
KubeClientset: kubeclientset,
53415334
Metrics: metrics,
53425335
}
@@ -6075,7 +6068,6 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) {
60756068
} {
60766069
t.Run(cc.name, func(t *testing.T) {
60776070
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
6078-
argoDBMock := dbmocks.ArgoDB{}
60796071

60806072
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cc.appSet).WithStatusSubresource(&cc.appSet).Build()
60816073
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
@@ -6085,7 +6077,7 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) {
60856077
Scheme: scheme,
60866078
Recorder: record.NewFakeRecorder(1),
60876079
Generators: map[string]generators.Generator{},
6088-
ArgoDB: &argoDBMock,
6080+
ArgoDB: &dbmocks.ArgoDB{},
60896081
KubeClientset: kubeclientset,
60906082
Metrics: metrics,
60916083
}
@@ -6286,7 +6278,6 @@ func TestUpdateResourceStatus(t *testing.T) {
62866278
} {
62876279
t.Run(cc.name, func(t *testing.T) {
62886280
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
6289-
argoDBMock := dbmocks.ArgoDB{}
62906281

62916282
client := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&cc.appSet).WithObjects(&cc.appSet).Build()
62926283
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
@@ -6296,7 +6287,7 @@ func TestUpdateResourceStatus(t *testing.T) {
62966287
Scheme: scheme,
62976288
Recorder: record.NewFakeRecorder(1),
62986289
Generators: map[string]generators.Generator{},
6299-
ArgoDB: &argoDBMock,
6290+
ArgoDB: &dbmocks.ArgoDB{},
63006291
KubeClientset: kubeclientset,
63016292
Metrics: metrics,
63026293
}
@@ -6376,7 +6367,6 @@ func TestResourceStatusAreOrdered(t *testing.T) {
63766367
} {
63776368
t.Run(cc.name, func(t *testing.T) {
63786369
kubeclientset := kubefake.NewSimpleClientset([]runtime.Object{}...)
6379-
argoDBMock := dbmocks.ArgoDB{}
63806370

63816371
client := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&cc.appSet).WithObjects(&cc.appSet).Build()
63826372
metrics := appsetmetrics.NewFakeAppsetMetrics(client)
@@ -6386,7 +6376,7 @@ func TestResourceStatusAreOrdered(t *testing.T) {
63866376
Scheme: scheme,
63876377
Recorder: record.NewFakeRecorder(1),
63886378
Generators: map[string]generators.Generator{},
6389-
ArgoDB: &argoDBMock,
6379+
ArgoDB: &dbmocks.ArgoDB{},
63906380
KubeClientset: kubeclientset,
63916381
Metrics: metrics,
63926382
}

‎applicationset/utils/clusterUtils.go

+22-5
Original file line numberDiff line numberDiff line change
@@ -51,24 +51,38 @@ const (
5151
// if we used destination name we infer the server url
5252
// if we used both name and server then we return an invalid spec error
5353
func ValidateDestination(ctx context.Context, dest *appv1.ApplicationDestination, clientset kubernetes.Interface, argoCDNamespace string) error {
54+
if dest.IsServerInferred() && dest.IsNameInferred() {
55+
return fmt.Errorf("application destination can't have both name and server inferred: %s %s", dest.Name, dest.Server)
56+
}
5457
if dest.Name != "" {
5558
if dest.Server == "" {
56-
server, err := getDestinationServer(ctx, dest.Name, clientset, argoCDNamespace)
59+
server, err := getDestinationBy(ctx, dest.Name, clientset, argoCDNamespace, true)
5760
if err != nil {
5861
return fmt.Errorf("unable to find destination server: %w", err)
5962
}
6063
if server == "" {
6164
return fmt.Errorf("application references destination cluster %s which does not exist", dest.Name)
6265
}
6366
dest.SetInferredServer(server)
64-
} else if !dest.IsServerInferred() {
67+
} else if !dest.IsServerInferred() && !dest.IsNameInferred() {
6568
return fmt.Errorf("application destination can't have both name and server defined: %s %s", dest.Name, dest.Server)
6669
}
70+
} else if dest.Server != "" {
71+
if dest.Name == "" {
72+
serverName, err := getDestinationBy(ctx, dest.Server, clientset, argoCDNamespace, false)
73+
if err != nil {
74+
return fmt.Errorf("unable to find destination server: %w", err)
75+
}
76+
if serverName == "" {
77+
return fmt.Errorf("application references destination cluster %s which does not exist", dest.Server)
78+
}
79+
dest.SetInferredName(serverName)
80+
}
6781
}
6882
return nil
6983
}
7084

71-
func getDestinationServer(ctx context.Context, clusterName string, clientset kubernetes.Interface, argoCDNamespace string) (string, error) {
85+
func getDestinationBy(ctx context.Context, cluster string, clientset kubernetes.Interface, argoCDNamespace string, byName bool) (string, error) {
7286
// settingsMgr := settings.NewSettingsManager(context.TODO(), clientset, namespace)
7387
// argoDB := db.NewDB(namespace, settingsMgr, clientset)
7488
// clusterList, err := argoDB.ListClusters(ctx)
@@ -78,14 +92,17 @@ func getDestinationServer(ctx context.Context, clusterName string, clientset kub
7892
}
7993
var servers []string
8094
for _, c := range clusterList.Items {
81-
if c.Name == clusterName {
95+
if byName && c.Name == cluster {
8296
servers = append(servers, c.Server)
8397
}
98+
if !byName && c.Server == cluster {
99+
servers = append(servers, c.Name)
100+
}
84101
}
85102
if len(servers) > 1 {
86103
return "", fmt.Errorf("there are %d clusters with the same name: %v", len(servers), servers)
87104
} else if len(servers) == 0 {
88-
return "", fmt.Errorf("there are no clusters with this name: %s", clusterName)
105+
return "", fmt.Errorf("there are no clusters with this name: %s", cluster)
89106
}
90107
return servers[0], nil
91108
}

0 commit comments

Comments
 (0)