Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Add CI/CD workflows for terraform automation #183
base: main
Are you sure you want to change the base?
Add CI/CD workflows for terraform automation #183
Changes from all commits
55c06e4
99a5ac2
4a6c180
546685a
2d35eef
2fe0c94
d865c7c
174064d
6ea4586
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem to be used, why setting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The env variable needs to be set when calling github api, otherwise an error is thrown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What and where calls those APIs? I don't see any usage of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GITHUB_TOKEN is needed in CI or bash cli for API calls. The API is called with curl via https://github.com/subspace/infra/pull/183/files/99a5ac25712fdda531fd88026a12f96aafeacb83#diff-b46b68b6df852ad5f8fc96162f55c2fe198f6d53eb2fe7d7ce12fcd4b2650ba6R39-R43
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you linked doesn't use
GITHUB_TOKEN
environment variable, it uses$token
that is obtained in a different way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nazar-pc removed it, here you go:
https://github.com/subspace/infra/actions/runs/6484473796/job/17608384650#step:5:24
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so it is not for
curl
, it was forgh
CLI command, now it makes sense.With that knowledge we understand why it is necessary. I think what we should do now in a secure way without using heavy tools like setting repo (I'm fairly certain simply setting environment variable wasn't) without using heavy tools like setting repo's secrets from workflow is to use outputs and mask them such that they are not visible in logs: https://github.com/orgs/community/discussions/25225#discussioncomment-3246942
That is the goal here: to pass the token from one step into another. What we had in earlier versions of this PR are various suboptimal/incorrect ways of achieving that ultimate goal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, github cli, I should have been more clear about that "The GITHUB_TOKEN is needed in CI or bash cli for API calls" but couldn't remember exactly since it was a while ago. That being said the solution of masking the secret and just passing it is nice if using the ephemeral runners maybe but this solution and workflow is for the dedicated runners, where i need to retain the secret so I can unregister and delete runner if need be and remove it from github. See https://docs.github.com/en/free-pro-team@latest/rest/actions/self-hosted-runners?apiVersion=2022-11-28#delete-a-self-hosted-runner-from-an-organization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you mentioned that tokens only live for 1 hour, so you'll have to retrieve fresh token anyway. Why retaining it then at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The token is not needed to deregister, a force delete can be done. So i've used the masking technique.