fix: Do not copy all labels from ArgoCD CR to resources we create (#414)

* fix: Remove application instance label from secrets we create

* fix: Do not copy all labels from ArgoCD CR to resources we create

* Include tests

api/v1alpha1/argocd_types.go

@@ -675,3 +675,13 @@ func (argocd *ArgoCD) IsDeletionFinalizerPresent() bool {
 	}
 	return false
 }
+
+// ApplicationInstanceLabelKey returns either the custom application instance
+// label key if set, or the default value.
+func (a *ArgoCD) ApplicationInstanceLabelKey() string {
+	if a.Spec.ApplicationInstanceLabelKey != "" {
+		return a.Spec.ApplicationInstanceLabelKey
+	} else {
+		return common.ArgoCDDefaultApplicationInstanceLabelKey
+	}
+}

api/v1alpha1/argocd_types_test.go

@@ -0,0 +1,17 @@
+package v1alpha1
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+
+	"github.com/argoproj-labs/argocd-operator/common"
+)
+
+func Test_ArgoCD_ApplicationInstanceLabelKey(t *testing.T) {
+	cr := &ArgoCD{}
+	cr.Spec.ApplicationInstanceLabelKey = "my.corp/instance"
+	assert.Equal(t, cr.ApplicationInstanceLabelKey(), "my.corp/instance")
+	cr = &ArgoCD{}
+	assert.Equal(t, cr.ApplicationInstanceLabelKey(), common.ArgoCDDefaultApplicationInstanceLabelKey)
+}

common/defaults.go

