Repository: tektoncd/operator (go)

Issue:
Operator Nightly failing
# Expected Behavior

Operator nightly should pass

# Actual Behavior

It has been failing the last 2 days. On both days, the failure is on the publish-images task.

```
 koparse --path /workspace/output/v20210701-e09614c741/release.yaml --base gcr.io/tekton-nightly/github.com/tektoncd/operator --images gcr.io/tekton-nightly/github.com/tektoncd/operator/cmd/kubernetes/operator:v20210701-e09614c741 gcr.io/tekton-nightly/github.com/tektoncd/operator/cmd/kubernetes/webhook:v20210701-e09614c741
Expected images did not match: Images ['gcr.io/tekton-nightly/github.com/tektoncd/operator/cmd/kubernetes/proxy-webhook:v20210701-e09614c741'] were present but not expected.
apiVersion: v1
```

It seems like a new image `proxy-webhook` was added but the release pipeline was not updated.


# Steps to Reproduce the Problem

1.
2.
3.

# Additional Info

- Kubernetes version:

  **Output of `kubectl version`:**

  ```
  (paste your output here)
  ```

- Tekton Pipeline version:

  **Output of `tkn version` or `kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'`**


<!-- Any other additional information -->


Pull request description:
Add proxy-webhook to the list of built images


# Changes

