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

Fetch repositories with remote ID if possible #1078

Merged
merged 60 commits into from
Sep 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
3d64cac
Fetch repositories with remote ID if possible
qwerty287 Aug 5, 2022
07791c4
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 5, 2022
f6d6548
Fixes and improvements
qwerty287 Aug 5, 2022
1fc7764
Implement redirections and some other fixes
qwerty287 Aug 5, 2022
c898101
Regenerate mocks + gofumpt
qwerty287 Aug 5, 2022
6a60ab5
Merge branch 'master' into fetch-withid
qwerty287 Aug 15, 2022
198c461
Add migration
qwerty287 Aug 15, 2022
e8207d8
Support BB cloud
qwerty287 Aug 15, 2022
77e6987
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 15, 2022
f87d4fa
Merge branch 'master' into fetch-withid
qwerty287 Aug 15, 2022
995ce82
Redirect renamed repos
qwerty287 Aug 19, 2022
d7fee1c
Improve arguments
qwerty287 Aug 19, 2022
f3ba6b1
fix repo sync
qwerty287 Aug 19, 2022
6123350
Resolve TODO by moving Perm to Repo
qwerty287 Aug 19, 2022
f42f33b
Fix possible sqlite lock
qwerty287 Aug 19, 2022
2c85c9d
Fix comment
qwerty287 Aug 19, 2022
e97b600
Fix lints + todos
qwerty287 Aug 19, 2022
873b05c
update format target
qwerty287 Aug 19, 2022
97d04f9
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 20, 2022
30d39c4
Fix prettier format
qwerty287 Aug 20, 2022
930708d
Fix tests
qwerty287 Aug 20, 2022
d6782d8
Fix tests
qwerty287 Aug 20, 2022
40b372e
Delete redirections with repo
qwerty287 Aug 20, 2022
843bd5e
Remote unique constraint
qwerty287 Aug 20, 2022
9372891
Add TableName method
qwerty287 Aug 20, 2022
5bc82e6
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 20, 2022
28e5ebf
Undo formatting changes
qwerty287 Aug 20, 2022
f8cf95e
Fix reset
qwerty287 Aug 20, 2022
6ee4e87
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 25, 2022
62daf1b
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 27, 2022
b3e9391
Fix a test
qwerty287 Aug 27, 2022
0ec9f8a
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 30, 2022
ad7bf96
Fix migration number
qwerty287 Aug 30, 2022
e4748ee
Fix migration type numbers
qwerty287 Aug 30, 2022
d9e44a5
Use native builder method
qwerty287 Aug 30, 2022
4bf6caf
Fix comment
qwerty287 Aug 30, 2022
6934540
Fix SQLite error & test
qwerty287 Aug 30, 2022
e8ca9cb
Merge branch 'master' into fetch-withid
6543 Aug 31, 2022
f0501ba
Add redirection tests
qwerty287 Aug 31, 2022
0380db6
Update server/remote/coding/coding.go
qwerty287 Aug 31, 2022
1c80525
Refer to todo issue
qwerty287 Aug 31, 2022
64a2324
Remove migration
qwerty287 Aug 31, 2022
fb07dc5
Merge remote-tracking branch 'upstream/master' into fetch-withid
qwerty287 Aug 31, 2022
708f8e0
Prefer fmt.Sprint
qwerty287 Aug 31, 2022
66d5c3d
Prefer fmt.Sprint
qwerty287 Aug 31, 2022
92b41c5
Fix migration
qwerty287 Aug 31, 2022
5ab3f36
Fix test var
qwerty287 Aug 31, 2022
8070c9c
Merge branch 'master' into fetch-withid
6543 Sep 1, 2022
f2f44f5
go generate ./...
6543 Sep 1, 2022
5b4d703
Apply suggestions from code review
6543 Sep 1, 2022
33ea365
Update server/api/repo.go
qwerty287 Sep 4, 2022
74bff65
Fix empty ID batch updating
qwerty287 Sep 4, 2022
51c7a28
Add redirection test
qwerty287 Sep 4, 2022
fb2c136
Merge branch 'master' into fetch-withid
qwerty287 Sep 4, 2022
11e499e
Remove useless test code
qwerty287 Sep 4, 2022
e00e69d
Check for id == "0"
qwerty287 Sep 4, 2022
c568e73
Merge branch 'master' into fetch-withid
6543 Sep 4, 2022
32c8aa3
add own type
6543 Sep 4, 2022
1e0f4f3
Merge branch 'master' into fetch-withid
6543 Sep 5, 2022
65fc724
IsSet -> IsValid
6543 Sep 5, 2022
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
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ vendor:
go mod tidy
go mod vendor

