Skip to content

Commit

Permalink
Fix gitlab hooks and simplify config extension (#2537)
Browse files Browse the repository at this point in the history
- closes #2534 
- remove `IsConfigured` func from config extension.
  If `server.Config.Services.ConfigService != nil` it is always configured
  • Loading branch information
qwerty287 authored Oct 7, 2023
1 parent bb17d25 commit 3bd53b3
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 13 deletions.
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 {
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

0 comments on commit 3bd53b3

Please sign in to comment.