Repository: kubesphere/ks-devops
Pull request #681 (2022-06-20 10:37:03)

Support to copy argocd-related annotations and labels
<!-- Thanks for sending a pull request! Here are some tips for you:

1. If you want **faster** PR reviews, read how: https://github.com/kubesphere/community/blob/master/developer-guide/development/the-pr-author-guide-to-getting-through-code-review.md
2. In case you want to know how your PR got reviewed, read: https://github.com/kubesphere/community/blob/master/developer-guide/development/code-review-guide.md
3. Here are some coding conventions followed by the KubeSphere community: https://github.com/kubesphere/community/blob/master/developer-guide/development/coding-conventions.md
4. Additional open-source best practice: https://github.com/LinuxSuRen/open-source-best-practice
-->

### What type of PR is this?
<!-- 
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind design
/kind chore

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->
/kind feature

### What this PR does / why we need it:
Some Argo CD features rely on annotations. For example, the image updater relies on the annotation `argocd-image-updater.argoproj.io/image-list`.

This is part of #676

Please feel free to help us to test it via:
```
ghcr.io/linuxsuren/devops-controller:dev-v3.2.1-6803647
```

### Which issue(s) this PR fixes:
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
Please leave it or change # to be None if there is no corresponding issue that exists
-->
Fixes #

### Special notes for reviewers:
<!--
You can use the following command to let the DevOps SIG members help you to review your PR.
/cc @kubesphere/sig-devops 
And please avoid cc any individual.
-->
Please check the following list before waiting reviewers:

