From 45dc3ed09d89aff6011b3dbcf67aecde49affb51 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 30 Nov 2023 21:29:46 -0800 Subject: [PATCH 1/2] fix: use keyset pagination only when it is supported https://github.com/mvisonneau/gitlab-ci-pipelines-exporter/pull/744 switched to keyset pagination for the `/api/v4/projects/:id/jobs` API endpoint, but this feature is only available in GitLab 15.9 and later (https://docs.gitlab.com/ee/api/jobs.html#list-project-jobs). This commit retrieves the instance metadata at startup to determine whether keyset pagination is available. If the metadata is not available or returns a version < 15.9, offset pagination will be used. --- pkg/controller/metadata.go | 53 +++++++++++++ pkg/controller/metadata_test.go | 61 +++++++++++++++ pkg/controller/scheduler.go | 4 + pkg/gitlab/client.go | 17 +++++ pkg/gitlab/jobs.go | 32 +++++--- pkg/gitlab/jobs_test.go | 129 +++++++++++++++++++------------- pkg/gitlab/version.go | 22 ++++++ pkg/gitlab/version_test.go | 54 +++++++++++++ 8 files changed, 313 insertions(+), 59 deletions(-) create mode 100644 pkg/controller/metadata.go create mode 100644 pkg/controller/metadata_test.go create mode 100644 pkg/gitlab/version.go create mode 100644 pkg/gitlab/version_test.go diff --git a/pkg/controller/metadata.go b/pkg/controller/metadata.go new file mode 100644 index 00000000..55d71364 --- /dev/null +++ b/pkg/controller/metadata.go @@ -0,0 +1,53 @@ +package controller + +import ( + "context" + "regexp" + "strconv" + + goGitlab "github.com/xanzy/go-gitlab" + + "github.com/mvisonneau/gitlab-ci-pipelines-exporter/pkg/gitlab" +) + +func parseVersionRegex(version string) (int, int, int, string) { + regexPattern := `^(\d+)\.(\d+)\.(\d+)(?:-(.*))?$` + r := regexp.MustCompile(regexPattern) + + matches := r.FindStringSubmatch(version) + + if matches == nil { + return 0, 0, 0, "Invalid version format" + } + + major, _ := strconv.Atoi(matches[1]) + minor, _ := strconv.Atoi(matches[2]) + patch, _ := strconv.Atoi(matches[3]) + + suffix := matches[4] + + return major, minor, patch, suffix +} + +func (c *Controller) GetGitLabMetadata(ctx context.Context) error { + options := []goGitlab.RequestOptionFunc{goGitlab.WithContext(ctx)} + + metadata, _, err := c.Gitlab.Metadata.GetMetadata(options...) + if err != nil { + return err + } + + if metadata.Version != "" { + major, minor, patch, suffix := parseVersionRegex(metadata.Version) + c.Gitlab.UpdateVersion( + gitlab.GitLabVersion{ + Major: major, + Minor: minor, + Patch: patch, + Suffix: suffix, + }, + ) + } + + return nil +} diff --git a/pkg/controller/metadata_test.go b/pkg/controller/metadata_test.go new file mode 100644 index 00000000..eb463b31 --- /dev/null +++ b/pkg/controller/metadata_test.go @@ -0,0 +1,61 @@ +package controller + +import ( + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mvisonneau/gitlab-ci-pipelines-exporter/pkg/config" + "github.com/mvisonneau/gitlab-ci-pipelines-exporter/pkg/gitlab" +) + +func TestGetGitLabMetadataSuccess(t *testing.T) { + tests := []struct { + name string + data string + expectedVersion gitlab.GitLabVersion + }{ + { + name: "successful parse", + data: ` +{ +"version":"16.7.0-pre", +"revision":"3fe364fe754", +"kas":{ + "enabled":true, + "externalUrl":"wss://kas.gitlab.com", + "version":"v16.7.0-rc2" +}, +"enterprise":true +} +`, + expectedVersion: gitlab.GitLabVersion{Major: 16, Minor: 7, Patch: 0, Suffix: "pre"}, + }, + { + name: "unsuccessful parse", + data: ` +{ +"version":"16.7" +} +`, + expectedVersion: gitlab.GitLabVersion{Major: 0, Minor: 0, Patch: 0, Suffix: "Invalid version format"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx, c, mux, srv := newTestController(config.Config{}) + defer srv.Close() + + mux.HandleFunc("/api/v4/metadata", + func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, tc.data) + }) + + assert.NoError(t, c.GetGitLabMetadata(ctx)) + assert.Equal(t, tc.expectedVersion, c.Gitlab.Version()) + }) + } +} diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 7a3ee763..37852af6 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -307,6 +307,10 @@ func (c *Controller) Schedule(ctx context.Context, pull config.Pull, gc config.G ctx, span := otel.Tracer(tracerName).Start(ctx, "controller:Schedule") defer span.End() + go func() { + c.GetGitLabMetadata(ctx) + }() + for tt, cfg := range map[schemas.TaskType]config.SchedulerConfig{ schemas.TaskTypePullProjectsFromWildcards: config.SchedulerConfig(pull.ProjectsFromWildcards), schemas.TaskTypePullEnvironmentsFromProjects: config.SchedulerConfig(pull.EnvironmentsFromProjects), diff --git a/pkg/gitlab/client.go b/pkg/gitlab/client.go index 64cefc69..2d979e28 100644 --- a/pkg/gitlab/client.go +++ b/pkg/gitlab/client.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strconv" + "sync" "sync/atomic" "time" @@ -36,6 +37,9 @@ type Client struct { RequestsCounter atomic.Uint64 RequestsLimit int RequestsRemaining int + + version GitLabVersion + mutex sync.RWMutex } // ClientConfig .. @@ -140,6 +144,19 @@ func (c *Client) rateLimit(ctx context.Context) { c.RequestsCounter.Add(1) } +func (c *Client) UpdateVersion(version GitLabVersion) { + c.mutex.Lock() + defer c.mutex.Unlock() + c.version = version +} + +func (c *Client) Version() GitLabVersion { + c.mutex.RLock() + defer c.mutex.RUnlock() + + return c.version +} + func (c *Client) requestsRemaining(response *goGitlab.Response) { if response == nil { return diff --git a/pkg/gitlab/jobs.go b/pkg/gitlab/jobs.go index e728db31..156caac5 100644 --- a/pkg/gitlab/jobs.go +++ b/pkg/gitlab/jobs.go @@ -231,13 +231,24 @@ func (c *Client) ListRefMostRecentJobs(ctx context.Context, ref schemas.Ref) (jo var ( foundJobs []*goGitlab.Job resp *goGitlab.Response + opt *goGitlab.ListJobsOptions ) - opt := &goGitlab.ListJobsOptions{ - ListOptions: goGitlab.ListOptions{ - Pagination: "keyset", - PerPage: 100, - }, + keysetPagination := c.Version().PipelineJobsKeysetPaginationSupported() + if keysetPagination { + opt = &goGitlab.ListJobsOptions{ + ListOptions: goGitlab.ListOptions{ + Pagination: "keyset", + PerPage: 100, + }, + } + } else { + opt = &goGitlab.ListJobsOptions{ + ListOptions: goGitlab.ListOptions{ + Page: 1, + PerPage: 100, + }, + } } options := []goGitlab.RequestOptionFunc{goGitlab.WithContext(ctx)} @@ -274,7 +285,8 @@ func (c *Client) ListRefMostRecentJobs(ctx context.Context, ref schemas.Ref) (jo } } - if resp.NextLink == "" { + if keysetPagination && resp.NextLink == "" || + (!keysetPagination && resp.CurrentPage >= resp.NextPage) { var notFoundJobs []string for k := range jobsToRefresh { @@ -295,9 +307,11 @@ func (c *Client) ListRefMostRecentJobs(ctx context.Context, ref schemas.Ref) (jo break } - options = []goGitlab.RequestOptionFunc{ - goGitlab.WithContext(ctx), - goGitlab.WithKeysetPaginationParameters(resp.NextLink), + if keysetPagination { + options = []goGitlab.RequestOptionFunc{ + goGitlab.WithContext(ctx), + goGitlab.WithKeysetPaginationParameters(resp.NextLink), + } } } diff --git a/pkg/gitlab/jobs_test.go b/pkg/gitlab/jobs_test.go index a7a45e4a..1d49a512 100644 --- a/pkg/gitlab/jobs_test.go +++ b/pkg/gitlab/jobs_test.go @@ -132,64 +132,93 @@ func TestListPipelineBridges(t *testing.T) { } func TestListRefMostRecentJobs(t *testing.T) { - ctx, mux, server, c := getMockedClient() - defer server.Close() - - ref := schemas.Ref{ - Project: schemas.NewProject("foo"), - Name: "yay", + tests := []struct { + name string + keysetPagination bool + expectedQueryParams url.Values + }{ + { + name: "offset pagination", + keysetPagination: false, + expectedQueryParams: url.Values{ + "page": []string{"1"}, + "per_page": []string{"100"}, + }, + }, + { + name: "keyset pagination", + keysetPagination: true, + expectedQueryParams: url.Values{ + "pagination": []string{"keyset"}, + "per_page": []string{"100"}, + }, + }, } - jobs, err := c.ListRefMostRecentJobs(ctx, ref) - assert.NoError(t, err) - assert.Len(t, jobs, 0) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx, mux, server, c := getMockedClient() + defer server.Close() - mux.HandleFunc("/api/v4/projects/foo/jobs", - func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "GET", r.Method) - expectedQueryParams := url.Values{ - "pagination": []string{"keyset"}, - "per_page": []string{"100"}, + if tc.keysetPagination { + c.UpdateVersion(GitLabVersion{Major: 16, Minor: 0}) + } else { + c.UpdateVersion(GitLabVersion{Major: 15, Minor: 0}) } - assert.Equal(t, expectedQueryParams, r.URL.Query()) - fmt.Fprint(w, `[{"id":3,"name":"foo","ref":"yay"},{"id":4,"name":"bar","ref":"yay"}]`) - }) - mux.HandleFunc(fmt.Sprintf("/api/v4/projects/bar/jobs"), - func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNotFound) - }) + ref := schemas.Ref{ + Project: schemas.NewProject("foo"), + Name: "yay", + } - ref.LatestJobs = schemas.Jobs{ - "foo": { - ID: 1, - Name: "foo", - }, - "bar": { - ID: 2, - Name: "bar", - }, - } + jobs, err := c.ListRefMostRecentJobs(ctx, ref) + assert.NoError(t, err) + assert.Len(t, jobs, 0) + + mux.HandleFunc("/api/v4/projects/foo/jobs", + func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "GET", r.Method) + assert.Equal(t, tc.expectedQueryParams, r.URL.Query()) + fmt.Fprint(w, `[{"id":3,"name":"foo","ref":"yay"},{"id":4,"name":"bar","ref":"yay"}]`) + }) + + mux.HandleFunc(fmt.Sprintf("/api/v4/projects/bar/jobs"), + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + ref.LatestJobs = schemas.Jobs{ + "foo": { + ID: 1, + Name: "foo", + }, + "bar": { + ID: 2, + Name: "bar", + }, + } - jobs, err = c.ListRefMostRecentJobs(ctx, ref) - assert.NoError(t, err) - assert.Len(t, jobs, 2) - assert.Equal(t, 3, jobs[0].ID) - assert.Equal(t, 4, jobs[1].ID) + jobs, err = c.ListRefMostRecentJobs(ctx, ref) + assert.NoError(t, err) + assert.Len(t, jobs, 2) + assert.Equal(t, 3, jobs[0].ID) + assert.Equal(t, 4, jobs[1].ID) - ref.LatestJobs["baz"] = schemas.Job{ - ID: 5, - Name: "baz", - } + ref.LatestJobs["baz"] = schemas.Job{ + ID: 5, + Name: "baz", + } - jobs, err = c.ListRefMostRecentJobs(ctx, ref) - assert.NoError(t, err) - assert.Len(t, jobs, 2) - assert.Equal(t, 3, jobs[0].ID) - assert.Equal(t, 4, jobs[1].ID) + jobs, err = c.ListRefMostRecentJobs(ctx, ref) + assert.NoError(t, err) + assert.Len(t, jobs, 2) + assert.Equal(t, 3, jobs[0].ID) + assert.Equal(t, 4, jobs[1].ID) - // Test invalid project id - ref.Project.Name = "bar" - _, err = c.ListRefMostRecentJobs(ctx, ref) - assert.Error(t, err) + // Test invalid project id + ref.Project.Name = "bar" + _, err = c.ListRefMostRecentJobs(ctx, ref) + assert.Error(t, err) + }) + } } diff --git a/pkg/gitlab/version.go b/pkg/gitlab/version.go new file mode 100644 index 00000000..f42d6f5d --- /dev/null +++ b/pkg/gitlab/version.go @@ -0,0 +1,22 @@ +package gitlab + +type GitLabVersion struct { + Major int + Minor int + Patch int + Suffix string +} + +// PipelineJobsKeysetPaginationSupported returns true if the GitLab instance +// is running 15.9 or later. +func (v GitLabVersion) PipelineJobsKeysetPaginationSupported() bool { + if v.Major == 0 { + return false + } else if v.Major < 15 { + return false + } else if v.Major > 15 { + return true + } else { + return v.Minor >= 9 + } +} diff --git a/pkg/gitlab/version_test.go b/pkg/gitlab/version_test.go new file mode 100644 index 00000000..0ebfd6ee --- /dev/null +++ b/pkg/gitlab/version_test.go @@ -0,0 +1,54 @@ +package gitlab + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPipelineJobsKeysetPaginationSupported(t *testing.T) { + tests := []struct { + name string + version GitLabVersion + expectedResult bool + }{ + { + name: "unknown", + version: GitLabVersion{Major: 0, Minor: 0, Patch: 0, Suffix: ""}, + expectedResult: false, + }, + { + name: "15.8.0", + version: GitLabVersion{Major: 15, Minor: 8, Patch: 0, Suffix: ""}, + expectedResult: false, + }, + { + name: "15.9.0", + version: GitLabVersion{Major: 15, Minor: 9, Patch: 0, Suffix: ""}, + expectedResult: true, + }, + { + name: "15.9.1", + version: GitLabVersion{Major: 15, Minor: 9, Patch: 1, Suffix: ""}, + expectedResult: true, + }, + { + name: "15.10.2", + version: GitLabVersion{Major: 15, Minor: 10, Patch: 12, Suffix: ""}, + expectedResult: true, + }, + { + name: "16.0.0", + version: GitLabVersion{Major: 16, Minor: 0, Patch: 0, Suffix: ""}, + expectedResult: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := tc.version.PipelineJobsKeysetPaginationSupported() + + assert.Equal(t, tc.expectedResult, result) + }) + } +} From 73dd2d3cf36ad21490e8b0cefd76e3af3b9a0811 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 4 Dec 2023 13:37:22 -0800 Subject: [PATCH 2/2] chore: Use golang.org/x/mod/semver for version parsing --- go.mod | 1 + go.sum | 2 ++ pkg/controller/metadata.go | 31 +------------------------------ pkg/controller/metadata_test.go | 6 +++--- pkg/gitlab/jobs_test.go | 4 ++-- pkg/gitlab/version.go | 32 +++++++++++++++++++++----------- pkg/gitlab/version_test.go | 22 ++++++++++++++++------ 7 files changed, 46 insertions(+), 52 deletions(-) diff --git a/go.mod b/go.mod index cea64cc3..dc157dd6 100644 --- a/go.mod +++ b/go.mod @@ -93,6 +93,7 @@ require ( go.opentelemetry.io/otel/metric v1.21.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect golang.org/x/crypto v0.16.0 // indirect + golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.19.0 // indirect golang.org/x/oauth2 v0.15.0 // indirect golang.org/x/sync v0.5.0 // indirect diff --git a/go.sum b/go.sum index fa5b3d92..62f8c347 100644 --- a/go.sum +++ b/go.sum @@ -227,6 +227,8 @@ golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq golang.org/x/exp v0.0.0-20231127185646-65229373498e h1:Gvh4YaCaXNs6dKTlfgismwWZKyjVZXwOPfIyUaqU3No= golang.org/x/exp v0.0.0-20231127185646-65229373498e/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= +golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= diff --git a/pkg/controller/metadata.go b/pkg/controller/metadata.go index 55d71364..f2485c2f 100644 --- a/pkg/controller/metadata.go +++ b/pkg/controller/metadata.go @@ -2,33 +2,12 @@ package controller import ( "context" - "regexp" - "strconv" goGitlab "github.com/xanzy/go-gitlab" "github.com/mvisonneau/gitlab-ci-pipelines-exporter/pkg/gitlab" ) -func parseVersionRegex(version string) (int, int, int, string) { - regexPattern := `^(\d+)\.(\d+)\.(\d+)(?:-(.*))?$` - r := regexp.MustCompile(regexPattern) - - matches := r.FindStringSubmatch(version) - - if matches == nil { - return 0, 0, 0, "Invalid version format" - } - - major, _ := strconv.Atoi(matches[1]) - minor, _ := strconv.Atoi(matches[2]) - patch, _ := strconv.Atoi(matches[3]) - - suffix := matches[4] - - return major, minor, patch, suffix -} - func (c *Controller) GetGitLabMetadata(ctx context.Context) error { options := []goGitlab.RequestOptionFunc{goGitlab.WithContext(ctx)} @@ -38,15 +17,7 @@ func (c *Controller) GetGitLabMetadata(ctx context.Context) error { } if metadata.Version != "" { - major, minor, patch, suffix := parseVersionRegex(metadata.Version) - c.Gitlab.UpdateVersion( - gitlab.GitLabVersion{ - Major: major, - Minor: minor, - Patch: patch, - Suffix: suffix, - }, - ) + c.Gitlab.UpdateVersion(gitlab.NewGitLabVersion(metadata.Version)) } return nil diff --git a/pkg/controller/metadata_test.go b/pkg/controller/metadata_test.go index eb463b31..5fe830f0 100644 --- a/pkg/controller/metadata_test.go +++ b/pkg/controller/metadata_test.go @@ -31,16 +31,16 @@ func TestGetGitLabMetadataSuccess(t *testing.T) { "enterprise":true } `, - expectedVersion: gitlab.GitLabVersion{Major: 16, Minor: 7, Patch: 0, Suffix: "pre"}, + expectedVersion: gitlab.NewGitLabVersion("v16.7.0-pre"), }, { name: "unsuccessful parse", data: ` { -"version":"16.7" +"revision":"3fe364fe754" } `, - expectedVersion: gitlab.GitLabVersion{Major: 0, Minor: 0, Patch: 0, Suffix: "Invalid version format"}, + expectedVersion: gitlab.NewGitLabVersion(""), }, } diff --git a/pkg/gitlab/jobs_test.go b/pkg/gitlab/jobs_test.go index 1d49a512..93b28813 100644 --- a/pkg/gitlab/jobs_test.go +++ b/pkg/gitlab/jobs_test.go @@ -161,9 +161,9 @@ func TestListRefMostRecentJobs(t *testing.T) { defer server.Close() if tc.keysetPagination { - c.UpdateVersion(GitLabVersion{Major: 16, Minor: 0}) + c.UpdateVersion(NewGitLabVersion("16.0.0")) } else { - c.UpdateVersion(GitLabVersion{Major: 15, Minor: 0}) + c.UpdateVersion(NewGitLabVersion("15.0.0")) } ref := schemas.Ref{ diff --git a/pkg/gitlab/version.go b/pkg/gitlab/version.go index f42d6f5d..efb5dcfb 100644 --- a/pkg/gitlab/version.go +++ b/pkg/gitlab/version.go @@ -1,22 +1,32 @@ package gitlab +import ( + "strings" + + "golang.org/x/mod/semver" +) + type GitLabVersion struct { - Major int - Minor int - Patch int - Suffix string + Version string +} + +func NewGitLabVersion(version string) GitLabVersion { + ver := "" + if strings.HasPrefix(version, "v") { + ver = version + } else if version != "" { + ver = "v" + version + } + + return GitLabVersion{Version: ver} } // PipelineJobsKeysetPaginationSupported returns true if the GitLab instance // is running 15.9 or later. func (v GitLabVersion) PipelineJobsKeysetPaginationSupported() bool { - if v.Major == 0 { + if v.Version == "" { return false - } else if v.Major < 15 { - return false - } else if v.Major > 15 { - return true - } else { - return v.Minor >= 9 } + + return semver.Compare(v.Version, "v15.9.0") >= 0 } diff --git a/pkg/gitlab/version_test.go b/pkg/gitlab/version_test.go index 0ebfd6ee..d12febf4 100644 --- a/pkg/gitlab/version_test.go +++ b/pkg/gitlab/version_test.go @@ -14,32 +14,42 @@ func TestPipelineJobsKeysetPaginationSupported(t *testing.T) { }{ { name: "unknown", - version: GitLabVersion{Major: 0, Minor: 0, Patch: 0, Suffix: ""}, + version: NewGitLabVersion(""), expectedResult: false, }, { name: "15.8.0", - version: GitLabVersion{Major: 15, Minor: 8, Patch: 0, Suffix: ""}, + version: NewGitLabVersion("15.8.0"), + expectedResult: false, + }, + { + name: "v15.8.0", + version: NewGitLabVersion("v15.8.0"), expectedResult: false, }, { name: "15.9.0", - version: GitLabVersion{Major: 15, Minor: 9, Patch: 0, Suffix: ""}, + version: NewGitLabVersion("15.9.0"), + expectedResult: true, + }, + { + name: "v15.9.0", + version: NewGitLabVersion("v15.9.0"), expectedResult: true, }, { name: "15.9.1", - version: GitLabVersion{Major: 15, Minor: 9, Patch: 1, Suffix: ""}, + version: NewGitLabVersion("15.9.1"), expectedResult: true, }, { name: "15.10.2", - version: GitLabVersion{Major: 15, Minor: 10, Patch: 12, Suffix: ""}, + version: NewGitLabVersion("15.10.2"), expectedResult: true, }, { name: "16.0.0", - version: GitLabVersion{Major: 16, Minor: 0, Patch: 0, Suffix: ""}, + version: NewGitLabVersion("16.0.0"), expectedResult: true, }, }