Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix gitlab hooks and simplify config extension #2537

Merged
merged 7 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/forge/configFetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er
continue
}

if cf.configExtension != nil && cf.configExtension.IsConfigured() {
if cf.configExtension != nil {
fetchCtx, cancel := context.WithTimeout(ctx, cf.timeout)
defer cancel() // ok here as we only try http fetching once, returning on fail and success

Expand Down
2 changes: 1 addition & 1 deletion server/forge/configFetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func TestFetch(t *testing.T) {
configFetcher := forge.NewConfigFetcher(
f,
time.Second*3,
config.NewHTTP("", ""),
nil,
&model.User{Token: "xxx"},
repo,
&model.Pipeline{Commit: "89ab7b2d6bfb347144ac7c557e638ab402848fee"},
Expand Down
2 changes: 1 addition & 1 deletion server/forge/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ func (g *GitLab) getTokenAndWebURL(link string) (token, webURL string, err error
return "", "", err
}
token = uri.Query().Get("access_token")
webURL = fmt.Sprintf("%s://%s/api/hook", uri.Scheme, uri.Host)
webURL = fmt.Sprintf("%s://%s/%s", uri.Scheme, uri.Host, strings.TrimPrefix(uri.Path, "/"))
return token, webURL, nil
}

Expand Down
6 changes: 3 additions & 3 deletions server/forge/gitlab/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ func Test_GitLab(t *testing.T) {
// Test activate method
g.Describe("Activate", func() {
g.It("Should be success", func() {
err := client.Activate(ctx, &user, &repo, "http://example.com/api/hook/test/test?access_token=token")
err := client.Activate(ctx, &user, &repo, "http://example.com/api/hook?access_token=token")
assert.NoError(t, err)
})

g.It("Should be failed, when token not given", func() {
err := client.Activate(ctx, &user, &repo, "http://example.com/api/hook/test/test")
err := client.Activate(ctx, &user, &repo, "http://example.com/api/hook")

g.Assert(err).IsNotNil()
})
Expand All @@ -124,7 +124,7 @@ func Test_GitLab(t *testing.T) {
// Test deactivate method
g.Describe("Deactivate", func() {
g.It("Should be success", func() {
err := client.Deactivate(ctx, &user, &repo, "http://example.com/api/hook/test/test?access_token=token")
err := client.Deactivate(ctx, &user, &repo, "http://example.com/api/hook?access_token=token")

g.Assert(err).IsNil()
})
Expand Down
4 changes: 2 additions & 2 deletions server/pipeline/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func Restart(ctx context.Context, store store.Store, lastPipeline *model.Pipelin
pipelineFiles = append(pipelineFiles, &forge_types.FileMeta{Data: y.Data, Name: y.Name})
}

// If config extension is active we should refetch the config in case something changed
if server.Config.Services.ConfigService != nil && server.Config.Services.ConfigService.IsConfigured() {
// If the config extension is active we should refetch the config in case something changed
if server.Config.Services.ConfigService != nil {
anbraten marked this conversation as resolved.
Show resolved Hide resolved
currentFileMeta := make([]*forge_types.FileMeta, len(configs))
for i, cfg := range configs {
currentFileMeta[i] = &forge_types.FileMeta{Name: cfg.Name, Data: cfg.Data}
Expand Down
1 change: 0 additions & 1 deletion server/plugins/config/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,5 @@ import (
)

type Extension interface {
IsConfigured() bool
FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error)
}
4 changes: 0 additions & 4 deletions server/plugins/config/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ func NewHTTP(endpoint string, privateKey crypto.PrivateKey) Extension {
return &http{endpoint, privateKey}
}

func (cp *http) IsConfigured() bool {
return cp.endpoint != ""
}

func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error) {
currentConfigs := make([]*config, len(currentFileMeta))
for i, pipe := range currentFileMeta {
Expand Down