Skip to content

Commit

Permalink
Use global cfg installation id for temporal worker github activities (#…
Browse files Browse the repository at this point in the history 
…711)

Let's pass the new installation id configuration throughout all use
cases of the GH id in the temporal worker.
  • Loading branch information
@samrabelachew
samrabelachew authored Jul 20, 2023
1 parent 5c40a34 commit 54778ce
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 116 deletions.
14 changes: 7 additions & 7 deletions server/neptune/temporalworker/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func NewServer(config *config.Config) (*Server, error) {
config.DataDir,
config.ServerCfg.URL,
config.TemporalCfg.TerraformTaskQueue,
config.GithubCfg.TemporalAppInstallationID,
jobStreamHandler,
)
if err != nil {
Expand Down Expand Up @@ -197,8 +198,8 @@ func NewServer(config *config.Config) (*Server, error) {
}

githubActivities, err := activities.NewGithub(
config.App,
scope.SubScope("app"),
clientCreator,
config.GithubCfg.TemporalAppInstallationID,
config.DataDir,
featureAllocator,
)
Expand All @@ -225,11 +226,10 @@ func NewServer(config *config.Config) (*Server, error) {
Executor: crons.NewRuntimeStats(scope).Run,
Frequency: 1 * time.Minute,
},
// TODO: use when we rollout new app for temporalworker all together
//{
// Executor: crons.NewRateLimitStats(scope, clientCreator, config.GithubCfg.TemporalAppInstallationID).Run,
// Frequency: 1 * time.Minute,
//},
{
Executor: crons.NewRateLimitStats(scope, clientCreator, config.GithubCfg.TemporalAppInstallationID).Run,
Frequency: 1 * time.Minute,
},
},
HTTPServerProxy: httpServerProxy,
Port: config.ServerCfg.Port,
Expand Down
45 changes: 20 additions & 25 deletions server/neptune/workflows/activities/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ import (
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
)

type ClientContext struct {
InstallationToken int64
context.Context
}

var HashiGetter = func(ctx context.Context, dst, src string) error {
return getter.Get(dst, src, getter.WithContext(ctx))
}
Expand All @@ -34,16 +29,16 @@ var HashiGetter = func(ctx context.Context, dst, src string) error {
type gogetter func(ctx context.Context, dst, src string) error

type githubClient interface { //nolint:interfacebloat
CreateCheckRun(ctx internal.Context, owner, repo string, opts github.CreateCheckRunOptions) (*github.CheckRun, *github.Response, error)
UpdateCheckRun(ctx internal.Context, owner, repo string, checkRunID int64, opts github.UpdateCheckRunOptions) (*github.CheckRun, *github.Response, error)
GetArchiveLink(ctx internal.Context, owner, repo string, archiveformat github.ArchiveFormat, opts *github.RepositoryContentGetOptions, followRedirects bool) (*url.URL, *github.Response, error)
CompareCommits(ctx internal.Context, owner, repo string, base, head string, opts *github.ListOptions) (*github.CommitsComparison, *github.Response, error)
ListReviews(ctx internal.Context, owner string, repo string, number int) ([]*github.PullRequestReview, error)
GetPullRequest(ctx internal.Context, owner, repo string, number int) (*github.PullRequest, *github.Response, error)
ListCommits(ctx internal.Context, owner string, repo string, number int) ([]*github.RepositoryCommit, error)
DismissReview(ctx internal.Context, owner, repo string, number int, reviewID int64, review *github.PullRequestReviewDismissalRequest) (*github.PullRequestReview, *github.Response, error)
ListTeamMembers(ctx internal.Context, org string, teamSlug string) ([]*github.User, error)
CreateComment(ctx internal.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error)
CreateCheckRun(ctx context.Context, owner, repo string, opts github.CreateCheckRunOptions) (*github.CheckRun, *github.Response, error)
UpdateCheckRun(ctx context.Context, owner, repo string, checkRunID int64, opts github.UpdateCheckRunOptions) (*github.CheckRun, *github.Response, error)
GetArchiveLink(ctx context.Context, owner, repo string, archiveformat github.ArchiveFormat, opts *github.RepositoryContentGetOptions, followRedirects bool) (*url.URL, *github.Response, error)
CompareCommits(ctx context.Context, owner, repo string, base, head string, opts *github.ListOptions) (*github.CommitsComparison, *github.Response, error)
ListReviews(ctx context.Context, owner string, repo string, number int) ([]*github.PullRequestReview, error)
GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, *github.Response, error)
ListCommits(ctx context.Context, owner string, repo string, number int) ([]*github.RepositoryCommit, error)
DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, review *github.PullRequestReviewDismissalRequest) (*github.PullRequestReview, *github.Response, error)
ListTeamMembers(ctx context.Context, org string, teamSlug string) ([]*github.User, error)
CreateComment(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error)
}

