Repository: kubesphere/ks-devops
Pull request #409 (2021-12-10 07:50:45)

[release-3.2] Fix the problem that could not proceed or break the pipeline created by others
This is an automated cherry-pick of #408


Patch:
diff --git a/pkg/client/devops/pipeline.go b/pkg/client/devops/pipeline.go
index 802ee95..2a945c9 100644
--- a/pkg/client/devops/pipeline.go
+++ b/pkg/client/devops/pipeline.go
@@ -1093,9 +1093,12 @@ func (i *Input) GetSubmitters() (submitters []string) {
 	}
 
 	submitterArray := strings.Split(fmt.Sprintf("%v", i.Submitter), ",")
-	submitters = make([]string, len(submitterArray))
-	for i, submitter := range submitterArray {
-		submitters[i] = strings.TrimSpace(submitter)
+	submitters = make([]string, 0, len(submitterArray))
+	for _, submitter := range submitterArray {
+		trimmedSubmitter := strings.TrimSpace(submitter)
+		if trimmedSubmitter != "" {
+			submitters = append(submitters, trimmedSubmitter)
+		}
 	}
 	return
 }
diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go
index 3cdc0a5..6d53c34 100644
--- a/pkg/kapis/devops/v1alpha2/devops.go
+++ b/pkg/kapis/devops/v1alpha2/devops.go
@@ -27,7 +27,6 @@ import (
 	"kubesphere.io/devops/pkg/apiserver/request"
 
 	"github.com/emicklei/go-restful"
-	"k8s.io/apiserver/pkg/authentication/user"
 	log "k8s.io/klog"
 	"k8s.io/klog/v2"
 
@@ -318,65 +317,37 @@ func (h *ProjectPipelineHandler) GetPipelineRunNodes(req *restful.Request, resp
 	resp.WriteAsJson(res)
 }
 
-// there're two situation here:
-// 1. the particular submitters exist
-// the users who are the owner of this Pipeline or the submitter of this Pipeline, or has the auth to create a DevOps project
-// 2. no particular submitters
-// only the owner of this Pipeline can approve or reject it
+// approvableCheck requires the users who have PipelineRun management permission to
+// approve a step. If the particular submitters exist, we also restrict the users
+// who are Pipeline creator or in the particular submitters can be able to approve or reject a step.
 func (h *ProjectPipelineHandler) approvableCheck(nodes []clientDevOps.NodesDetail, pipe pipelineParam) {
-	var userInfo user.Info
-	var ok bool
-	var isAdmin bool
-	// check if current user belong to the admin group, grant it if it's true
-	if userInfo, ok = request.UserFrom(pipe.Context); ok {
-		//createAuth := authorizer.AttributesRecord{
-		//	User:            userInfo,
-		//	Verb:            authorizer.VerbDelete,
-		//	Workspace:       pipe.Workspace,
-		//	DevOps:          pipe.ProjectName,
-		//	Resource:        "devopsprojects",
-		//	ResourceRequest: true,
-		//	ResourceScope:   request.DevOpsScope,
-		//}
-		//
-		//if decision, _, err := h.authorizer.Authorize(createAuth); err == nil {
-		//	isAdmin = decision == authorizer.DecisionAllow
-		//} else {
-		//	// this is an expected case, printing the debug info for troubleshooting
-		//	klog.V(8).Infof("authorize failed with '%v', error is '%v'",
-		//		createAuth, err)
-		//}
-	} else {
+	userInfo, ok := request.UserFrom(pipe.Context)
+	if !ok {
 		klog.V(6).Infof("cannot get the current user when checking the approvable with pipeline '%s/%s'",
 			pipe.ProjectName, pipe.Name)
 		return
 	}
 
-	var createdByCurrentUser bool // indicate if the current user is the owner
-	if pipeline, err := h.devopsOperator.GetPipelineObj(pipe.ProjectName, pipe.Name); err == nil {
-		if creator, ok := pipeline.GetAnnotations()[constants.CreatorAnnotationKey]; ok {
-			createdByCurrentUser = userInfo.GetName() == creator
-		} else {
-			klog.V(6).Infof("annotation '%s' is necessary but it is missing from '%s/%s'",
-				constants.CreatorAnnotationKey, pipe.ProjectName, pipe.Name)
-		}
-	} else {
-		klog.V(6).Infof("cannot find pipeline '%s/%s', error is '%v'", pipe.ProjectName, pipe.Name, err)
-		return
-	}
-
 	// check every input steps if it's approvable
-	for i, node := range nodes {
+	for i := range nodes {
+		node := &nodes[i]
 		if node.State != clientDevOps.StatePaused {
 			continue
 		}
 
-		for j, step := range node.Steps {
+		for j := range node.Steps {
+			step := &node.Steps[j]
 			if step.State != clientDevOps.StatePaused || step.Input == nil {
 				continue
 			}
-
-			nodes[i].Steps[j].Approvable = isAdmin || createdByCurrentUser || step.Input.Approvable(userInfo.GetName())
+			if len(step.Input.GetSubmitters()) == 0 {
+				// TODO: no particular submitters, we make the PipelineRun approvable by default.
+				// Until we can obtain the permissions of the currently logged in user
+				step.Approvable = true
+			} else {
+				isCreator := h.createdBy(pipe.ProjectName, pipe.Name, userInfo.GetName())
+				step.Approvable = isCreator || step.Input.Approvable(userInfo.GetName())
+			}
 		}
 	}
 }
@@ -437,9 +408,12 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP
 		}
 	} else {
 		log.V(4).Infof("cannot get nodes detail, error: %v", err)
-		err = errors.New("cannot get the submitters of current pipeline run")
+		err = errors.New("cannot get the submitters of current step")
 		return
 	}
+	if !hasPermit && err == nil {
+		err = fmt.Errorf("you have no permission to approve this step")
+	}
 	return
 }
 


Test patch:
diff --git a/pkg/client/devops/pipeline_test.go b/pkg/client/devops/pipeline_test.go
index c33210f..4e96bde 100644
--- a/pkg/client/devops/pipeline_test.go
+++ b/pkg/client/devops/pipeline_test.go
@@ -12,6 +12,9 @@ func TestGetSubmitters(t *testing.T) {
 	assert.Equal(t, len(input.GetSubmitters()), 0,
 		"errors happen when try to get submitters without any submitters")
 
+	input.Submitter = ""
+	assert.Equal(t, len(input.GetSubmitters()), 0, "Should be empty submitters when submitter not set")
+
 	input.Submitter = "a , b, c,d"
 	submitters := input.GetSubmitters()
 	assert.Equal(t, len(submitters), 4, "get incorrect number of submitters")
diff --git a/pkg/kapis/devops/v1alpha2/devops_test.go b/pkg/kapis/devops/v1alpha2/devops_test.go
index 1a98b56..292061f 100644
--- a/pkg/kapis/devops/v1alpha2/devops_test.go
+++ b/pkg/kapis/devops/v1alpha2/devops_test.go
@@ -1,11 +1,11 @@
 package v1alpha2
 
 import (
-	"github.com/emicklei/go-restful"
-	"github.com/stretchr/testify/assert"
 	"net/http"
 	"testing"
 
+	"github.com/emicklei/go-restful"
+	"github.com/stretchr/testify/assert"
 	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
 )