Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: GitHub - Support loading git token from disk #4928

Merged
merged 1 commit into from
Nov 4, 2024
+142 −54
Merged

feat: GitHub - Support loading git token from disk #4928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions cmd/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const (
GHHostnameFlag = "gh-hostname"
GHTeamAllowlistFlag = "gh-team-allowlist"
GHTokenFlag = "gh-token"
GHTokenFileFlag = "gh-token-file" // nolint: gosec
GHUserFlag = "gh-user"
GHAppIDFlag = "gh-app-id"
GHAppKeyFlag = "gh-app-key"
Expand Down Expand Up @@ -317,6 +318,9 @@ var stringFlags = map[string]stringFlag{
GHTokenFlag: {
description: "GitHub token of API user. Can also be specified via the ATLANTIS_GH_TOKEN environment variable.",
},
GHTokenFileFlag: {
description: "A path to a file containing the GitHub token of API user. Can also be specified via the ATLANTIS_GH_TOKEN_FILE environment variable.",
},
GHAppKeyFlag: {
description: "The GitHub App's private key",
defaultValue: "",
Expand Down Expand Up @@ -965,26 +969,29 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error {
}

// The following combinations are valid.
// 1. github user and token set
// 1. github user and (token or token file)
// 2. github app ID and (key file set or key set)
// 3. gitea user and token set
// 4. gitlab user and token set
// 5. bitbucket user and token set
// 6. azuredevops user and token set
// 7. any combination of the above
vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GHAppIDFlag, GHAppKeyFileFlag, GHAppIDFlag, GHAppKeyFlag, GiteaUserFlag, GiteaTokenFlag, GitlabUserFlag, GitlabTokenFlag, BitbucketUserFlag, BitbucketTokenFlag, ADUserFlag, ADTokenFlag)
if ((userConfig.GithubUser == "") != (userConfig.GithubToken == "")) ||
((userConfig.GiteaUser == "") != (userConfig.GiteaToken == "")) ||
vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GHUserFlag, GHTokenFileFlag, GHAppIDFlag, GHAppKeyFileFlag, GHAppIDFlag, GHAppKeyFlag, GiteaUserFlag, GiteaTokenFlag, GitlabUserFlag, GitlabTokenFlag, BitbucketUserFlag, BitbucketTokenFlag, ADUserFlag, ADTokenFlag)
if ((userConfig.GiteaUser == "") != (userConfig.GiteaToken == "")) ||
((userConfig.GitlabUser == "") != (userConfig.GitlabToken == "")) ||
((userConfig.BitbucketUser == "") != (userConfig.BitbucketToken == "")) ||
((userConfig.AzureDevopsUser == "") != (userConfig.AzureDevopsToken == "")) {
return vcsErr
}
if (userConfig.GithubAppID != 0) && ((userConfig.GithubAppKey == "") && (userConfig.GithubAppKeyFile == "")) {
return vcsErr
if userConfig.GithubUser != "" {
if (userConfig.GithubToken == "") == (userConfig.GithubTokenFile == "") {
return vcsErr
}
meringu marked this conversation as resolved.
Show resolved Hide resolved
}
if (userConfig.GithubAppID == 0) && ((userConfig.GithubAppKey != "") || (userConfig.GithubAppKeyFile != "")) {
return vcsErr
if userConfig.GithubAppID != 0 {
if (userConfig.GithubAppKey == "") == (userConfig.GithubAppKeyFile == "") {
return vcsErr
}
}
// At this point, we know that there can't be a single user/token without
// its partner, but we haven't checked if any user/token is set at all.
Expand Down Expand Up @@ -1026,6 +1033,7 @@ func (s *ServerCmd) validate(userConfig server.UserConfig) error {
// Warn if any tokens have newlines.
for name, token := range map[string]string{
GHTokenFlag: userConfig.GithubToken,
GHTokenFileFlag: userConfig.GithubTokenFile,
GHWebhookSecretFlag: userConfig.GithubWebhookSecret,
GitlabTokenFlag: userConfig.GitlabToken,
GitlabWebhookSecretFlag: userConfig.GitlabWebhookSecret,
Expand Down
20 changes: 19 additions & 1 deletion cmd/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ var testFlags = map[string]interface{}{
GHHostnameFlag: "ghhostname",
GHTeamAllowlistFlag: "",
GHTokenFlag: "token",
GHTokenFileFlag: "",
GHUserFlag: "user",
GHAppIDFlag: int64(0),
GHAppKeyFlag: "",
Expand Down Expand Up @@ -433,7 +434,7 @@ func TestExecute_ValidateSSLConfig(t *testing.T) {
}

func TestExecute_ValidateVCSConfig(t *testing.T) {
expErr := "--gh-user/--gh-token or --gh-app-id/--gh-app-key-file or --gh-app-id/--gh-app-key or --gitea-user/--gitea-token or --gitlab-user/--gitlab-token or --bitbucket-user/--bitbucket-token or --azuredevops-user/--azuredevops-token must be set"
expErr := "--gh-user/--gh-token or --gh-user/--gh-token-file or --gh-app-id/--gh-app-key-file or --gh-app-id/--gh-app-key or --gitea-user/--gitea-token or --gitlab-user/--gitlab-token or --bitbucket-user/--bitbucket-token or --azuredevops-user/--azuredevops-token must be set"
cases := []struct {
description string
flags map[string]interface{}
Expand Down Expand Up @@ -583,6 +584,23 @@ func TestExecute_ValidateVCSConfig(t *testing.T) {
},
false,
},
{
"github user and github token file and should be successful",
map[string]interface{}{
GHUserFlag: "user",
GHTokenFileFlag: "/path/to/token",
},
false,
},
{
"github user, github token, and github token file and should fail",
map[string]interface{}{
GHUserFlag: "user",
GHTokenFlag: "token",
GHTokenFileFlag: "/path/to/token",
},
true,
},
{
"gitea user and gitea token set and should be successful",
map[string]interface{}{
Expand Down
10 changes: 10 additions & 0 deletions runatlantis.io/docs/server-configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,16 @@ based on the organization or user that triggered the webhook.

GitHub token of API user.

### `--gh-token-file`

```bash
atlantis server --gh-token-file="/path/to/token"
# or
ATLANTIS_GH_TOKEN_FILE="/path/to/token"
```

GitHub token of API user. The token is loaded from disk regularly to allow for rotation of the token without the need to restart the Atlantis server.

### `--gh-user`

```bash
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/git_cred_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func WriteGitCreds(gitUser string, gitToken string, gitHostname string, home str
if err := fileLineReplace(config, gitUser, gitHostname, credsFile); err != nil {
return errors.Wrap(err, "replacing git credentials line for github app")
}
logger.Info("updated git app credentials in %s", credsFile)
logger.Info("updated git credentials in %s", credsFile)
} else {
if err := fileAppend(config, credsFile); err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions server/events/vcs/github_client_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ import (

// If the hostname is github.com, should use normal BaseURL.
func TestNewGithubClient_GithubCom(t *testing.T) {
client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := NewGithubClient("github.com", &GithubUserCredentials{"user", "pass", ""}, GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
Equals(t, "https://api.github.com/", client.client.BaseURL.String())
}

// If the hostname is a non-github hostname should use the right BaseURL.
func TestNewGithubClient_NonGithub(t *testing.T) {
client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass"}, GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := NewGithubClient("example.com", &GithubUserCredentials{"user", "pass", ""}, GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
Equals(t, "https://example.com/api/v3/", client.client.BaseURL.String())
// If possible in the future, test the GraphQL client's URL as well. But at the
Expand Down
36 changes: 18 additions & 18 deletions server/events/vcs/github_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestGithubClient_GetModifiedFiles(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logger)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logger)
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -121,7 +121,7 @@ func TestGithubClient_GetModifiedFilesMovedFile(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -218,7 +218,7 @@ func TestGithubClient_PaginatesComments(t *testing.T) {
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)

client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -334,7 +334,7 @@ func TestGithubClient_HideOldComments(t *testing.T) {
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)

client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{atlantisUser, "pass"}, vcs.GithubConfig{}, 0,
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{atlantisUser, "pass", ""}, vcs.GithubConfig{}, 0,
logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
Expand Down Expand Up @@ -407,7 +407,7 @@ func TestGithubClient_UpdateStatus(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -496,7 +496,7 @@ func TestGithubClient_PullIsApproved(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -591,7 +591,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -874,7 +874,7 @@ func TestGithubClient_PullIsMergeableWithAllowMergeableBypassApply(t *testing.T)
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{AllowMergeableBypassApply: true}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{AllowMergeableBypassApply: true}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -959,7 +959,7 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -1139,7 +1139,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -1173,7 +1173,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
}

func TestGithubClient_MarkdownPullLink(t *testing.T) {
client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient("hostname", &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
pull := models.PullRequest{Num: 1}
s, _ := client.MarkdownPullLink(pull)
Expand Down Expand Up @@ -1229,7 +1229,7 @@ func TestGithubClient_SplitComments(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
pull := models.PullRequest{Num: 1}
Expand Down Expand Up @@ -1288,7 +1288,7 @@ func TestGithubClient_Retry404(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
repo := models.Repo{
Expand Down Expand Up @@ -1334,7 +1334,7 @@ func TestGithubClient_Retry404Files(t *testing.T) {

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
repo := models.Repo{
Expand Down Expand Up @@ -1387,7 +1387,7 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) {
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logger)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logger)
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -1585,7 +1585,7 @@ func TestGithubClient_DiscardReviews(t *testing.T) {
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
if err := client.DiscardReviews(tt.args.repo, tt.args.pull); (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -1654,7 +1654,7 @@ func TestGithubClient_GetPullLabels(t *testing.T) {
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logger)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logger)
Ok(t, err)
defer disableSSLVerification()()

Expand Down Expand Up @@ -1691,7 +1691,7 @@ func TestGithubClient_GetPullLabels_EmptyResponse(t *testing.T) {
}))
testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, vcs.GithubConfig{}, 0, logger)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass", ""}, vcs.GithubConfig{}, 0, logger)
Ok(t, err)
defer disableSSLVerification()()

Expand Down
50 changes: 44 additions & 6 deletions server/events/vcs/github_credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"net/url"
"os"
"strings"

"github.com/bradleyfalzon/ghinstallation/v2"
Expand Down Expand Up @@ -42,17 +43,45 @@ func (c *GithubAnonymousCredentials) GetToken() (string, error) {

// GithubUserCredentials implements GithubCredentials for the personal auth token flow.
type GithubUserCredentials struct {
User string
Token string
User string
Token string
TokenFile string
}

type GitHubUserTransport struct {
Credentials *GithubUserCredentials
Transport *github.BasicAuthTransport
}

func (t *GitHubUserTransport) RoundTrip(req *http.Request) (*http.Response, error) {
// update token
token, err := t.Credentials.GetToken()
if err != nil {
return nil, err
}
t.Transport.Password = token

// defer to the underlying transport
return t.Transport.RoundTrip(req)
}

// Client returns a client for basic auth user credentials.
func (c *GithubUserCredentials) Client() (*http.Client, error) {
tr := &github.BasicAuthTransport{
Username: strings.TrimSpace(c.User),
Password: strings.TrimSpace(c.Token),
password, err := c.GetToken()
if err != nil {
return nil, err
}

client := &http.Client{
Transport: &GitHubUserTransport{
Credentials: c,
Transport: &github.BasicAuthTransport{
Username: strings.TrimSpace(c.User),
Password: strings.TrimSpace(password),
},
},
}
return tr.Client(), nil
return client, nil
}

// GetUser returns the username for these credentials.
Expand All @@ -62,6 +91,15 @@ func (c *GithubUserCredentials) GetUser() (string, error) {

// GetToken returns the user token.
func (c *GithubUserCredentials) GetToken() (string, error) {
if c.TokenFile != "" {
content, err := os.ReadFile(c.TokenFile)
if err != nil {
return "", fmt.Errorf("failed reading github token file: %w", err)
}

return string(content), nil
}

return c.Token, nil
}

Expand Down
Loading
Loading