feat: Add support for uninitialized custom params

docs/content/docs/guide/customparams.md

@@ -45,11 +45,25 @@ spec:
         key: companyname
 ```
 
+Lastly, if no default value makes sense for a custom param, it can be defined
+without a value:
+
+```yaml
+spec:
+  params:
+    - name: start_time
+```
+
+If the custom parameter is not defined with any value, it is only expanded
+if a value is supplied via [a GitOps command]({{< relref "/docs/guide/gitops_commands#passing-parameters-to-gitops-commands-as-arguments" >}}).
+
 {{< hint info >}}
 
 - If you have a `value` and a `secret_ref` defined, the `value` will be used.
-- If you don't have a `value` or a `secret_ref`, the parameter will not be
-  parsed, and it will be shown as `{{ param }}` in the `PipelineRun`.
+- If you don't have a `value` or a `secret_ref`, and the parameter is not
+  [overridden by a GitOps command]({{< relref "/docs/guide/gitops_commands#passing-parameters-to-gitops-commands-as-arguments" >}}),
+  the parameter will not be parsed, and it will be shown as `{{ param }}` in
+  the `PipelineRun`.
 - If you don't have a `name` in the `params`, the parameter will not be parsed.
 - If you have multiple `params` with the same `name`, the last one will be used.
 {{< /hint >}}

pkg/customparams/customparams.go

@@ -64,7 +64,7 @@ func (p *CustomParams) applyIncomingParams(ret map[string]string) map[string]str
 // matched true.
 func (p *CustomParams) GetParams(ctx context.Context) (map[string]string, map[string]interface{}, error) {
 	stdParams, changedFiles := p.makeStandardParamsFromEvent(ctx)
-	ret, mapFilters, parsedFromComment := map[string]string{}, map[string]string{}, map[string]string{}
+	resolvedParams, mapFilters, parsedFromComment := map[string]string{}, map[string]string{}, map[string]string{}
 	if p.event.TriggerComment != "" {
 		parsedFromComment = opscomments.ParseKeyValueArgs(p.event.TriggerComment)
 		for k, v := range parsedFromComment {
@@ -121,36 +121,45 @@ func (p *CustomParams) GetParams(ctx context.Context) (map[string]string, map[st
 				"ParamsFilterUsedValue",
 				fmt.Sprintf("repo %s, param name %s has a value and secretref, picking value", p.repo.GetName(), value.Name))
 		}
-		if value.Value != "" {
-			ret[value.Name] = value.Value
-		} else if value.SecretRef != nil {
+
+		_, paramIsStd := stdParams[value.Name]
+		_, paramParsedFromContent := parsedFromComment[value.Name]
+
+		switch {
+		case value.Value != "":
+			resolvedParams[value.Name] = value.Value
+		case paramParsedFromContent && !paramIsStd:
+			// If the param is standard, it's initial value will be set later so we don't set it here.
+			// Setting to empty string allows the parsedFromComment overrides to set the overridden value below.
+			resolvedParams[value.Name] = ""
+		case value.SecretRef != nil:
 			secretValue, err := p.k8int.GetSecret(ctx, sectypes.GetSecretOpt{
 				Namespace: p.repo.GetNamespace(),
 				Name:      value.SecretRef.Name,
 				Key:       value.SecretRef.Key,
 			})
 			if err != nil {
-				return ret, changedFiles, err
+				return resolvedParams, changedFiles, err
 			}
-			ret[value.Name] = secretValue
+			resolvedParams[value.Name] = secretValue
 		}
 	}
 
 	// TODO: Should we let the user override the standard params?
 	// we don't let them here
 	for k, v := range stdParams {
 		// check if not already there
-		if _, ok := ret[k]; !ok && v != "" {
-			ret[k] = v
+		if _, ok := resolvedParams[k]; !ok && v != "" {
+			resolvedParams[k] = v
 		}
 	}
 
 	// overwrite stdParams with parsed ones from the trigger comment
 	for k, v := range parsedFromComment {
-		if _, ok := ret[k]; ok && v != "" {
-			ret[k] = v
+		if _, ok := resolvedParams[k]; ok && v != "" {
+			resolvedParams[k] = v
 		}
 	}
 
-	return p.applyIncomingParams(ret), changedFiles, nil
+	return p.applyIncomingParams(resolvedParams), changedFiles, nil
 }

pkg/customparams/customparams_test.go

@@ -264,6 +264,22 @@ func TestProcessTemplates(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "params/override params with no value via gitops arguments",
+			expected: map[string]string{
+				"event_type":      "push",
+				"hello":           `"yolo"`,
+				"trigger_comment": triggerCommentArgs,
+			},
+			event: &info.Event{EventType: "pull_request", TriggerComment: triggerCommentArgs},
+			repository: &v1alpha1.Repository{
+				Spec: v1alpha1.RepositorySpec{
+					Params: &[]v1alpha1.Params{
+						{Name: "hello"},
+					},
+				},
+			},
+		},
 		{
 			name:               "params/skip with no name",
 			expectedLogSnippet: "no name has been set in params[0] of repo",
@@ -277,6 +293,19 @@ func TestProcessTemplates(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "params/skip with no value",
+			expected: map[string]string{},
+			repository: &v1alpha1.Repository{
+				Spec: v1alpha1.RepositorySpec{
+					Params: &[]v1alpha1.Params{
+						{
+							Name: "empty-param",
+						},
+					},
+				},
+			},
+		},
 		{
 			name:               "params/pick value when value and secret set",
 			expected:           map[string]string{"params": "batman"},

test/gitea_gitops_commands_test.go

@@ -65,8 +65,10 @@ func TestGiteaOnCommentAnnotation(t *testing.T) {
 				Value: "bar",
 			},
 			{
-				Name:  "custom3",
-				Value: "moto",
+				Name: "custom_no_initial_value",
+			},
+			{
+				Name: "custom_never_value",
 			},
 		},
 	}
@@ -111,7 +113,7 @@ func TestGiteaOnCommentAnnotation(t *testing.T) {
 	err = twait.RegexpMatchingInPodLog(context.Background(), topts.ParamsRun, topts.TargetNS, fmt.Sprintf("tekton.dev/pipelineRun=%s", last.PipelineRunName), "step-task", *regexp.MustCompile(triggerComment), "", 2)
 	assert.NilError(t, err)
 
-	tgitea.PostCommentOnPullRequest(t, topts, fmt.Sprintf(`%s revision=main custom1=thisone custom2="another one" custom3="a \"quote\""`, triggerComment))
+	tgitea.PostCommentOnPullRequest(t, topts, fmt.Sprintf(`%s revision=main custom1=thisone custom2="another one" custom_no_initial_value="a \"quote\""`, triggerComment))
 	waitOpts.MinNumberStatus = 4
 	repo, err = twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
 	assert.NilError(t, err)

test/gitea_params_test.go

@@ -294,6 +294,9 @@ func TestGiteaParamsOnRepoCR(t *testing.T) {
 					Key:  "unknowsecret",
 				},
 			},
+			{
+				Name: "no_initial_value",
+			},
 		},
 	}
 	topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
@@ -317,7 +320,7 @@ func TestGiteaParamsOnRepoCR(t *testing.T) {
 	assert.NilError(t,
 		twait.RegexpMatchingInPodLog(context.Background(), topts.ParamsRun, topts.TargetNS, fmt.Sprintf("tekton.dev/pipelineRun=%s,tekton.dev/pipelineTask=params",
 			repo.Status[0].PipelineRunName), "step-test-params-value", *regexp.MustCompile(
-			"I am the most Kawaī params\nSHHHHHHH\nFollow me on my ig #nofilter\n{{ no_match }}\nHey I show up from a payload match\n{{ secret_nothere }}"), "", 2))
+			"I am the most Kawaī params\nSHHHHHHH\nFollow me on my ig #nofilter\n{{ no_match }}\nHey I show up from a payload match\n{{ secret_nothere }}\n{{ no_initial_value }}"), "", 2))
 }
 
 // TestGiteaParamsBodyHeadersCEL Test that we can access the pull request body and headers in params

test/testdata/params.yaml

@@ -22,3 +22,4 @@ spec:
                 echo "{{ no_match }}"
                 echo "{{ filter_on_body }}"
                 echo "{{ secret_nothere }}"
+                echo "{{ no_initial_value }}"

test/testdata/pipelinerun-on-comment-annotation.yaml

@@ -24,4 +24,5 @@ spec:
                 echo "The revision is {{ revision }}"
                 echo "The custom1 value is {{ custom1 }}"
                 echo "The custom2 value is {{ custom2 }}"
-                echo "The custom3 value is {{ custom3 }}"
+                echo "The custom_no_initial_value value is {{ custom_no_initial_value }}"
+                echo "The custom_never_value value is {{ custom_never_value }}"