Repository: kubesphere/ks-devops
Pull request #494 (2022-03-10 04:05:36)

Redefine template delimiters to "$( )"
### What type of PR is this?

/kind feature

### What this PR does / why we need it:

As mentioned in the title.

By default, Helm is using go template to render its templates. If we using the same template delimiters as Helm, they will have conflicts. See error below when I tried to add a ClusterTemplate into Helm charts at [here](https://github.com/kubesphere-sigs/ks-devops-helm-chart/pull/62/commits/235e3e1cbaa4e347339ccb7954075e389d3161be):

```bash
helm lint charts/ks-devops
Error: 1 chart(s) linted, 1 chart(s) failed
==> Linting charts/ks-devops
[INFO] Chart.yaml: icon is recommended
Error:  templates/: template: ks-devops/templates/cluster-template-nodejs.yaml:44:[6](https://github.com/kubesphere-sigs/ks-devops-helm-chart/runs/5476016572?check_suite_focus=true#step:4:6)[8](https://github.com/kubesphere-sigs/ks-devops-helm-chart/runs/5476016572?check_suite_focus=true#step:4:8): executing "ks-devops/templates/cluster-template-nodejs.yaml" at <.params.CloneRevision>: nil pointer evaluating interface {}.CloneRevision

make: *** [Makefile:2: lint-chart] Error 1
Error: Process completed with exit code 2.
```

### Which issue(s) this PR fixes:

Fixes #

### Special notes for reviewers:

Please check the following list before waiting reviewers:

- [ ] Already committed the CRD files to [the Helm Chart](https://github.com/kubesphere-sigs/ks-devops-helm-chart/) if you created some new CRDs
- [ ] Already [added the permission](https://github.com/kubesphere/ks-installer/blob/9e063b085a0e43fdb3d0d9e3e7f4149146f14b9c/roles/ks-core/prepare/files/ks-init/role-templates.yaml) for the new API
- [ ] Already added the RBAC markers for the new controllers

### Does this PR introduce a user-facing change??

```release-note
None
```

/cc @kubesphere/sig-devops 

ks-ci-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/494#" title="Author self-approved">JohnNiang</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **linuxsuren** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @linuxsuren` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

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

- **[OWNERS](https://github.com/kubesphere/ks-devops/blob/master/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":["linuxsuren"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere) Report
> Merging [#494](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere) (bc662d0) into [master](https://codecov.io/gh/kubesphere/ks-devops/commit/e744581b2a8f7461042e6ab41e58919a1c28210c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere) (e744581) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/kubesphere/ks-devops/pull/494/graphs/tree.svg?width=650&height=150&src=pr&token=qSK4RBbyNg&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere)](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere)

```diff
@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   77.18%   77.21%   +0.02%     
==========================================
  Files         133      133              
  Lines       22764    22765       +1     
==========================================
+ Hits        17571    17577       +6     
+ Misses       4997     4993       -4     
+ Partials      196      195       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `77.21% <100.00%> (+0.02%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere) | Coverage Δ | |
|---|---|---|
| [pkg/kapis/devops/v1alpha3/template/render.go](https://codecov.io/gh/kubesphere/ks-devops/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere#diff-cGtnL2thcGlzL2Rldm9wcy92MWFscGhhMy90ZW1wbGF0ZS9yZW5kZXIuZ28=) | `87.50% <100.00%> (+0.54%)` | :arrow_up: |
| [...ontrollers/jenkins/pipeline/pipeline\_controller.go](https://codecov.io/gh/kubesphere/ks-devops/pull/494/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere#diff-Y29udHJvbGxlcnMvamVua2lucy9waXBlbGluZS9waXBlbGluZV9jb250cm9sbGVyLmdv) | `43.18% <0.00%> (+3.78%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere). Last update [e744581...bc662d0](https://codecov.io/gh/kubesphere/ks-devops/pull/494?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubesphere).

ks-ci-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/494#" title="Author self-approved">JohnNiang</a>*, *<a href="https://github.com/kubesphere/ks-devops/pull/494#pullrequestreview-905368448" title="Approved">LinuxSuRen</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

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/kubesphere/ks-devops/blob/master/OWNERS)~~ [LinuxSuRen]

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/config/samples/devops_v1alpha1_clustertemplate.yaml b/config/samples/devops_v1alpha1_clustertemplate.yaml
index ce304ff..d892d14 100644
--- a/config/samples/devops_v1alpha1_clustertemplate.yaml
+++ b/config/samples/devops_v1alpha1_clustertemplate.yaml
@@ -4,7 +4,7 @@ metadata:
   name: clustertemplate-sample
 spec:
   parameters:
-    - name: gitCloneURL
+    - name: cloneURL
       description: What is your repository URL you want to clone?
       type: string # ignorable
       validation:
@@ -17,11 +17,6 @@ spec:
       description: Do we really need build stage only?
       default: false # Valid JSON value
       type: bool
-    - name: matrix
-      description: Matrix versions of gradle
-      type: string-array
-      default: ["6.9.1-jdk11", "7.0.0-jdk11", "7.3.3-jdk11"]
-
   template: | # Go template
     pipeline {
         agent {
@@ -36,7 +31,7 @@ spec:
         stages {
             stage('Checkout') {
                 steps {
-                    git branch: '{{.params.revision}}', url: '{{.params.cloneURL}}'
+                    git branch: '$(.params.revision)', url: '$(.params.cloneURL)'
                 }
             }
             stage('Gradle Check') {
@@ -53,12 +48,12 @@ spec:
                     }
                 }
             }
-            {{if ne .params.buildOnly "true"}}
+            $(if ne .params.buildOnly "true")
             stage('Archive Assets') {
                 steps {
                     archiveArtifacts '**/build/libs/*.jar'
                 }
             }
-            {{end}}
+            $(end)
         }
     }
\ No newline at end of file
diff --git a/pkg/kapis/devops/v1alpha3/template/render.go b/pkg/kapis/devops/v1alpha3/template/render.go
index 199f1fa..cca5050 100644
--- a/pkg/kapis/devops/v1alpha3/template/render.go
+++ b/pkg/kapis/devops/v1alpha3/template/render.go
@@ -41,6 +41,8 @@ func render(templateObject v1alpha3.TemplateObject, parameters []Parameter) (v1a
 		Namespace: templateObject.GetNamespace(),
 	}.String()
 	template := tmpl.New(templateName)
+	//TODO Make delimiters configurable
+	template.Delims("$(", ")")
 	if _, err := template.Parse(rawTemplate); err != nil {
 		klog.Errorf("failed to parse template: %s, and err = %v", templateName, err)
 		return nil, errors.NewBadRequest("Failed to render template, please check the pipeline template for syntax error.")


Test patch:
diff --git a/pkg/kapis/devops/v1alpha3/template/render_test.go b/pkg/kapis/devops/v1alpha3/template/render_test.go
index a2cdc5e..2bc5ba1 100644
--- a/pkg/kapis/devops/v1alpha3/template/render_test.go
+++ b/pkg/kapis/devops/v1alpha3/template/render_test.go
@@ -58,7 +58,7 @@ func Test_render(t *testing.T) {
 	}, {
 		name: "Should render template with parameters",
 		args: args{
-			template: createTemplate("fake-name", "The number should be {{ .params.number }}"),
+			template: createTemplate("fake-name", "The number should be $(.params.number)"),
 			parameters: []Parameter{{
 				Name:  "number",
 				Value: "233",
@@ -72,7 +72,7 @@ func Test_render(t *testing.T) {
 	}, {
 		name: "Should render incorrectly without corresponding parameter",
 		args: args{
-			template: createTemplate("fake-name", "The number should be: {{ .params.number }}"),
+			template: createTemplate("fake-name", "The number should be: $(.params.number)"),
 			parameters: []Parameter{{
 				Name:  "name",
 				Value: "fake-name",
@@ -86,7 +86,7 @@ func Test_render(t *testing.T) {
 	}, {
 		name: "Should return error if the template is invalid",
 		args: args{
-			template: createTemplate("fake-name", "{{}}"),
+			template: createTemplate("fake-name", "$()"),
 			parameters: []Parameter{{
 				Name:  "name",
 				Value: "fake-name",
@@ -96,6 +96,33 @@ func Test_render(t *testing.T) {
 			assert.Nil(t, template)
 		},
 		wantErr: assert.Error,
+	}, {
+		name: "Should escape delim",
+		args: args{
+			template: createTemplate("fake-name", "$(`$(pwd)`)"),
+		},
+		verify: func(t *testing.T, template v1alpha3.TemplateObject) {
+			got := template.GetAnnotations()[devops.GroupName+devops.RenderResultAnnoKey]
+			assert.Equal(t, "$(pwd)", got)
+		},
+		wantErr: assert.NoError,
+	}, {
+		name: "Should render conditionally",
+		args: args{
+			// ${}
+			template: createTemplate("fake-name", "$(if .params.valid)Valid$(else)Invalid$(end)"),
+			parameters: []Parameter{
+				{
+					Name:  "valid",
+					Value: true,
+				},
+			},
+		},
+		verify: func(t *testing.T, template v1alpha3.TemplateObject) {
+			got := template.GetAnnotations()[devops.GroupName+devops.RenderResultAnnoKey]
+			assert.Equal(t, "Valid", got)
+		},
+		wantErr: assert.NoError,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {