From 8f4a6d9b9315336ff0cd8a72693d7abc23687b58 Mon Sep 17 00:00:00 2001 From: rick Date: Fri, 27 Nov 2020 22:37:46 +0800 Subject: [PATCH 1/5] Restrict only specific users or admin can approve a pipeline Signed-off-by: rick --- pkg/apiserver/apiserver.go | 3 +- pkg/kapis/devops/v1alpha2/devops.go | 139 ++++++++++++++++++++++++-- pkg/kapis/devops/v1alpha2/handler.go | 5 +- pkg/kapis/devops/v1alpha2/register.go | 9 +- pkg/models/devops/devops.go | 4 - 5 files changed, 144 insertions(+), 16 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 60aa85ea2..70a0e76fb 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -210,7 +210,8 @@ func (s *APIServer) installKubeSphereAPIs() { s.SonarClient, s.KubernetesClient.KubeSphere(), s.S3Client, - s.Config.DevopsOptions.Host)) + s.Config.DevopsOptions.Host, + am.NewOperator(s.InformerFactory, s.KubernetesClient.KubeSphere(), s.KubernetesClient.Kubernetes()))) urlruntime.Must(devopsv1alpha3.AddToContainer(s.container, s.DevopsClient, s.KubernetesClient.Kubernetes(), diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 382cf972c..7f8eef92d 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -17,10 +17,17 @@ limitations under the License. package v1alpha2 import ( + "encoding/json" + "errors" + "fmt" "github.com/emicklei/go-restful" + "k8s.io/apiserver/pkg/authentication/user" log "k8s.io/klog" "kubesphere.io/kubesphere/pkg/api" + iamv1alpha2 "kubesphere.io/kubesphere/pkg/apis/iam/v1alpha2" + "kubesphere.io/kubesphere/pkg/apiserver/request" "kubesphere.io/kubesphere/pkg/models/devops" + clientDevOps "kubesphere.io/kubesphere/pkg/simple/client/devops" "net/http" "strings" ) @@ -202,6 +209,78 @@ func (h *ProjectPipelineHandler) GetPipelineRunNodes(req *restful.Request, resp resp.WriteAsJson(res) } +func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasPermit bool, err error) { + var currentUserName string + var userInfo user.Info + var ok bool + ctx := req.Request.Context() + if userInfo, ok = request.UserFrom(ctx); ok { + // check if current user belong to the admin group, grant it if it's true + var role *iamv1alpha2.GlobalRole + currentUserName = userInfo.GetName() + if role, err = h.abc.GetGlobalRoleOfUser(currentUserName); err == nil { + if role.Name == iamv1alpha2.PlatformAdmin { + hasPermit = true + return + } + } else { + return + } + } + + // step 2, check if current user if was addressed + httpReq := &http.Request{ + URL: req.Request.URL, + Header: req.Request.Header, + Form: req.Request.Form, + PostForm: req.Request.PostForm, + } + + projectName := req.PathParameter("devops") + pipelineName := req.PathParameter("pipeline") + runId := req.PathParameter("run") + nodeId := req.PathParameter("node") + stepId := req.PathParameter("step") + + // find the expected submitter list which separated by common + var expectedSubmitter string + var res []clientDevOps.NodesDetail + if res, err = h.devopsOperator.GetNodesDetail(projectName, pipelineName, runId, httpReq); err == nil { + for _, node := range res { + if node.ID != nodeId { + continue + } + + for _, step := range node.Steps { + if step.ID != stepId || step.Input == nil { + continue + } + + expectedSubmitter = fmt.Sprintf("%v", step.Input.Submitter) + break + } + break + } + } else { + log.Errorf("cannot get nodes detail, error: %v", err) + err = errors.New("cannot get the submitters of current pipeline run") + return + } + + // grant all users if there's no specific one + if expectedSubmitter == "" { + hasPermit = true + } else { + for _, submitter := range strings.Split(expectedSubmitter, ",") { + if strings.TrimSpace(submitter) == currentUserName { + hasPermit = true + break + } + } + } + return +} + func (h *ProjectPipelineHandler) SubmitInputStep(req *restful.Request, resp *restful.Response) { projectName := req.PathParameter("devops") pipelineName := req.PathParameter("pipeline") @@ -209,13 +288,27 @@ func (h *ProjectPipelineHandler) SubmitInputStep(req *restful.Request, resp *res nodeId := req.PathParameter("node") stepId := req.PathParameter("step") - res, err := h.devopsOperator.SubmitInputStep(projectName, pipelineName, runId, nodeId, stepId, req.Request) - if err != nil { - parseErr(err, resp) - return - } + var ( + response []byte + err error + ok bool + ) - resp.Write(res) + if ok, err = h.hasSubmitPermission(req); !ok || err != nil { + msg := map[string]string{ + "allow": "false", + "message": fmt.Sprintf("%v", err), + } + + response, _ = json.Marshal(msg) + } else { + response, err = h.devopsOperator.SubmitInputStep(projectName, pipelineName, runId, nodeId, stepId, req.Request) + if err != nil { + parseErr(err, resp) + return + } + } + resp.Write(response) } func (h *ProjectPipelineHandler) GetNodesDetail(req *restful.Request, resp *restful.Response) { @@ -401,6 +494,40 @@ func (h *ProjectPipelineHandler) SubmitBranchInputStep(req *restful.Request, res nodeId := req.PathParameter("node") stepId := req.PathParameter("step") + var currentUesrName string + ctx := req.Request.Context() + if user, ok := request.UserFrom(ctx); ok { + currentUesrName = user.GetName() + } + + fmt.Println("current user", currentUesrName, "nodeId", nodeId, "stepid", stepId) + req.Request.UserAgent() + if res, err := h.devopsOperator.GetNodesDetail(projectName, pipelineName, runId, req.Request); err == nil { + for _, node := range res { + fmt.Println("nodeid", node.ID) + if node.ID != nodeId { + continue + } + + for _, step := range node.Steps { + fmt.Println("stepid", step.ID, step.Input) + if step.ID != stepId && step.Input != nil { + continue + } + + submitter := step.Input.Submitter + fmt.Println(submitter) + + if currentUesrName != submitter { + resp.Write([]byte("no permission")) + return + } + } + } + } else { + log.Infof("cannot get the nodes detail when submit a branch input step") + } + res, err := h.devopsOperator.SubmitBranchInputStep(projectName, pipelineName, branchName, runId, nodeId, stepId, req.Request) if err != nil { parseErr(err, resp) diff --git a/pkg/kapis/devops/v1alpha2/handler.go b/pkg/kapis/devops/v1alpha2/handler.go index 45c6d0e41..447b0f6ac 100644 --- a/pkg/kapis/devops/v1alpha2/handler.go +++ b/pkg/kapis/devops/v1alpha2/handler.go @@ -20,6 +20,7 @@ import ( "kubesphere.io/kubesphere/pkg/client/clientset/versioned" "kubesphere.io/kubesphere/pkg/client/informers/externalversions" "kubesphere.io/kubesphere/pkg/models/devops" + "kubesphere.io/kubesphere/pkg/models/iam/am" devopsClient "kubesphere.io/kubesphere/pkg/simple/client/devops" "kubesphere.io/kubesphere/pkg/simple/client/s3" "kubesphere.io/kubesphere/pkg/simple/client/sonarqube" @@ -28,16 +29,18 @@ import ( type ProjectPipelineHandler struct { devopsOperator devops.DevopsOperator projectCredentialGetter devops.ProjectCredentialGetter + abc am.AccessManagementInterface } type PipelineSonarHandler struct { pipelineSonarGetter devops.PipelineSonarGetter } -func NewProjectPipelineHandler(devopsClient devopsClient.Interface) ProjectPipelineHandler { +func NewProjectPipelineHandler(devopsClient devopsClient.Interface, abc am.AccessManagementInterface) ProjectPipelineHandler { return ProjectPipelineHandler{ devopsOperator: devops.NewDevopsOperator(devopsClient, nil, nil, nil, nil), projectCredentialGetter: devops.NewProjectCredentialOperator(devopsClient), + abc: abc, } } diff --git a/pkg/kapis/devops/v1alpha2/register.go b/pkg/kapis/devops/v1alpha2/register.go index 5b6e7ec37..e8916b09d 100644 --- a/pkg/kapis/devops/v1alpha2/register.go +++ b/pkg/kapis/devops/v1alpha2/register.go @@ -28,6 +28,7 @@ import ( "kubesphere.io/kubesphere/pkg/client/clientset/versioned" "kubesphere.io/kubesphere/pkg/client/informers/externalversions" "kubesphere.io/kubesphere/pkg/constants" + "kubesphere.io/kubesphere/pkg/models/iam/am" "kubesphere.io/kubesphere/pkg/simple/client/devops/jenkins" "kubesphere.io/kubesphere/pkg/simple/client/s3" "kubesphere.io/kubesphere/pkg/simple/client/sonarqube" @@ -46,10 +47,10 @@ const ( var GroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha2"} -func AddToContainer(container *restful.Container, ksInformers externalversions.SharedInformerFactory, devopsClient devops.Interface, sonarqubeClient sonarqube.SonarInterface, ksClient versioned.Interface, s3Client s3.Interface, endpoint string) error { +func AddToContainer(container *restful.Container, ksInformers externalversions.SharedInformerFactory, devopsClient devops.Interface, sonarqubeClient sonarqube.SonarInterface, ksClient versioned.Interface, s3Client s3.Interface, endpoint string, abc am.AccessManagementInterface) error { ws := runtime.NewWebService(GroupVersion) - err := AddPipelineToWebService(ws, devopsClient) + err := AddPipelineToWebService(ws, devopsClient, abc) if err != nil { return err } @@ -74,12 +75,12 @@ func AddToContainer(container *restful.Container, ksInformers externalversions.S return nil } -func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface) error { +func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface, abc am.AccessManagementInterface) error { projectPipelineEnable := devopsClient != nil if projectPipelineEnable { - projectPipelineHandler := NewProjectPipelineHandler(devopsClient) + projectPipelineHandler := NewProjectPipelineHandler(devopsClient, abc) webservice.Route(webservice.GET("/devops/{devops}/credentials/{credential}/usage"). To(projectPipelineHandler.GetProjectCredentialUsage). diff --git a/pkg/models/devops/devops.go b/pkg/models/devops/devops.go index 8fdf15922..550fc3a33 100644 --- a/pkg/models/devops/devops.go +++ b/pkg/models/devops/devops.go @@ -487,20 +487,16 @@ func (d devopsOperator) GetNodeSteps(projectName, pipelineName, runId, nodeId st } func (d devopsOperator) GetPipelineRunNodes(projectName, pipelineName, runId string, req *http.Request) ([]devops.PipelineRunNodes, error) { - res, err := d.devopsClient.GetPipelineRunNodes(projectName, pipelineName, runId, convertToHttpParameters(req)) if err != nil { klog.Error(err) return nil, err } - fmt.Println() - return res, err } func (d devopsOperator) SubmitInputStep(projectName, pipelineName, runId, nodeId, stepId string, req *http.Request) ([]byte, error) { - newBody, err := getInputReqBody(req.Body) if err != nil { klog.Error(err) From df34ee9978013928d59d3e051e112177643b1172 Mon Sep 17 00:00:00 2001 From: rick Date: Mon, 30 Nov 2020 11:42:41 +0800 Subject: [PATCH 2/5] Adding approvable field to indicate if current user can approve a particular step Signed-off-by: rick --- pkg/kapis/devops/v1alpha2/devops.go | 67 +++++++++++++++++----------- pkg/simple/client/devops/pipeline.go | 41 ++++++++++++++++- tools/cmd/doc-gen/main.go | 2 +- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 7f8eef92d..8b0cf0033 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -209,24 +209,49 @@ func (h *ProjectPipelineHandler) GetPipelineRunNodes(req *restful.Request, resp resp.WriteAsJson(res) } -func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasPermit bool, err error) { - var currentUserName string +func (h *ProjectPipelineHandler) approvableCheck(nodes []clientDevOps.NodesDetail, req *restful.Request) { + currentUserName, roleName := h.getCurrentUser(req) + // check if current user belong to the admin group, grant it if it's true + isAdmin := roleName == iamv1alpha2.PlatformAdmin + + for i, node := range nodes { + if node.State != clientDevOps.StatePaused { + continue + } + + for j, step := range node.Steps { + if step.State != clientDevOps.StatePaused || step.Input == nil { + continue + } + + nodes[i].Steps[j].Approvable = isAdmin || step.Input.Approvable(currentUserName) + } + } +} + +func (h *ProjectPipelineHandler) getCurrentUser(req *restful.Request) (username, roleName string) { var userInfo user.Info var ok bool + var err error + ctx := req.Request.Context() if userInfo, ok = request.UserFrom(ctx); ok { - // check if current user belong to the admin group, grant it if it's true var role *iamv1alpha2.GlobalRole - currentUserName = userInfo.GetName() - if role, err = h.abc.GetGlobalRoleOfUser(currentUserName); err == nil { - if role.Name == iamv1alpha2.PlatformAdmin { - hasPermit = true - return - } - } else { - return + username = userInfo.GetName() + if role, err = h.abc.GetGlobalRoleOfUser(username); err == nil { + roleName = role.Name } } + return +} + +func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasPermit bool, err error) { + currentUserName, roleName := h.getCurrentUser(req) + // check if current user belong to the admin group, grant it if it's true + if roleName == iamv1alpha2.PlatformAdmin { + hasPermit = true + return + } // step 2, check if current user if was addressed httpReq := &http.Request{ @@ -242,8 +267,7 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP nodeId := req.PathParameter("node") stepId := req.PathParameter("step") - // find the expected submitter list which separated by common - var expectedSubmitter string + // check if current user can approve this input var res []clientDevOps.NodesDetail if res, err = h.devopsOperator.GetNodesDetail(projectName, pipelineName, runId, httpReq); err == nil { for _, node := range res { @@ -256,7 +280,7 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP continue } - expectedSubmitter = fmt.Sprintf("%v", step.Input.Submitter) + hasPermit = step.Input.Approvable(currentUserName) break } break @@ -266,18 +290,6 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP err = errors.New("cannot get the submitters of current pipeline run") return } - - // grant all users if there's no specific one - if expectedSubmitter == "" { - hasPermit = true - } else { - for _, submitter := range strings.Split(expectedSubmitter, ",") { - if strings.TrimSpace(submitter) == currentUserName { - hasPermit = true - break - } - } - } return } @@ -321,6 +333,8 @@ func (h *ProjectPipelineHandler) GetNodesDetail(req *restful.Request, resp *rest parseErr(err, resp) return } + h.approvableCheck(res, req) + resp.WriteAsJson(res) } @@ -548,6 +562,7 @@ func (h *ProjectPipelineHandler) GetBranchNodesDetail(req *restful.Request, resp parseErr(err, resp) return } + h.approvableCheck(res, req) resp.WriteAsJson(res) } diff --git a/pkg/simple/client/devops/pipeline.go b/pkg/simple/client/devops/pipeline.go index 800220d71..0eb78db04 100644 --- a/pkg/simple/client/devops/pipeline.go +++ b/pkg/simple/client/devops/pipeline.go @@ -17,9 +17,11 @@ limitations under the License. package devops import ( + "fmt" "io" "net/http" "net/url" + "strings" ) type PipelineList struct { @@ -979,6 +981,8 @@ type NodeSteps struct { StartTime string `json:"startTime,omitempty" description:"the time of starts"` State string `json:"state,omitempty" description:"run state. e.g. SKIPPED"` Type string `json:"type,omitempty" description:"type"` + // Approvable indicates if this step can be approved by current user + Approvable bool `json:"aprovable" description:"indicate if this step can be approved by current user"` } // CheckScriptCompile @@ -1075,6 +1079,11 @@ type NodesDetail struct { Steps []NodeSteps `json:"steps,omitempty" description:"steps"` } +const ( + // StatePaused indicates a node or a step was paused, for example it's waiting for an iput + StatePaused = "PAUSED" +) + type NodesStepsIndex struct { Id int `json:"id,omitempty" description:"id"` Steps []NodeSteps `json:"steps,omitempty" description:"steps"` @@ -1095,6 +1104,37 @@ type Input struct { Submitter interface{} `json:"submitter,omitempty" description:"check submitter"` } +// GetSubmitters returns the all submitters related to this input +func (i *Input) GetSubmitters() (submitters []string) { + if i.Submitter == nil { + return + } + + submitterArray := strings.Split(fmt.Sprintf("%v", i.Submitter), ",") + submitters = make([]string, len(submitterArray)) + for i, submitter := range submitterArray { + submitters[i] = strings.TrimSpace(submitter) + } + return +} + +// Approvable returns the result if the given identify (username or group name) can approve this input +func (i *Input) Approvable(identify string) (ok bool) { + submitters := i.GetSubmitters() + + // it means anyone can approve this if there's no specific one + if len(submitters) == 0 { + ok = true + } else { + for _, submitter := range submitters { + if submitter == identify { + ok = true + } + } + } + return +} + type HttpParameters struct { Method string `json:"method,omitempty"` Header http.Header `json:"header,omitempty"` @@ -1105,7 +1145,6 @@ type HttpParameters struct { } type PipelineOperator interface { - // Pipelinne operator interface GetPipeline(projectName, pipelineName string, httpParameters *HttpParameters) (*Pipeline, error) ListPipelines(httpParameters *HttpParameters) (*PipelineList, error) diff --git a/tools/cmd/doc-gen/main.go b/tools/cmd/doc-gen/main.go index a0b9c226a..52138cbcc 100644 --- a/tools/cmd/doc-gen/main.go +++ b/tools/cmd/doc-gen/main.go @@ -119,7 +119,7 @@ func generateSwaggerJson() []byte { urlruntime.Must(oauth.AddToContainer(container, nil, nil, nil, nil, nil)) urlruntime.Must(clusterkapisv1alpha1.AddToContainer(container, informerFactory.KubernetesSharedInformerFactory(), informerFactory.KubeSphereSharedInformerFactory(), "", "", "")) - urlruntime.Must(devopsv1alpha2.AddToContainer(container, informerFactory.KubeSphereSharedInformerFactory(), &fakedevops.Devops{}, nil, clientsets.KubeSphere(), fakes3.NewFakeS3(), "")) + urlruntime.Must(devopsv1alpha2.AddToContainer(container, informerFactory.KubeSphereSharedInformerFactory(), &fakedevops.Devops{}, nil, clientsets.KubeSphere(), fakes3.NewFakeS3(), "", am.NewReadOnlyOperator(informerFactory))) urlruntime.Must(devopsv1alpha3.AddToContainer(container, &fakedevops.Devops{}, clientsets.Kubernetes(), clientsets.KubeSphere(), informerFactory.KubeSphereSharedInformerFactory(), informerFactory.KubernetesSharedInformerFactory())) urlruntime.Must(iamv1alpha2.AddToContainer(container, im.NewOperator(clientsets.KubeSphere(), informerFactory, nil), am.NewReadOnlyOperator(informerFactory), group.New(informerFactory, clientsets.KubeSphere(), clientsets.Kubernetes()), authoptions.NewAuthenticateOptions())) urlruntime.Must(monitoringv1alpha3.AddToContainer(container, clientsets.Kubernetes(), nil, informerFactory, nil)) From 8451c182771c6391fa7f455c3d5732fe98ec7038 Mon Sep 17 00:00:00 2001 From: rick Date: Mon, 30 Nov 2020 14:03:42 +0800 Subject: [PATCH 3/5] Add unit tests for pipeline approve functions Signed-off-by: rick --- pkg/kapis/devops/v1alpha2/devops.go | 55 ++++++++--------------- pkg/kapis/devops/v1alpha2/handler.go | 6 +-- pkg/kapis/devops/v1alpha2/register.go | 8 ++-- pkg/simple/client/devops/pipeline_test.go | 34 ++++++++++++++ 4 files changed, 59 insertions(+), 44 deletions(-) create mode 100644 pkg/simple/client/devops/pipeline_test.go diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 8b0cf0033..58c858587 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -238,7 +238,7 @@ func (h *ProjectPipelineHandler) getCurrentUser(req *restful.Request) (username, if userInfo, ok = request.UserFrom(ctx); ok { var role *iamv1alpha2.GlobalRole username = userInfo.GetName() - if role, err = h.abc.GetGlobalRoleOfUser(username); err == nil { + if role, err = h.amInterface.GetGlobalRoleOfUser(username); err == nil { roleName = role.Name } } @@ -508,47 +508,28 @@ func (h *ProjectPipelineHandler) SubmitBranchInputStep(req *restful.Request, res nodeId := req.PathParameter("node") stepId := req.PathParameter("step") - var currentUesrName string - ctx := req.Request.Context() - if user, ok := request.UserFrom(ctx); ok { - currentUesrName = user.GetName() - } + var ( + response []byte + err error + ok bool + ) - fmt.Println("current user", currentUesrName, "nodeId", nodeId, "stepid", stepId) - req.Request.UserAgent() - if res, err := h.devopsOperator.GetNodesDetail(projectName, pipelineName, runId, req.Request); err == nil { - for _, node := range res { - fmt.Println("nodeid", node.ID) - if node.ID != nodeId { - continue - } - - for _, step := range node.Steps { - fmt.Println("stepid", step.ID, step.Input) - if step.ID != stepId && step.Input != nil { - continue - } - - submitter := step.Input.Submitter - fmt.Println(submitter) - - if currentUesrName != submitter { - resp.Write([]byte("no permission")) - return - } - } + if ok, err = h.hasSubmitPermission(req); !ok || err != nil { + msg := map[string]string{ + "allow": "false", + "message": fmt.Sprintf("%v", err), } + + response, _ = json.Marshal(msg) } else { - log.Infof("cannot get the nodes detail when submit a branch input step") + response, err = h.devopsOperator.SubmitBranchInputStep(projectName, pipelineName, branchName, runId, nodeId, stepId, req.Request) + if err != nil { + parseErr(err, resp) + return + } } - res, err := h.devopsOperator.SubmitBranchInputStep(projectName, pipelineName, branchName, runId, nodeId, stepId, req.Request) - if err != nil { - parseErr(err, resp) - return - } - - resp.Write(res) + resp.Write(response) } func (h *ProjectPipelineHandler) GetBranchNodesDetail(req *restful.Request, resp *restful.Response) { diff --git a/pkg/kapis/devops/v1alpha2/handler.go b/pkg/kapis/devops/v1alpha2/handler.go index 447b0f6ac..35044a8b0 100644 --- a/pkg/kapis/devops/v1alpha2/handler.go +++ b/pkg/kapis/devops/v1alpha2/handler.go @@ -29,18 +29,18 @@ import ( type ProjectPipelineHandler struct { devopsOperator devops.DevopsOperator projectCredentialGetter devops.ProjectCredentialGetter - abc am.AccessManagementInterface + amInterface am.AccessManagementInterface } type PipelineSonarHandler struct { pipelineSonarGetter devops.PipelineSonarGetter } -func NewProjectPipelineHandler(devopsClient devopsClient.Interface, abc am.AccessManagementInterface) ProjectPipelineHandler { +func NewProjectPipelineHandler(devopsClient devopsClient.Interface, amInterface am.AccessManagementInterface) ProjectPipelineHandler { return ProjectPipelineHandler{ devopsOperator: devops.NewDevopsOperator(devopsClient, nil, nil, nil, nil), projectCredentialGetter: devops.NewProjectCredentialOperator(devopsClient), - abc: abc, + amInterface: amInterface, } } diff --git a/pkg/kapis/devops/v1alpha2/register.go b/pkg/kapis/devops/v1alpha2/register.go index e8916b09d..dcb30747b 100644 --- a/pkg/kapis/devops/v1alpha2/register.go +++ b/pkg/kapis/devops/v1alpha2/register.go @@ -47,10 +47,10 @@ const ( var GroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha2"} -func AddToContainer(container *restful.Container, ksInformers externalversions.SharedInformerFactory, devopsClient devops.Interface, sonarqubeClient sonarqube.SonarInterface, ksClient versioned.Interface, s3Client s3.Interface, endpoint string, abc am.AccessManagementInterface) error { +func AddToContainer(container *restful.Container, ksInformers externalversions.SharedInformerFactory, devopsClient devops.Interface, sonarqubeClient sonarqube.SonarInterface, ksClient versioned.Interface, s3Client s3.Interface, endpoint string, amInterface am.AccessManagementInterface) error { ws := runtime.NewWebService(GroupVersion) - err := AddPipelineToWebService(ws, devopsClient, abc) + err := AddPipelineToWebService(ws, devopsClient, amInterface) if err != nil { return err } @@ -75,12 +75,12 @@ func AddToContainer(container *restful.Container, ksInformers externalversions.S return nil } -func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface, abc am.AccessManagementInterface) error { +func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface, amInterface am.AccessManagementInterface) error { projectPipelineEnable := devopsClient != nil if projectPipelineEnable { - projectPipelineHandler := NewProjectPipelineHandler(devopsClient, abc) + projectPipelineHandler := NewProjectPipelineHandler(devopsClient, amInterface) webservice.Route(webservice.GET("/devops/{devops}/credentials/{credential}/usage"). To(projectPipelineHandler.GetProjectCredentialUsage). diff --git a/pkg/simple/client/devops/pipeline_test.go b/pkg/simple/client/devops/pipeline_test.go new file mode 100644 index 000000000..2f9bb0ed6 --- /dev/null +++ b/pkg/simple/client/devops/pipeline_test.go @@ -0,0 +1,34 @@ +package devops + +import ( + "gotest.tools/assert" + "testing" +) + +func TestGetSubmitters(t *testing.T) { + input := &Input{} + assert.Equal(t, len(input.GetSubmitters()), 0, + "errors happen when try to get submitters without any submitters") + + input.Submitter = "a , b, c,d" + submitters := input.GetSubmitters() + assert.Equal(t, len(submitters), 4, "get incorrect number of submitters") + assert.DeepEqual(t, submitters, []string{"a", "b", "c", "d"}) +} + +func TestApprovable(t *testing.T) { + input := &Input{} + + assert.Equal(t, input.Approvable(""), true, "should allow anyone to approve it if there's no submitter given") + assert.Equal(t, input.Approvable("fake"), true, "should allow anyone to approve it if there's no submitter given") + + input.Submitter = "fake" + assert.Equal(t, input.Approvable(""), false, "should not approve by nobody if there's a particular submitter") + assert.Equal(t, input.Approvable("rick"), false, "should not approve by who is not the specific one") + assert.Equal(t, input.Approvable("fake"), true, "should be approvable") + + input.Submitter = "fake, good ,bad" + assert.Equal(t, input.Approvable("fake"), true, "should be approvable") + assert.Equal(t, input.Approvable("good"), true, "should be approvable") + assert.Equal(t, input.Approvable("bad"), true, "should be approvable") +} From 92e7349cf99b06f1fb234bd361144ab7d7121b3f Mon Sep 17 00:00:00 2001 From: rick Date: Wed, 2 Dec 2020 14:07:55 +0800 Subject: [PATCH 4/5] Fix an issue which the pipeline owner cannot approve his pipeline Signed-off-by: rick --- go.mod | 2 ++ pkg/kapis/devops/v1alpha2/devops.go | 20 ++++++++++++++++---- pkg/kapis/devops/v1alpha2/handler.go | 4 ++-- pkg/kapis/devops/v1alpha2/register.go | 6 +++--- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 3da567126..97a4bea9d 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( github.com/kubernetes-csi/external-snapshotter/v2 v2.1.0 github.com/kubesphere/sonargo v0.0.2 github.com/lib/pq v1.2.0 // indirect + github.com/mitchellh/mapstructure v1.2.2 github.com/onsi/ginkgo v1.12.0 github.com/onsi/gomega v1.9.0 github.com/open-policy-agent/opa v0.18.0 @@ -80,6 +81,7 @@ require ( gopkg.in/src-d/go-git.v4 v4.11.0 gopkg.in/yaml.v2 v2.3.0 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c + gotest.tools v2.2.0+incompatible istio.io/api v0.0.0-20191111210003-35e06ef8d838 istio.io/client-go v0.0.0-20191113122552-9bd0ba57c3d2 k8s.io/api v0.17.5 diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 58c858587..76022084e 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -26,6 +26,7 @@ import ( "kubesphere.io/kubesphere/pkg/api" iamv1alpha2 "kubesphere.io/kubesphere/pkg/apis/iam/v1alpha2" "kubesphere.io/kubesphere/pkg/apiserver/request" + "kubesphere.io/kubesphere/pkg/constants" "kubesphere.io/kubesphere/pkg/models/devops" clientDevOps "kubesphere.io/kubesphere/pkg/simple/client/devops" "net/http" @@ -229,6 +230,17 @@ func (h *ProjectPipelineHandler) approvableCheck(nodes []clientDevOps.NodesDetai } } +func (h *ProjectPipelineHandler) createdBy(projectName string, pipelineName string, currentUserName string) bool { + if pipeline, err := h.devopsOperator.GetPipelineObj(projectName, pipelineName); err == nil { + if creator, ok := pipeline.Annotations[constants.CreatorAnnotationKey]; ok { + return creator == currentUserName + } + } else { + log.Error(fmt.Sprintf("cannot get pipeline %s/%s, error %#v", projectName, pipelineName, err)) + } + return false +} + func (h *ProjectPipelineHandler) getCurrentUser(req *restful.Request) (username, roleName string) { var userInfo user.Info var ok bool @@ -247,8 +259,10 @@ func (h *ProjectPipelineHandler) getCurrentUser(req *restful.Request) (username, func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasPermit bool, err error) { currentUserName, roleName := h.getCurrentUser(req) - // check if current user belong to the admin group, grant it if it's true - if roleName == iamv1alpha2.PlatformAdmin { + projectName := req.PathParameter("devops") + pipelineName := req.PathParameter("pipeline") + // check if current user belong to the admin group or he's the owner, grant it if it's true + if roleName == iamv1alpha2.PlatformAdmin || h.createdBy(projectName, pipelineName, currentUserName) { hasPermit = true return } @@ -261,8 +275,6 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP PostForm: req.Request.PostForm, } - projectName := req.PathParameter("devops") - pipelineName := req.PathParameter("pipeline") runId := req.PathParameter("run") nodeId := req.PathParameter("node") stepId := req.PathParameter("step") diff --git a/pkg/kapis/devops/v1alpha2/handler.go b/pkg/kapis/devops/v1alpha2/handler.go index 35044a8b0..56ea6147f 100644 --- a/pkg/kapis/devops/v1alpha2/handler.go +++ b/pkg/kapis/devops/v1alpha2/handler.go @@ -36,9 +36,9 @@ type PipelineSonarHandler struct { pipelineSonarGetter devops.PipelineSonarGetter } -func NewProjectPipelineHandler(devopsClient devopsClient.Interface, amInterface am.AccessManagementInterface) ProjectPipelineHandler { +func NewProjectPipelineHandler(devopsClient devopsClient.Interface, ksInformers externalversions.SharedInformerFactory, amInterface am.AccessManagementInterface) ProjectPipelineHandler { return ProjectPipelineHandler{ - devopsOperator: devops.NewDevopsOperator(devopsClient, nil, nil, nil, nil), + devopsOperator: devops.NewDevopsOperator(devopsClient, nil, nil, ksInformers, nil), projectCredentialGetter: devops.NewProjectCredentialOperator(devopsClient), amInterface: amInterface, } diff --git a/pkg/kapis/devops/v1alpha2/register.go b/pkg/kapis/devops/v1alpha2/register.go index dcb30747b..f659b6f6d 100644 --- a/pkg/kapis/devops/v1alpha2/register.go +++ b/pkg/kapis/devops/v1alpha2/register.go @@ -50,7 +50,7 @@ var GroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha2"} func AddToContainer(container *restful.Container, ksInformers externalversions.SharedInformerFactory, devopsClient devops.Interface, sonarqubeClient sonarqube.SonarInterface, ksClient versioned.Interface, s3Client s3.Interface, endpoint string, amInterface am.AccessManagementInterface) error { ws := runtime.NewWebService(GroupVersion) - err := AddPipelineToWebService(ws, devopsClient, amInterface) + err := AddPipelineToWebService(ws, devopsClient, ksInformers, amInterface) if err != nil { return err } @@ -75,12 +75,12 @@ func AddToContainer(container *restful.Container, ksInformers externalversions.S return nil } -func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface, amInterface am.AccessManagementInterface) error { +func AddPipelineToWebService(webservice *restful.WebService, devopsClient devops.Interface, ksInformers externalversions.SharedInformerFactory, amInterface am.AccessManagementInterface) error { projectPipelineEnable := devopsClient != nil if projectPipelineEnable { - projectPipelineHandler := NewProjectPipelineHandler(devopsClient, amInterface) + projectPipelineHandler := NewProjectPipelineHandler(devopsClient, ksInformers, amInterface) webservice.Route(webservice.GET("/devops/{devops}/credentials/{credential}/usage"). To(projectPipelineHandler.GetProjectCredentialUsage). From 4d19c4dbb400a4a6c4372777fcaef3f3e3d2a90c Mon Sep 17 00:00:00 2001 From: rick Date: Mon, 7 Dec 2020 11:09:37 +0800 Subject: [PATCH 5/5] Provide a specific log level instead of using log.error unifi variable declare style Signed-off-by: rick --- pkg/kapis/devops/v1alpha2/devops.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 76022084e..f9d8ca52c 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -236,7 +236,7 @@ func (h *ProjectPipelineHandler) createdBy(projectName string, pipelineName stri return creator == currentUserName } } else { - log.Error(fmt.Sprintf("cannot get pipeline %s/%s, error %#v", projectName, pipelineName, err)) + log.V(4).Infof("cannot get pipeline %s/%s, error %#v", projectName, pipelineName, err) } return false } @@ -298,7 +298,7 @@ func (h *ProjectPipelineHandler) hasSubmitPermission(req *restful.Request) (hasP break } } else { - log.Errorf("cannot get nodes detail, error: %v", err) + log.V(4).Infof("cannot get nodes detail, error: %v", err) err = errors.New("cannot get the submitters of current pipeline run") return } @@ -312,11 +312,9 @@ func (h *ProjectPipelineHandler) SubmitInputStep(req *restful.Request, resp *res nodeId := req.PathParameter("node") stepId := req.PathParameter("step") - var ( - response []byte - err error - ok bool - ) + var response []byte + var err error + var ok bool if ok, err = h.hasSubmitPermission(req); !ok || err != nil { msg := map[string]string{ @@ -520,11 +518,9 @@ func (h *ProjectPipelineHandler) SubmitBranchInputStep(req *restful.Request, res nodeId := req.PathParameter("node") stepId := req.PathParameter("step") - var ( - response []byte - err error - ok bool - ) + var response []byte + var err error + var ok bool if ok, err = h.hasSubmitPermission(req); !ok || err != nil { msg := map[string]string{