@@ -49,7 +49,7 @@ const (
 	ArgoCDDefaultApplicationSetVersion = "v0.1.0"
 
 	// ArgoCDDefaultApplicationInstanceLabelKey is the default app name as a tracking label.
-	ArgoCDDefaultApplicationInstanceLabelKey = "mycompany.com/appname"
+	ArgoCDDefaultApplicationInstanceLabelKey = "app.kubernetes.io/instance"
 
 	// ArgoCDDefaultArgoImage is the ArgoCD container image to use when not specified.
 	ArgoCDDefaultArgoImage = "argoproj/argocd"
@@ -284,3 +284,20 @@ ssh.dev.azure.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hr1oTWqNqOlzGJOfGJ4Nak
 vs-ssh.visualstudio.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC7Hr1oTWqNqOlzGJOfGJ4NakVyIzf1rXYd4d7wo6jBlkLvCA4odBlL0mDUyZ0/QUfTTqeu+tm22gOsv+VrVTMk6vwRU75gY/y9ut5Mb3bR5BV58dKXyq9A9UeB5Cakehn5Zgm6x1mKoVyf+FFn26iYqXJRgzIZZcZ5V6hrE0Qg39kZm4az48o0AUbf6Sp4SLdvnuMa2sVNwHBboS7EJkm57XQPVU3/QpyNLHbWDdzwtrlS+ez30S3AdYhLKEOxAG8weOnyrtLJAUen9mTkol8oII1edf7mWWbWVf0nBmly21+nZcmCTISQBtdcyPaEno7fFQMDD26/s0lfKob4Kw8H
 `
 )
+
+// DefaultLabels returns the default set of labels for controllers.
+func DefaultLabels(name string) map[string]string {
+	return map[string]string{
+		ArgoCDKeyName:      name,
+		ArgoCDKeyPartOf:    ArgoCDAppName,
+		ArgoCDKeyManagedBy: name,
+	}
+}
+
+// DefaultAnnotations returns the default set of annotations for child resources of ArgoCD
+func DefaultAnnotations(name string, namespace string) map[string]string {
+	return map[string]string{
+		AnnotationName:      name,
+		AnnotationNamespace: namespace,
+	}
+}

controllers/argocd/configmap.go

@@ -223,7 +223,7 @@ func newConfigMap(cr *argoprojv1a1.ArgoCD) *corev1.ConfigMap {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }
@@ -287,7 +287,7 @@ func (r *ReconcileArgoCD) reconcileCAConfigMap(cr *argoprojv1a1.ArgoCD) error {
 		return nil // ConfigMap found, do nothing
 	}
 
-	caSecret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, common.ArgoCDCASuffix)
+	caSecret := argoutil.NewSecretWithSuffix(cr, common.ArgoCDCASuffix)
 	if !argoutil.IsObjectFound(r.Client, cr.Namespace, caSecret.Name, caSecret) {
 		log.Info(fmt.Sprintf("ca secret [%s] not found, waiting to reconcile ca configmap [%s]", caSecret.Name, cm.Name))
 		return nil
@@ -506,7 +506,7 @@ func (r *ReconcileArgoCD) reconcileGrafanaConfiguration(cr *argoprojv1a1.ArgoCD)
 		return nil // ConfigMap found, do nothing
 	}
 
-	secret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "grafana")
+	secret := argoutil.NewSecretWithSuffix(cr, "grafana")
 	secret, err := argoutil.FetchSecret(r.Client, cr.ObjectMeta, secret.Name)
 	if err != nil {
 		return err

controllers/argocd/configmap_test.go

@@ -99,7 +99,7 @@ func TestReconcileArgoCD_reconcileArgoConfigMap(t *testing.T) {
 	assert.NilError(t, err)
 
 	want := map[string]string{
-		"application.instanceLabelKey": "mycompany.com/appname",
+		"application.instanceLabelKey": common.ArgoCDDefaultApplicationInstanceLabelKey,
 		"admin.enabled":                "true",
 		"configManagementPlugins":      "",
 		"dex.config":                   "",
@@ -224,7 +224,7 @@ func TestReconcileArgoCD_reconcileArgoConfigMap_withDexConnector(t *testing.T) {
 		}},
 	}
 
-	secret := argoutil.NewSecretWithName(metav1.ObjectMeta{Name: "token", Namespace: "argocd"}, "token")
+	secret := argoutil.NewSecretWithName(a, "token")
 	r := makeTestReconciler(t, a, sa, secret)
 	err := r.reconcileArgoConfigMap(a)
 	assert.NilError(t, err)

controllers/argocd/deployment.go

@@ -247,7 +247,7 @@ func newDeployment(cr *argoprojv1a1.ArgoCD) *appsv1.Deployment {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/hpa.go

@@ -30,7 +30,7 @@ func newHorizontalPodAutoscaler(cr *argoprojv1a1.ArgoCD) *autoscaling.Horizontal
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/ingress.go

@@ -50,7 +50,7 @@ func newIngress(cr *argoprojv1a1.ArgoCD) *extv1beta1.Ingress {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/prometheus.go

@@ -89,7 +89,7 @@ func newPrometheus(cr *argoprojv1a1.ArgoCD) *monitoringv1.Prometheus {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }
@@ -100,7 +100,7 @@ func newServiceMonitor(cr *argoprojv1a1.ArgoCD) *monitoringv1.ServiceMonitor {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/role.go

@@ -15,6 +15,7 @@ import (
 
 	argoprojv1a1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1"
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 )
 
 const (
@@ -30,7 +31,7 @@ func newRole(name string, rules []v1.PolicyRule, cr *argoprojv1a1.ArgoCD) *v1.Ro
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      generateResourceName(name, cr),
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 		Rules: rules,
 	}
@@ -49,8 +50,8 @@ func newClusterRole(name string, rules []v1.PolicyRule, cr *argoprojv1a1.ArgoCD)
 	return &v1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        GenerateUniqueResourceName(name, cr),
-			Labels:      labelsForCluster(cr),
-			Annotations: annotationsForCluster(cr),
+			Labels:      argoutil.LabelsForCluster(cr),
+			Annotations: argoutil.AnnotationsForCluster(cr),
 		},
 		Rules: rules,
 	}

controllers/argocd/rolebinding.go

@@ -15,15 +15,16 @@ import (
 
 	argoprojv1a1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1"
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 )
 
 // newClusterRoleBinding returns a new ClusterRoleBinding instance.
 func newClusterRoleBinding(name string, cr *argoprojv1a1.ArgoCD) *v1.ClusterRoleBinding {
 	return &v1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        cr.Name,
-			Labels:      labelsForCluster(cr),
-			Annotations: annotationsForCluster(cr),
+			Labels:      argoutil.LabelsForCluster(cr),
+			Annotations: argoutil.AnnotationsForCluster(cr),
 		},
 	}
 }
@@ -45,8 +46,8 @@ func newRoleBinding(cr *argoprojv1a1.ArgoCD) *v1.RoleBinding {
 	return &v1.RoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        cr.Name,
-			Labels:      labelsForCluster(cr),
-			Annotations: annotationsForCluster(cr),
+			Labels:      argoutil.LabelsForCluster(cr),
+			Annotations: argoutil.AnnotationsForCluster(cr),
 			Namespace:   cr.Namespace,
 		},
 	}

controllers/argocd/route.go

@@ -51,7 +51,7 @@ func newRoute(cr *argoprojv1a1.ArgoCD) *routev1.Route {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/secret.go

@@ -85,7 +85,7 @@ func nowNano() string {
 
 // newCASecret creates a new CA secret with the given suffix for the given ArgoCD.
 func newCASecret(cr *argoprojv1a1.ArgoCD) (*corev1.Secret, error) {
-	secret := argoutil.NewTLSSecret(cr.ObjectMeta, "ca")
+	secret := argoutil.NewTLSSecret(cr, "ca")
 
 	key, err := argoutil.NewPrivateKey()
 	if err != nil {
@@ -109,7 +109,7 @@ func newCASecret(cr *argoprojv1a1.ArgoCD) (*corev1.Secret, error) {
 
 // newCertificateSecret creates a new secret using the given name suffix for the given TLS certificate.
 func newCertificateSecret(suffix string, caCert *x509.Certificate, caKey *rsa.PrivateKey, cr *argoprojv1a1.ArgoCD) (*corev1.Secret, error) {
-	secret := argoutil.NewTLSSecret(cr.ObjectMeta, suffix)
+	secret := argoutil.NewTLSSecret(cr, suffix)
 
 	key, err := argoutil.NewPrivateKey()
 	if err != nil {
@@ -151,15 +151,15 @@ func newCertificateSecret(suffix string, caCert *x509.Certificate, caKey *rsa.Pr
 
 // reconcileArgoSecret will ensure that the Argo CD Secret is present.
 func (r *ReconcileArgoCD) reconcileArgoSecret(cr *argoprojv1a1.ArgoCD) error {
-	clusterSecret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "cluster")
-	secret := argoutil.NewSecretWithName(cr.ObjectMeta, common.ArgoCDSecretName)
+	clusterSecret := argoutil.NewSecretWithSuffix(cr, "cluster")
+	secret := argoutil.NewSecretWithName(cr, common.ArgoCDSecretName)
 
 	if !argoutil.IsObjectFound(r.Client, cr.Namespace, clusterSecret.Name, clusterSecret) {
 		log.Info(fmt.Sprintf("cluster secret [%s] not found, waiting to reconcile argo secret [%s]", clusterSecret.Name, secret.Name))
 		return nil
 	}
 
-	tlsSecret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "tls")
+	tlsSecret := argoutil.NewSecretWithSuffix(cr, "tls")
 	if !argoutil.IsObjectFound(r.Client, cr.Namespace, tlsSecret.Name, tlsSecret) {
 		log.Info(fmt.Sprintf("tls secret [%s] not found, waiting to reconcile argo secret [%s]", tlsSecret.Name, secret.Name))
 		return nil
@@ -196,7 +196,7 @@ func (r *ReconcileArgoCD) reconcileArgoSecret(cr *argoprojv1a1.ArgoCD) error {
 
 // reconcileClusterMainSecret will ensure that the main Secret is present for the Argo CD cluster.
 func (r *ReconcileArgoCD) reconcileClusterMainSecret(cr *argoprojv1a1.ArgoCD) error {
-	secret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "cluster")
+	secret := argoutil.NewSecretWithSuffix(cr, "cluster")
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, secret.Name, secret) {
 		return nil // Secret found, do nothing
 	}
@@ -218,12 +218,12 @@ func (r *ReconcileArgoCD) reconcileClusterMainSecret(cr *argoprojv1a1.ArgoCD) er
 
 // reconcileClusterTLSSecret ensures the TLS Secret is created for the ArgoCD cluster.
 func (r *ReconcileArgoCD) reconcileClusterTLSSecret(cr *argoprojv1a1.ArgoCD) error {
-	secret := argoutil.NewTLSSecret(cr.ObjectMeta, "tls")
+	secret := argoutil.NewTLSSecret(cr, "tls")
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, secret.Name, secret) {
 		return nil // Secret found, do nothing
 	}
 
-	caSecret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "ca")
+	caSecret := argoutil.NewSecretWithSuffix(cr, "ca")
 	caSecret, err := argoutil.FetchSecret(r.Client, cr.ObjectMeta, caSecret.Name)
 	if err != nil {
 		return err
@@ -253,7 +253,7 @@ func (r *ReconcileArgoCD) reconcileClusterTLSSecret(cr *argoprojv1a1.ArgoCD) err
 
 // reconcileClusterCASecret ensures the CA Secret is created for the ArgoCD cluster.
 func (r *ReconcileArgoCD) reconcileClusterCASecret(cr *argoprojv1a1.ArgoCD) error {
-	secret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "ca")
+	secret := argoutil.NewSecretWithSuffix(cr, "ca")
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, secret.Name, secret) {
 		return nil // Secret found, do nothing
 	}
@@ -335,8 +335,8 @@ func (r *ReconcileArgoCD) reconcileGrafanaSecret(cr *argoprojv1a1.ArgoCD) error
 		return nil // Grafana not enabled, do nothing.
 	}
 
-	clusterSecret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "cluster")
-	secret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "grafana")
+	clusterSecret := argoutil.NewSecretWithSuffix(cr, "cluster")
+	secret := argoutil.NewSecretWithSuffix(cr, "grafana")
 
 	if !argoutil.IsObjectFound(r.Client, cr.Namespace, clusterSecret.Name, clusterSecret) {
 		log.Info(fmt.Sprintf("cluster secret [%s] not found, waiting to reconcile grafana secret [%s]", clusterSecret.Name, secret.Name))
@@ -394,7 +394,7 @@ func (r *ReconcileArgoCD) reconcileGrafanaSecret(cr *argoprojv1a1.ArgoCD) error
 // reconcileClusterPermissionsSecret ensures ArgoCD instance is namespace-scoped
 func (r *ReconcileArgoCD) reconcileClusterPermissionsSecret(cr *argoprojv1a1.ArgoCD) error {
 	var clusterConfigInstance bool
-	secret := argoutil.NewSecretWithSuffix(cr.ObjectMeta, "default-cluster-config")
+	secret := argoutil.NewSecretWithSuffix(cr, "default-cluster-config")
 	secret.Labels[common.ArgoCDSecretTypeLabel] = "cluster"
 	dataBytes, _ := json.Marshal(map[string]interface{}{
 		"tlsClientConfig": map[string]interface{}{

controllers/argocd/secret_test.go

@@ -214,7 +214,7 @@ func Test_ReconcileArgoCD_ClusterPermissionsSecret(t *testing.T) {
 	r := makeTestReconciler(t, a)
 	assert.NilError(t, createNamespace(r, a.Namespace, a.Namespace))
 
-	testSecret := argoutil.NewSecretWithSuffix(a.ObjectMeta, "default-cluster-config")
+	testSecret := argoutil.NewSecretWithSuffix(a, "default-cluster-config")
 	assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: testSecret.Name, Namespace: testSecret.Namespace}, testSecret), "not found")
 
 	assert.NilError(t, r.reconcileClusterPermissionsSecret(a))

controllers/argocd/service.go

@@ -42,7 +42,7 @@ func newService(cr *argoprojv1a1.ArgoCD) *corev1.Service {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/service_account.go

@@ -41,7 +41,7 @@ func newServiceAccount(cr *argoprojv1a1.ArgoCD) *corev1.ServiceAccount {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/statefulset.go

@@ -43,7 +43,7 @@ func newStatefulSet(cr *argoprojv1a1.ArgoCD) *appsv1.StatefulSet {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
 			Namespace: cr.Namespace,
-			Labels:    labelsForCluster(cr),
+			Labels:    argoutil.LabelsForCluster(cr),
 		},
 	}
 }

controllers/argocd/util.go

@@ -335,7 +335,7 @@ func (r *ReconcileArgoCD) getDexOAuthClientSecret(cr *argoprojv1a1.ArgoCD) (*str
 	}
 
 	// Fetch the secret to obtain the token
-	secret := argoutil.NewSecretWithName(cr.ObjectMeta, tokenSecret.Name)
+	secret := argoutil.NewSecretWithName(cr, tokenSecret.Name)
 	if err := argoutil.FetchObject(r.Client, cr.Namespace, secret.Name, secret); err != nil {
 		return nil, err
 	}
@@ -846,24 +846,6 @@ func removeString(slice []string, s string) []string {
 	return result
 }
 
-// labelsForCluster returns the labels for all cluster resources.
-func labelsForCluster(cr *argoprojv1a1.ArgoCD) map[string]string {
-	labels := argoutil.DefaultLabels(cr.Name)
-	for key, val := range cr.ObjectMeta.Labels {
-		labels[key] = val
-	}
-	return labels
-}
-
-// annotationsForCluster returns the annotations for all cluster resources.
-func annotationsForCluster(cr *argoprojv1a1.ArgoCD) map[string]string {
-	annotations := argoutil.DefaultAnnotations(cr)
-	for key, val := range cr.ObjectMeta.Annotations {
-		annotations[key] = val
-	}
-	return annotations
-}
-
 // setResourceWatches will register Watches for each of the supported Resources.
 func setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper handler.MapFunc) *builder.Builder {
 
@@ -993,7 +975,7 @@ func setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretM
 
 // withClusterLabels will add the given labels to the labels for the cluster and return the result.
 func withClusterLabels(cr *argoprojv1a1.ArgoCD, addLabels map[string]string) map[string]string {
-	labels := labelsForCluster(cr)
+	labels := argoutil.LabelsForCluster(cr)
 	for key, val := range addLabels {
 		labels[key] = val
 	}

controllers/argocd/util_test.go

@@ -427,7 +427,7 @@ func TestDeleteRBACsForNamespace(t *testing.T) {
 	_, err = testClient.RbacV1().RoleBindings(testNameSpace).Create(context.TODO(), roleBinding2, metav1.CreateOptions{})
 	assert.NilError(t, err)
 
-	secret := argoutil.NewSecretWithSuffix(a.ObjectMeta, "xyz")
+	secret := argoutil.NewSecretWithSuffix(a, "xyz")
 	secret.Labels = map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}
 	secret.Data = map[string][]byte{
 		"server":     []byte(common.ArgoCDDefaultServer),