Replace fmt.Sprintf() with labels.NewSelector() for building labelsel…

…ectors

(cherry picked from commit f580edf)

pkg/reconciler/common/labels.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2022 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"fmt"
+	"strings"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
+)
+
+func LabelSelector(ls metav1.LabelSelector) (string, error) {
+	var (
+		err error
+		req *labels.Requirement
+		s   []string
+	)
+	for k, v := range ls.MatchLabels {
+		req, err = labels.NewRequirement(k, selection.Equals, []string{v})
+		if err != nil {
+			return "", fmt.Errorf("failed to create requirement: %w", err)
+		}
+		s = append(s, labels.NewSelector().Add(*req).String())
+	}
+	return strings.Join(s, ","), err
+}

pkg/reconciler/common/labels_test.go

@@ -0,0 +1,51 @@
+/*
+Copyright 2022 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestLabelSelector(t *testing.T) {
+	for _, c := range []struct {
+		name string
+		ls   metav1.LabelSelector
+		want string
+	}{{
+		name: "empty label selector",
+		ls:   metav1.LabelSelector{},
+		want: "",
+	}, {
+		name: "non empty label selector",
+		ls: metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"createdByKey":     "createdByValue",
+				"installerSetType": "pipelineResourceName",
+			},
+		},
+		want: "createdByKey=createdByValue,installerSetType=pipelineResourceName",
+	}} {
+		t.Run(c.name, func(t *testing.T) {
+			got, _ := LabelSelector(c.ls)
+			if got != c.want {
+				t.Errorf("LabelSelector:\n got %q\nwant %q", got, c.want)
+			}
+		})
+	}
+}

pkg/reconciler/kubernetes/tektondashboard/tektondashboard.go

@@ -33,8 +33,12 @@ import (
 	pipelineinformer "github.com/tektoncd/operator/pkg/client/informers/externalversions/operator/v1alpha1"
 	tektondashboardreconciler "github.com/tektoncd/operator/pkg/client/injection/reconciler/operator/v1alpha1/tektondashboard"
 	"github.com/tektoncd/operator/pkg/reconciler/common"
+	"github.com/tektoncd/operator/pkg/reconciler/kubernetes/tektoninstallerset"
+	"github.com/tektoncd/operator/pkg/reconciler/shared/hash"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
 	pkgreconciler "knative.dev/pkg/reconciler"
@@ -67,14 +71,7 @@ var _ tektondashboardreconciler.Finalizer = (*Reconciler)(nil)
 
 var watchedResourceName = "dashboard"
 
-const (
-	// TektonInstallerSet keys
-	lastAppliedHashKey = "operator.tekton.dev/last-applied-hash"
-	createdByKey       = "operator.tekton.dev/created-by"
-	createdByValue     = "TektonDashboard"
-	releaseVersionKey  = "operator.tekton.dev/release-version"
-	targetNamespaceKey = "operator.tekton.dev/target-namespace"
-)
+const createdByValue = "TektonDashboard"
 
 // FinalizeKind removes all resources after deletion of a TektonDashboards.
 func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1alpha1.TektonDashboard) pkgreconciler.Event {
@@ -96,9 +93,18 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1alpha1.Tekton
 		return err
 	}
 
+	ls := metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			tektoninstallerset.CreatedByKey: createdByValue,
+		},
+	}
+	labelSelector, err := common.LabelSelector(ls)
+	if err != nil {
+		return err
+	}
 	if err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
 		DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
-			LabelSelector: fmt.Sprintf("%s=%s", createdByKey, createdByValue),
+			LabelSelector: labelSelector,
 		}); err != nil {
 		logger.Error("Failed to delete installer set created by TektonDashboard", err)
 		return err
@@ -179,8 +185,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 		return err
 	}
 
-	installerSetTargetNamespace := installedTIS.Annotations[targetNamespaceKey]
-	installerSetReleaseVersion := installedTIS.Annotations[releaseVersionKey]
+	installerSetTargetNamespace := installedTIS.Annotations[tektoninstallerset.TargetNamespaceKey]
+	installerSetReleaseVersion := installedTIS.Annotations[tektoninstallerset.ReleaseVersionKey]
 
 	// Check if TargetNamespace of existing TektonInstallerSet is same as expected
 	// Check if Release Version in TektonInstallerSet is same as expected
@@ -223,7 +229,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 		}
 
 		// spec hash stored on installerSet
-		lastAppliedHash := installedTIS.GetAnnotations()[lastAppliedHashKey]
+		lastAppliedHash := installedTIS.GetAnnotations()[tektoninstallerset.LastAppliedHashKey]
 
 		if lastAppliedHash != expectedSpecHash {
 
@@ -241,7 +247,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 
 			// Update the spec hash
 			current := installedTIS.GetAnnotations()
-			current[lastAppliedHashKey] = expectedSpecHash
+			current[tektoninstallerset.LastAppliedHashKey] = expectedSpecHash
 			installedTIS.SetAnnotations(current)
 
 			// Update the manifests
@@ -332,6 +338,7 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, td *v1alpha1.Tekton
 		td.Status.MarkNotReady("transformation failed: " + err.Error())
 		return nil, err
 	}
+
 	// compute the hash of tektondashboard spec and store as an annotation
 	// in further reconciliation we compute hash of td spec and check with
 	// annotation, if they are same then we skip updating the object
@@ -340,6 +347,7 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, td *v1alpha1.Tekton
 	if err != nil {
 		return nil, err
 	}
+
 	// create installer set
 	tis := makeInstallerSet(td, manifest, specHash, r.operatorVersion)
 	createdIs, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().

pkg/reconciler/kubernetes/tektoninstallerset/query_test.go

@@ -18,15 +18,30 @@ package tektoninstallerset
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	"github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
+	"github.com/tektoncd/operator/pkg/reconciler/common"
 	"gotest.tools/v3/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var (
+	pipelineLS = metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			CreatedByKey:     "TektonPipeline",
+			InstallerSetType: "pipeline",
+		},
+	}
+	triggersLS = metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			CreatedByKey:     "TektonTriggers",
+			InstallerSetType: "triggers",
+		},
+	}
+)
+
 func TestCurrentInstallerSetName(t *testing.T) {
 
 	iSets := v1alpha1.TektonInstallerSetList{
@@ -44,10 +59,10 @@ func TestCurrentInstallerSetName(t *testing.T) {
 		},
 	}
 	client := fake.NewSimpleClientset(&iSets)
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s",
-		CreatedByKey, "TektonPipeline",
-		InstallerSetType, "pipeline",
-	)
+	labelSelector, err := common.LabelSelector(pipelineLS)
+	if err != nil {
+		t.Error(err)
+	}
 	name, err := CurrentInstallerSetName(context.TODO(), client, labelSelector)
 
 	assert.NilError(t, err)
@@ -71,10 +86,10 @@ func TestCurrentInstallerSetNameNoMatching(t *testing.T) {
 		},
 	}
 	client := fake.NewSimpleClientset(&iSets)
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s",
-		CreatedByKey, "TektonTriggers",
-		InstallerSetType, "triggers",
-	)
+	labelSelector, err := common.LabelSelector(triggersLS)
+	if err != nil {
+		t.Error(err)
+	}
 	name, err := CurrentInstallerSetName(context.TODO(), client, labelSelector)
 
 	assert.NilError(t, err)
@@ -107,10 +122,10 @@ func TestCurrentInstallerSetNameWithDuplicates(t *testing.T) {
 		},
 	}
 	client := fake.NewSimpleClientset(&iSets)
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s",
-		CreatedByKey, "TektonPipeline",
-		InstallerSetType, "pipeline",
-	)
+	labelSelector, err := common.LabelSelector(pipelineLS)
+	if err != nil {
+		t.Error(err)
+	}
 
 	name, err := CurrentInstallerSetName(context.TODO(), client, labelSelector)
 	assert.Error(t, err, v1alpha1.RECONCILE_AGAIN_ERR.Error())

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go

@@ -68,6 +68,15 @@ type Reconciler struct {
 	pipelineVersion string
 }
 
+var (
+	ls = metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			tektoninstallerset.CreatedByKey:     createdByValue,
+			tektoninstallerset.InstallerSetType: v1alpha1.PipelineResourceName,
+		},
+	}
+)
+
 // Check that our Reconciler implements controller.Reconciler
 var _ tektonpipelinereconciler.Interface = (*Reconciler)(nil)
 var _ tektonpipelinereconciler.Finalizer = (*Reconciler)(nil)
@@ -84,11 +93,13 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1alpha1.Tekton
 		return err
 	}
 
+	labelSelector, err := common.LabelSelector(ls)
+	if err != nil {
+		return err
+	}
 	if err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
 		DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
-			LabelSelector: fmt.Sprintf("%s=%s,%s=%s",
-				tektoninstallerset.CreatedByKey, createdByValue,
-				tektoninstallerset.InstallerSetType, v1alpha1.PipelineResourceName),
+			LabelSelector: labelSelector,
 		}); err != nil {
 		logger.Error("Failed to delete installer set created by TektonPipeline", err)
 		return err
@@ -133,10 +144,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 	tp.Status.MarkPreReconcilerComplete()
 
 	// Check if an tekton installer set already exists, if not then create
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s",
-		tektoninstallerset.CreatedByKey, createdByValue,
-		tektoninstallerset.InstallerSetType, v1alpha1.PipelineResourceName,
-	)
+	labelSelector, err := common.LabelSelector(ls)
+	if err != nil {
+		return err
+	}
 	existingInstallerSet, err := tektoninstallerset.CurrentInstallerSetName(ctx, r.operatorClientSet, labelSelector)
 	if err != nil {
 		return err

pkg/reconciler/kubernetes/tektontrigger/tektontrigger.go

@@ -69,6 +69,15 @@ type Reconciler struct {
 	operatorVersion string
 }
 
+var (
+	ls = metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			tektoninstallerset.CreatedByKey:     createdByValue,
+			tektoninstallerset.InstallerSetType: v1alpha1.TriggerResourceName,
+		},
+	}
+)
+
 // Check that our Reconciler implements controller.Reconciler
 var _ tektontriggerreconciler.Interface = (*Reconciler)(nil)
 var _ tektontriggerreconciler.Finalizer = (*Reconciler)(nil)
@@ -85,11 +94,13 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1alpha1.Tekton
 		return err
 	}
 
+	labelSelector, err := common.LabelSelector(ls)
+	if err != nil {
+		return err
+	}
 	if err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
 		DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
-			LabelSelector: fmt.Sprintf("%s=%s,%s=%s",
-				tektoninstallerset.CreatedByKey, createdByValue,
-				tektoninstallerset.InstallerSetType, v1alpha1.TriggerResourceName),
+			LabelSelector: labelSelector,
 		}); err != nil {
 		logger.Error("Failed to delete installer set created by TektonTrigger", err)
 		return err
@@ -148,10 +159,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tt *v1alpha1.TektonTrigg
 	tt.Status.MarkPreReconcilerComplete()
 
 	// Check if an tekton installer set already exists, if not then create
-	labelSelector := fmt.Sprintf("%s=%s,%s=%s",
-		tektoninstallerset.CreatedByKey, createdByValue,
-		tektoninstallerset.InstallerSetType, v1alpha1.TriggerResourceName,
-	)
+	labelSelector, err := common.LabelSelector(ls)
+	if err != nil {
+		return err
+	}
 	existingInstallerSet, err := tektoninstallerset.CurrentInstallerSetName(ctx, r.operatorClientSet, labelSelector)
 	if err != nil {
 		return err