type DiffDirection string
Expand Down Expand Up @@ -142,7 +137,7 @@ func (a *githubActivities) GithubUpdateCheckRun(ctx context.Context, request Upd
}

run, _, err := a.Client.UpdateCheckRun(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner, request.Repo.Name, request.ID, opts,
)

Expand Down Expand Up @@ -200,7 +195,7 @@ func (a *githubActivities) GithubCreateCheckRun(ctx context.Context, request Cre
}

run, _, err := a.Client.CreateCheckRun(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner, request.Repo.Name, opts,
)

Expand Down Expand Up @@ -275,7 +270,7 @@ func (a *githubActivities) GithubFetchRoot(ctx context.Context, request FetchRoo
Ref: request.Revision,
}
// note: this link exists for 5 minutes when fetching a private repository archive
archiveLink, resp, err := a.Client.GetArchiveLink(internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken), request.Repo.Owner, request.Repo.Name, github.Zipball, opts, true)
archiveLink, resp, err := a.Client.GetArchiveLink(ctx, request.Repo.Owner, request.Repo.Name, github.Zipball, opts, true)
if err != nil {
return FetchRootResponse{}, errors.Wrap(err, "getting repo archive link")
}
Expand Down Expand Up @@ -315,7 +310,7 @@ type CompareCommitResponse struct {
}

func (a *githubActivities) GithubCompareCommit(ctx context.Context, request CompareCommitRequest) (CompareCommitResponse, error) {
comparison, resp, err := a.Client.CompareCommits(internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken), request.Repo.Owner, request.Repo.Name, request.LatestDeployedRevision, request.DeployRequestRevision, &github.ListOptions{})
comparison, resp, err := a.Client.CompareCommits(ctx, request.Repo.Owner, request.Repo.Name, request.LatestDeployedRevision, request.DeployRequestRevision, &github.ListOptions{})

