Adding GitHub token into all env files #2863
Conversation
TDDR
added
type: enhancement
|
|
tools/autodeployment/deploy.sh
Outdated
| @@ -34,7 +34,7 @@ fi | |||
| echo "GIT_COMMIT=$2" >> $ENV_FILE | |||
|
|
|||
| echo "Building $ENV Container" | |||
| docker-compose --env-file $ENV_FILE --project-name=$ENV build | |||
| GITHUB_TOKEN={$GITHUB_TOKEN} docker-compose --env-file $ENV_FILE --project-name=$ENV build | |||
There was a problem hiding this comment.
is there anyway we can see the value of GITHUB_TOKEN here?
There was a problem hiding this comment.
It would have to be set outside the script. @manekenpix can speak to what's possible in the env of the build machines.
There was a problem hiding this comment.
I think it should be set in here so the autodeployment server can see it, and then passed into the script.
There was a problem hiding this comment.
Oh, you already did, the only step left is passing it to the script here
There was a problem hiding this comment.
Not sure if it belongs in this PR, but we could also document this change in our docs
|
I made some changes. Still not sure how to test this out? |
|
@TDDR Ping me on slack for testing your PR on |
|
I somehow deleted the previous two commits I had originally shared when I did a |
There was a problem hiding this comment.
LGTM 👍
We'll have to update the docs to reflect these changes, here or in a different PR.
|
@TDDR, let's rebase this and ping the Sheriffs after |
TDDR
dismissed stale reviews from Kevan-Y, humphd, manekenpix, and tcvan0707
via
a789d2d
|
Nice |
Issue This PR Addresses
Fixes #2799
Description
This is my attempt at getting the ball rolling for issue #2799.
From what I can tell, adding the
GITHUB_TOKEN=to theenv.examplewill auto-populate the GitHub token.I then expose that token in the Docker-compose by adding it to the top level
environmentThe thing I am least certain of is my attempt to use the token in the
deploy.shfile. I am fairly sure that is wrong, but I'm not sure what exactly needs to happen, or how to test it.Steps to test the PR
@manekenpix had to take down staging in order to test this.
Checklist