Repository: tektoncd/operator
Pull request #1405 (2023-03-29 08:02:50)

Moves `default-forbidden-env` to optional Pipelines properties
- Initially the field `default-forbidden-env` was added as a part
of `feature-flag` configMap whereas it has to be included in
`config-defaults` configMap

- Hence, this patch moves the field to optional Pipelines properties
struct so that the field gets added in `config-defaults` configMap

Signed-off-by: Puneet Punamiya ppunamiy@redhat.com

# Changes

<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

<!-- Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! -->

# Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

- [ ] Run `make test lint` before submitting a PR
- [ ] 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
```
-->


tekton-robot: [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 assign **houshengbo** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @houshengbo` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://prow.tekton.dev/command-help?repo=tektoncd%2Foperator).

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

- **[OWNERS](https://github.com/tektoncd/operator/blob/main/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":["houshengbo"]} -->
PuneetPunamiya: /cherry-pick release-v0.65.x
PuneetPunamiya: /cherry-pick release-v0.66.x
tekton-robot: @PuneetPunamiya: once the present PR merges, I will cherry-pick it on top of release-v0.66.x in a new PR and assign it to you.

<details>

In response to [this](https://github.com/tektoncd/operator/pull/1405#issuecomment-1488125861):

>/cherry-pick release-v0.66.x


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
tekton-robot: @PuneetPunamiya: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-tekton-operator-build-tests | e265e523e89559d201e958e3c38f611753ca742f | [link](https://prow.tekton.dev/view/gs/tekton-prow/pr-logs/pull/tektoncd_operator/1405/pull-tekton-operator-build-tests/1640992510354395136) | true | `/test pull-tekton-operator-build-tests`

[Full PR test history](https://prow.tekton.dev/pr-history?org=tektoncd&repo=operator&pr=1405). [Your PR dashboard](https://prow.tekton.dev/pr?query=is%3Apr%20state%3Aopen%20author%3APuneetPunamiya).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
tekton-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tektoncd/operator/pull/1405#pullrequestreview-1362604496" title="Approved">vdemeester</a>*

The full list of commands accepted by this bot can be found [here](https://prow.tekton.dev/command-help?repo=tektoncd%2Foperator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

- ~~[OWNERS](https://github.com/tektoncd/operator/blob/main/OWNERS)~~ [vdemeester]

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":[]} -->

Patch:
diff --git a/docs/TektonChain.md b/docs/TektonChain.md
index e71a82133..a78f2f282 100644
--- a/docs/TektonChain.md
+++ b/docs/TektonChain.md
@@ -73,12 +73,15 @@ metadata:
   name: chain
 spec:
   targetNamespace: tekton-pipelines
-  artifacts.taskrun.format: tekton
+  artifacts.taskrun.format: in-toto
   artifacts.taskrun.storage: tekton,oci (comma separated values)
   artifacts.taskrun.signer: x509
   artifacts.oci.storage: oci (comma separated values)
   artifacts.oci.format: simplesigning
   artifacts.oci.signer: x509
+  artifacts.pipelinerun.format: in-toto
+  artifacts.pipelinerun.storage: tekton,oci (comma separated values)
+  artifacts.pipelinerun.signer: x509
   storage.gcs.bucket: #value
   storage.oci.repository: #value
   storage.oci.repository.insecure: #value (boolean - true/false)
diff --git a/pkg/apis/operator/v1alpha1/tektonchain_types.go b/pkg/apis/operator/v1alpha1/tektonchain_types.go
index c40b3dda6..35b2fb9a6 100644
--- a/pkg/apis/operator/v1alpha1/tektonchain_types.go
+++ b/pkg/apis/operator/v1alpha1/tektonchain_types.go
@@ -65,6 +65,11 @@ type Chain struct {
 	ArtifactsTaskRunStorage string `json:"artifacts.taskrun.storage,omitempty"`
 	ArtifactsTaskRunSigner  string `json:"artifacts.taskrun.signer,omitempty"`
 
+	// pipelinerun artifacts config
+	ArtifactsPipelineRunFormat  string `json:"artifacts.pipelinerun.format,omitempty"`
+	ArtifactsPipelineRunStorage string `json:"artifacts.pipelinerun.storage,omitempty"`
+	ArtifactsPipelineRunSigner  string `json:"artifacts.pipelinerun.signer,omitempty"`
+
 	// oci artifacts config
 	ArtifactsOCIFormat  string `json:"artifacts.oci.format,omitempty"`
 	ArtifactsOCIStorage string `json:"artifacts.oci.storage,omitempty"`
diff --git a/pkg/apis/operator/v1alpha1/tektonchain_validation.go b/pkg/apis/operator/v1alpha1/tektonchain_validation.go
index b0b8a3859..8ca5e46ec 100644
--- a/pkg/apis/operator/v1alpha1/tektonchain_validation.go
+++ b/pkg/apis/operator/v1alpha1/tektonchain_validation.go
@@ -68,6 +68,28 @@ func (tcs *TektonChainSpec) ValidateChainConfig(path string) (errs *apis.FieldEr
 		}
 	}
 
+	if tcs.ArtifactsPipelineRunFormat != "" {
+		if tcs.ArtifactsPipelineRunFormat != "in-toto" {
+			errs = errs.Also(apis.ErrInvalidValue(tcs.ArtifactsPipelineRunFormat, path+".artifacts.pipelinerun.format"))
+		}
+	}
+
+	if tcs.ArtifactsPipelineRunStorage != "" {
+		input := strings.Split(tcs.ArtifactsPipelineRunStorage, ",")
+		for i, v := range input {
+			input[i] = strings.TrimSpace(v)
+			if !allowedArtifactsStorage.Has(input[i]) {
+				errs = errs.Also(apis.ErrInvalidValue(input[i], path+".artifacts.pipelinerun.storage"))
+			}
+		}
+	}
+
+	if tcs.ArtifactsPipelineRunSigner != "" {
+		if tcs.ArtifactsPipelineRunSigner != "x509" && tcs.ArtifactsPipelineRunSigner != "kms" {
+			errs = errs.Also(apis.ErrInvalidValue(tcs.ArtifactsPipelineRunSigner, path+".artifacts.pipelinerun.signer"))
+		}
+	}
+
 	if tcs.ArtifactsOCIFormat != "" {
 		if tcs.ArtifactsOCIFormat != "simplesigning" {
 			errs = errs.Also(apis.ErrInvalidValue(tcs.ArtifactsOCIFormat, path+".artifacts.oci.format"))
diff --git a/pkg/apis/operator/v1alpha1/tektonpipeline_types.go b/pkg/apis/operator/v1alpha1/tektonpipeline_types.go
index e7624a3f7..d480a0514 100644
--- a/pkg/apis/operator/v1alpha1/tektonpipeline_types.go
+++ b/pkg/apis/operator/v1alpha1/tektonpipeline_types.go
@@ -97,7 +97,6 @@ type PipelineProperties struct {
 	SendCloudEventsForRuns                   *bool  `json:"send-cloudevents-for-runs,omitempty"`
 	VerificationMode                         string `json:"verification-mode,omitempty"`
 	EnableProvenanceInStatus                 *bool  `json:"enable-provenance-in-status,omitempty"`
-	DefaultForbiddenEnv                      string `json:"default-forbidden-env,omitempty"`
 
 	// ScopeWhenExpressionsToTask Deprecated: remove in next release
 	ScopeWhenExpressionsToTask *bool `json:"scope-when-expressions-to-task,omitempty"`
@@ -119,6 +118,7 @@ type OptionalPipelineProperties struct {
 	DefaultAffinityAssistantPodTemplate string `json:"default-affinity-assistant-pod-template,omitempty"`
 	DefaultTaskRunWorkspaceBinding      string `json:"default-task-run-workspace-binding,omitempty"`
 	DefaultMaxMatrixCombinationsCount   string `json:"default-max-matrix-combinations-count,omitempty"`
+	DefaultForbiddenEnv                 string `json:"default-forbidden-env,omitempty"`
 }
 
 // PipelineMetricsProperties defines the fields which are configurable for


Test patch:
diff --git a/pkg/apis/operator/v1alpha1/tektonchain_validation_test.go b/pkg/apis/operator/v1alpha1/tektonchain_validation_test.go
index 6f5196ec5..c9ba588c0 100644
--- a/pkg/apis/operator/v1alpha1/tektonchain_validation_test.go
+++ b/pkg/apis/operator/v1alpha1/tektonchain_validation_test.go
@@ -59,7 +59,7 @@ func Test_ValidateTektonChain_OnDelete(t *testing.T) {
 	}
 }
 
-func Test_ValidateTektonChain_ConfigFormat(t *testing.T) {
+func Test_ValidateTektonChain_ConfigTaskRunFormat(t *testing.T) {
 	td := &TektonChain{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "chain",
@@ -79,7 +79,7 @@ func Test_ValidateTektonChain_ConfigFormat(t *testing.T) {
 	assert.Equal(t, "invalid value: test: spec.artifacts.taskrun.format", err.Error())
 }
 
-func Test_ValidateTektonChain_ConfigStorage(t *testing.T) {
+func Test_ValidateTektonChain_ConfigTaskRunStorage(t *testing.T) {
 	td := &TektonChain{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "chain",
@@ -99,7 +99,7 @@ func Test_ValidateTektonChain_ConfigStorage(t *testing.T) {
 	assert.Equal(t, "invalid value: test: spec.artifacts.taskrun.storage", err.Error())
 }
 
-func Test_ValidateTektonChain_ConfigStorageInvalidValidMix(t *testing.T) {
+func Test_ValidateTektonChain_ConfigTaskRunStorageInvalidValidMix(t *testing.T) {
 	td := &TektonChain{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "chain",
@@ -119,7 +119,7 @@ func Test_ValidateTektonChain_ConfigStorageInvalidValidMix(t *testing.T) {
 	assert.Equal(t, "invalid value: test: spec.artifacts.taskrun.storage", err.Error())
 }
 
-func Test_ValidateTektonChain_ConfigStorageValid(t *testing.T) {
+func Test_ValidateTektonChain_ConfigTaskRunStorageValid(t *testing.T) {
 	td := &TektonChain{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "chain",
@@ -140,3 +140,85 @@ func Test_ValidateTektonChain_ConfigStorageValid(t *testing.T) {
 		t.Errorf("ValidateTektonChain.Validate() expected no error for the given config, but got one, ValidateTektonChain: %v", err)
 	}
 }
+
+func Test_ValidateTektonChain_ConfigPipelineRunFormat(t *testing.T) {
+	td := &TektonChain{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "chain",
+			Namespace: "namespace",
+		},
+		Spec: TektonChainSpec{
+			CommonSpec: CommonSpec{
+				TargetNamespace: "namespace",
+			},
+			Chain: Chain{
+				ArtifactsPipelineRunFormat: "test",
+			},
+		},
+	}
+
+	err := td.Validate(context.TODO())
+	assert.Equal(t, "invalid value: test: spec.artifacts.pipelinerun.format", err.Error())
+}
+
+func Test_ValidateTektonChain_ConfigPipelineRunStorage(t *testing.T) {
+	td := &TektonChain{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "chain",
+			Namespace: "namespace",
+		},
+		Spec: TektonChainSpec{
+			CommonSpec: CommonSpec{
+				TargetNamespace: "namespace",
+			},
+			Chain: Chain{
+				ArtifactsPipelineRunStorage: "test",
+			},
+		},
+	}
+
+	err := td.Validate(context.TODO())
+	assert.Equal(t, "invalid value: test: spec.artifacts.pipelinerun.storage", err.Error())
+}
+
+func Test_ValidateTektonChain_ConfigPipelineRunStorageInvalidValidMix(t *testing.T) {
+	td := &TektonChain{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "chain",
+			Namespace: "namespace",
+		},
+		Spec: TektonChainSpec{
+			CommonSpec: CommonSpec{
+				TargetNamespace: "namespace",
+			},
+			Chain: Chain{
+				ArtifactsPipelineRunStorage: "tekton, test",
+			},
+		},
+	}
+
+	err := td.Validate(context.TODO())
+	assert.Equal(t, "invalid value: test: spec.artifacts.pipelinerun.storage", err.Error())
+}
+
+func Test_ValidateTektonChain_ConfigPipelineRunStorageValid(t *testing.T) {
+	td := &TektonChain{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "chain",
+			Namespace: "namespace",
+		},
+		Spec: TektonChainSpec{
+			CommonSpec: CommonSpec{
+				TargetNamespace: "namespace",
+			},
+			Chain: Chain{
+				ArtifactsPipelineRunStorage: "tekton, oci",
+			},
+		},
+	}
+
+	err := td.Validate(context.TODO())
+	if err != nil {
+		t.Errorf("ValidateTektonChain.Validate() expected no error for the given config, but got one, ValidateTektonChain: %v", err)
+	}
+}