refactor: change providers' inner Client privacy to private

Requiring the client object be accessed via an accessor method allows
for hooking into API usage for things like API metrics, since the
github-client library API is designed in a way difficult to decorate.

Author: aThorp96

pkg/matcher/annotation_matcher_test.go

@@ -1350,9 +1350,9 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 			fakeclient, mux, ghTestServerURL, teardown := ghtesthelper.SetupGH()
 			defer teardown()
 			vcx := &ghprovider.Provider{
-				Client: fakeclient,
-				Token:  github.Ptr("None"),
+				Token: github.Ptr("None"),
 			}
+			vcx.SetGithubClient(fakeclient)
 			if tt.args.runevent.Request == nil {
 				tt.args.runevent.Request = &info.Request{Header: http.Header{}, Payload: nil}
 			}

pkg/pipelineascode/match_test.go

@@ -254,10 +254,10 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 				ConsoleURL: "https://console.url",
 			}
 			vcx := &ghprovider.Provider{
-				Client: fakeclient,
 				Token:  github.Ptr("None"),
 				Logger: logger,
 			}
+			vcx.SetGithubClient(fakeclient)
 			pacInfo := &info.PacOpts{
 				Settings: settings.Settings{
 					ApplicationName:    "Pipelines as Code CI",

pkg/pipelineascode/pipelineascode_test.go

@@ -606,11 +606,11 @@ func TestRun(t *testing.T) {
 				},
 			}
 			vcx := &ghprovider.Provider{
-				Client: fakeclient,
 				Run:    cs,
 				Token:  github.Ptr("None"),
 				Logger: logger,
 			}
+			vcx.SetGithubClient(fakeclient)
 			vcx.SetPacInfo(pacInfo)
 			p := NewPacs(&tt.runevent, vcx, cs, pacInfo, k8int, logger, nil)
 			err := p.Run(ctx)

pkg/provider/bitbucketcloud/acl.go

@@ -27,7 +27,7 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, erro
 }
 
 func (v *Provider) isWorkspaceMember(event *info.Event) (bool, error) {
-	members, err := v.Client.Workspaces.Members(event.Organization)
+	members, err := v.Client().Workspaces.Members(event.Organization)
 	if err != nil {
 		return false, err
 	}
@@ -83,7 +83,7 @@ func (v *Provider) checkMember(ctx context.Context, event *info.Event) (bool, er
 }
 
 func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, event *info.Event) (bool, error) {
-	commentsIntf, err := v.Client.Repositories.PullRequests.GetComments(&bitbucket.PullRequestsOptions{
+	commentsIntf, err := v.Client().Repositories.PullRequests.GetComments(&bitbucket.PullRequestsOptions{
 		Owner:    event.Organization,
 		RepoSlug: event.Repository,
 		ID:       strconv.Itoa(event.PullRequestNumber),

pkg/provider/bitbucketcloud/acl_test.go

@@ -179,7 +179,7 @@ func TestIsAllowed(t *testing.T) {
 			bbcloudtest.MuxComments(t, mux, tt.event, tt.fields.comments)
 			bbcloudtest.MuxFiles(t, mux, tt.event, tt.fields.filescontents, "")
 
-			v := &Provider{Client: bbclient}
+			v := &Provider{bbClient: bbclient}
 			got, err := v.IsAllowed(ctx, tt.event)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Provider.IsAllowed() error = %v, wantErr %v", err, tt.wantErr)

pkg/provider/bitbucketcloud/bitbucket.go

@@ -22,7 +22,7 @@ import (
 var _ provider.Interface = (*Provider)(nil)
 
 type Provider struct {
-	Client        *bitbucket.Client
+	bbClient      *bitbucket.Client
 	Logger        *zap.SugaredLogger
 	run           *params.Run
 	pacInfo       *info.PacOpts
@@ -31,6 +31,10 @@ type Provider struct {
 	provenance    string
 }
 
+func (v Provider) Client() *bitbucket.Client {
+	return v.bbClient
+}
+
 // CheckPolicyAllowing TODO: Implement ME.
 func (v *Provider) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) {
 	return false, ""
@@ -104,11 +108,11 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
 		Revision: event.SHA,
 	}
 
-	if v.Client == nil {
+	if v.bbClient == nil {
 		return fmt.Errorf("no token has been set, cannot set status")
 	}
 
-	_, err := v.Client.Repositories.Commits.CreateCommitStatus(cmo, cso)
+	_, err := v.Client().Repositories.Commits.CreateCommitStatus(cmo, cso)
 	if err != nil {
 		return err
 	}
@@ -121,7 +125,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
 		if statusopts.OriginalPipelineRunName != "" {
 			onPr = "/" + statusopts.OriginalPipelineRunName
 		}
-		_, err = v.Client.Repositories.PullRequests.AddComment(
+		_, err = v.Client().Repositories.PullRequests.AddComment(
 			&bitbucket.PullRequestCommentOptions{
 				Owner:         event.Organization,
 				RepoSlug:      event.Repository,
@@ -161,7 +165,7 @@ func (v *Provider) getDir(event *info.Event, path string) ([]bitbucket.Repositor
 		Path:     path,
 	}
 
-	repositoryFiles, err := v.Client.Repositories.Repository.ListFiles(repoFileOpts)
+	repositoryFiles, err := v.Client().Repositories.Repository.ListFiles(repoFileOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -183,7 +187,7 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Eve
 	if event.Provider.User == "" {
 		return fmt.Errorf("no git_provider.user has been in repo crd")
 	}
-	v.Client = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
+	v.bbClient = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
 	v.Token = &event.Provider.Token
 	v.Username = &event.Provider.User
 	v.run = run
@@ -195,7 +199,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
 	if branchortag == "" {
 		branchortag = event.HeadBranch
 	}
-	response, err := v.Client.Repositories.Commits.GetCommits(&bitbucket.CommitsOptions{
+	response, err := v.Client().Repositories.Commits.GetCommits(&bitbucket.CommitsOptions{
 		Owner:       event.Organization,
 		RepoSlug:    event.Repository,
 		Branchortag: branchortag,
@@ -226,7 +230,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
 	event.SHA = commitinfo.Hash
 
 	// now to get the default branch from repository.Get
-	repo, err := v.Client.Repositories.Repository.Get(&bitbucket.RepositoryOptions{
+	repo, err := v.Client().Repositories.Repository.Get(&bitbucket.RepositoryOptions{
 		Owner:    event.Organization,
 		RepoSlug: event.Repository,
 	})
@@ -278,7 +282,7 @@ func (v *Provider) concatAllYamlFiles(objects []bitbucket.RepositoryFile, event
 }
 
 func (v *Provider) getBlob(runevent *info.Event, ref, path string) (string, error) {
-	blob, err := v.Client.Repositories.Repository.GetFileBlob(&bitbucket.RepositoryBlobOptions{
+	blob, err := v.Client().Repositories.Repository.GetFileBlob(&bitbucket.RepositoryBlobOptions{
 		Owner:    runevent.Organization,
 		RepoSlug: runevent.Repository,
 		Ref:      ref,

pkg/provider/bitbucketcloud/bitbucket_test.go

@@ -75,7 +75,7 @@ func TestGetTektonDir(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
 			bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
 			defer tearDown()
-			v := &Provider{Logger: fakelogger, Client: bbclient}
+			v := &Provider{Logger: fakelogger, bbClient: bbclient}
 			bbcloudtest.MuxDirContent(t, mux, tt.event, tt.testDirPath, tt.provenance)
 			content, err := v.GetTektonDir(ctx, tt.event, ".tekton", tt.provenance)
 			if tt.wantErr != "" {
@@ -202,7 +202,7 @@ func TestGetCommitInfo(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
 			bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
 			defer tearDown()
-			v := &Provider{Client: bbclient}
+			v := &Provider{bbClient: bbclient}
 			bbcloudtest.MuxCommits(t, mux, tt.event, []types.Commit{
 				tt.commitinfo,
 			})
@@ -295,8 +295,8 @@ func TestCreateStatus(t *testing.T) {
 			bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
 			defer tearDown()
 			v := &Provider{
-				Client: bbclient,
-				run:    params.New(),
+				bbClient: bbclient,
+				run:      params.New(),
 				pacInfo: &info.PacOpts{
 					Settings: settings.Settings{
 						ApplicationName: settings.PACApplicationNameDefaultValue,

pkg/provider/bitbucketdatacenter/acl.go

@@ -53,7 +53,7 @@ func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, e
 		if nextPage > 0 {
 			localVarOptionals["start"] = int(nextPage)
 		}
-		return v.Client.DefaultApi.GetActivities(v.projectKey, event.Repository, v.pullRequestNumber, localVarOptionals)
+		return v.Client().DefaultApi.GetActivities(v.projectKey, event.Repository, v.pullRequestNumber, localVarOptionals)
 	})
 	if err != nil {
 		return false, err
@@ -120,7 +120,7 @@ func (v *Provider) checkMemberShip(ctx context.Context, event *info.Event) (bool
 		if nextPage > 0 {
 			localVarOptionals["start"] = int(nextPage)
 		}
-		return v.Client.DefaultApi.GetUsersWithAnyPermission_23(v.projectKey, localVarOptionals)
+		return v.Client().DefaultApi.GetUsersWithAnyPermission_23(v.projectKey, localVarOptionals)
 	})
 	if err != nil {
 		return false, err
@@ -139,7 +139,7 @@ func (v *Provider) checkMemberShip(ctx context.Context, event *info.Event) (bool
 		if nextPage > 0 {
 			localVarOptionals["start"] = int(nextPage)
 		}
-		return v.Client.DefaultApi.GetUsersWithAnyPermission_24(v.projectKey, event.Repository, localVarOptionals)
+		return v.Client().DefaultApi.GetUsersWithAnyPermission_24(v.projectKey, event.Repository, localVarOptionals)
 	})
 	if err != nil {
 		return false, err

pkg/provider/bitbucketdatacenter/acl_test.go

@@ -212,8 +212,8 @@ func TestIsAllowed(t *testing.T) {
 
 			v := &Provider{
 				baseURL:                   tURL,
-				Client:                    bbclient,
-				ScmClient:                 scmClient,
+				bbClient:                  bbclient,
+				scmClient:                 scmClient,
 				defaultBranchLatestCommit: tt.fields.defaultBranchLatestCommit,
 				pullRequestNumber:         tt.fields.pullRequestNumber,
 				projectKey:                tt.event.Organization,

pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

@@ -31,8 +31,8 @@ const apiResponseLimit = 100
 var _ provider.Interface = (*Provider)(nil)
 
 type Provider struct {
-	Client                    *bbv1.APIClient // temporarily keeping it after the refactor finishes, will be removed
-	ScmClient                 *scm.Client
+	bbClient                  *bbv1.APIClient // temporarily keeping it after the refactor finishes, will be removed
+	scmClient                 *scm.Client
 	Logger                    *zap.SugaredLogger
 	run                       *params.Run
 	pacInfo                   *info.PacOpts
@@ -44,6 +44,22 @@ type Provider struct {
 	projectKey                string
 }
 
+func (v Provider) Client() *bbv1.APIClient {
+	return v.bbClient
+}
+
+func (v *Provider) SetBitBucketClient(client *bbv1.APIClient) {
+	v.bbClient = client
+}
+
+func (v Provider) ScmClient() *scm.Client {
+	return v.scmClient
+}
+
+func (v *Provider) SetScmClient(client *scm.Client) {
+	v.scmClient = client
+}
+
 func (v *Provider) SetPacInfo(pacInfo *info.PacOpts) {
 	v.pacInfo = pacInfo
 }
@@ -99,7 +115,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
 	if statusOpts.DetailsURL != "" {
 		detailsURL = statusOpts.DetailsURL
 	}
-	if v.ScmClient == nil {
+	if v.scmClient == nil {
 		return fmt.Errorf("no token has been set, cannot set status")
 	}
 
@@ -119,7 +135,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
 		Desc:  statusOpts.Title,
 		Link:  detailsURL,
 	}
-	_, _, err := v.ScmClient.Repositories.CreateStatus(ctx, OrgAndRepo, event.SHA, opts)
+	_, _, err := v.ScmClient().Repositories.CreateStatus(ctx, OrgAndRepo, event.SHA, opts)
 	if err != nil {
 		return err
 	}
@@ -135,7 +151,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
 		input := &scm.CommentInput{
 			Body: bbComment,
 		}
-		_, _, err := v.ScmClient.PullRequests.CreateComment(ctx, OrgAndRepo, event.PullRequestNumber, input)
+		_, _, err := v.ScmClient().PullRequests.CreateComment(ctx, OrgAndRepo, event.PullRequestNumber, input)
 		if err != nil {
 			return err
 		}
@@ -185,7 +201,7 @@ func (v *Provider) concatAllYamlFiles(ctx context.Context, objects []string, sha
 
 func (v *Provider) getRaw(ctx context.Context, runevent *info.Event, revision, path string) (string, error) {
 	repo := fmt.Sprintf("%s/%s", runevent.Organization, runevent.Repository)
-	content, _, err := v.ScmClient.Contents.Find(ctx, repo, path, revision)
+	content, _, err := v.ScmClient().Contents.Find(ctx, repo, path, revision)
 	if err != nil {
 		return "", fmt.Errorf("cannot find %s inside the %s repository: %w", path, runevent.Repository, err)
 	}
@@ -206,7 +222,7 @@ func (v *Provider) GetTektonDir(ctx context.Context, event *info.Event, path, pr
 	var fileEntries []*scm.FileEntry
 	opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
 	for {
-		entries, _, err := v.ScmClient.Contents.List(ctx, orgAndRepo, path, at, opts)
+		entries, _, err := v.ScmClient().Contents.List(ctx, orgAndRepo, path, at, opts)
 		if err != nil {
 			return "", fmt.Errorf("cannot list content of %s directory: %w", path, err)
 		}
@@ -281,11 +297,11 @@ 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)
-	if v.Client == nil {
-		v.Client = bbv1.NewAPIClient(ctx, cfg)
+	if v.bbClient == nil {
+		v.bbClient = bbv1.NewAPIClient(ctx, cfg)
 	}
 
-	if v.ScmClient == nil {
+	if v.scmClient == nil {
 		client, err := stash.New(removeLastSegment(event.Provider.URL)) // remove `/rest` from url
 		if err != nil {
 			return err
@@ -299,10 +315,10 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 				),
 			},
 		}
-		v.ScmClient = client
+		v.scmClient = client
 	}
 	v.run = run
-	_, resp, err := v.ScmClient.Users.FindLogin(ctx, event.Provider.User)
+	_, resp, err := v.ScmClient().Users.FindLogin(ctx, event.Provider.User)
 	if resp != nil && resp.Status == http.StatusUnauthorized {
 		return fmt.Errorf("cannot get user %s with token: %w", event.Provider.User, err)
 	}
@@ -315,14 +331,14 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 
 func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
 	OrgAndRepo := fmt.Sprintf("%s/%s", event.Organization, event.Repository)
-	commit, _, err := v.ScmClient.Git.FindCommit(context.Background(), OrgAndRepo, event.SHA)
+	commit, _, err := v.ScmClient().Git.FindCommit(context.Background(), OrgAndRepo, event.SHA)
 	if err != nil {
 		return err
 	}
 	event.SHATitle = sanitizeTitle(commit.Message)
 	event.SHAURL = fmt.Sprintf("%s/projects/%s/repos/%s/commits/%s", v.baseURL, v.projectKey, event.Repository, event.SHA)
 
-	ref, _, err := v.ScmClient.Git.GetDefaultBranch(context.Background(), OrgAndRepo)
+	ref, _, err := v.ScmClient().Git.GetDefaultBranch(context.Background(), OrgAndRepo)
 	if err != nil {
 		return err
 	}
@@ -345,7 +361,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf
 		opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
 		changedFiles := changedfiles.ChangedFiles{}
 		for {
-			changes, _, err := v.ScmClient.PullRequests.ListChanges(ctx, OrgAndRepo, runevent.PullRequestNumber, opts)
+			changes, _, err := v.ScmClient().PullRequests.ListChanges(ctx, OrgAndRepo, runevent.PullRequestNumber, opts)
 			if err != nil {
 				return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for pull request: %w", err)
 			}
@@ -383,7 +399,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf
 		opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
 		changedFiles := changedfiles.ChangedFiles{}
 		for {
-			changes, _, err := v.ScmClient.Git.ListChanges(ctx, OrgAndRepo, runevent.SHA, opts)
+			changes, _, err := v.ScmClient().Git.ListChanges(ctx, OrgAndRepo, runevent.SHA, opts)
 			if err != nil {
 				return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for commit %s: %w", runevent.SHA, err)
 			}