format:
@gofmt -s -w ${GOFILES_NOVENDOR}
format: install-tools
6543 marked this conversation as resolved.
Show resolved Hide resolved
@gofumpt -extra -w ${GOFILES_NOVENDOR}

.PHONY: docs
docs:
Expand Down Expand Up @@ -140,6 +140,9 @@ install-tools:
fi ; \
hash lint > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
go install github.com/rs/zerolog/cmd/lint@latest; \
fi ; \
hash gofumpt > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
go install mvdan.cc/gofumpt@latest; \
fi

cross-compile-server:
Expand Down
35 changes: 32 additions & 3 deletions server/api/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ func BlockTilQueueHasRunningItem(c *gin.Context) {
// PostHook start a pipeline triggered by a forges post webhook
func PostHook(c *gin.Context) {
_store := store.FromContext(c)
remote := server.Config.Services.Remote

tmpRepo, tmpBuild, err := server.Config.Services.Remote.Hook(c, c.Request)
tmpRepo, tmpBuild, err := remote.Hook(c, c.Request)
if err != nil {
msg := "failure to parse hook"
log.Debug().Err(err).Msg(msg)
Expand Down Expand Up @@ -100,7 +101,7 @@ func PostHook(c *gin.Context) {
return
}

repo, err := _store.GetRepoName(tmpRepo.Owner + "/" + tmpRepo.Name)
repo, err := _store.GetRepoNameFallback(tmpRepo.RemoteID, tmpRepo.FullName)
if err != nil {
msg := fmt.Sprintf("failure to get repo %s from store", tmpRepo.FullName)
log.Error().Err(err).Msg(msg)
Expand All @@ -114,6 +115,23 @@ func PostHook(c *gin.Context) {
return
}

oldFullName := repo.FullName
if oldFullName != tmpRepo.FullName {
// create a redirection
err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
if err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
}
}

repo.Update(tmpRepo)
err = _store.UpdateRepo(repo)
if err != nil {
c.String(http.StatusInternalServerError, err.Error())
return
}

// get the token and verify the hook is authorized
parsed, err := token.ParseRequest(c.Request, func(_ *token.Token) (string, error) {
return repo.Hash, nil
Expand All @@ -124,7 +142,18 @@ func PostHook(c *gin.Context) {
c.String(http.StatusBadRequest, msg)
return
}
if parsed.Text != repo.FullName {
verifiedKey := parsed.Text == oldFullName
if !verifiedKey {
verifiedKey, err = _store.HasRedirectionForRepo(repo.ID, repo.FullName)
if err != nil {
msg := "failure to verify token from hook. Could not check for redirections of the repo"
log.Error().Err(err).Msg(msg)
c.String(http.StatusInternalServerError, msg)
return
}
}

if !verifiedKey {
msg := fmt.Sprintf("failure to verify token from hook. Expected %s, got %s", repo.FullName, parsed.Text)
log.Debug().Msg(msg)
c.String(http.StatusForbidden, msg)
Expand Down
72 changes: 39 additions & 33 deletions server/api/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,25 @@ func PostRepo(c *gin.Context) {
sig,
)

from, err := remote.Repo(c, user, repo.RemoteID, repo.Owner, repo.Name)
if err == nil {
if repo.FullName != from.FullName {
// create a redirection
err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
if err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
}
}
repo.Update(from)
}

err = remote.Activate(c, user, repo, link)
if err != nil {
c.String(http.StatusInternalServerError, err.Error())
return
}

from, err := remote.Repo(c, user, repo.Owner, repo.Name)
if err == nil {
repo.Update(from)
}

err = _store.UpdateRepo(repo)
if err != nil {
c.String(http.StatusInternalServerError, err.Error())
Expand Down Expand Up @@ -251,35 +259,36 @@ func RepairRepo(c *gin.Context) {
sig,
)

if err := remote.Deactivate(c, user, repo, host); err != nil {
log.Trace().Err(err).Msgf("deactivate repo '%s' to repair failed", repo.FullName)
}
if err := remote.Activate(c, user, repo, link); err != nil {
c.String(500, err.Error())
return
}

from, err := remote.Repo(c, user, repo.Owner, repo.Name)
from, err := remote.Repo(c, user, repo.RemoteID, repo.Owner, repo.Name)
if err != nil {
log.Error().Err(err).Msgf("get repo '%s/%s' from remote", repo.Owner, repo.Name)
c.AbortWithStatus(http.StatusInternalServerError)
return
}
repo.Name = from.Name
repo.Owner = from.Owner
repo.FullName = from.FullName
repo.Avatar = from.Avatar
repo.Link = from.Link
repo.Clone = from.Clone
repo.IsSCMPrivate = from.IsSCMPrivate
if repo.IsSCMPrivate != from.IsSCMPrivate {
repo.ResetVisibility()

if repo.FullName != from.FullName {
// create a redirection
err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
if err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
}
}

repo.Update(from)
if err := _store.UpdateRepo(repo); err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
}

if err := remote.Deactivate(c, user, repo, host); err != nil {
log.Trace().Err(err).Msgf("deactivate repo '%s' to repair failed", repo.FullName)
}
if err := remote.Activate(c, user, repo, link); err != nil {
c.String(500, err.Error())
return
}

c.Writer.WriteHeader(http.StatusOK)
}

Expand All @@ -302,7 +311,7 @@ func MoveRepo(c *gin.Context) {
return
}

from, err := remote.Repo(c, user, owner, name)
from, err := remote.Repo(c, user, "", owner, name)
if err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
Expand All @@ -312,17 +321,14 @@ func MoveRepo(c *gin.Context) {
return
}

repo.Name = from.Name
repo.Owner = from.Owner
repo.FullName = from.FullName
repo.Avatar = from.Avatar
repo.Link = from.Link
repo.Clone = from.Clone
repo.IsSCMPrivate = from.IsSCMPrivate
if repo.IsSCMPrivate != from.IsSCMPrivate {
repo.ResetVisibility()
err = _store.CreateRedirection(&model.Redirection{RepoID: repo.ID, FullName: repo.FullName})
if err != nil {
_ = c.AbortWithError(http.StatusInternalServerError, err)
return
}

repo.Update(from)

errStore := _store.UpdateRepo(repo)
if errStore != nil {
_ = c.AbortWithError(http.StatusInternalServerError, errStore)
Expand Down
18 changes: 9 additions & 9 deletions server/model/perm.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ type PermStore interface {

// Perm defines a repository permission for an individual user.
type Perm struct {
UserID int64 `json:"-" xorm:"UNIQUE(s) INDEX NOT NULL 'perm_user_id'"`
RepoID int64 `json:"-" xorm:"UNIQUE(s) INDEX NOT NULL 'perm_repo_id'"`
Repo string `json:"-" xorm:"-"` // TODO: better caching (use type *Repo)
Pull bool `json:"pull" xorm:"perm_pull"`
Push bool `json:"push" xorm:"perm_push"`
Admin bool `json:"admin" xorm:"perm_admin"`
Synced int64 `json:"synced" xorm:"perm_synced"`
Created int64 `json:"created" xorm:"created"`
Updated int64 `json:"updated" xorm:"updated"`
UserID int64 `json:"-" xorm:"UNIQUE(s) INDEX NOT NULL 'perm_user_id'"`
RepoID int64 `json:"-" xorm:"UNIQUE(s) INDEX NOT NULL 'perm_repo_id'"`
Repo *Repo `json:"-" xorm:"-"`
6543 marked this conversation as resolved.
Show resolved Hide resolved
Pull bool `json:"pull" xorm:"perm_pull"`
Push bool `json:"push" xorm:"perm_push"`
Admin bool `json:"admin" xorm:"perm_admin"`
Synced int64 `json:"synced" xorm:"perm_synced"`
Created int64 `json:"created" xorm:"created"`
Updated int64 `json:"updated" xorm:"updated"`
}

// TableName return database table name for xorm
Expand Down
11 changes: 11 additions & 0 deletions server/model/redirection.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package model

type Redirection struct {
ID int64 `xorm:"pk autoincr 'redirection_id'"`
RepoID int64 `xorm:"'repo_id'"`
FullName string `xorm:"UNIQUE INDEX 'repo_full_name'"`
}

func (r Redirection) TableName() string {
return "redirections"
}
11 changes: 11 additions & 0 deletions server/model/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
type Repo struct {
ID int64 `json:"id,omitempty" xorm:"pk autoincr 'repo_id'"`
UserID int64 `json:"-" xorm:"repo_user_id"`
RemoteID RemoteID `json:"-" xorm:"'remote_id'"`
Owner string `json:"owner" xorm:"UNIQUE(name) 'repo_owner'"`
Name string `json:"name" xorm:"UNIQUE(name) 'repo_name'"`
FullName string `json:"full_name" xorm:"UNIQUE 'repo_full_name'"`
Expand Down Expand Up @@ -74,6 +75,10 @@ func ParseRepo(str string) (user, repo string, err error) {

// Update updates the repository with values from the given Repo.
func (r *Repo) Update(from *Repo) {
r.RemoteID = from.RemoteID
r.Owner = from.Owner
r.Name = from.Name
r.FullName = from.FullName
r.Avatar = from.Avatar
r.Link = from.Link
r.SCMKind = from.SCMKind
Expand All @@ -99,3 +104,9 @@ type RepoPatch struct {
AllowPull *bool `json:"allow_pr,omitempty"`
CancelPreviousPipelineEvents *[]WebhookEvent `json:"cancel_previous_pipeline_events"`
}

type RemoteID string
6543 marked this conversation as resolved.
Show resolved Hide resolved

func (r RemoteID) IsValid() bool {
return r != "" && r != "0"
}
5 changes: 4 additions & 1 deletion server/remote/bitbucket/bitbucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ func (c *config) Teams(ctx context.Context, u *model.User) ([]*model.Team, error
}

// Repo returns the named Bitbucket repository.
func (c *config) Repo(ctx context.Context, u *model.User, owner, name string) (*model.Repo, error) {
func (c *config) Repo(ctx context.Context, u *model.User, id model.RemoteID, owner, name string) (*model.Repo, error) {
if id.IsValid() {
name = string(id)
}
repo, err := c.newClient(ctx, u).FindRepo(owner, name)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions server/remote/bitbucket/bitbucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ func Test_bitbucket(t *testing.T) {

g.Describe("When requesting a repository", func() {
g.It("Should return the details", func() {
repo, err := c.Repo(ctx, fakeUser, fakeRepo.Owner, fakeRepo.Name)
repo, err := c.Repo(ctx, fakeUser, "", fakeRepo.Owner, fakeRepo.Name)
g.Assert(err).IsNil()
g.Assert(repo.FullName).Equal(fakeRepo.FullName)
})
g.It("Should handle not found errors", func() {
_, err := c.Repo(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name)
_, err := c.Repo(ctx, fakeUser, "", fakeRepoNotFound.Owner, fakeRepoNotFound.Name)
g.Assert(err).IsNotNil()
})
})
Expand Down
1 change: 1 addition & 0 deletions server/remote/bitbucket/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func convertStatus(status model.StatusValue) string {
// structure to the common Woodpecker repository structure.
func convertRepo(from *internal.Repo) *model.Repo {
repo := model.Repo{
RemoteID: model.RemoteID(from.UUID),
Clone: cloneLink(from),
Owner: strings.Split(from.FullName, "/")[0],
Name: strings.Split(from.FullName, "/")[1],
Expand Down
1 change: 1 addition & 0 deletions server/remote/bitbucket/internal/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type LinkClone struct {
}

type Repo struct {
UUID string `json:"uuid"`
Owner Account `json:"owner"`
Name string `json:"name"`
FullName string `json:"full_name"`
Expand Down
2 changes: 1 addition & 1 deletion server/remote/bitbucketserver/bitbucketserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (*Config) TeamPerm(u *model.User, org string) (*model.Perm, error) {
return nil, nil
}

func (c *Config) Repo(ctx context.Context, u *model.User, owner, name string) (*model.Repo, error) {
func (c *Config) Repo(ctx context.Context, u *model.User, _ model.RemoteID, owner, name string) (*model.Repo, error) {
repo, err := internal.NewClientWithToken(ctx, c.URL, c.Consumer, u.Token).FindRepo(owner, name)
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions server/remote/bitbucketserver/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func convertStatus(status model.StatusValue) string {
// structure to the common Woodpecker repository structure.
func convertRepo(from *internal.Repo) *model.Repo {
repo := model.Repo{
RemoteID: model.RemoteID(fmt.Sprint(from.ID)),
Name: from.Slug,
Owner: from.Project.Key,
Branch: "master",
Expand Down
1 change: 1 addition & 0 deletions server/remote/bitbucketserver/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func parseHook(r *http.Request, baseURL string) (*model.Repo, *model.Build, erro
}
build := convertPushHook(hook, baseURL)
repo := &model.Repo{
RemoteID: model.RemoteID(fmt.Sprint(hook.Repository.ID)),
Name: hook.Repository.Slug,
Owner: hook.Repository.Project.Key,
FullName: fmt.Sprintf("%s/%s", hook.Repository.Project.Key, hook.Repository.Slug),
Expand Down
6 changes: 4 additions & 2 deletions server/remote/coding/coding.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ func (c *Coding) TeamPerm(u *model.User, org string) (*model.Perm, error) {
return nil, nil
}

// Repo fetches the named repository from the remote system.
func (c *Coding) Repo(ctx context.Context, u *model.User, owner, name string) (*model.Repo, error) {
// Repo fetches the repository from the remote system.
func (c *Coding) Repo(ctx context.Context, u *model.User, _ model.RemoteID, owner, name string) (*model.Repo, error) {
client := c.newClient(ctx, u)
project, err := client.GetProject(owner, name)
if err != nil {
Expand All @@ -172,6 +172,7 @@ func (c *Coding) Repo(ctx context.Context, u *model.User, owner, name string) (*
return nil, err
}
return &model.Repo{
// TODO(1138) RemoteID: project.ID,
Owner: project.Owner,
Name: project.Name,
FullName: projectFullName(project.Owner, project.Name),
Expand Down Expand Up @@ -199,6 +200,7 @@ func (c *Coding) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error
return nil, err
}
repo := &model.Repo{
// TODO(1138) RemoteID: project.ID,
Owner: project.Owner,
Name: project.Name,
FullName: projectFullName(project.Owner, project.Name),
Expand Down
4 changes: 2 additions & 2 deletions server/remote/coding/coding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func Test_coding(t *testing.T) {

g.Describe("When requesting a repository", func() {
g.It("Should return the details", func() {
repo, err := c.Repo(ctx, fakeUser, fakeRepo.Owner, fakeRepo.Name)
repo, err := c.Repo(ctx, fakeUser, "", fakeRepo.Owner, fakeRepo.Name)
g.Assert(err).IsNil()
g.Assert(repo.FullName).Equal(fakeRepo.FullName)
g.Assert(repo.Avatar).Equal(s.URL + fakeRepo.Avatar)
Expand All @@ -119,7 +119,7 @@ func Test_coding(t *testing.T) {
g.Assert(repo.IsSCMPrivate).Equal(fakeRepo.IsSCMPrivate)
})
g.It("Should handle not found errors", func() {
_, err := c.Repo(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name)
_, err := c.Repo(ctx, fakeUser, "", fakeRepoNotFound.Owner, fakeRepoNotFound.Name)
g.Assert(err).IsNotNil()
})
})
Expand Down
1 change: 1 addition & 0 deletions server/remote/coding/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func convertRepository(repo *Repository) (*model.Repo, error) {
}

return &model.Repo{
// TODO RemoteID: repo.ID,
Owner: matches[1],
Name: repo.Name,
FullName: projectFullName(repo.Owner.GlobalKey, repo.Name),
Expand Down
Loading