Nightly pipeline was failing since a new proxy-webhook binary was added (#330)
Fixes #344


# Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

- [ ] Includes [tests](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if functionality changed/added)
- [ ] Includes [docs](https://github.com/tektoncd/community/blob/master/standards.md#principles) (if user facing)
- [ ] Commit messages follow [commit message best practices](https://github.com/tektoncd/community/blob/master/standards.md#commit-messages)

_See [the contribution guide](https://github.com/tektoncd/operator/blob/master/CONTRIBUTING.md) for more details._

# Release Notes

<!--
Describe any user facing changes here, or delete this block.

Examples of user facing changes:
- API changes
- Bug fixes
- Any changes in behavior
- Changes requiring upgrade notices or deprecation warnings

For pull requests with a release note:

```release-note
Your release note here
```

For pull requests that require additional action from users switching to the new release, include the string "action required" (case insensitive) in the release note:

```release-note
action required: your release note here
```

For pull requests that don't need to be mentioned at release time, use the `/release-note-none` Prow command to add the `release-note-none` label to the PR. You can also write the string "NONE" as a release note in your PR description:

```release-note
NONE
```
-->


Patch:
diff --git a/pkg/reconciler/common/common.go b/pkg/reconciler/common/common.go
index 9ba80cd4a..d371569a9 100644
--- a/pkg/reconciler/common/common.go
+++ b/pkg/reconciler/common/common.go
@@ -40,10 +40,11 @@ var (
 )
 
 const (
-	PipelineNotReady = "tekton-pipelines not ready"
-	PipelineNotFound = "tekton-pipelines not installed"
-	TriggerNotReady  = "tekton-triggers not ready"
-	TriggerNotFound  = "tekton-triggers not installed"
+	PipelineNotReady       = "tekton-pipelines not ready"
+	PipelineNotFound       = "tekton-pipelines not installed"
+	TriggerNotReady        = "tekton-triggers not ready"
+	TriggerNotFound        = "tekton-triggers not installed"
+	NamespaceIgnorePattern = "^(openshift|kube)-"
 )
 
 func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPipeline, error) {
diff --git a/pkg/reconciler/common/name.go b/pkg/reconciler/common/name.go
new file mode 100644
index 000000000..a1d7d8593
--- /dev/null
+++ b/pkg/reconciler/common/name.go
@@ -0,0 +1,72 @@
+/*
+Copyright 2020 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"
+	"regexp"
+
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
+)
+
+// NameGenerator generates names for objects. Some backends may have more information
+// available to guide selection of new names and this interface hides those details.
+type NameGenerator interface {
+	// RestrictLengthWithRandomSuffix generates a valid name from the base name, adding a random suffix to the
+	// the base. If base is valid, the returned name must also be valid. The generator is
+	// responsible for knowing the maximum valid name length.
+	RestrictLengthWithRandomSuffix(base string) string
+
+	// RestrictLength generates a valid name from the name of a step specified in a Task,
+	// shortening it to the maximum valid name length if needed.
+	RestrictLength(base string) string
+}
+
+// simpleNameGenerator generates random names.
+type simpleNameGenerator struct{}
+
+// SimpleNameGenerator is a generator that returns the name plus a random suffix of five alphanumerics
+// when a name is requested. The string is guaranteed to not exceed the length of a standard Kubernetes
+// name (63 characters)
+var SimpleNameGenerator NameGenerator = simpleNameGenerator{}
+
+const (
+	// TODO: make this flexible for non-core resources with alternate naming rules.
+	maxNameLength          = 63
+	randomLength           = 5
+	maxGeneratedNameLength = maxNameLength - randomLength - 1
+)
+
+func (simpleNameGenerator) RestrictLengthWithRandomSuffix(base string) string {
+	if len(base) > maxGeneratedNameLength {
+		base = base[:maxGeneratedNameLength]
+	}
+	return fmt.Sprintf("%s-%s", base, utilrand.String(randomLength))
+}
+
+var alphaNumericRE = regexp.MustCompile(`^[a-zA-Z0-9]+$`)
+
+func (simpleNameGenerator) RestrictLength(base string) string {
+	if len(base) > maxNameLength {
+		base = base[:maxNameLength]
+	}
+
+	for !alphaNumericRE.MatchString(base[len(base)-1:]) {
+		base = base[:len(base)-1]
+	}
+	return base
+}
diff --git a/pkg/reconciler/common/prune.go b/pkg/reconciler/common/prune.go
index acf944989..c4fafca6b 100644
--- a/pkg/reconciler/common/prune.go
+++ b/pkg/reconciler/common/prune.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"regexp"
 	"strings"
 
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
@@ -32,8 +33,6 @@ import (
 )
 
 const (
-	openshiftPrefix  = "openshift"
-	kubePrefix       = "kube"
 	tektonSA         = "tekton-pipelines-controller"
 	CronName         = "resource-pruner"
 	JobsTKNImageName = "IMAGE_JOB_PRUNER_TKN"
@@ -42,29 +41,33 @@ const (
 )
 
 func Prune(k kubernetes.Interface, ctx context.Context, tC *v1alpha1.TektonConfig) error {
-	if 0 < len(tC.Spec.Pruner.Resources) {
-		tknImage := os.Getenv(JobsTKNImageName)
-		if tknImage == "" {
-			return fmt.Errorf("%s environment variable not found", JobsTKNImageName)
-		}
-		pru := tC.Spec.Pruner
-		logger := logging.FromContext(ctx)
-		ownerRef := v1.OwnerReference{
-			APIVersion: ownerAPIVer,
-			Kind:       ownerKind,
-			Name:       tC.Name,
-			UID:        tC.ObjectMeta.UID,
-		}
 
-		pruningNamespaces, err := GetPrunableNamespaces(k, ctx)
-		if err != nil {
-			return err
-		}
-		if err := createCronJob(k, ctx, pru, tC.Spec.TargetNamespace, pruningNamespaces, ownerRef, tknImage); err != nil {
-			logger.Error("failed to create cronjob ", err)
+	if len(tC.Spec.Pruner.Resources) == 0 || tC.Spec.Pruner.Schedule == "" {
+		return checkAndDelete(k, ctx, tC.Spec.TargetNamespace)
+	}
+
+	tknImage := os.Getenv(JobsTKNImageName)
+	if tknImage == "" {
+		return fmt.Errorf("%s environment variable not found", JobsTKNImageName)
+	}
+	pru := tC.Spec.Pruner
+	logger := logging.FromContext(ctx)
+	ownerRef := v1.OwnerReference{
+		APIVersion: ownerAPIVer,
+		Kind:       ownerKind,
+		Name:       tC.Name,
+		UID:        tC.ObjectMeta.UID,
+	}
+
+	pruningNamespaces, err := GetPrunableNamespaces(k, ctx)
+	if err != nil {
+		return err
+	}
+	if err := createCronJob(k, ctx, pru, tC.Spec.TargetNamespace, pruningNamespaces, ownerRef, tknImage); err != nil {
+		logger.Error("failed to create cronjob ", err)
 
-		}
 	}
+
 	return nil
 }
 
@@ -75,10 +78,12 @@ func GetPrunableNamespaces(k kubernetes.Interface, ctx context.Context) ([]strin
 	}
 
 	var allNameSpaces []string
+	re := regexp.MustCompile(NamespaceIgnorePattern)
 	for _, ns := range nsList.Items {
-		if !strings.HasPrefix(ns.Name, openshiftPrefix) && !strings.HasPrefix(ns.Name, kubePrefix) {
-			allNameSpaces = append(allNameSpaces, ns.Name)
+		if ignore := re.MatchString(ns.GetName()); ignore {
+			continue
 		}
+		allNameSpaces = append(allNameSpaces, ns.Name)
 	}
 	return allNameSpaces, nil
 }
@@ -132,9 +137,9 @@ func getPruningContainers(resources, namespaces []string, keep int, tknImage str
 	containers := []corev1.Container{}
 	for _, ns := range namespaces {
 		cmdArgs := deleteCommand(resources, keep, ns)
-		cName := "pruner-tkn-" + ns
+		jobName := SimpleNameGenerator.RestrictLengthWithRandomSuffix("pruner-tkn-" + ns)
 		container := corev1.Container{
-			Name:                     cName,
+			Name:                     jobName,
 			Image:                    tknImage,
 			Command:                  []string{"/bin/sh", "-c"},
 			Args:                     cmdArgs,
@@ -154,3 +159,16 @@ func deleteCommand(resources []string, keep int, ns string) []string {
 	}
 	return cmdArgs
 }
+
+func checkAndDelete(k kubernetes.Interface, ctx context.Context, targetNamespace string) error {
+	if _, err := k.BatchV1beta1().CronJobs(targetNamespace).Get(ctx, CronName, v1.GetOptions{}); err != nil {
+		if strings.Contains(err.Error(), "not found") {
+			return nil
+		} else {
+			return err
+		}
+	}
+
+	//if there is no error it means cron is exists, but no prune in config it means delete it
+	return k.BatchV1beta1().CronJobs(targetNamespace).Delete(ctx, CronName, v1.DeleteOptions{})
+}
diff --git a/pkg/reconciler/openshift/tektonconfig/rbac.go b/pkg/reconciler/openshift/tektonconfig/rbac.go
index 0fdded9c4..bcc490e58 100644
--- a/pkg/reconciler/openshift/tektonconfig/rbac.go
+++ b/pkg/reconciler/openshift/tektonconfig/rbac.go
@@ -20,6 +20,8 @@ import (
 	"context"
 	"regexp"
 
+	"github.com/tektoncd/operator/pkg/reconciler/common"
+
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -39,7 +41,6 @@ const (
 	pipelineSA               = "pipeline"
 	serviceCABundleConfigMap = "config-service-cabundle"
 	trustedCABundleConfigMap = "config-trusted-cabundle"
-	namespaceIgnorePattern   = "^(openshift|kube)-"
 	clusterInterceptors      = "openshift-pipelines-clusterinterceptors"
 )
 
@@ -66,7 +67,7 @@ func (r *rbac) createResources(ctx context.Context) error {
 		return err
 	}
 
-	re := regexp.MustCompile(namespaceIgnorePattern)
+	re := regexp.MustCompile(common.NamespaceIgnorePattern)
 
 	for _, n := range namespaces.Items {
 
diff --git a/tekton/bases/build-publish-images-manifests.yaml b/tekton/bases/build-publish-images-manifests.yaml
index e1cf5df15..b4aa657f6 100644
--- a/tekton/bases/build-publish-images-manifests.yaml
+++ b/tekton/bases/build-publish-images-manifests.yaml
@@ -9,7 +9,7 @@ spec:
       default: github.com/tektoncd/operator
     - name: images
       description: List of cmd/<kubeDistro>/* paths to be published as images
-      default: "operator webhook"
+      default: "operator webhook proxy-webhook"
     - name: versionTag
       description: The vX.Y.Z version that the artifacts should be tagged with (including `v`)
     - name: imageRegistry
diff --git a/tekton/overlays/nightly-releases/operator-nightly-release-pipeline.yaml b/tekton/overlays/nightly-releases/operator-nightly-release-pipeline.yaml
index 4b75f0fe9..febcb2e79 100644
--- a/tekton/overlays/nightly-releases/operator-nightly-release-pipeline.yaml
+++ b/tekton/overlays/nightly-releases/operator-nightly-release-pipeline.yaml
@@ -130,7 +130,7 @@ spec:
         - name: serviceAccountPath
           value: $(params.serviceAccountPath)
         - name: images
-          value: "operator webhook"
+          value: "operator webhook proxy-webhook"
         - name: kubeDistro
           value: $(params.kubeDistro)
       workspaces:


Test patch:
diff --git a/pkg/reconciler/common/name_test.go b/pkg/reconciler/common/name_test.go
new file mode 100644
index 000000000..1f4505fd3
--- /dev/null
+++ b/pkg/reconciler/common/name_test.go
@@ -0,0 +1,71 @@
+/*
+Copyright 2020 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 (
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
+	"strings"
+	"testing"
+)
+
+func TestRestrictLengthWithRandomSuffix(t *testing.T) {
+	for _, c := range []struct {
+		in, want string
+	}{{
+		in:   "hello",
+		want: "hello-9l9zj",
+	}, {
+		in:   strings.Repeat("a", 100),
+		want: strings.Repeat("a", 57) + "-9l9zj",
+	}} {
+		t.Run(c.in, func(t *testing.T) {
+			TestingSeed()
+			got := SimpleNameGenerator.RestrictLengthWithRandomSuffix(c.in)
+			if got != c.want {
+				t.Errorf("RestrictLengthWithRandomSuffix:\n got %q\nwant %q", got, c.want)
+			}
+		})
+	}
+}
+
+func TestRestrictLength(t *testing.T) {
+	for _, c := range []struct {
+		in, want string
+	}{{
+		in:   "hello",
+		want: "hello",
+	}, {
+		in:   strings.Repeat("a", 100),
+		want: strings.Repeat("a", maxNameLength),
+	}, {
+		// Values that don't end with an alphanumeric value are
+		// trimmed until they do.
+		in:   "abcdefg   !@#!$",
+		want: "abcdefg",
+	}} {
+		t.Run(c.in, func(t *testing.T) {
+			got := SimpleNameGenerator.RestrictLength(c.in)
+			if got != c.want {
+				t.Errorf("RestrictLength:\n got %q\nwant %q", got, c.want)
+			}
+		})
+	}
+}
+
+func TestingSeed() {
+	utilrand.Seed(12345)
+}
diff --git a/pkg/reconciler/common/prune_test.go b/pkg/reconciler/common/prune_test.go
index 4c7a2d179..330be3934 100644
--- a/pkg/reconciler/common/prune_test.go
+++ b/pkg/reconciler/common/prune_test.go
@@ -49,7 +49,7 @@ func TestGetPrunableNamespaces(t *testing.T) {
 
 func TestCreateCronJob(t *testing.T) {
 	cronName := "resource-pruner"
-	resource := "pipelinerun"
+	resource := []string{"pipelinerun", "taskrun"}
 	cronJob := &v1beta1.CronJob{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "CronJob",
@@ -67,13 +67,14 @@ func TestCreateCronJob(t *testing.T) {
 		},
 	}
 
-	nsObj := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-one"}}
+	nsObj1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-one"}}
+
 	pru := v1alpha1.Prune{
-		Resources: []string{resource},
+		Resources: resource,
 		Keep:      2,
 		Schedule:  "*/5 * * * *",
 	}
-	client := fake.NewSimpleClientset(cronJob, nsObj)
+	client := fake.NewSimpleClientset(cronJob, nsObj1)
 	nsList := []string{"ns-one"}
 	if err := createCronJob(client, context.TODO(), pru, nsList[0], nsList, metav1.OwnerReference{}, "some-image"); err != nil {
 		t.Error("failed creating cronjob")
@@ -86,8 +87,8 @@ func TestCreateCronJob(t *testing.T) {
 		t.Error("cronjob not matched")
 	}
 	jobName := "pruner-tkn-" + nsList[0]
-	if cron.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Name != jobName {
+	nameAfterRemovingRand := cron.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Name[:len(jobName)]
+	if nameAfterRemovingRand != jobName {
 		t.Error("Job Name not matched")
 	}
-
 }