- [ ] Already committed the CRD files to [the Helm Chart](https://github.com/kubesphere-sigs/ks-devops-helm-chart/) if you created some new CRDs
- [ ] Already [added the permission](https://github.com/kubesphere/ks-installer/blob/9e063b085a0e43fdb3d0d9e3e7f4149146f14b9c/roles/ks-core/prepare/files/ks-init/role-templates.yaml) for the new API
- [ ] Already added the RBAC markers for the new controllers

### Does this PR introduce a user-facing change??
<!--
If no, just write "None" in the release-note block below.
If yes, a release note is required:
Enter your extended-release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

Please keep the note be same as your PR title if you believe it should be in the release notes.
-->
```release-note
None
```


ks-ci-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **linuxsuren** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubesphere/ks-devops/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["linuxsuren"]} -->

Patch:
diff --git a/controllers/argocd/application_controller.go b/controllers/argocd/application_controller.go
index ca2b0b3..d2e0a13 100644
--- a/controllers/argocd/application_controller.go
+++ b/controllers/argocd/application_controller.go
@@ -22,6 +22,7 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
@@ -154,6 +155,10 @@ func (r *ApplicationReconciler) reconcileArgoApplication(app *v1alpha1.Applicati
 			if newArgoApp, err = createUnstructuredApplication(app); err == nil {
 				argoApp.Object["spec"] = newArgoApp.Object["spec"]
 				argoApp.Object["operation"] = newArgoApp.Object["operation"]
+
+				// append annotations and labels
+				copyArgoAnnotationsAndLabels(newArgoApp, argoApp)
+
 				argoApp.SetFinalizers(newArgoApp.GetFinalizers())
 				err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
 					latestArgoApp := createBareArgoCDApplicationObject()
@@ -205,6 +210,9 @@ func createUnstructuredApplication(app *v1alpha1.Application) (result *unstructu
 	newArgoApp.SetName(app.GetName())
 	newArgoApp.SetNamespace(app.GetNamespace())
 
+	// make sure all Argo CD supported annotations and labels exist
+	copyArgoAnnotationsAndLabels(app, newArgoApp)
+
 	// copy all potential finalizers
 	finalizers := app.GetFinalizers()
 	targetFinalizers := make([]string, 0)
@@ -228,6 +236,30 @@ func createUnstructuredApplication(app *v1alpha1.Application) (result *unstructu
 	return newArgoApp, nil
 }
 
+func copyArgoAnnotationsAndLabels(app metav1.Object, argoApp metav1.Object) {
+	var annotations map[string]string
+	if annotations = argoApp.GetAnnotations(); annotations == nil {
+		annotations = map[string]string{}
+	}
+	for k, v := range app.GetAnnotations() {
+		if strings.Contains(k, "argoproj.io") {
+			annotations[k] = v
+		}
+	}
+	argoApp.SetAnnotations(annotations)
+
+	var labels map[string]string
+	if labels = argoApp.GetLabels(); labels == nil {
+		labels = map[string]string{}
+	}
+	for k, v := range app.GetLabels() {
+		if strings.Contains(k, "argoproj.io") {
+			labels[k] = v
+		}
+	}
+	argoApp.SetLabels(labels)
+}
+
 func (r *ApplicationReconciler) addArgoAppNameIntoLabels(namespace, name, argoAppName string) (err error) {
 	app := &v1alpha1.Application{}
 	ctx := context.Background()
@@ -306,13 +338,44 @@ func (finalizersChangedPredicate) Update(e event.UpdateEvent) bool {
 	return !reflect.DeepEqual(e.MetaNew.GetFinalizers(), e.MetaOld.GetFinalizers())
 }
 
+type specificAnnotationsOrLabelsChangedPredicate struct {
+	predicate.Funcs
+	filter string
+}
+
+func (p specificAnnotationsOrLabelsChangedPredicate) Update(e event.UpdateEvent) (changed bool) {
+	changed = !reflect.DeepEqual(e.MetaNew.GetAnnotations(), e.MetaOld.GetAnnotations())
+	if !changed {
+		if changed = !reflect.DeepEqual(e.MetaNew.GetLabels(), e.MetaOld.GetLabels()); changed {
+			changed = mapKeysContains(p.filter, e.MetaNew.GetLabels(), e.MetaOld.GetLabels())
+		}
+	} else {
+		changed = mapKeysContains(p.filter, e.MetaNew.GetAnnotations(), e.MetaOld.GetAnnotations())
+	}
+	return
+}
+
+func mapKeysContains(filter string, annotations ...map[string]string) (has bool) {
+	for _, anno := range annotations {
+		for k := range anno {
+			if has = strings.Contains(k, filter); has {
+				return
+			}
+		}
+	}
+	return
+}
+
 // SetupWithManager setups the reconciler with a manager
 // setup the logger, recorder
 func (r *ApplicationReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	r.log = ctrl.Log.WithName(r.GetName())
 	r.recorder = mgr.GetEventRecorderFor(r.GetName())
 	return ctrl.NewControllerManagedBy(mgr).
-		WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, finalizersChangedPredicate{})).
+		WithEventFilter(predicate.Or(
+			predicate.GenerationChangedPredicate{},
+			finalizersChangedPredicate{},
+			specificAnnotationsOrLabelsChangedPredicate{filter: "argoproj.io"})).
 		For(&v1alpha1.Application{}).
 		Complete(r)
 }
diff --git a/controllers/jenkins/config/README.md b/controllers/jenkins/config/README.md
index ce8a9b4..20c01ed 100644
--- a/controllers/jenkins/config/README.md
+++ b/controllers/jenkins/config/README.md
@@ -54,7 +54,7 @@ An example of target ConfigMap:
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  annotation:
+  annotations:
     devops.kubesphere.io/ks-jenkins-config: ks-jenkins.yaml
     devops.kubesphere.io/jenkins-config-formula: custom
     devops.kubesphere.io/jenkins-config-customized: "true"


Test patch:
diff --git a/controllers/argocd/application_controller_test.go b/controllers/argocd/application_controller_test.go
index 11c5672..f3cdc22 100644
--- a/controllers/argocd/application_controller_test.go
+++ b/controllers/argocd/application_controller_test.go
@@ -64,6 +64,16 @@ func Test_createUnstructuredApplication(t *testing.T) {
 		name: "with some specific fields, with default values",
 		args: args{
 			app: &v1alpha1.Application{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd-image-updater.argoproj.io/image-list": "nginx",
+						"other": "other",
+					},
+					Labels: map[string]string{
+						"argocd.argoproj.io/instance": "instance",
+						"other":                       "other",
+					},
+				},
 				Spec: v1alpha1.ApplicationSpec{
 					ArgoApp: &v1alpha1.ArgoApplication{
 						Spec: v1alpha1.ArgoApplicationSpec{
@@ -106,6 +116,20 @@ func Test_createUnstructuredApplication(t *testing.T) {
 			assert.Equal(t, true, dryRun)
 			initiatedBy, _, _ := unstructured.NestedString(gotResult.Object, "operation", "initiatedBy", "username")
 			assert.Equal(t, "admin", initiatedBy)
+
+			// check annotations
+			annotations, _, _ := unstructured.NestedStringMap(gotResult.Object, "metadata", "annotations")
+			assert.Equal(t, map[string]string{
+				"argocd-image-updater.argoproj.io/image-list": "nginx",
+			}, annotations)
+			// check labels
+			labels, _, _ := unstructured.NestedStringMap(gotResult.Object, "metadata", "labels")
+			assert.Equal(t, map[string]string{
+				"argocd.argoproj.io/instance":                        "instance",
+				"gitops.kubesphere.io/application-name":              "",
+				"gitops.kubesphere.io/application-namespace":         "",
+				"gitops.kubesphere.io/argocd-application-control-by": "ks-devops",
+			}, labels)
 		},
 	}}
 	for _, tt := range tests {
@@ -127,6 +151,9 @@ func TestApplicationReconciler_reconcileArgoApplication(t *testing.T) {
 			Labels: map[string]string{
 				v1alpha1.ArgoCDLocationLabelKey: "fake",
 			},
+			Annotations: map[string]string{
+				"argocd-image-updater.argoproj.io/image-list": "nginx",
+			},
 		},
 		Spec: v1alpha1.ApplicationSpec{
 			ArgoApp: &v1alpha1.ArgoApplication{
@@ -141,6 +168,11 @@ func TestApplicationReconciler_reconcileArgoApplication(t *testing.T) {
 			},
 		},
 	}
+	appWithLabel := app.DeepCopy()
+	appWithLabel.SetLabels(map[string]string{
+		v1alpha1.ArgoCDAppNameLabelKey:  "fake",
+		v1alpha1.ArgoCDLocationLabelKey: "fake",
+	})
 
 	argoApp := &unstructured.Unstructured{}
 	argoApp.SetKind("Application")
@@ -148,6 +180,16 @@ func TestApplicationReconciler_reconcileArgoApplication(t *testing.T) {
 	argoApp.SetName("fake")
 	argoApp.SetNamespace("fake")
 
+	argoAppWithLabel := argoApp.DeepCopy()
+	argoAppWithLabel.SetLabels(map[string]string{
+		v1alpha1.ArgoCDAppNameLabelKey: "fake",
+	})
+
+	argoAppList := &unstructured.UnstructuredList{}
+	argoAppList.SetKind("ApplicationList")
+	argoAppList.SetAPIVersion("argoproj.io/v1alpha1")
+	argoAppList.Items = append(argoAppList.Items, *argoApp)
+
 	type fields struct {
 		Client client.Client
 		log    logr.Logger
@@ -203,6 +245,44 @@ func TestApplicationReconciler_reconcileArgoApplication(t *testing.T) {
 			assert.Nil(t, err)
 			project, _, _ := unstructured.NestedString(app.Object, "spec", "project")
 			assert.Equal(t, "fake", project)
+			annotations, _, _ := unstructured.NestedStringMap(app.Object, "metadata", "annotations")
+			assert.Equal(t, map[string]string{
+				"argocd-image-updater.argoproj.io/image-list": "nginx",
+			}, annotations)
+		},
+	}, {
+		name: "update the existing argo application which not have labels",
+		fields: fields{
+			Client: fake.NewFakeClientWithScheme(schema, app.DeepCopy(), argoAppList.DeepCopy()),
+		},
+		args: args{
+			app: app.DeepCopy(),
+		},
+		verify: func(t *testing.T, Client client.Client, err error) {
+			assert.Nil(t, err)
+		},
+	}, {
+		name: "update the existing argo application which has labels",
+		fields: fields{
+			Client: fake.NewFakeClientWithScheme(schema, appWithLabel.DeepCopy(), argoAppWithLabel.DeepCopy()),
+		},
+		args: args{
+			app: appWithLabel.DeepCopy(),
+		},
+		verify: func(t *testing.T, Client client.Client, err error) {
+			assert.Nil(t, err)
+
+			app := argoApp.DeepCopy()
+
+			err = Client.Get(context.TODO(), types.NamespacedName{
+				Namespace: "fake",
+				Name:      "fake",
+			}, app)
+			assert.Nil(t, err)
+			annotations, _, _ := unstructured.NestedStringMap(app.Object, "metadata", "annotations")
+			assert.Equal(t, map[string]string{
+				"argocd-image-updater.argoproj.io/image-list": "nginx",
+			}, annotations)
 		},
 	}}
 	for _, tt := range tests {
@@ -310,3 +390,226 @@ func Test_finalizersChangedPredicate_Update(t *testing.T) {
 		})
 	}
 }
+
+func Test_copyArgoAnnotationsAndLabels(t *testing.T) {
+	type args struct {
+		app     *v1alpha1.Application
+		argoApp *unstructured.Unstructured
+	}
+	tests := []struct {
+		name   string
+		args   args
+		verify func(*testing.T, *unstructured.Unstructured)
+	}{{
+		name: "normal",
+		args: args{
+			app: &v1alpha1.Application{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"other":           "other",
+						"a.argoproj.io/b": "ab",
+					},
+					Labels: map[string]string{
+						"other":           "other",
+						"a.argoproj.io/b": "ab",
+					},
+				},
+			},
+			argoApp: &unstructured.Unstructured{},
+		},
+		verify: func(t *testing.T, u *unstructured.Unstructured) {
+			annotations, _, _ := unstructured.NestedStringMap(u.Object, "metadata", "annotations")
+			assert.Equal(t, map[string]string{
+				"a.argoproj.io/b": "ab",
+			}, annotations)
+			labels, _, _ := unstructured.NestedStringMap(u.Object, "metadata", "annotations")
+			assert.Equal(t, map[string]string{
+				"a.argoproj.io/b": "ab",
+			}, labels)
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			copyArgoAnnotationsAndLabels(tt.args.app, tt.args.argoApp)
+		})
+	}
+}
+
+func Test_mapKeysContains(t *testing.T) {
+	type args struct {
+		filter       string
+		annotations  map[string]string
+		annotations2 map[string]string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantHas bool
+	}{{
+		name: "do not contain the key string in one map",
+		args: args{
+			filter: "fake",
+			annotations: map[string]string{
+				"name": "linuxsuren",
+			},
+		},
+		wantHas: false,
+	}, {
+		name: "do not contain the key string in two maps",
+		args: args{
+			filter: "fake",
+			annotations: map[string]string{
+				"name": "linuxsuren",
+			},
+			annotations2: map[string]string{
+				"name": "linuxsuren-bot",
+			},
+		},
+		wantHas: false,
+	}, {
+		name: "contain the key string in one map",
+		args: args{
+			filter: "fake",
+			annotations: map[string]string{
+				"name.fake.com": "linuxsuren",
+				"age":           "10",
+			},
+		},
+		wantHas: true,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.wantHas, mapKeysContains(tt.args.filter, tt.args.annotations, tt.args.annotations2), "mapKeysContains(%v, %v, %v)", tt.args.filter, tt.args.annotations, tt.args.annotations2)
+		})
+	}
+}
+
+func Test_specificAnnotationsOrLabelsChangedPredicate_Update(t *testing.T) {
+	type fields struct {
+		Funcs  predicate.Funcs
+		filter string
+	}
+
+	defaultPredicate := fields{filter: "fake"}
+	type args struct {
+		e event.UpdateEvent
+	}
+	tests := []struct {
+		name        string
+		fields      fields
+		args        args
+		wantChanged bool
+	}{{
+		name:   "everything is same",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{},
+				MetaOld: &metav1.ObjectMeta{},
+			},
+		},
+		wantChanged: false,
+	}, {
+		name:   "annotations are different, but do not have key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"name": "fake",
+					},
+				},
+				MetaOld: &metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"name": "linuxsuren",
+					},
+				},
+			},
+		},
+		wantChanged: false,
+	}, {
+		name:   "labels are different, but do not have key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{
+					Labels: map[string]string{
+						"name": "fake",
+					},
+				},
+				MetaOld: &metav1.ObjectMeta{
+					Labels: map[string]string{
+						"name": "linuxsuren",
+					},
+				},
+			},
+		},
+		wantChanged: false,
+	}, {
+		name:   "old meta's annotations has key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{},
+				MetaOld: &metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"this.is.fake": "",
+					},
+				},
+			},
+		},
+		wantChanged: true,
+	}, {
+		name:   "new meta's annotations has key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"this.is.fake": "",
+					},
+				},
+				MetaOld: &metav1.ObjectMeta{},
+			},
+		},
+		wantChanged: true,
+	}, {
+		name:   "old meta's labels has key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{},
+				MetaOld: &metav1.ObjectMeta{
+					Labels: map[string]string{
+						"this.is.fake": "",
+					},
+				},
+			},
+		},
+		wantChanged: true,
+	}, {
+		name:   "new meta's labels has key",
+		fields: defaultPredicate,
+		args: args{
+			e: event.UpdateEvent{
+				MetaNew: &metav1.ObjectMeta{
+					Labels: map[string]string{
+						"this.is.fake": "",
+					},
+				},
+				MetaOld: &metav1.ObjectMeta{},
+			},
+		},
+		wantChanged: true,
+	}}
+	t.Logf("there are [%d] test cases\n", len(tests))
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := specificAnnotationsOrLabelsChangedPredicate{
+				Funcs:  tt.fields.Funcs,
+				filter: tt.fields.filter,
+			}
+			assert.Equalf(t, tt.wantChanged, p.Update(tt.args.e), "Update(%v)", tt.args.e)
+		})
+	}
+}