From 02abcb55056b0353a06d406cee140c0abac8001b Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Wed, 15 Mar 2023 11:37:12 +0100 Subject: [PATCH 01/14] first naive implementation --- server/events/github_app_working_dir.go | 13 --- server/events/vcs/gh_app_creds_rotator.go | 82 +++++++++++++++++++ server/events/{ => vcs}/git_cred_writer.go | 2 +- .../events/{ => vcs}/git_cred_writer_test.go | 22 ++--- server/server.go | 15 +++- 5 files changed, 105 insertions(+), 29 deletions(-) create mode 100644 server/events/vcs/gh_app_creds_rotator.go rename server/events/{ => vcs}/git_cred_writer.go (99%) rename server/events/{ => vcs}/git_cred_writer_test.go (83%) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 05e2055e33..dc8b8a75d2 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/mitchellh/go-homedir" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" @@ -23,23 +22,11 @@ type GithubAppWorkingDir struct { // Clone writes a fresh token for Github App authentication func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { - log.Info("Refreshing git tokens for Github App") - token, err := g.Credentials.GetToken() if err != nil { return "", false, errors.Wrap(err, "getting github token") } - home, err := homedir.Dir() - if err != nil { - return "", false, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") - } - - // https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#http-based-git-access-by-an-installation - if err := WriteGitCreds("x-access-token", token, g.GithubHostname, home, log, true); err != nil { - return "", false, err - } - baseRepo := &p.BaseRepo // Realistically, this is a super brittle way of supporting clones using gh app installation tokens diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go new file mode 100644 index 0000000000..f92daad968 --- /dev/null +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -0,0 +1,82 @@ +package vcs + +import ( + "time" + + "github.com/mitchellh/go-homedir" + "github.com/pkg/errors" + + "github.com/runatlantis/atlantis/server/logging" +) + +// GitCredsTokenRotator continuously rotates the github app token ever 9 minutes and writes the ~/.git-credentials file +type GitCredsTokenRotator interface { + Start() error + Stop() + rotate() error +} + +type githubAppTokenRotator struct { + stop chan struct{} + log logging.SimpleLogging + githubCredentials GithubCredentials + githubHostname string +} + +func NewGithubAppTokenRotator( + logger logging.SimpleLogging, + githubCredentials GithubCredentials, + githubHostname string) GitCredsTokenRotator { + return &githubAppTokenRotator{ + log: logger, + githubCredentials: githubCredentials, + githubHostname: githubHostname, + } +} + +// make sure interface is implemented correctly +var _ GitCredsTokenRotator = (*githubAppTokenRotator)(nil) + +func (r *githubAppTokenRotator) Start() error { + quit := make(chan struct{}) + + // github tokens are valid for ~10 mins + ticker := time.NewTicker(9 * time.Minute) + + go func() { + for { + select { + case <-ticker.C: + r.rotate() + case <-quit: + ticker.Stop() + return + } + } + }() + + return r.rotate() +} + +func (r *githubAppTokenRotator) Stop() { + close(r.stop) +} + +func (r *githubAppTokenRotator) rotate() error { + r.log.Info("Refreshing git tokens for Github App") + token, err := r.githubCredentials.GetToken() + if err != nil { + return errors.Wrap(err, "getting github token") + } + + home, err := homedir.Dir() + if err != nil { + return errors.Wrap(err, "getting home dir to write ~/.git-credentials file") + } + + // https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#http-based-git-access-by-an-installation + if err := WriteGitCreds("x-access-token", token, r.githubHostname, home, r.log, true); err != nil { + return errors.Wrap(err, "writing ~/.git-credentials file") + } + return nil +} diff --git a/server/events/git_cred_writer.go b/server/events/vcs/git_cred_writer.go similarity index 99% rename from server/events/git_cred_writer.go rename to server/events/vcs/git_cred_writer.go index 945dfcecb2..6d6cf85317 100644 --- a/server/events/git_cred_writer.go +++ b/server/events/vcs/git_cred_writer.go @@ -1,4 +1,4 @@ -package events +package vcs import ( "fmt" diff --git a/server/events/git_cred_writer_test.go b/server/events/vcs/git_cred_writer_test.go similarity index 83% rename from server/events/git_cred_writer_test.go rename to server/events/vcs/git_cred_writer_test.go index c87637b037..0aefd703b1 100644 --- a/server/events/git_cred_writer_test.go +++ b/server/events/vcs/git_cred_writer_test.go @@ -1,4 +1,4 @@ -package events_test +package vcs_test import ( "fmt" @@ -7,7 +7,7 @@ import ( "path/filepath" "testing" - "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" ) @@ -18,7 +18,7 @@ var logger logging.SimpleLogging func TestWriteGitCreds_WriteFile(t *testing.T) { tmp := t.TempDir() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expContents := `https://user:token@hostname` @@ -37,7 +37,7 @@ func TestWriteGitCreds_Appends(t *testing.T) { err := os.WriteFile(credsFile, []byte("contents"), 0600) Ok(t, err) - err = events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + err = vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expContents := "contents\nhttps://user:token@hostname" @@ -56,7 +56,7 @@ func TestWriteGitCreds_NoModification(t *testing.T) { err := os.WriteFile(credsFile, []byte(contents), 0600) Ok(t, err) - err = events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + err = vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) actContents, err := os.ReadFile(filepath.Join(tmp, ".git-credentials")) Ok(t, err) @@ -72,7 +72,7 @@ func TestWriteGitCreds_ReplaceApp(t *testing.T) { err := os.WriteFile(credsFile, []byte(contents), 0600) Ok(t, err) - err = events.WriteGitCreds("x-access-token", "token", "github.com", tmp, logger, true) + err = vcs.WriteGitCreds("x-access-token", "token", "github.com", tmp, logger, true) Ok(t, err) expContets := "line1\nhttps://x-access-token:token@github.com\nline2" actContents, err := os.ReadFile(filepath.Join(tmp, ".git-credentials")) @@ -89,7 +89,7 @@ func TestWriteGitCreds_AppendApp(t *testing.T) { err := os.WriteFile(credsFile, []byte(contents), 0600) Ok(t, err) - err = events.WriteGitCreds("x-access-token", "token", "github.com", tmp, logger, true) + err = vcs.WriteGitCreds("x-access-token", "token", "github.com", tmp, logger, true) Ok(t, err) expContets := "https://x-access-token:token@github.com" actContents, err := os.ReadFile(filepath.Join(tmp, ".git-credentials")) @@ -107,7 +107,7 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { Ok(t, err) expErr := fmt.Sprintf("open %s: permission denied", credsFile) - actErr := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + actErr := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) ErrContains(t, expErr, actErr) } @@ -115,7 +115,7 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { func TestWriteGitCreds_ErrIfCannotWrite(t *testing.T) { credsFile := "/this/dir/does/not/exist/.git-credentials" // nolint: gosec expErr := fmt.Sprintf("writing generated .git-credentials file with user, token and hostname to %s: open %s: no such file or directory", credsFile, credsFile) - actErr := events.WriteGitCreds("user", "token", "hostname", "/this/dir/does/not/exist", logger, false) + actErr := vcs.WriteGitCreds("user", "token", "hostname", "/this/dir/does/not/exist", logger, false) ErrEquals(t, expErr, actErr) } @@ -123,7 +123,7 @@ func TestWriteGitCreds_ErrIfCannotWrite(t *testing.T) { func TestWriteGitCreds_ConfigureGitCredentialHelper(t *testing.T) { tmp := t.TempDir() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expOutput := `store` @@ -136,7 +136,7 @@ func TestWriteGitCreds_ConfigureGitCredentialHelper(t *testing.T) { func TestWriteGitCreds_ConfigureGitUrlOverride(t *testing.T) { tmp := t.TempDir() - err := events.WriteGitCreds("user", "token", "hostname", tmp, logger, false) + err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) Ok(t, err) expOutput := `ssh://git@hostname` diff --git a/server/server.go b/server/server.go index 27825b6776..07403baf23 100644 --- a/server/server.go +++ b/server/server.go @@ -169,6 +169,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { var bitbucketCloudClient *bitbucketcloud.Client var bitbucketServerClient *bitbucketserver.Client var azuredevopsClient *vcs.AzureDevopsClient + var githubAppTokenRotator vcs.GitCredsTokenRotator policyChecksEnabled := false if userConfig.EnablePolicyChecksFlag { @@ -304,12 +305,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") } if userConfig.GithubUser != "" { - if err := events.WriteGitCreds(userConfig.GithubUser, userConfig.GithubToken, userConfig.GithubHostname, home, logger, false); err != nil { + if err := vcs.WriteGitCreds(userConfig.GithubUser, userConfig.GithubToken, userConfig.GithubHostname, home, logger, false); err != nil { return nil, err } } if userConfig.GitlabUser != "" { - if err := events.WriteGitCreds(userConfig.GitlabUser, userConfig.GitlabToken, userConfig.GitlabHostname, home, logger, false); err != nil { + if err := vcs.WriteGitCreds(userConfig.GitlabUser, userConfig.GitlabToken, userConfig.GitlabHostname, home, logger, false); err != nil { return nil, err } } @@ -320,12 +321,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if bitbucketBaseURL == "https://api.bitbucket.org" { bitbucketBaseURL = "bitbucket.org" } - if err := events.WriteGitCreds(userConfig.BitbucketUser, userConfig.BitbucketToken, bitbucketBaseURL, home, logger, false); err != nil { + if err := vcs.WriteGitCreds(userConfig.BitbucketUser, userConfig.BitbucketToken, bitbucketBaseURL, home, logger, false); err != nil { return nil, err } } if userConfig.AzureDevopsUser != "" { - if err := events.WriteGitCreds(userConfig.AzureDevopsUser, userConfig.AzureDevopsToken, "dev.azure.com", home, logger, false); err != nil { + if err := vcs.WriteGitCreds(userConfig.AzureDevopsUser, userConfig.AzureDevopsToken, "dev.azure.com", home, logger, false); err != nil { return nil, err } } @@ -471,6 +472,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { Credentials: githubCredentials, GithubHostname: userConfig.GithubHostname, } + + githubAppTokenRotator = vcs.NewGithubAppTokenRotator(logger, githubCredentials, userConfig.GithubHostname) + err := githubAppTokenRotator.Start() + if err != nil { + return nil, errors.Wrap(err, "Could not write credentials") + } } projectLocker := &events.DefaultProjectLocker{ From b098860b9b3f01629bd85490788d1135970ac3c2 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Wed, 15 Mar 2023 14:26:11 +0100 Subject: [PATCH 02/14] fix typos --- server/events/vcs/gh_app_creds_rotator.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go index f92daad968..f12cba9a53 100644 --- a/server/events/vcs/gh_app_creds_rotator.go +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -9,7 +9,7 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) -// GitCredsTokenRotator continuously rotates the github app token ever 9 minutes and writes the ~/.git-credentials file +// GitCredsTokenRotator continuously tries to rotate the github app access token every 30 seconds and writes the ~/.git-credentials file type GitCredsTokenRotator interface { Start() error Stop() @@ -28,6 +28,7 @@ func NewGithubAppTokenRotator( githubCredentials GithubCredentials, githubHostname string) GitCredsTokenRotator { return &githubAppTokenRotator{ + stop: make(chan struct{}), log: logger, githubCredentials: githubCredentials, githubHostname: githubHostname, @@ -38,17 +39,14 @@ func NewGithubAppTokenRotator( var _ GitCredsTokenRotator = (*githubAppTokenRotator)(nil) func (r *githubAppTokenRotator) Start() error { - quit := make(chan struct{}) - - // github tokens are valid for ~10 mins - ticker := time.NewTicker(9 * time.Minute) + ticker := time.NewTicker(30 * time.Second) go func() { for { select { case <-ticker.C: r.rotate() - case <-quit: + case <-r.stop: ticker.Stop() return } @@ -63,12 +61,12 @@ func (r *githubAppTokenRotator) Stop() { } func (r *githubAppTokenRotator) rotate() error { - r.log.Info("Refreshing git tokens for Github App") + r.log.Debug("Refreshing git tokens for Github App") token, err := r.githubCredentials.GetToken() if err != nil { return errors.Wrap(err, "getting github token") } - + r.log.Debug("token %s", token) home, err := homedir.Dir() if err != nil { return errors.Wrap(err, "getting home dir to write ~/.git-credentials file") From ba49842e8e938e104677a26f34e6fbf3b1fe5543 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Thu, 16 Mar 2023 10:09:18 +0100 Subject: [PATCH 03/14] integration with executor_service --- docker-compose.yml | 4 ++- go.mod | 3 +-- go.sum | 8 ++---- server/events/vcs/gh_app_creds_rotator.go | 31 ++++++++--------------- server/scheduled/executor_service.go | 14 +++++++--- server/server.go | 13 ++++++---- 6 files changed, 34 insertions(+), 39 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 9266996801..70552e8540 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,9 +7,11 @@ services: - 4040:4040 environment: # https://dashboard.ngrok.com/get-started/your-authtoken - NGROK_AUTH: REPLACE-WITH-YOUR-TOKEN + # NGROK_AUTH: REPLACE-WITH-YOUR-TOKEN // set this in atlantis.env NGROK_PROTOCOL: http NGROK_PORT: atlantis:4141 + env_file: + - ./atlantis.env depends_on: - atlantis redis: diff --git a/go.mod b/go.mod index 96d80a63d6..500372b121 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( github.com/Masterminds/sprig/v3 v3.2.3 github.com/alicebob/miniredis/v2 v2.30.0 - github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 + github.com/bradleyfalzon/ghinstallation/v2 v2.2.0 github.com/briandowns/spinner v1.21.0 github.com/elazarl/go-bindata-assetfs v1.0.1 github.com/go-ozzo/ozzo-validation v3.6.0+incompatible @@ -81,7 +81,6 @@ require ( github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/google/go-cmp v0.5.9 // indirect - github.com/google/go-github/v45 v45.2.0 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/gorilla/css v1.0.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect diff --git a/go.sum b/go.sum index 961fa6ad89..58895caf10 100644 --- a/go.sum +++ b/go.sum @@ -69,8 +69,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4= -github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 h1:5+NghM1Zred9Z078QEZtm28G/kfDfZN/92gkDlLwGVA= -github.com/bradleyfalzon/ghinstallation/v2 v2.1.0/go.mod h1:Xg3xPRN5Mcq6GDqeUVhFbjEWMb4JHCyWEeeBGEYQoTU= +github.com/bradleyfalzon/ghinstallation/v2 v2.2.0 h1:AVvVU33rE8wdTS1aNnenwpigEBA9mvzI5OhjhZfH/LU= +github.com/bradleyfalzon/ghinstallation/v2 v2.2.0/go.mod h1:xo3iIfK0lDKECe0s19nbxT0KKvk7LsrGc4NxR5ckKMA= github.com/briandowns/spinner v1.21.0 h1:2lVBzf3iZ3cT/ulVXljc4BzlL3yTWZDzsGsamI7si+A= github.com/briandowns/spinner v1.21.0/go.mod h1:TcwZHb7Wb6vn/+bcVv1UXEzaA4pLS7yznHlkY/HzH44= github.com/bsm/ginkgo/v2 v2.5.0 h1:aOAnND1T40wEdAtkGSkvSICWeQ8L3UASX7YVCqQx+eQ= @@ -140,7 +140,6 @@ github.com/go-test/deep v1.0.4/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3a github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg= github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= -github.com/golang-jwt/jwt/v4 v4.4.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= @@ -184,11 +183,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= -github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-github/v45 v45.2.0 h1:5oRLszbrkvxDDqBCNj2hjDZMKmvexaZ1xw/FCD+K3FI= -github.com/google/go-github/v45 v45.2.0/go.mod h1:FObaZJEDSTa/WGCzZ2Z3eoCDXWJKMenWWTrd8jrta28= github.com/google/go-github/v50 v50.1.0 h1:hMUpkZjklC5GJ+c3GquSqOP/T4BNsB7XohaPhtMOzRk= github.com/google/go-github/v50 v50.1.0/go.mod h1:Ev4Tre8QoKiolvbpOSG3FIi4Mlon3S2Nt9W5JYqKiwA= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go index f12cba9a53..263a0fc967 100644 --- a/server/events/vcs/gh_app_creds_rotator.go +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -7,17 +7,17 @@ import ( "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/scheduled" ) // GitCredsTokenRotator continuously tries to rotate the github app access token every 30 seconds and writes the ~/.git-credentials file type GitCredsTokenRotator interface { - Start() error - Stop() + Run() rotate() error + GenerateJob() (scheduled.JobDefinition, error) } type githubAppTokenRotator struct { - stop chan struct{} log logging.SimpleLogging githubCredentials GithubCredentials githubHostname string @@ -28,7 +28,6 @@ func NewGithubAppTokenRotator( githubCredentials GithubCredentials, githubHostname string) GitCredsTokenRotator { return &githubAppTokenRotator{ - stop: make(chan struct{}), log: logger, githubCredentials: githubCredentials, githubHostname: githubHostname, @@ -38,26 +37,16 @@ func NewGithubAppTokenRotator( // make sure interface is implemented correctly var _ GitCredsTokenRotator = (*githubAppTokenRotator)(nil) -func (r *githubAppTokenRotator) Start() error { - ticker := time.NewTicker(30 * time.Second) +func (r *githubAppTokenRotator) GenerateJob() (scheduled.JobDefinition, error) { - go func() { - for { - select { - case <-ticker.C: - r.rotate() - case <-r.stop: - ticker.Stop() - return - } - } - }() - - return r.rotate() + return scheduled.JobDefinition{ + Job: r, + Period: 30 * time.Second, + }, r.rotate() } -func (r *githubAppTokenRotator) Stop() { - close(r.stop) +func (r *githubAppTokenRotator) Run() { + r.rotate() } func (r *githubAppTokenRotator) rotate() error { diff --git a/server/scheduled/executor_service.go b/server/scheduled/executor_service.go index 2d522d472e..a4be0f266e 100644 --- a/server/scheduled/executor_service.go +++ b/server/scheduled/executor_service.go @@ -16,7 +16,7 @@ type ExecutorService struct { log logging.SimpleLogging // jobs - runtimeStatsPublisher JobDefinition + jobs []JobDefinition } func NewExecutorService( @@ -33,11 +33,15 @@ func NewExecutorService( } return &ExecutorService{ - log: log, - runtimeStatsPublisher: runtimeStatsPublisherJob, + log: log, + jobs: []JobDefinition{runtimeStatsPublisherJob}, } } +func (s *ExecutorService) AddJob(jd JobDefinition) { + s.jobs = append(s.jobs, jd) +} + type JobDefinition struct { Job Job Period time.Duration @@ -50,7 +54,9 @@ func (s *ExecutorService) Run() { var wg sync.WaitGroup - s.runScheduledJob(ctx, &wg, s.runtimeStatsPublisher) + for _, jd := range s.jobs { + s.runScheduledJob(ctx, &wg, jd) + } interrupt := make(chan os.Signal, 1) diff --git a/server/server.go b/server/server.go index 07403baf23..82a1d98501 100644 --- a/server/server.go +++ b/server/server.go @@ -462,6 +462,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { CheckoutDepth: userConfig.CheckoutDepth, GithubAppEnabled: githubAppEnabled, } + + scheduledExecutorService := scheduled.NewExecutorService( + statsScope, + logger, + ) + // provide fresh tokens before clone from the GitHub Apps integration, proxy workingDir if githubAppEnabled { if !userConfig.WriteGitCreds { @@ -474,10 +480,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } githubAppTokenRotator = vcs.NewGithubAppTokenRotator(logger, githubCredentials, userConfig.GithubHostname) - err := githubAppTokenRotator.Start() + tokenJd, err := githubAppTokenRotator.GenerateJob() if err != nil { return nil, errors.Wrap(err, "Could not write credentials") } + scheduledExecutorService.AddJob(tokenJd) } projectLocker := &events.DefaultProjectLocker{ @@ -868,10 +875,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GithubHostname: userConfig.GithubHostname, GithubOrg: userConfig.GithubOrg, } - scheduledExecutorService := scheduled.NewExecutorService( - statsScope, - logger, - ) return &Server{ AtlantisVersion: config.AtlantisVersion, From 7defbe3af1dfff71e5182b279a533755caf84925 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Thu, 16 Mar 2023 11:44:34 +0100 Subject: [PATCH 04/14] adds unit tests --- server/events/vcs/gh_app_creds_rotator.go | 23 +++-- .../events/vcs/gh_app_creds_rotator_test.go | 84 +++++++++++++++++++ server/server.go | 12 +-- 3 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 server/events/vcs/gh_app_creds_rotator_test.go diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go index 263a0fc967..1c1dc32543 100644 --- a/server/events/vcs/gh_app_creds_rotator.go +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -3,9 +3,7 @@ package vcs import ( "time" - "github.com/mitchellh/go-homedir" "github.com/pkg/errors" - "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/scheduled" ) @@ -13,7 +11,6 @@ import ( // GitCredsTokenRotator continuously tries to rotate the github app access token every 30 seconds and writes the ~/.git-credentials file type GitCredsTokenRotator interface { Run() - rotate() error GenerateJob() (scheduled.JobDefinition, error) } @@ -21,16 +18,20 @@ type githubAppTokenRotator struct { log logging.SimpleLogging githubCredentials GithubCredentials githubHostname string + homeDirPath string } func NewGithubAppTokenRotator( - logger logging.SimpleLogging, + log logging.SimpleLogging, githubCredentials GithubCredentials, - githubHostname string) GitCredsTokenRotator { + githubHostname string, + homeDirPath string) GitCredsTokenRotator { + return &githubAppTokenRotator{ - log: logger, + log: log, githubCredentials: githubCredentials, githubHostname: githubHostname, + homeDirPath: homeDirPath, } } @@ -53,17 +54,13 @@ func (r *githubAppTokenRotator) rotate() error { r.log.Debug("Refreshing git tokens for Github App") token, err := r.githubCredentials.GetToken() if err != nil { - return errors.Wrap(err, "getting github token") + return errors.Wrap(err, "Getting github token") } r.log.Debug("token %s", token) - home, err := homedir.Dir() - if err != nil { - return errors.Wrap(err, "getting home dir to write ~/.git-credentials file") - } // https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#http-based-git-access-by-an-installation - if err := WriteGitCreds("x-access-token", token, r.githubHostname, home, r.log, true); err != nil { - return errors.Wrap(err, "writing ~/.git-credentials file") + if err := WriteGitCreds("x-access-token", token, r.githubHostname, r.homeDirPath, r.log, true); err != nil { + return errors.Wrap(err, "Writing ~/.git-credentials file") } return nil } diff --git a/server/events/vcs/gh_app_creds_rotator_test.go b/server/events/vcs/gh_app_creds_rotator_test.go new file mode 100644 index 0000000000..4c5f4eb1e2 --- /dev/null +++ b/server/events/vcs/gh_app_creds_rotator_test.go @@ -0,0 +1,84 @@ +package vcs_test + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/events/vcs/testdata" + "github.com/runatlantis/atlantis/server/logging" + . "github.com/runatlantis/atlantis/testing" +) + +func Test_githubAppTokenRotator_GenerateJob(t *testing.T) { + defer disableSSLVerification()() + testServer, err := testdata.GithubAppTestServer(t) + Ok(t, err) + + anonCreds := &vcs.GithubAnonymousCredentials{} + anonClient, err := vcs.NewGithubClient(testServer, anonCreds, vcs.GithubConfig{}, logging.NewNoopLogger(t)) + Ok(t, err) + tempSecrets, err := anonClient.ExchangeCode("good-code") + Ok(t, err) + type fields struct { + githubCredentials vcs.GithubCredentials + } + tests := []struct { + name string + fields fields + credsFileWritten bool + wantErr bool + }{ + { + name: "Should write .git-credentials file on start", + fields: fields{&vcs.GithubAppCredentials{ + AppID: tempSecrets.ID, + Key: []byte(testdata.GithubPrivateKey), + Hostname: testServer, + }}, + credsFileWritten: true, + wantErr: false, + }, + { + name: "Should return an error if pem data is missing or wrong", + fields: fields{&vcs.GithubAppCredentials{ + AppID: tempSecrets.ID, + Key: []byte("some bad formatted pem key"), + Hostname: testServer, + }}, + credsFileWritten: false, + wantErr: true, + }, + { + name: "Should return an error if app id is missing or wrong", + fields: fields{&vcs.GithubAppCredentials{ + AppID: 3819, + Key: []byte(testdata.GithubPrivateKey), + Hostname: testServer, + }}, + credsFileWritten: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + r := vcs.NewGithubAppTokenRotator(logging.NewNoopLogger(t), tt.fields.githubCredentials, testServer, tmpDir) + got, err := r.GenerateJob() + if (err != nil) != tt.wantErr { + t.Errorf("githubAppTokenRotator.GenerateJob() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.credsFileWritten { + credsFileContent := fmt.Sprintf(`https://x-access-token:some-token@%s`, testServer) + actContents, err := os.ReadFile(filepath.Join(tmpDir, ".git-credentials")) + Ok(t, err) + Equals(t, credsFileContent, string(actContents)) + } + Equals(t, 30*time.Second, got.Period) + }) + } +} diff --git a/server/server.go b/server/server.go index 82a1d98501..8b5e6a4dbf 100644 --- a/server/server.go +++ b/server/server.go @@ -169,7 +169,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { var bitbucketCloudClient *bitbucketcloud.Client var bitbucketServerClient *bitbucketserver.Client var azuredevopsClient *vcs.AzureDevopsClient - var githubAppTokenRotator vcs.GitCredsTokenRotator policyChecksEnabled := false if userConfig.EnablePolicyChecksFlag { @@ -299,11 +298,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } } + home, err := homedir.Dir() + if err != nil { + return nil, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") + } + if userConfig.WriteGitCreds { - home, err := homedir.Dir() - if err != nil { - return nil, errors.Wrap(err, "getting home dir to write ~/.git-credentials file") - } if userConfig.GithubUser != "" { if err := vcs.WriteGitCreds(userConfig.GithubUser, userConfig.GithubToken, userConfig.GithubHostname, home, logger, false); err != nil { return nil, err @@ -479,7 +479,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GithubHostname: userConfig.GithubHostname, } - githubAppTokenRotator = vcs.NewGithubAppTokenRotator(logger, githubCredentials, userConfig.GithubHostname) + githubAppTokenRotator := vcs.NewGithubAppTokenRotator(logger, githubCredentials, userConfig.GithubHostname, home) tokenJd, err := githubAppTokenRotator.GenerateJob() if err != nil { return nil, errors.Wrap(err, "Could not write credentials") From c77b239c69a1876b6372dd9ee87d473a48168fb4 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Fri, 17 Mar 2023 11:44:33 +0100 Subject: [PATCH 05/14] fix --- .gitignore | 2 ++ server/events/github_app_working_dir.go | 23 +++++++++++------------ server/events/vcs/gh_app_creds_rotator.go | 1 + 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index fbf411260f..b06420a328 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,5 @@ dist/ tmp-CHANGELOG.md .envrc + +Dockerfile.local \ No newline at end of file diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index dc8b8a75d2..153e7d8e8e 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/logging" @@ -21,22 +20,22 @@ type GithubAppWorkingDir struct { // Clone writes a fresh token for Github App authentication func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { - - token, err := g.Credentials.GetToken() - if err != nil { - return "", false, errors.Wrap(err, "getting github token") - } - baseRepo := &p.BaseRepo // Realistically, this is a super brittle way of supporting clones using gh app installation tokens // This URL should be built during Repo creation and the struct should be immutable going forward. // Doing this requires a larger refactor however, and can probably be coupled with supporting > 1 installation - authURL := fmt.Sprintf("://x-access-token:%s", token) - baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:", authURL, 1) - baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:", "://x-access-token:", 1) - headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:", authURL, 1) - headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:", "://x-access-token:", 1) + + // This removes the credential part from the url and leaves us with the raw http url + // git will then pick up credentials from the credential store which is set in vcs.WriteGitCreds. + // Git credentials will then be rotated by vcs.GitCredsTokenRotator + authURL := fmt.Sprintf("://") + baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", authURL, 1) + baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", "://", 1) + headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", authURL, 1) + headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://@", "://", 1) + + log.Info("headRepo _ after: %s", headRepo) return g.WorkingDir.Clone(log, headRepo, p, workspace) } diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go index 1c1dc32543..a4eeea4d18 100644 --- a/server/events/vcs/gh_app_creds_rotator.go +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -52,6 +52,7 @@ func (r *githubAppTokenRotator) Run() { func (r *githubAppTokenRotator) rotate() error { r.log.Debug("Refreshing git tokens for Github App") + token, err := r.githubCredentials.GetToken() if err != nil { return errors.Wrap(err, "Getting github token") From 7dd7d34ce8e60c676d95586aeb77ec0856ec4a98 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Fri, 17 Mar 2023 11:45:28 +0100 Subject: [PATCH 06/14] fix --- server/events/github_app_working_dir.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 153e7d8e8e..aadaa9f45d 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -35,7 +35,5 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", authURL, 1) headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://@", "://", 1) - log.Info("headRepo _ after: %s", headRepo) - return g.WorkingDir.Clone(log, headRepo, p, workspace) } From 5ec0a62a6fba018330b1999b00a3ae6f122044ed Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Fri, 17 Mar 2023 13:04:48 +0100 Subject: [PATCH 07/14] fix tests --- server/events/apply_command_runner_test.go | 2 ++ .../events/approve_policies_command_runner_test.go | 1 + server/events/command_runner_test.go | 2 +- server/events/github_app_working_dir.go | 13 +++++++------ server/events/github_app_working_dir_test.go | 4 ++-- server/events/import_command_runner_test.go | 1 + server/events/plan_command_runner_test.go | 1 + server/events/pull_closed_executor_test.go | 2 ++ server/events/vcs/git_cred_writer_test.go | 11 +++++++++-- 9 files changed, 26 insertions(+), 11 deletions(-) diff --git a/server/events/apply_command_runner_test.go b/server/events/apply_command_runner_test.go index 5d461d2db8..6bbe25d0a1 100644 --- a/server/events/apply_command_runner_test.go +++ b/server/events/apply_command_runner_test.go @@ -19,6 +19,7 @@ import ( ) func TestApplyCommandRunner_IsLocked(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) cases := []struct { @@ -78,6 +79,7 @@ func TestApplyCommandRunner_IsLocked(t *testing.T) { } func TestApplyCommandRunner_IsSilenced(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) cases := []struct { diff --git a/server/events/approve_policies_command_runner_test.go b/server/events/approve_policies_command_runner_test.go index 8db1ad1216..a5ac56e57a 100644 --- a/server/events/approve_policies_command_runner_test.go +++ b/server/events/approve_policies_command_runner_test.go @@ -15,6 +15,7 @@ import ( ) func TestApproveCommandRunner_IsOwner(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) cases := []struct { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 227dca01cc..4eab741941 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -113,7 +113,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock githubGetter = mocks.NewMockGithubPullGetter() gitlabGetter = mocks.NewMockGitlabMergeRequestGetter() azuredevopsGetter = mocks.NewMockAzureDevopsPullGetter() - logger = logging.NewNoopLogger(t) + logger := logging.NewNoopLogger(t) projectCommandRunner = mocks.NewMockProjectCommandRunner() workingDir = mocks.NewMockWorkingDir() pendingPlanFinder = mocks.NewMockPendingPlanFinder() diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index aadaa9f45d..c3d5c3601c 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -1,7 +1,6 @@ package events import ( - "fmt" "strings" "github.com/runatlantis/atlantis/server/events/models" @@ -29,11 +28,13 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R // This removes the credential part from the url and leaves us with the raw http url // git will then pick up credentials from the credential store which is set in vcs.WriteGitCreds. // Git credentials will then be rotated by vcs.GitCredsTokenRotator - authURL := fmt.Sprintf("://") - baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", authURL, 1) - baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", "://", 1) - headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", authURL, 1) - headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://@", "://", 1) + replacement := "://" + baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", replacement, 1) + baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) + headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", replacement, 1) + headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://@", replacement, 1) + + log.Info("%v", headRepo) return g.WorkingDir.Clone(log, headRepo, p, workspace) } diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go index 9598a23044..58f732aa44 100644 --- a/server/events/github_app_working_dir_test.go +++ b/server/events/github_app_working_dir_test.go @@ -82,8 +82,8 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { headRepo := baseRepo modifiedBaseRepo := baseRepo - modifiedBaseRepo.CloneURL = "https://x-access-token:token@github.com/runatlantis/atlantis.git" - modifiedBaseRepo.SanitizedCloneURL = "https://x-access-token:@github.com/runatlantis/atlantis.git" + modifiedBaseRepo.CloneURL = "https://github.com/runatlantis/atlantis.git" + modifiedBaseRepo.SanitizedCloneURL = "https://:@github.com/runatlantis/atlantis.git" // stays as is / unmodified When(credentials.GetToken()).ThenReturn("token", nil) When(workingDir.Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default")).ThenReturn( diff --git a/server/events/import_command_runner_test.go b/server/events/import_command_runner_test.go index 1acaecbfdb..2d435b692a 100644 --- a/server/events/import_command_runner_test.go +++ b/server/events/import_command_runner_test.go @@ -14,6 +14,7 @@ import ( ) func TestImportCommandRunner_Run(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) tests := []struct { diff --git a/server/events/plan_command_runner_test.go b/server/events/plan_command_runner_test.go index 799dd54b2a..6e5c510566 100644 --- a/server/events/plan_command_runner_test.go +++ b/server/events/plan_command_runner_test.go @@ -16,6 +16,7 @@ import ( ) func TestPlanCommandRunner_IsSilenced(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) cases := []struct { diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index 6a49686c86..41fbd913bf 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -20,6 +20,7 @@ import ( "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/db" "github.com/runatlantis/atlantis/server/jobs" + "github.com/runatlantis/atlantis/server/logging" "github.com/stretchr/testify/assert" bolt "go.etcd.io/bbolt" @@ -187,6 +188,7 @@ func TestCleanUpPullComments(t *testing.T) { } func TestCleanUpLogStreaming(t *testing.T) { + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) t.Run("Should Clean Up Log Streaming Resources When PR is closed", func(t *testing.T) { diff --git a/server/events/vcs/git_cred_writer_test.go b/server/events/vcs/git_cred_writer_test.go index 0aefd703b1..6826c1cc84 100644 --- a/server/events/vcs/git_cred_writer_test.go +++ b/server/events/vcs/git_cred_writer_test.go @@ -12,10 +12,9 @@ import ( . "github.com/runatlantis/atlantis/testing" ) -var logger logging.SimpleLogging - // Test that we write the file as expected func TestWriteGitCreds_WriteFile(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) @@ -31,6 +30,7 @@ func TestWriteGitCreds_WriteFile(t *testing.T) { // Test that if the file already exists and it doesn't have the line we would // have written, we write it. func TestWriteGitCreds_Appends(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() credsFile := filepath.Join(tmp, ".git-credentials") @@ -49,6 +49,7 @@ func TestWriteGitCreds_Appends(t *testing.T) { // Test that if the file already exists and it already has the line expected // we do nothing. func TestWriteGitCreds_NoModification(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() credsFile := filepath.Join(tmp, ".git-credentials") @@ -65,6 +66,7 @@ func TestWriteGitCreds_NoModification(t *testing.T) { // Test that the github app credentials get replaced. func TestWriteGitCreds_ReplaceApp(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() credsFile := filepath.Join(tmp, ".git-credentials") @@ -82,6 +84,7 @@ func TestWriteGitCreds_ReplaceApp(t *testing.T) { // Test that the github app credentials get updated when cred file is empty. func TestWriteGitCreds_AppendApp(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() credsFile := filepath.Join(tmp, ".git-credentials") @@ -100,6 +103,7 @@ func TestWriteGitCreds_AppendApp(t *testing.T) { // Test that if we can't read the existing file to see if the contents will be // the same that we just error out. func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() credsFile := filepath.Join(tmp, ".git-credentials") @@ -113,6 +117,7 @@ func TestWriteGitCreds_ErrIfCannotRead(t *testing.T) { // Test that if we can't write, we error out. func TestWriteGitCreds_ErrIfCannotWrite(t *testing.T) { + logger := logging.NewNoopLogger(t) credsFile := "/this/dir/does/not/exist/.git-credentials" // nolint: gosec expErr := fmt.Sprintf("writing generated .git-credentials file with user, token and hostname to %s: open %s: no such file or directory", credsFile, credsFile) actErr := vcs.WriteGitCreds("user", "token", "hostname", "/this/dir/does/not/exist", logger, false) @@ -121,6 +126,7 @@ func TestWriteGitCreds_ErrIfCannotWrite(t *testing.T) { // Test that git is actually configured to use the credentials func TestWriteGitCreds_ConfigureGitCredentialHelper(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) @@ -134,6 +140,7 @@ func TestWriteGitCreds_ConfigureGitCredentialHelper(t *testing.T) { // Test that git is configured to use https instead of ssh func TestWriteGitCreds_ConfigureGitUrlOverride(t *testing.T) { + logger := logging.NewNoopLogger(t) tmp := t.TempDir() err := vcs.WriteGitCreds("user", "token", "hostname", tmp, logger, false) From 57cb82bede7ab268641e458eabab55fe131e2f6e Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 09:59:45 +0100 Subject: [PATCH 08/14] Update server/server.go Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> --- server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index 8b5e6a4dbf..9613de5067 100644 --- a/server/server.go +++ b/server/server.go @@ -482,7 +482,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { githubAppTokenRotator := vcs.NewGithubAppTokenRotator(logger, githubCredentials, userConfig.GithubHostname, home) tokenJd, err := githubAppTokenRotator.GenerateJob() if err != nil { - return nil, errors.Wrap(err, "Could not write credentials") + return nil, errors.Wrap(err, "could not write credentials") } scheduledExecutorService.AddJob(tokenJd) } From 8cd641fa3d8ca2f0ea3d769fddf58f91ca200ffe Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 10:06:14 +0100 Subject: [PATCH 09/14] fix --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b06420a328..2d9dad12f1 100644 --- a/.gitignore +++ b/.gitignore @@ -17,11 +17,10 @@ atlantis atlantis.env *.act package-lock.json +Dockerfile.local # gitreleaser dist/ tmp-CHANGELOG.md .envrc - -Dockerfile.local \ No newline at end of file From 058470a103bc87ac333551d3e0d87ac5c067bd19 Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 10:36:51 +0100 Subject: [PATCH 10/14] update tests --- server/events/github_app_working_dir.go | 6 ++---- server/events/github_app_working_dir_test.go | 9 ++++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index c3d5c3601c..68bbfd9241 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -30,11 +30,9 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R // Git credentials will then be rotated by vcs.GitCredsTokenRotator replacement := "://" baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", replacement, 1) - baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) + baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", replacement, 1) - headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://@", replacement, 1) - - log.Info("%v", headRepo) + headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) return g.WorkingDir.Clone(log, headRepo, p, workspace) } diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go index 58f732aa44..fe3bea74e2 100644 --- a/server/events/github_app_working_dir_test.go +++ b/server/events/github_app_working_dir_test.go @@ -5,6 +5,7 @@ import ( "testing" . "github.com/petergtz/pegomock" + pegomock "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events" eventMocks "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/models" @@ -57,6 +58,9 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { } func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { + + pegomock.RegisterMockTestingT(t) + workingDir := eventMocks.NewMockWorkingDir() credentials := vcsMocks.NewMockGithubCredentials() @@ -82,8 +86,9 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { headRepo := baseRepo modifiedBaseRepo := baseRepo + // remove credentials from both urls since we want to use the credential store modifiedBaseRepo.CloneURL = "https://github.com/runatlantis/atlantis.git" - modifiedBaseRepo.SanitizedCloneURL = "https://:@github.com/runatlantis/atlantis.git" // stays as is / unmodified + modifiedBaseRepo.SanitizedCloneURL = "https://github.com/runatlantis/atlantis.git" When(credentials.GetToken()).ThenReturn("token", nil) When(workingDir.Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default")).ThenReturn( @@ -92,5 +97,7 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { _, success, _ := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default") + workingDir.VerifyWasCalledOnce().Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default") + Assert(t, success == true, "clone url mutation error") } From fc9f0fd7bcafd0809680288235172f0f8a314e3c Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 11:50:10 +0100 Subject: [PATCH 11/14] add test for scheduled executior_service --- server/events/github_app_working_dir_test.go | 1 - server/scheduled/executor_service.go | 1 + server/scheduled/executor_service_test.go | 48 ++++++++++ .../mocks/mock_executor_service_job.go | 87 +++++++++++++++++++ 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 server/scheduled/executor_service_test.go create mode 100644 server/scheduled/mocks/mock_executor_service_job.go diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go index fe3bea74e2..af1d823873 100644 --- a/server/events/github_app_working_dir_test.go +++ b/server/events/github_app_working_dir_test.go @@ -58,7 +58,6 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { } func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { - pegomock.RegisterMockTestingT(t) workingDir := eventMocks.NewMockWorkingDir() diff --git a/server/scheduled/executor_service.go b/server/scheduled/executor_service.go index a4be0f266e..2bcdf2012d 100644 --- a/server/scheduled/executor_service.go +++ b/server/scheduled/executor_service.go @@ -102,6 +102,7 @@ func (s *ExecutorService) runScheduledJob(ctx context.Context, wg *sync.WaitGrou } +//go:generate pegomock generate -m --package mocks -o mocks/mock_executor_service_job.go Job type Job interface { Run() } diff --git a/server/scheduled/executor_service_test.go b/server/scheduled/executor_service_test.go new file mode 100644 index 0000000000..a2705815da --- /dev/null +++ b/server/scheduled/executor_service_test.go @@ -0,0 +1,48 @@ +package scheduled + +import ( + "testing" + "time" + + "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/scheduled/mocks" +) + +func TestExecutorService_Run(t *testing.T) { + pegomock.RegisterMockTestingT(t) + mockJob := mocks.NewMockJob() + type fields struct { + log logging.SimpleLogging + jobs []JobDefinition + } + tests := []struct { + name string + fields fields + }{ + { + name: "test", + fields: fields{ + log: logging.NewNoopLogger(t), + jobs: []JobDefinition{ + { + Job: mockJob, + Period: 1 * time.Second, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &ExecutorService{ + log: tt.fields.log, + jobs: make([]JobDefinition, 0), + } + s.AddJob(tt.fields.jobs[0]) + go s.Run() + time.Sleep(1050 * time.Millisecond) + mockJob.VerifyWasCalledOnce().Run() + }) + } +} diff --git a/server/scheduled/mocks/mock_executor_service_job.go b/server/scheduled/mocks/mock_executor_service_job.go new file mode 100644 index 0000000000..3266ab0a71 --- /dev/null +++ b/server/scheduled/mocks/mock_executor_service_job.go @@ -0,0 +1,87 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/scheduled (interfaces: Job) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + "reflect" + "time" +) + +type MockJob struct { + fail func(message string, callerSkip ...int) +} + +func NewMockJob(options ...pegomock.Option) *MockJob { + mock := &MockJob{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockJob) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockJob) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockJob) Run() { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockJob().") + } + params := []pegomock.Param{} + pegomock.GetGenericMockFrom(mock).Invoke("Run", params, []reflect.Type{}) +} + +func (mock *MockJob) VerifyWasCalledOnce() *VerifierMockJob { + return &VerifierMockJob{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockJob) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockJob { + return &VerifierMockJob{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockJob) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockJob { + return &VerifierMockJob{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockJob) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockJob { + return &VerifierMockJob{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockJob struct { + mock *MockJob + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockJob) Run() *MockJob_Run_OngoingVerification { + params := []pegomock.Param{} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Run", params, verifier.timeout) + return &MockJob_Run_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockJob_Run_OngoingVerification struct { + mock *MockJob + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockJob_Run_OngoingVerification) GetCapturedArguments() { +} + +func (c *MockJob_Run_OngoingVerification) GetAllCapturedArguments() { +} From 8ee8e64efd69a37536cefd47755c29d5939fa49f Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 14:40:18 +0100 Subject: [PATCH 12/14] move old string into a global const --- server/events/github_app_working_dir.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 68bbfd9241..0e94ba2f37 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -8,6 +8,8 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) +const RedactedReplacement = "://:@" + // GithubAppWorkingDir implements WorkingDir. // It acts as a proxy to an instance of WorkingDir that refreshes the app's token // before every clone, given Github App tokens expire quickly @@ -30,9 +32,9 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R // Git credentials will then be rotated by vcs.GitCredsTokenRotator replacement := "://" baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", replacement, 1) - baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) + baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, RedactedReplacement, replacement, 1) headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", replacement, 1) - headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:@", replacement, 1) + headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, RedactedReplacement, replacement, 1) return g.WorkingDir.Clone(log, headRepo, p, workspace) } From 3a9c06819864acc0e75c04c7ee4d2d2341c498bf Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Mon, 20 Mar 2023 14:41:04 +0100 Subject: [PATCH 13/14] don't export const --- server/events/github_app_working_dir.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 0e94ba2f37..adb54d353c 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -8,7 +8,7 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) -const RedactedReplacement = "://:@" +const redactedReplacement = "://:@" // GithubAppWorkingDir implements WorkingDir. // It acts as a proxy to an instance of WorkingDir that refreshes the app's token @@ -32,9 +32,9 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R // Git credentials will then be rotated by vcs.GitCredsTokenRotator replacement := "://" baseRepo.CloneURL = strings.Replace(baseRepo.CloneURL, "://:@", replacement, 1) - baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, RedactedReplacement, replacement, 1) + baseRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, redactedReplacement, replacement, 1) headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", replacement, 1) - headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, RedactedReplacement, replacement, 1) + headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, redactedReplacement, replacement, 1) return g.WorkingDir.Clone(log, headRepo, p, workspace) } From 2e5f3dd66d0ddbb6c15299416f53cb353bdd188b Mon Sep 17 00:00:00 2001 From: Jonathan Wiemers Date: Tue, 21 Mar 2023 08:56:06 +0100 Subject: [PATCH 14/14] fix lint error --- server/events/vcs/gh_app_creds_rotator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/events/vcs/gh_app_creds_rotator.go b/server/events/vcs/gh_app_creds_rotator.go index a4eeea4d18..6522f33118 100644 --- a/server/events/vcs/gh_app_creds_rotator.go +++ b/server/events/vcs/gh_app_creds_rotator.go @@ -47,7 +47,11 @@ func (r *githubAppTokenRotator) GenerateJob() (scheduled.JobDefinition, error) { } func (r *githubAppTokenRotator) Run() { - r.rotate() + err := r.rotate() + if err != nil { + // at least log the error message here, as we want to notify the that user that the key rotation wasn't successful + r.log.Err(err.Error()) + } } func (r *githubAppTokenRotator) rotate() error {