Repository: kubesphere/ks-devops
Pull request #378 (2021-11-25 02:56:59)

[release-3.2] Fix the problem of rendering choice parameter incorrectly
This is an automated cherry-pick of #371

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/378#" title="Author self-approved">ks-ci-bot</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/release-3.2/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"]} -->
LinuxSuRen: /label tide/merge-method-squash
ks-ci-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/378#" title="Author self-approved">ks-ci-bot</a>*, *<a href="https://github.com/kubesphere/ks-devops/pull/378#pullrequestreview-815518143" 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/release-3.2/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/cmd/controller/app/server.go b/cmd/controller/app/server.go
index 3345a91..f66c07f 100644
--- a/cmd/controller/app/server.go
+++ b/cmd/controller/app/server.go
@@ -18,8 +18,8 @@ package app
 
 import (
 	"fmt"
-
 	"github.com/jenkins-zh/jenkins-client/pkg/core"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"kubesphere.io/devops/cmd/controller/app/options"
 	"kubesphere.io/devops/pkg/apis"
 	"kubesphere.io/devops/pkg/client/devops"
@@ -28,6 +28,7 @@ import (
 	"kubesphere.io/devops/pkg/client/s3"
 	"kubesphere.io/devops/pkg/config"
 	"kubesphere.io/devops/pkg/informers"
+	"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -36,7 +37,6 @@ import (
 	"k8s.io/klog/klogr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
 )
 
 func NewControllerManagerCommand() *cobra.Command {
@@ -65,9 +65,11 @@ func NewControllerManagerCommand() *cobra.Command {
 		Use:   "controller-manager",
 		Short: `KubeSphere DevOps controller manager`,
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
-			if errs := s.Validate(); len(errs) == 0 {
-				err = Run(s, signals.SetupSignalHandler())
+			if errs := s.Validate(); len(errs) != 0 {
+				return utilerrors.NewAggregate(errs)
 			}
+
+			err = Run(s, signals.SetupSignalHandler())
 			return
 		},
 		SilenceUsage: true,
diff --git a/config/dockerfiles/tools/Dockerfile b/config/dockerfiles/tools/Dockerfile
index b3601f9..ef997ee 100644
--- a/config/dockerfiles/tools/Dockerfile
+++ b/config/dockerfiles/tools/Dockerfile
@@ -19,7 +19,7 @@ RUN CGO_ENABLED=0 GO111MODULE=on go build -a -o jwt cmd/tools/jwt/jwt_cmd.go
 
 FROM golang:1.16 as downloader
 RUN go install github.com/linuxsuren/http-downloader@v0.0.49
-RUN http-downloader install kubesphere-sigs/ks@v0.0.55
+RUN http-downloader install kubesphere-sigs/ks@v0.0.58
 
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details
diff --git a/controllers/jenkins/pipeline/metadata_converter.go b/controllers/jenkins/pipeline/metadata_converter.go
index 7881aa1..293f75a 100644
--- a/controllers/jenkins/pipeline/metadata_converter.go
+++ b/controllers/jenkins/pipeline/metadata_converter.go
@@ -2,6 +2,7 @@ package pipeline
 
 import (
 	"github.com/jenkins-zh/jenkins-client/pkg/job"
+	"kubesphere.io/devops/pkg/client/devops/jenkins"
 	"kubesphere.io/devops/pkg/models/pipeline"
 )
 
@@ -9,7 +10,7 @@ func convertPipeline(jobPipeline *job.Pipeline) *pipeline.Metadata {
 	return &pipeline.Metadata{
 		WeatherScore:                   jobPipeline.WeatherScore,
 		EstimatedDurationInMillis:      jobPipeline.EstimatedDurationInMillis,
-		Parameters:                     jobPipeline.Parameters,
+		Parameters:                     convertParameterDefinitions(jobPipeline.Parameters),
 		Name:                           jobPipeline.Name,
 		Disabled:                       jobPipeline.Disabled,
 		NumberOfPipelines:              jobPipeline.NumberOfPipelines,
@@ -27,6 +28,18 @@ func convertPipeline(jobPipeline *job.Pipeline) *pipeline.Metadata {
 	}
 }
 
+func convertParameterDefinitions(paramDefs []job.ParameterDefinition) []job.ParameterDefinition {
+	newParamDefs := []job.ParameterDefinition{}
+	for _, paramDef := range paramDefs {
+		// copy the parameter definition
+		if simpleType, ok := jenkins.ParameterTypeMap["hudson.model."+paramDef.Type]; ok {
+			paramDef.Type = simpleType
+		}
+		newParamDefs = append(newParamDefs, paramDef)
+	}
+	return newParamDefs
+}
+
 func convertLatestRun(jobLatestRun *job.PipelineRunSummary) *pipeline.LatestRun {
 	if jobLatestRun == nil {
 		return nil
diff --git a/go.mod b/go.mod
index 3c491d4..a5f2361 100644
--- a/go.mod
+++ b/go.mod
@@ -21,7 +21,7 @@ require (
 	github.com/golang/mock v1.6.0
 	github.com/google/go-cmp v0.5.5
 	github.com/google/go-querystring v1.1.0 // indirect
-	github.com/jenkins-zh/jenkins-client v0.0.5
+	github.com/jenkins-zh/jenkins-client v0.0.6
 	github.com/kubesphere/sonargo v0.0.2
 	github.com/onsi/ginkgo v1.16.4
 	github.com/onsi/gomega v1.15.0
diff --git a/go.sum b/go.sum
index 657163c..45c0f0a 100644
--- a/go.sum
+++ b/go.sum
@@ -317,8 +317,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
 github.com/jenkins-zh/jenkins-cli v0.0.32/go.mod h1:uE1mH9PNITrg0sugv6HXuM/CSddg0zxXoYu3w57I3JY=
-github.com/jenkins-zh/jenkins-client v0.0.5 h1:oKsVlcqrq7mSg/qk0gG16Ev7g23ego+X0VDiQNZrbRY=
-github.com/jenkins-zh/jenkins-client v0.0.5/go.mod h1:ICBk7OOoTafVP//f/VfKZ34c0ff8vJwVnOsF9btiMYU=
+github.com/jenkins-zh/jenkins-client v0.0.6 h1:Rs10HIXBP6evnmW+zeASJye/VSwrtT1bgXj+IC+oXXk=
+github.com/jenkins-zh/jenkins-client v0.0.6/go.mod h1:ICBk7OOoTafVP//f/VfKZ34c0ff8vJwVnOsF9btiMYU=
 github.com/jenkins-zh/jenkins-formulas v0.0.5/go.mod h1:zS8fm8u5L6FcjZM0QznXsLV9T2UtSVK+hT6Sm76iUZ4=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
 github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
diff --git a/pkg/client/devops/jenkins/constants.go b/pkg/client/devops/jenkins/constants.go
index 99f9590..e862c82 100644
--- a/pkg/client/devops/jenkins/constants.go
+++ b/pkg/client/devops/jenkins/constants.go
@@ -35,6 +35,7 @@ const (
 	PROJECT_ROLE = "projectRoles"
 )
 
+// ParameterTypeMap aims for simplifying representation of parameter definition type.
 var ParameterTypeMap = map[string]string{
 	"hudson.model.StringParameterDefinition":   "string",
 	"hudson.model.ChoiceParameterDefinition":   "choice",
diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go
index 7195f32..3cdc0a5 100644
--- a/pkg/kapis/devops/v1alpha2/devops.go
+++ b/pkg/kapis/devops/v1alpha2/devops.go
@@ -20,11 +20,12 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"kubesphere.io/devops/pkg/apiserver/query"
-	"kubesphere.io/devops/pkg/apiserver/request"
 	"net/http"
 	"strings"
 
+	"kubesphere.io/devops/pkg/apiserver/query"
+	"kubesphere.io/devops/pkg/apiserver/request"
+
 	"github.com/emicklei/go-restful"
 	"k8s.io/apiserver/pkg/authentication/user"
 	log "k8s.io/klog"
@@ -35,6 +36,7 @@ import (
 	"kubesphere.io/devops/pkg/api"
 	clientDevOps "kubesphere.io/devops/pkg/client/devops"
 	"kubesphere.io/devops/pkg/client/devops/jenkins"
+
 	//"kubesphere.io/devops/pkg/apiserver/authorization/authorizer"
 	//"kubesphere.io/devops/pkg/apiserver/request"
 	"kubesphere.io/devops/pkg/constants"
@@ -422,10 +424,12 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP
 			}
 
 			for _, step := range node.Steps {
-				if step.ID != stepId || step.Input == nil {
+				if step.ID != stepId {
 					continue
 				}
-
+				if step.Input == nil {
+					err = fmt.Errorf("the step '%s' is not approvable", step.DisplayName)
+				}
 				hasPermit = step.Approvable
 				break
 			}


Test patch:
diff --git a/controllers/jenkins/pipeline/metadata_converter_test.go b/controllers/jenkins/pipeline/metadata_converter_test.go
new file mode 100644
index 0000000..0a9e4bc
--- /dev/null
+++ b/controllers/jenkins/pipeline/metadata_converter_test.go
@@ -0,0 +1,138 @@
+package pipeline
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/jenkins-zh/jenkins-client/pkg/job"
+)
+
+func Test_convertParameterDefinitions(t *testing.T) {
+	type args struct {
+		paramDefs []job.ParameterDefinition
+	}
+	tests := []struct {
+		name string
+		args args
+		want []job.ParameterDefinition
+	}{{
+		name: "Convert nil parameter definitions",
+		args: args{},
+		want: []job.ParameterDefinition{},
+	}, {
+		name: "Convert empty parameter definitions",
+		args: args{
+			paramDefs: []job.ParameterDefinition{},
+		},
+		want: []job.ParameterDefinition{},
+	}, {
+		name: "Convert string parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "string-name",
+				Type: "StringParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "string-name",
+			Type: "string",
+		}},
+	}, {
+		name: "Convert choice parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "choice-name",
+				Type: "ChoiceParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "choice-name",
+			Type: "choice",
+		}},
+	}, {
+		name: "Convert text parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "text-name",
+				Type: "TextParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "text-name",
+			Type: "text",
+		}},
+	}, {
+		name: "Convert boolean parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "boolean-name",
+				Type: "BooleanParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "boolean-name",
+			Type: "boolean",
+		}},
+	}, {
+		name: "Convert file parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "file-name",
+				Type: "FileParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "file-name",
+			Type: "file",
+		}},
+	}, {
+		name: "Convert password parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "password-name",
+				Type: "PasswordParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "password-name",
+			Type: "password",
+		}},
+	}, {
+		name: "Convert multi parameter definitions",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "password-name",
+				Type: "PasswordParameterDefinition",
+			}, {
+				Name: "file-name",
+				Type: "FileParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "password-name",
+			Type: "password",
+		}, {
+			Name: "file-name",
+			Type: "file",
+		}},
+	}, {
+		name: "Convert invalid parameter definition",
+		args: args{
+			paramDefs: []job.ParameterDefinition{{
+				Name: "invalid-name",
+				Type: "InvalidParameterDefinition",
+			}},
+		},
+		want: []job.ParameterDefinition{{
+			Name: "invalid-name",
+			Type: "InvalidParameterDefinition",
+		}},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := convertParameterDefinitions(tt.args.paramDefs); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("convertParameterDefinitions() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}