Merge pull request #399 from ks-ci-bot/cherry-pick-398-to-release-3.2

[release-3.2] Fix the problem caused by strange branch names that contain special characters

cmd/controller/app/server.go

@@ -28,6 +28,7 @@ import (
 	"kubesphere.io/devops/pkg/client/k8s"
 	"kubesphere.io/devops/pkg/client/s3"
 	"kubesphere.io/devops/pkg/config"
+	"kubesphere.io/devops/pkg/indexers"
 	"kubesphere.io/devops/pkg/informers"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
 
@@ -189,6 +190,10 @@ func Run(s *options.DevOpsControllerManagerOptions, stopCh <-chan struct{}) erro
 		return fmt.Errorf("unable to register controllers to the manager: %v", err)
 	}
 
+	if err := indexers.CreatePipelineRunSCMRefNameIndexer(mgr.GetCache()); err != nil {
+		return err
+	}
+
 	// Start cache data after all informer is registered
 	klog.V(0).Info("Starting cache resource from apiserver...")
 	informerFactory.Start(stopCh)

controllers/jenkins/pipeline/metadata_converter.go

@@ -75,6 +75,7 @@ func convertBranches(jobBranches []job.PipelineBranch) []pipeline.Branch {
 	for _, jobBranch := range jobBranches {
 		branches = append(branches, pipeline.Branch{
 			Name:         jobBranch.Name,
+			RawName:      jobBranch.DisplayName,
 			WeatherScore: jobBranch.WeatherScore,
 			Branch:       jobBranch.Branch,
 			PullRequest:  jobBranch.PullRequest,

controllers/jenkins/pipelinerun/pipelinerun_controller.go

@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -30,7 +29,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
@@ -119,9 +117,6 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		pipelineRunCopied.Labels = make(map[string]string)
 	}
 	pipelineRunCopied.Labels[v1alpha3.PipelineNameLabelKey] = pipelineName
-	if refName, err := getSCMRefName(&pipelineRunCopied.Spec); err == nil && refName != "" {
-		pipelineRunCopied.Labels[v1alpha3.SCMRefNameLabelKey] = refName
-	}
 
 	log = log.WithValues("namespace", namespaceName, "Pipeline", pipelineName)
 
@@ -141,7 +136,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 			// See also: https://book-v1.book.kubebuilder.io/basics/status_subresource.html
 			if err := r.updateStatus(ctx, status, req.NamespacedName); err != nil {
 				log.Error(err, "unable to update PipelineRun status.")
-				return ctrl.Result{RequeueAfter: time.Second}, err
+				return ctrl.Result{}, err
 			}
 		}
 
@@ -169,7 +164,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		// update labels and annotations
 		if err := r.updateLabelsAndAnnotations(ctx, pipelineRunCopied); err != nil {
 			log.Error(err, "unable to update PipelineRun labels and annotations.")
-			return ctrl.Result{RequeueAfter: time.Second}, err
+			return ctrl.Result{}, err
 		}
 
 		r.recorder.Eventf(pipelineRunCopied, corev1.EventTypeNormal, v1alpha3.Updated, "Updated running data for PipelineRun %s", req.NamespacedName)
@@ -231,7 +226,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 	r.recorder.Eventf(pipelineRunCopied, corev1.EventTypeNormal, v1alpha3.Started, "Started PipelineRun %s", req.NamespacedName)
 	// requeue after 1 second
-	return ctrl.Result{RequeueAfter: 1 * time.Second}, nil
+	return ctrl.Result{}, nil
 }
 
 func (r *Reconciler) hasSamePipelineRun(jobRun *job.PipelineRun, pipeline *v1alpha3.Pipeline) (exists bool, err error) {
@@ -243,7 +238,7 @@ func (r *Reconciler) hasSamePipelineRun(jobRun *job.PipelineRun, pipeline *v1alp
 	}
 	if pipeline.Spec.Type == v1alpha3.MultiBranchPipelineType {
 		// add SCM reference name into list options for multi-branch Pipeline
-		listOptions = append(listOptions, client.MatchingLabels{v1alpha3.SCMRefNameLabelKey: jobRun.Pipeline})
+		listOptions = append(listOptions, client.MatchingFields{v1alpha3.PipelineRunSCMRefNameField: jobRun.Pipeline})
 	}
 	if err = r.Client.List(context.Background(), pipelineRuns, listOptions...); err == nil {
 		isMultiBranch := pipeline.Spec.Type == v1alpha3.MultiBranchPipelineType
@@ -259,9 +254,6 @@ func getSCMRefName(prSpec *v1alpha3.PipelineRunSpec) (string, error) {
 		if prSpec.SCM == nil || prSpec.SCM.RefName == "" {
 			return "", fmt.Errorf("failed to obtain SCM reference name for multi-branch Pipeline")
 		}
-		if errs := validation.IsValidLabelValue(prSpec.SCM.RefName); len(errs) != 0 {
-			return "", fmt.Errorf(strings.Join(errs, "; "))
-		}
 		branch = prSpec.SCM.RefName
 	}
 	return branch, nil

controllers/jenkins/pipelinerun/pipelinerun_controller_test.go

@@ -82,7 +82,7 @@ func Test_getBranch(t *testing.T) {
 		},
 		want: "main",
 	}, {
-		name: "Multi-branch Pipeline and SCM set, but the name is invalid",
+		name: "Multi-branch Pipeline and SCM set, but the name is written in Chinese",
 		args: args{
 			prSpec: &v1alpha3.PipelineRunSpec{
 				PipelineSpec: &v1alpha3.PipelineSpec{
@@ -94,7 +94,7 @@ func Test_getBranch(t *testing.T) {
 				},
 			},
 		},
-		wantErr: true,
+		want: "测试分支",
 	},
 	}
 	for _, tt := range tests {
@@ -159,7 +159,6 @@ var _ = Describe("TestReconciler_hasSamePipelineRun", func() {
 					v1alpha3.JenkinsPipelineRunIDAnnoKey: "123",
 				},
 				Labels: map[string]string{
-					v1alpha3.SCMRefNameLabelKey:   "main",
 					v1alpha3.PipelineNameLabelKey: "multi-branch-pipeline",
 				},
 			},

controllers/jenkins/pipelinerun/pipelinerun_synchronizer_test.go

@@ -143,7 +143,6 @@ func Test_createBarePipelineRun(t *testing.T) {
 				},
 				Labels: map[string]string{
 					v1alpha3.PipelineNameLabelKey: "fake-pipeline",
-					v1alpha3.SCMRefNameLabelKey:   "main",
 				},
 			},
 			Spec: v1alpha3.PipelineRunSpec{

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.6
+	github.com/jenkins-zh/jenkins-client v0.0.7
 	github.com/kubesphere/sonargo v0.0.2
 	github.com/onsi/ginkgo v1.16.4
 	github.com/onsi/gomega v1.15.0

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.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-client v0.0.7 h1:E0EUOx1G3RVEUrvj8WGvI5WjDTRD4tcLm8FAg0iqJJ8=
+github.com/jenkins-zh/jenkins-client v0.0.7/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=

pkg/api/devops/v1alpha3/groupversion_info.go

@@ -35,10 +35,10 @@ const (
 	PipelineRunOrphanLabelKey = GroupName + "/jenkins-pipelinerun-orphan"
 	// PipelineNameLabelKey is label key of Pipeline name.
 	PipelineNameLabelKey = GroupName + "/pipeline"
-	// SCMRefNameLabelKey is label key of SCM reference name.
-	SCMRefNameLabelKey = GroupName + "/scm-ref-name"
 	// PipelineRunCreatorAnnoKey is annotation key of PipelineRun's creator
 	PipelineRunCreatorAnnoKey = GroupName + "/creator"
+	// PipelineRunSCMRefNameField is the field name of SCM reference name in PipelineRun spec.
+	PipelineRunSCMRefNameField = "spec.scm.ref-name"
 )
 
 var (

pkg/apiserver/apiserver.go

@@ -34,6 +34,7 @@ import (
 	"kubesphere.io/devops/pkg/apiserver/authentication/request/anonymous"
 	"kubesphere.io/devops/pkg/apiserver/filters"
 	"kubesphere.io/devops/pkg/apiserver/request"
+	"kubesphere.io/devops/pkg/indexers"
 	"kubesphere.io/devops/pkg/kapis/oauth"
 	"kubesphere.io/devops/pkg/models/auth"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -154,7 +155,9 @@ func (s *APIServer) installKubeSphereAPIs() {
 }
 
 func (s *APIServer) Run(stopCh <-chan struct{}) (err error) {
-
+	if err := indexers.CreatePipelineRunSCMRefNameIndexer(s.RuntimeCache); err != nil {
+		return err
+	}
 	err = s.waitForResourceSync(stopCh)
 	if err != nil {
 		return err

pkg/indexers/indexers.go

@@ -0,0 +1,26 @@
+package indexers
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+)
+
+// CreatePipelineRunSCMRefNameIndexer creates field indexer which could speed up listing PipelineRun by SCM reference name.
+func CreatePipelineRunSCMRefNameIndexer(runtimeCache cache.Cache) error {
+	return runtimeCache.IndexField(context.Background(),
+		&v1alpha3.PipelineRun{},
+		v1alpha3.PipelineRunSCMRefNameField,
+		func(o runtime.Object) []string {
+			pipelineRun, ok := o.(*v1alpha3.PipelineRun)
+			if !ok || pipelineRun == nil {
+				return []string{}
+			}
+			if pipelineRun.Spec.SCM == nil {
+				return []string{}
+			}
+			return []string{pipelineRun.Spec.SCM.RefName}
+		})
+}

pkg/kapis/devops/v1alpha3/pipeline/branch_filter.go

@@ -2,7 +2,6 @@ package pipeline
 
 import (
 	"github.com/jenkins-zh/jenkins-client/pkg/job"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"kubesphere.io/devops/pkg/models/pipeline"
 )
 
@@ -13,9 +12,6 @@ type branchSlice []pipeline.Branch
 func (branches branchSlice) filter(predicate branchPredicate) []pipeline.Branch {
 	var resultBranches []pipeline.Branch
 	for _, branch := range branches {
-		if errors := validation.IsValidLabelValue(branch.Name); len(errors) != 0 {
-			continue
-		}
 		if predicate != nil && predicate(branch) {
 			resultBranches = append(resultBranches, branch)
 		}

pkg/kapis/devops/v1alpha3/pipeline/branch_filter_test.go

@@ -64,7 +64,7 @@ func Test_filterBranches(t *testing.T) {
 			PullRequest: &job.PullRequest{},
 		}},
 	}, {
-		name: "With filter: origin, but name is invalid",
+		name: "With filter: origin, but name is written in Chinese",
 		args: args{
 			branches: []pipeline.Branch{{
 				Name:        "main1",
@@ -83,6 +83,9 @@ func Test_filterBranches(t *testing.T) {
 		want: []pipeline.Branch{{
 			Name:        "main1",
 			PullRequest: nil,
+		}, {
+			Name:        "主分支2",
+			PullRequest: &job.PullRequest{},
 		}},
 	}, {
 		name: "With filter: pull-requests",
@@ -108,7 +111,7 @@ func Test_filterBranches(t *testing.T) {
 			},
 		}},
 	}, {
-		name: "With filter: pull-requests， but name is invalid",
+		name: "With filter: pull-requests, but name is written in Chinese",
 		args: args{
 			branches: []pipeline.Branch{{
 				Name:        "main1",
@@ -134,6 +137,11 @@ func Test_filterBranches(t *testing.T) {
 			PullRequest: &job.PullRequest{
 				ID: "1",
 			},
+		}, {
+			Name: "分支2",
+			PullRequest: &job.PullRequest{
+				ID: "2",
+			},
 		}},
 	}, {
 		name: "With other filter",

pkg/kapis/devops/v1alpha3/pipelinerun/handler.go

@@ -56,17 +56,22 @@ func (h *apiHandler) listPipelineRuns(request *restful.Request, response *restfu
 	}
 
 	// build label selector
-	labelSelector, err := buildLabelSelector(queryParam, pipeline.Name, branchName)
+	labelSelector, err := buildLabelSelector(queryParam, pipeline.Name)
 	if err != nil {
 		api.HandleError(request, response, err)
 		return
 	}
 
+	opts := make([]client.ListOption, 0, 3)
+	opts = append(opts, client.InNamespace(pipeline.Namespace))
+	opts = append(opts, client.MatchingLabelsSelector{Selector: labelSelector})
+	if branchName != "" {
+		opts = append(opts, client.MatchingFields{v1alpha3.PipelineRunSCMRefNameField: branchName})
+	}
+
 	var prs v1alpha3.PipelineRunList
 	// fetch PipelineRuns
-	if err := h.client.List(context.Background(), &prs,
-		client.InNamespace(pipeline.Namespace),
-		client.MatchingLabelsSelector{Selector: labelSelector}); err != nil {
+	if err := h.client.List(context.Background(), &prs, opts...); err != nil {
 		api.HandleError(request, response, err)
 		return
 	}

pkg/kapis/devops/v1alpha3/pipelinerun/util.go

@@ -3,38 +3,25 @@ package pipelinerun
 import (
 	"errors"
 	"fmt"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/selection"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
 	"kubesphere.io/devops/pkg/apiserver/query"
 	"kubesphere.io/devops/pkg/client/devops"
 )
 
-func buildLabelSelector(queryParam *query.Query, pipelineName, branchName string) (labels.Selector, error) {
+func buildLabelSelector(queryParam *query.Query, pipelineName string) (labels.Selector, error) {
 	labelSelector := queryParam.Selector()
 	rq, err := labels.NewRequirement(v1alpha3.PipelineNameLabelKey, selection.Equals, []string{pipelineName})
 	if err != nil {
 		// should never happen
 		return nil, err
 	}
 	labelSelector = labelSelector.Add(*rq)
-	if branchName != "" {
-		if errs := validation.IsValidLabelValue(branchName); len(errs) != 0 {
-			return nil, fmt.Errorf(strings.Join(errs, "; "))
-		}
-		rq, err = labels.NewRequirement(v1alpha3.SCMRefNameLabelKey, selection.Equals, []string{branchName})
-		if err != nil {
-			// should never happen
-			return nil, err
-		}
-		labelSelector = labelSelector.Add(*rq)
-	}
 	return labelSelector, nil
 }
 
@@ -109,8 +96,5 @@ func CreatePipelineRun(pipeline *v1alpha3.Pipeline, payload *devops.RunPayload,
 			SCM:          scm,
 		},
 	}
-	if scm != nil && scm.RefName != "" {
-		pipelineRun.Labels[v1alpha3.SCMRefNameLabelKey] = scm.RefName
-	}
 	return pipelineRun
 }

pkg/kapis/devops/v1alpha3/pipelinerun/util_test.go

@@ -25,7 +25,6 @@ func Test_buildLabelSelector(t *testing.T) {
 	type args struct {
 		queryParam   *query.Query
 		pipelineName string
-		branchName   string
 	}
 	tests := []struct {
 		name    string
@@ -37,32 +36,21 @@ func Test_buildLabelSelector(t *testing.T) {
 		args: args{
 			queryParam:   &query.Query{},
 			pipelineName: "pipelineA",
-			branchName:   "branchA",
 		},
-		want: parseSelector(fmt.Sprintf("%s=pipelineA,%s=branchA", v1alpha3.PipelineNameLabelKey, v1alpha3.SCMRefNameLabelKey)),
+		want: parseSelector(fmt.Sprintf("%s=pipelineA", v1alpha3.PipelineNameLabelKey)),
 	}, {
 		name: "Label selector was provided",
 		args: args{
 			queryParam: &query.Query{
 				LabelSelector: "a=b",
 			},
 			pipelineName: "pipelineA",
-			branchName:   "branchA",
 		},
-		want: parseSelector(fmt.Sprintf("%s=pipelineA,%s=branchA,a=b", v1alpha3.PipelineNameLabelKey, v1alpha3.SCMRefNameLabelKey)),
-	}, {
-		name: "No label selector was provided",
-		args: args{
-			queryParam:   &query.Query{},
-			pipelineName: "pipelineA",
-			branchName:   "分支A",
-		},
-		wantErr: true,
-	},
-	}
+		want: parseSelector(fmt.Sprintf("%s=pipelineA,a=b", v1alpha3.PipelineNameLabelKey)),
+	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := buildLabelSelector(tt.args.queryParam, tt.args.pipelineName, tt.args.branchName)
+			got, err := buildLabelSelector(tt.args.queryParam, tt.args.pipelineName)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("buildLabelSelector() error = %v, wantErr %v", err, tt.wantErr)
 				return