From cab4915795b9a0891108108e50390a204af14dd7 Mon Sep 17 00:00:00 2001 From: rick Date: Tue, 2 Feb 2021 15:19:55 +0800 Subject: [PATCH] Fix the Pipeline name filter issues Signed-off-by: rick --- pkg/kapis/devops/v1alpha2/devops.go | 87 +++++++++------- pkg/kapis/devops/v1alpha3/handler.go | 4 +- pkg/models/devops/devops.go | 20 ++-- pkg/server/params/params.go | 36 ++++++- pkg/server/params/params_test.go | 102 +++++++++++++++++++ pkg/simple/client/devops/jenkins/pipeline.go | 4 +- pkg/simple/client/devops/jenkins/utils.go | 4 +- 7 files changed, 197 insertions(+), 60 deletions(-) diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go index 96da69a7b..ee4a244dd 100644 --- a/pkg/kapis/devops/v1alpha2/devops.go +++ b/pkg/kapis/devops/v1alpha2/devops.go @@ -29,9 +29,10 @@ import ( "kubesphere.io/kubesphere/pkg/apiserver/request" "kubesphere.io/kubesphere/pkg/constants" "kubesphere.io/kubesphere/pkg/models/devops" + "kubesphere.io/kubesphere/pkg/server/params" clientDevOps "kubesphere.io/kubesphere/pkg/simple/client/devops" + "kubesphere.io/kubesphere/pkg/simple/client/devops/jenkins" "net/http" - "strconv" "strings" ) @@ -54,32 +55,39 @@ func (h *ProjectPipelineHandler) GetPipeline(req *restful.Request, resp *restful func (h *ProjectPipelineHandler) getPipelinesByRequest(req *restful.Request) (api.ListResult, error) { // this is a very trick way, but don't have a better solution for now var ( - err error - start int - limit int - namespace string + start int + limit int + namespace string + nameFilter string ) // parse query from the request query := req.QueryParameter("q") for _, val := range strings.Split(query, ";") { if strings.HasPrefix(val, "pipeline:") { - namespace = strings.TrimLeft(val, "pipeline:") - namespace = strings.Split(namespace, "/")[0] + nsAndName := strings.TrimLeft(val, "pipeline:") + filterMeta := strings.Split(nsAndName, "/") + if len(filterMeta) >= 2 { + namespace = filterMeta[0] + nameFilter = filterMeta[1] // the format is '*keyword*' + nameFilter = strings.TrimSuffix(nameFilter, "*") + nameFilter = strings.TrimPrefix(nameFilter, "*") + } else if len(filterMeta) > 0 { + namespace = filterMeta[0] + } } } + pipelineFilter := func(pipeline *v1alpha3.Pipeline) bool { + return strings.Contains(pipeline.Name, nameFilter) + } + if nameFilter == "" { + pipelineFilter = nil + } + // make sure we have an appropriate value - if start, err = strconv.Atoi(req.QueryParameter("start")); err != nil { - start = 0 - } - if limit, err = strconv.Atoi(req.QueryParameter("limit")); err != nil { - limit = 10 - } - - defer req.Request.Form.Set("limit", "10000") // assume the pipelines no more than 10k - - return h.devopsOperator.ListPipelineObj(namespace, func(list []*v1alpha3.Pipeline, i int, j int) bool { + limit, start = params.ParsePaging(req) + return h.devopsOperator.ListPipelineObj(namespace, pipelineFilter, func(list []*v1alpha3.Pipeline, i int, j int) bool { return strings.Compare(strings.ToUpper(list[i].Name), strings.ToUpper(list[j].Name)) < 0 }, limit, start) } @@ -96,31 +104,32 @@ func (h *ProjectPipelineHandler) ListPipelines(req *restful.Request, resp *restf Total: objs.TotalItems, Items: make([]clientDevOps.Pipeline, len(objs.Items)), } - if pipelineList.Total > 0 && len(objs.Items) > 0 { - pipelineMap := make(map[string]int, pipelineList.Total) - for i, item := range objs.Items { - if pipeline, ok := item.(v1alpha3.Pipeline); !ok { - continue - } else { - pip := clientDevOps.Pipeline{ - Name: pipeline.Name, - } - - pipelineMap[pipeline.Name] = i - pipelineList.Items[i] = pip + pipelineMap := make(map[string]int, pipelineList.Total) + for i, _ := range objs.Items { + if pipeline, ok := objs.Items[i].(v1alpha3.Pipeline); !ok { + continue + } else { + pipelineMap[pipeline.Name] = i + pipelineList.Items[i] = clientDevOps.Pipeline{ + Name: pipeline.Name, } } + } - // get all pipelines which come from Jenkins - // fill out the rest fields - res, err := h.devopsOperator.ListPipelines(req.Request) - if err != nil { - log.Error(err) - } else { - for _, item := range res.Items { - if index, ok := pipelineMap[item.Name]; ok { - pipelineList.Items[index] = item - } + // get all pipelines which come from Jenkins + // fill out the rest fields + if query, err := jenkins.ParseJenkinsQuery(req.Request.URL.RawQuery); err == nil { + query.Set("limit", "10000") + query.Set("start", "0") + req.Request.URL.RawQuery = query.Encode() + } + res, err := h.devopsOperator.ListPipelines(req.Request) + if err != nil { + log.Error(err) + } else { + for i, _ := range res.Items { + if index, ok := pipelineMap[res.Items[i].Name]; ok { + pipelineList.Items[index] = res.Items[i] } } } diff --git a/pkg/kapis/devops/v1alpha3/handler.go b/pkg/kapis/devops/v1alpha3/handler.go index 52cb29500..10af9fb3f 100644 --- a/pkg/kapis/devops/v1alpha3/handler.go +++ b/pkg/kapis/devops/v1alpha3/handler.go @@ -181,10 +181,10 @@ func (h *devopsHandler) GetPipeline(request *restful.Request, response *restful. } func (h *devopsHandler) ListPipeline(request *restful.Request, response *restful.Response) { - devops := request.PathParameter("devops") + devopsProject := request.PathParameter("devops") limit, offset := params.ParsePaging(request) - objs, err := h.devops.ListPipelineObj(devops, nil, limit, offset) + objs, err := h.devops.ListPipelineObj(devopsProject, nil, nil, limit, offset) if err != nil { klog.Error(err) if errors.IsNotFound(err) { diff --git a/pkg/models/devops/devops.go b/pkg/models/devops/devops.go index ac0e927fb..040c3259c 100644 --- a/pkg/models/devops/devops.go +++ b/pkg/models/devops/devops.go @@ -61,7 +61,7 @@ type DevopsOperator interface { GetPipelineObj(projectName string, pipelineName string) (*v1alpha3.Pipeline, error) DeletePipelineObj(projectName string, pipelineName string) error UpdatePipelineObj(projectName string, pipeline *v1alpha3.Pipeline) (*v1alpha3.Pipeline, error) - ListPipelineObj(projectName string, sortFunc func([]*v1alpha3.Pipeline, int, int) bool, limit, offset int) (api.ListResult, error) + ListPipelineObj(projectName string, filterFunc func(*v1alpha3.Pipeline) bool, sortFunc func([]*v1alpha3.Pipeline, int, int) bool, limit, offset int) (api.ListResult, error) CreateCredentialObj(projectName string, s *v1.Secret) (*v1.Secret, error) GetCredentialObj(projectName string, secretName string) (*v1.Secret, error) @@ -256,7 +256,8 @@ func (d devopsOperator) UpdatePipelineObj(projectName string, pipeline *v1alpha3 return d.ksclient.DevopsV1alpha3().Pipelines(projectObj.Status.AdminNamespace).Update(context.Background(), pipeline, metav1.UpdateOptions{}) } -func (d devopsOperator) ListPipelineObj(projectName string, sortFunc func([]*v1alpha3.Pipeline, int, int) bool, limit, offset int) (api.ListResult, error) { +func (d devopsOperator) ListPipelineObj(projectName string, filterFunc func(pipeline *v1alpha3.Pipeline) bool, + sortFunc func([]*v1alpha3.Pipeline, int, int) bool, limit, offset int) (api.ListResult, error) { projectObj, err := d.ksInformers.Devops().V1alpha3().DevOpsProjects().Lister().Get(projectName) if err != nil { return api.ListResult{}, err @@ -266,26 +267,25 @@ func (d devopsOperator) ListPipelineObj(projectName string, sortFunc func([]*v1a return api.ListResult{}, err } - // sort the pipeline list according to the request if sortFunc != nil { + //sort the pipeline list according to the request sort.SliceStable(data, func(i, j int) bool { return sortFunc(data, i, j) }) } - items := make([]interface{}, 0) var result []interface{} - for _, item := range data { - result = append(result, *item) + for i, _ := range data { + if filterFunc != nil && !filterFunc(data[i]) { + continue + } + result = append(result, *data[i]) } if limit == -1 || limit+offset > len(result) { limit = len(result) - offset } - items = result[offset : offset+limit] - if items == nil { - items = []interface{}{} - } + items := result[offset : offset+limit] return api.ListResult{TotalItems: len(result), Items: items}, nil } diff --git a/pkg/server/params/params.go b/pkg/server/params/params.go index 47c418552..f8712bed0 100644 --- a/pkg/server/params/params.go +++ b/pkg/server/params/params.go @@ -31,18 +31,44 @@ const ( ReverseParam = "reverse" ) +const ( + DefaultLimit = 10 + DefaultPage = 1 +) + +// ParsePaging parse the paging parameters from the request, then returns the limit and offset +// supported format: ?limit=1&page=1 +// supported format: ?paging=limit=100,page=1 func ParsePaging(req *restful.Request) (limit, offset int) { paging := req.QueryParameter(PagingParam) limit = 10 - offset = 0 - if groups := regexp.MustCompile(`^limit=(-?\d+),page=(\d+)$`).FindStringSubmatch(paging); len(groups) == 3 { - limit, _ = strconv.Atoi(groups[1]) - page, _ := strconv.Atoi(groups[2]) - offset = (page - 1) * limit + page := DefaultPage + if paging != "" { + if groups := regexp.MustCompile(`^limit=(-?\d+),page=(\d+)$`).FindStringSubmatch(paging); len(groups) == 3 { + limit = AtoiOrDefault(groups[1], DefaultLimit) + page = AtoiOrDefault(groups[2], DefaultPage) + } + } else { + // try to parse from format ?limit=10&page=1 + limit = AtoiOrDefault(req.QueryParameter("limit"), DefaultLimit) + page = AtoiOrDefault(req.QueryParameter("page"), DefaultPage) + } + offset = (page - 1) * limit + + // use the explict offset + if start := req.QueryParameter("start"); start != "" { + offset = AtoiOrDefault(start, offset) } return } +func AtoiOrDefault(str string, defVal int) int { + if result, err := strconv.Atoi(str); err == nil { + return result + } + return defVal +} + var ( invalidKeyRegex = regexp.MustCompile(`[\s(){}\[\]]`) ) diff --git a/pkg/server/params/params_test.go b/pkg/server/params/params_test.go index 1055c93a1..7fc09e86f 100644 --- a/pkg/server/params/params_test.go +++ b/pkg/server/params/params_test.go @@ -17,6 +17,7 @@ limitations under the License. package params import ( + "gotest.tools/assert" "net/http" "net/url" "reflect" @@ -171,3 +172,104 @@ func Test_parseConditions(t *testing.T) { }) } } + +func TestParsePaging(t *testing.T) { + type testData struct { + req *restful.Request + limit int + offset int + } + + table := []testData{{ + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "", + }}}, + limit: 10, offset: 0, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "paging=limit=11,page=1", + }}}, + limit: 11, offset: 0, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "paging=limit=10,page=2", + }}}, + limit: 10, offset: 10, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "paging=limit=a,page=2", + }}}, + limit: 10, offset: 0, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "paging=limit=10,page=a", + }}}, + limit: 10, offset: 0, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "paging=limit=a,page=a", + }}}, + limit: 10, offset: 0, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "limit=10&page=2", + }}}, + limit: 10, offset: 10, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "page=2", + }}}, + limit: 10, offset: 10, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "page=3", + }}}, + limit: 10, offset: 20, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "start=10&limit10", + }}}, + limit: 10, offset: 10, + }, { + req: &restful.Request{Request: &http.Request{URL: &url.URL{ + RawQuery: "page=3&start=10&limit10", + }}}, + limit: 10, offset: 10, + }} + + for index, item := range table { + limit, offset := ParsePaging(item.req) + assert.Equal(t, item.limit, limit, "index: [%d], wrong limit", index) + assert.Equal(t, item.offset, offset, "index: [%d], wrong offset", index) + } +} + +func TestAtoiOrDefault(t *testing.T) { + type testData struct { + msg string + str string + def int + expected int + } + table := []testData{{ + msg: "non-numerical", + str: "a", + def: 1, + expected: 1, + }, { + msg: "non-numerical, empty string", + str: "", + def: 1, + expected: 1, + }, { + msg: "numerical", + str: "2", + def: 1, + expected: 2, + }} + + for index, item := range table { + result := AtoiOrDefault(item.str, item.def) + assert.Equal(t, item.expected, result, "test case[%d]: '%s' is wrong", index, item.msg) + } +} diff --git a/pkg/simple/client/devops/jenkins/pipeline.go b/pkg/simple/client/devops/jenkins/pipeline.go index 6856eab2d..0e68fa545 100644 --- a/pkg/simple/client/devops/jenkins/pipeline.go +++ b/pkg/simple/client/devops/jenkins/pipeline.go @@ -132,7 +132,7 @@ func (p *Pipeline) ListPipelines() (*devops.PipelineList, error) { } func (p *Pipeline) searchPipelineCount() (int, error) { - query, _ := parseJenkinsQuery(p.HttpParameters.Url.RawQuery) + query, _ := ParseJenkinsQuery(p.HttpParameters.Url.RawQuery) query.Set("start", "0") query.Set("limit", "1000") query.Set("depth", "-1") @@ -192,7 +192,7 @@ func (p *Pipeline) ListPipelineRuns() (*devops.PipelineRunList, error) { } func (p *Pipeline) searchPipelineRunsCount() (int, error) { - query, _ := parseJenkinsQuery(p.HttpParameters.Url.RawQuery) + query, _ := ParseJenkinsQuery(p.HttpParameters.Url.RawQuery) query.Set("start", "0") query.Set("limit", "1000") query.Set("depth", "-1") diff --git a/pkg/simple/client/devops/jenkins/utils.go b/pkg/simple/client/devops/jenkins/utils.go index e7893fee4..10ea4c7e8 100644 --- a/pkg/simple/client/devops/jenkins/utils.go +++ b/pkg/simple/client/devops/jenkins/utils.go @@ -72,9 +72,9 @@ func getRespBody(resp *http.Response) ([]byte, error) { } -// parseJenkinsQuery Parse the special query of jenkins. +// ParseJenkinsQuery Parse the special query of jenkins. // ParseQuery in the standard library makes the query not re-encode -func parseJenkinsQuery(query string) (url.Values, error) { +func ParseJenkinsQuery(query string) (url.Values, error) { m := make(url.Values) err := error(nil) for query != "" {