if err != nil {
return CompareCommitResponse{}, errors.Wrap(err, "comparing commits")
Expand All @@ -341,7 +336,7 @@ type GetPullRequestStateResponse struct {

func (a *githubActivities) GithubGetPullRequestState(ctx context.Context, request GetPullRequestStateRequest) (GetPullRequestStateResponse, error) {
resp, _, err := a.Client.GetPullRequest(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner,
request.Repo.Name,
request.PRNumber,
Expand All @@ -365,7 +360,7 @@ type ListPRReviewsResponse struct {

func (a *githubActivities) GithubListPRReviews(ctx context.Context, request ListPRReviewsRequest) (ListPRReviewsResponse, error) {
reviews, err := a.Client.ListReviews(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner,
request.Repo.Name,
request.PRNumber,
Expand All @@ -389,7 +384,7 @@ type ListPRCommitsResponse struct {

func (a *githubActivities) GithubListPRCommits(ctx context.Context, request ListPRCommitsRequest) (ListPRCommitsResponse, error) {
commits, err := a.Client.ListCommits(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner,
request.Repo.Name,
request.PRNumber,
Expand Down Expand Up @@ -426,7 +421,7 @@ func (a *githubActivities) GithubDismiss(ctx context.Context, request DismissReq
Message: github.String(request.DismissReason),
}
_, _, err = a.Client.DismissReview(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner,
request.Repo.Name,
request.PRNumber,
Expand All @@ -451,7 +446,7 @@ type ListTeamMembersResponse struct {

func (a *githubActivities) GithubListTeamMembers(ctx context.Context, request ListTeamMembersRequest) (ListTeamMembersResponse, error) {
users, err := a.Client.ListTeamMembers(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Org,
request.TeamSlug,
)
Expand Down Expand Up @@ -480,7 +475,7 @@ func (a *githubActivities) GithubCreateComment(ctx context.Context, request Crea
Body: github.String(request.CommentBody),
}
_, _, err := a.Client.CreateComment(
internal.ContextWithInstallationToken(ctx, request.Repo.Credentials.InstallationToken),
ctx,
request.Repo.Owner,
request.Repo.Name,
request.PRNumber,
Expand Down
64 changes: 22 additions & 42 deletions server/neptune/workflows/activities/github/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,50 +11,30 @@ import (
)

type Client struct {
ClientCreator githubapp.ClientCreator
ClientCreator githubapp.ClientCreator
InstallationID int64
}

type Context interface {
GetInstallationToken() int64
context.Context
}

type contextWithToken struct {
InstallationToken int64
context.Context
}

func (c *contextWithToken) GetInstallationToken() int64 {
return c.InstallationToken
}

func ContextWithInstallationToken(ctx context.Context, installationToken int64) Context {
return &contextWithToken{
InstallationToken: installationToken,
Context: ctx,
}
}

func (c *Client) CreateCheckRun(ctx Context, owner, repo string, opts github.CreateCheckRunOptions) (*github.CheckRun, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) CreateCheckRun(ctx context.Context, owner, repo string, opts github.CreateCheckRunOptions) (*github.CheckRun, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)

if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
}

return client.Checks.CreateCheckRun(ctx, owner, repo, opts)
}
func (c *Client) UpdateCheckRun(ctx Context, owner, repo string, checkRunID int64, opts github.UpdateCheckRunOptions) (*github.CheckRun, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) UpdateCheckRun(ctx context.Context, owner, repo string, checkRunID int64, opts github.UpdateCheckRunOptions) (*github.CheckRun, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)

if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
}

return client.Checks.UpdateCheckRun(ctx, owner, repo, checkRunID, opts)
}
func (c *Client) GetArchiveLink(ctx Context, owner, repo string, archiveformat github.ArchiveFormat, opts *github.RepositoryContentGetOptions, followRedirects bool) (*url.URL, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) GetArchiveLink(ctx context.Context, owner, repo string, archiveformat github.ArchiveFormat, opts *github.RepositoryContentGetOptions, followRedirects bool) (*url.URL, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)

if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
Expand All @@ -63,8 +43,8 @@ func (c *Client) GetArchiveLink(ctx Context, owner, repo string, archiveformat g
return client.Repositories.GetArchiveLink(ctx, owner, repo, archiveformat, opts, followRedirects)
}

func (c *Client) CompareCommits(ctx Context, owner, repo string, base, head string, opts *github.ListOptions) (*github.CommitsComparison, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) CompareCommits(ctx context.Context, owner, repo string, base, head string, opts *github.ListOptions) (*github.CommitsComparison, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)

if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
Expand All @@ -73,8 +53,8 @@ func (c *Client) CompareCommits(ctx Context, owner, repo string, base, head stri
return client.Repositories.CompareCommits(ctx, owner, repo, base, head, opts)
}

func (c *Client) ListReviews(ctx Context, owner string, repo string, number int) ([]*github.PullRequestReview, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) ListReviews(ctx context.Context, owner string, repo string, number int) ([]*github.PullRequestReview, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, errors.Wrap(err, "creating client from installation")
}
Expand All @@ -89,16 +69,16 @@ func (c *Client) ListReviews(ctx Context, owner string, repo string, number int)
return gh_helper.Iterate(ctx, run)
}

func (c *Client) GetPullRequest(ctx Context, owner, repo string, number int) (*github.PullRequest, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
}
return client.PullRequests.Get(ctx, owner, repo, number)
}

func (c *Client) ListCommits(ctx Context, owner string, repo string, number int) ([]*github.RepositoryCommit, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) ListCommits(ctx context.Context, owner string, repo string, number int) ([]*github.RepositoryCommit, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, errors.Wrap(err, "creating client from installation")
}
Expand All @@ -113,16 +93,16 @@ func (c *Client) ListCommits(ctx Context, owner string, repo string, number int)
return gh_helper.Iterate(ctx, run)
}

func (c *Client) DismissReview(ctx Context, owner string, repo string, number int, reviewID int64, review *github.PullRequestReviewDismissalRequest) (*github.PullRequestReview, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) DismissReview(ctx context.Context, owner string, repo string, number int, reviewID int64, review *github.PullRequestReviewDismissalRequest) (*github.PullRequestReview, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, nil, errors.Wrap(err, "creating client from installation")
}
return client.PullRequests.DismissReview(ctx, owner, repo, number, reviewID, review)
}

func (c *Client) ListTeamMembers(ctx Context, org string, teamSlug string) ([]*github.User, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) ListTeamMembers(ctx context.Context, org string, teamSlug string) ([]*github.User, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, errors.Wrap(err, "creating client from installation")
}
Expand All @@ -139,8 +119,8 @@ func (c *Client) ListTeamMembers(ctx Context, org string, teamSlug string) ([]*g
return gh_helper.Iterate(ctx, run)
}

func (c *Client) CreateComment(ctx Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(ctx.GetInstallationToken())
func (c *Client) CreateComment(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) {
client, err := c.ClientCreator.NewInstallationClient(c.InstallationID)
if err != nil {
return nil, nil, err
}
Expand Down
20 changes: 5 additions & 15 deletions server/neptune/workflows/activities/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ import (
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/command"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/deployment"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/file"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
internal "github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github/cli"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github/link"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/slack-go/slack"
"github.com/uber-go/tally/v4"
)

const (
Expand Down Expand Up @@ -95,7 +93,7 @@ type PolicySet struct {
Paths []string
}

func NewTerraform(tfConfig config.TerraformConfig, validationConfig config.ValidationConfig, ghAppConfig githubapp.Config, dataDir string, serverURL *url.URL, taskQueue string, streamHandler StreamCloser, opts ...TerraformOptions) (*Terraform, error) {
func NewTerraform(tfConfig config.TerraformConfig, validationConfig config.ValidationConfig, ghAppConfig githubapp.Config, dataDir string, serverURL *url.URL, taskQueue string, installationID int64, streamHandler StreamCloser, opts ...TerraformOptions) (*Terraform, error) {
binDir, err := mkSubDir(dataDir, BinDirName)
if err != nil {
return nil, err
Expand Down Expand Up @@ -187,6 +185,7 @@ func NewTerraform(tfConfig config.TerraformConfig, validationConfig config.Valid
GitCredentialsFileLock: gitCredentialsFileLock,
FileWriter: &file.Writer{},
CacheDir: cacheDir,
InstallationID: installationID,
},
conftestActivity: &conftestActivity{
DefaultConftestVersion: defaultConftestVersion,
Expand Down Expand Up @@ -221,19 +220,10 @@ func NewGithubWithClient(client githubClient, dataDir string, getter gogetter, a
}, nil
}

func NewGithub(appConfig githubapp.Config, scope tally.Scope, dataDir string, allocator feature.Allocator) (*Github, error) {
clientCreator, err := githubapp.NewDefaultCachingClientCreator(
appConfig,
githubapp.WithClientMiddleware(
github.ClientMetrics(scope.SubScope("app")),
))

if err != nil {
return nil, errors.Wrap(err, "initializing client creator")
}

func NewGithub(clientCreator githubapp.ClientCreator, installationID int64, dataDir string, allocator feature.Allocator) (*Github, error) {
client := &internal.Client{
ClientCreator: clientCreator,
ClientCreator: clientCreator,
InstallationID: installationID,
}

return NewGithubWithClient(client, dataDir, HashiGetter, allocator)
Expand Down
5 changes: 4 additions & 1 deletion server/neptune/workflows/activities/terraform.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type terraformActivities struct {
GitCredentialsFileLock *file.RWLock
FileWriter writer
CacheDir string
InstallationID int64
}

func NewTerraformActivities(
Expand All @@ -107,13 +108,15 @@ func NewTerraformActivities(
gitCredentialsFileLock *file.RWLock,
fileWriter writer,
cacheDir string,
installationID int64,
) *terraformActivities { //nolint:revive // avoiding refactor while adding linter action
return &terraformActivities{
TerraformClient: client,
DefaultTFVersion: defaultTfVersion,
StreamHandler: streamHandler,
GitCLICredentials: gitCredentialsRefresher,
GitCredentialsFileLock: gitCredentialsFileLock,
InstallationID: installationID,
FileWriter: fileWriter,
CacheDir: cacheDir,
}
Expand Down Expand Up @@ -176,7 +179,7 @@ func (t *terraformActivities) TerraformInit(ctx context.Context, request Terrafo
Version: tfVersion,
}

err = t.GitCLICredentials.Refresh(ctx, request.GithubInstallationID)
err = t.GitCLICredentials.Refresh(ctx, t.InstallationID)
if err != nil {
activity.GetLogger(ctx).Error("Error refreshing git cli credentials. This is bug and will likely cause fetching of private modules to fail", key.ErrKey, err)
}
Expand Down
Loading

0 comments on commit 54778ce

Please sign in to comment.