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.

Sign up for GitHub

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

bitbucket: Ensure token is valid before proceeding #1803

Merged
merged 1 commit into from
Oct 31, 2024
Merged

bitbucket: Ensure token is valid before proceeding #1803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion pkg/provider/bitbucketserver/acl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,15 @@ func TestIsAllowed(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
bbclient, mux, tearDown := bbv1test.SetupBBServerClient(ctx)
bbclient, mux, tearDown, tURL := bbv1test.SetupBBServerClient(ctx)
defer tearDown()
bbv1test.MuxProjectMemberShip(t, mux, tt.event, tt.fields.projectMembers)
bbv1test.MuxRepoMemberShip(t, mux, tt.event, tt.fields.repoMembers)
bbv1test.MuxPullRequestActivities(t, mux, tt.event, tt.fields.pullRequestNumber, tt.fields.activities)
bbv1test.MuxFiles(t, mux, tt.event, tt.fields.defaultBranchLatestCommit, "", tt.fields.filescontents)

v := &Provider{
baseURL: tURL,
Client: bbclient,
defaultBranchLatestCommit: tt.fields.defaultBranchLatestCommit,
pullRequestNumber: tt.fields.pullRequestNumber,
Expand Down
18 changes: 14 additions & 4 deletions pkg/provider/bitbucketserver/bitbucketserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package bitbucketserver
import (
"context"
"fmt"
"net/http"
"path/filepath"
"strings"

Expand Down Expand Up @@ -215,13 +216,13 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path,

func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error {
if event.Provider.User == "" {
return fmt.Errorf("no provider.user has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.user has been set in the repo crd")
}
if event.Provider.Token == "" {
return fmt.Errorf("no provider.secret has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.secret has been set in the repo crd")
}
if event.Provider.URL == "" {
return fmt.Errorf("no provider.url has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.url has been set in the repo crd")
}

// make sure we have /rest at the end of the url
Expand All @@ -237,8 +238,17 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E

ctx = context.WithValue(ctx, bbv1.ContextBasicAuth, basicAuth)
cfg := bbv1.NewConfiguration(event.Provider.URL)
v.Client = bbv1.NewAPIClient(ctx, cfg)
if v.Client == nil {
v.Client = bbv1.NewAPIClient(ctx, cfg)
}
v.run = run
resp, err := v.Client.DefaultApi.GetUser(event.Provider.User)
if resp.Response != nil && resp.StatusCode == http.StatusUnauthorized {
return fmt.Errorf("cannot get user %s with token: %w", event.Provider.User, err)
}
if err != nil {
return fmt.Errorf("cannot get user %s: %w", event.Provider.User, err)
}

return nil
}
Expand Down
65 changes: 53 additions & 12 deletions pkg/provider/bitbucketserver/bitbucketserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ func TestGetTektonDir(t *testing.T) {
observer, _ := zapobserver.New(zap.InfoLevel)
logger := zap.New(observer).Sugar()
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
v := &Provider{Logger: logger, Client: client, projectKey: tt.event.Organization}
v := &Provider{Logger: logger, baseURL: tURL, Client: client, projectKey: tt.event.Organization}
bbtest.MuxDirContent(t, mux, tt.event, tt.testDirPath, tt.path)
content, err := v.GetTektonDir(ctx, tt.event, tt.path, "")
if tt.wantErr {
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestCreateStatus(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
if tt.nilClient {
client = nil
Expand All @@ -177,6 +177,7 @@ func TestCreateStatus(t *testing.T) {
event.EventType = "pull_request"
event.Provider.Token = "token"
v := &Provider{
baseURL: tURL,
Client: client,
pullRequestNumber: pullRequestNumber,
projectKey: event.Organization,
Expand Down Expand Up @@ -229,9 +230,9 @@ func TestGetFileInsideRepo(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
v := &Provider{Client: client, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
v := &Provider{Client: client, baseURL: tURL, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
bbtest.MuxFiles(t, mux, tt.event, tt.targetbranch, filepath.Dir(tt.path), tt.filescontents)
fc, err := v.GetFileInsideRepo(ctx, tt.event, tt.path, tt.targetbranch)
assert.NilError(t, err)
Expand All @@ -246,11 +247,12 @@ func TestSetClient(t *testing.T) {
apiURL string
opts *info.Event
wantErrSubstr string
muxUser func(w http.ResponseWriter, r *http.Request)
}{
{
name: "bad/no username",
opts: info.NewEvent(),
wantErrSubstr: "no provider.user",
wantErrSubstr: "no spec.git_provider.user",
},
{
name: "bad/no secret",
Expand All @@ -259,7 +261,7 @@ func TestSetClient(t *testing.T) {
User: "foo",
},
},
wantErrSubstr: "no provider.secret",
wantErrSubstr: "no spec.git_provider.secret",
},
{
name: "bad/no url",
Expand All @@ -269,7 +271,39 @@ func TestSetClient(t *testing.T) {
Token: "bar",
},
},
wantErrSubstr: "no provider.url",
wantErrSubstr: "no spec.git_provider.url",
},
{
name: "bad/invalid user",
opts: &info.Event{
Provider: &info.Provider{
User: "foo",
Token: "bar",
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"errors": [{"message": "Unauthorized"}]}`))
},
apiURL: "https://foo.bar/rest",
wantErrSubstr: "cannot get user foo with token",
},
{
name: "bad/unknown error",
opts: &info.Event{
Provider: &info.Provider{
User: "foo",
Token: "bar",
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
_, _ = w.Write([]byte(`{"errors": [{"message": "Internal Server Error"}]}`))
},
apiURL: "https://foo.bar/rest",
wantErrSubstr: "cannot get user foo: Status: 500",
},
{
name: "good/url append /rest",
Expand All @@ -280,13 +314,21 @@ func TestSetClient(t *testing.T) {
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
fmt.Fprint(w, `{"name": "foo"}`)
},
apiURL: "https://foo.bar/rest",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
v := &Provider{}
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
if tt.muxUser != nil {
mux.HandleFunc("/users/foo", tt.muxUser)
}
v := &Provider{Client: bbclient, baseURL: tURL}
err := v.SetClient(ctx, nil, tt.opts, nil, nil)
if tt.wantErrSubstr != "" {
assert.ErrorContains(t, err, tt.wantErrSubstr)
Expand All @@ -299,7 +341,6 @@ func TestSetClient(t *testing.T) {
}

func TestGetCommitInfo(t *testing.T) {
defaultBaseURL := "https://base"
tests := []struct {
name string
event *info.Event
Expand All @@ -325,11 +366,11 @@ func TestGetCommitInfo(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
bbclient, mux, tearDown := bbtest.SetupBBServerClient(ctx)
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
bbtest.MuxCommitInfo(t, mux, tt.event, tt.commit)
bbtest.MuxDefaultBranch(t, mux, tt.event, tt.defaultBranch, tt.latestCommit)
defer tearDown()
v := &Provider{Client: bbclient, baseURL: defaultBaseURL, projectKey: tt.event.Organization}
v := &Provider{Client: bbclient, baseURL: tURL, projectKey: tt.event.Organization}
err := v.GetCommitInfo(ctx, tt.event)
assert.NilError(t, err)
assert.Equal(t, tt.defaultBranch, tt.event.DefaultBranch)
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/bitbucketserver/test/test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var (
buildAPIURL = "/build-status/1.0"
)

func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func()) {
func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func(), string) {
mux := http.NewServeMux()
apiHandler := http.NewServeMux()
apiHandler.Handle(defaultAPIURL+"/", http.StripPrefix(defaultAPIURL, mux))
Expand All @@ -49,7 +49,7 @@ func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux,
cfg := bbv1.NewConfiguration(server.URL)
cfg.HTTPClient = server.Client()
client := bbv1.NewAPIClient(ctx, cfg)
return client, mux, tearDown
return client, mux, tearDown, server.URL
}

func MuxCreateComment(t *testing.T, mux *http.ServeMux, event *info.Event, expectedCommentSubstr string, prID int) {
Expand Down
Loading