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

Jump to bottom

feat: Rotate Github App Token outside of Atlantis commands #3208

Merged
merged 22 commits into from
Mar 23, 2023
Merged

feat: Rotate Github App Token outside of Atlantis commands #3208

merged 22 commits into from
Mar 23, 2023

Conversation

jonathanwiemers
Copy link
Contributor

@jonathanwiemers jonathanwiemers commented Mar 15, 2023 •
edited
Loading

what

  • move server/events/git_cred_writer.go out of the git clone flow, when using GitHub app authentication
  • implements a token rotator that tries to rotate the token every 30 seconds - it will only rotate the token when it's actually about to expire.
  • I thought about just trying to refresh the token when it's about to expire, (we could get the information from the installation lib (https://github.com/bradleyfalzon/ghinstallation/blob/master/transport.go#L182). But that would need more error handling when it fails. This way it would try again after 30 seconds which might be successful, if it failed before due to temporary problems (eg. network problem / gh incidents / rate limiting).

why

The GitHub app credentials can expire if a PR was open for a long time and no other clone happened in the meantime on Atlantis. Terraform init would then fail to download downstream modules from git.
This PR changes the behaviour of Github app token refreshes to happen outside of the usual Atlantis commands and ensures that the token is always valid.

tests

I have tested my changes by

  • running it locally
  • unit tests
  • running in our infrastructure

references

fixes #3186

@jonathanwiemers jonathanwiemers changed the title first naive implementation Rotate Github App Token outside of Atlantis commands Mar 15, 2023
@jonathanwiemers
Copy link
Contributor Author

@nitrocode Hey could you have a quick look and give some feedback?
Do you think this is a valid way?

@jonathanwiemers jonathanwiemers marked this pull request as ready for review March 16, 2023 10:47
@jonathanwiemers jonathanwiemers requested a review from a team as a code owner March 16, 2023 10:47
@github-actions github-actions bot added go Pull requests that update Go code provider/github labels Mar 16, 2023
krrrr38
krrrr38 approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@krrrr38 krrrr38 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

server/scheduled/executor_service.go
}
}

func (s *ExecutorService) AddJob(jd JobDefinition) {
s.jobs = append(s.jobs, jd)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

@jonathanwiemers
Copy link
Contributor Author

@nitrocode pls review

nitrocode
nitrocode reviewed Mar 18, 2023
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
nitrocode
nitrocode reviewed Mar 18, 2023
View reviewed changes
server/server.go Outdated Show resolved Hide resolved
@nitrocode nitrocode changed the title Rotate Github App Token outside of Atlantis commands feat: Rotate Github App Token outside of Atlantis commands Mar 18, 2023
@nitrocode nitrocode added this to the v0.23.3 milestone Mar 18, 2023
@nitrocode
Copy link
Member

@jonathanwiemers please fix linter errors

@nitrocode nitrocode added the waiting-on-response Waiting for a response from the user label Mar 20, 2023
@nitrocode nitrocode modified the milestones: v0.23.3, v0.23.4 Mar 20, 2023
@jonathanwiemers
Copy link
Contributor Author

@nitrocode fixed it

@nitrocode nitrocode enabled auto-merge (squash) March 23, 2023 00:46
@nitrocode nitrocode removed the waiting-on-response Waiting for a response from the user label Mar 23, 2023
@nitrocode nitrocode merged commit 8b90c8b into runatlantis:main Mar 23, 2023
@jonathanwiemers jonathanwiemers deleted the gh-app-refresh-token-fix branch March 23, 2023 08:06
nitrocode added a commit that referenced this pull request May 5, 2023
@jonathanwiemers @nitrocode @jamengual
feat: Rotate Github App Token outside of Atlantis commands (#3208)
a63a7d4 
Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
@albertorm95 albertorm95 mentioned this pull request May 19, 2023
Merged
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
@jonathanwiemers @nitrocode
@jamengual @ijames-gc
feat: Rotate Github App Token outside of Atlantis commands (runatlant…
6ae1323 
…is#3208)

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
@jonathanwiemers @nitrocode
@jamengual @ijames-gc
feat: Rotate Github App Token outside of Atlantis commands (runatlant…
6e7861d 
…is#3208)

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
@meringu meringu mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JulianSuttner JulianSuttner JulianSuttner approved these changes

@nitrocode nitrocode nitrocode approved these changes

@krrrr38 krrrr38 krrrr38 approved these changes

Assignees
No one assigned
Labels
go Pull requests that update Go code provider/github security
Projects
None yet
Milestone
v0.23.4
Development

Successfully merging this pull request may close these issues.

Occasionally terraform module download error: Could not download module - No such device or address
5 participants
@jonathanwiemers @nitrocode @krrrr38 @JulianSuttner @jamengual