-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[Github Actions] Rename docker secrets and parameterize docker user #13297
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
Conversation
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.
Looks good, one question on token generation for acceptance_test_kube_gke
@@ -44,7 +44,7 @@ function findAndDeleteTag () { | |||
} | |||
|
|||
function cleanUpImages () { | |||
TOKEN=$(curl --request POST 'https://hub.docker.com/v2/users/login/' --header 'Content-Type: application/json' --data-raw '{"username":"airbytebot","password":"'$DOCKER_PASSWORD'"}' | jq '.token') | |||
TOKEN=$(curl --request POST 'https://hub.docker.com/v2/users/login/' --header 'Content-Type: application/json' --data-raw '{"username":"'$DOCKER_HUB_USERNAME'","password":"'$DOCKER_HUB_PASSWORD'"}' | jq '.token') |
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.
hmm - this one is also grabbing a token through curl, do we need to set +x
set -x
here as well?
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.
I don't think so, because no where in the file do we turn that on (set +x)?
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 maybe we should?
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.
Ah! I see, it was specifically enabled on manage.sh
- should be ok then
Co-authored-by: Pedro S. Lopez <[email protected]>
…irbytehq#13297) * Rename docker secrets and parameterize docker user * Apply suggestions from code review Co-authored-by: Pedro S. Lopez <[email protected]> Co-authored-by: Pedro S. Lopez <[email protected]>
Related to https://github.com/airbytehq/airbyte-cloud/issues/1763
This PR renames the secrets so that old branches stop working and folks will need to rebase from master to publish from here on out. I also require a secret for the docker username.
Note: DO NOT update the secrets until this PR is merged to master!