Make DOCKER_USERNAME and _PASSWORD optional #73
Conversation
This comment has been minimized.
This comment has been minimized.
The idea is that by making these fields optional, we support letting the user of the action pre-configure credentials or use already configured credentials instead.
consideRatio
force-pushed
the
pr/make-docker-username-password-optional
branch
from
8e601fa
to
6d3ca73
Compare
There was a problem hiding this comment.
I wrote some notes as all the changes are bundled to a single commit. Ping @yuvipanda who opened #70 that this PR is meant to close.
| MYBINDERORG_TAG: ${{ github.sha }} | ||
|
|
There was a problem hiding this comment.
action.yml declared DOCKER_USERNAME as required, and this test didn't explicitly set DOCKER_USERNAME but instead relied on a script to populate the INPUT_DOCKER_USERNAME variable with the github actor.
I think it is cleaner to do it like this though. I was clueless about the scripts logic as it seemed like something that shouldn't be able to happen until I saw the GitHub workflow CI failure.
README.md
Outdated
| - **`DOCKER_USERNAME`**: | ||
| description: Docker registry username. If not supplied, credentials must be setup ahead of time. | ||
| - **`DOCKER_PASSWORD`**: | ||
| description: Docker registry password or [access token (recommended)](https://docs.docker.com/docker-hub/access-tokens/). If not supplied, credentials must be setup ahead of time. | ||
| - **`DOCKER_REGISTRY`**: | ||
| description: name of the docker registry. If not supplied, this defaults to [DockerHub](https://hub.docker.com/) | ||
| - **`IMAGE_NAME`**: | ||
| name of the image. Example - myusername/myContainer. If not supplied, this defaults to `<DOCKER_USERNAME/GITHUB_REPOSITORY_NAME>`. |
There was a problem hiding this comment.
I moved DOCKER_USERNAME and PASSWORD from the mandatory section to the optional section. The other inputs were relocated to have the same order as in the action.yml file.
| # Set Docker username to the actor name not provided | ||
| if [ -z "$INPUT_DOCKER_USERNAME" ]; then | ||
| INPUT_DOCKER_USERNAME="$GITHUB_ACTOR" | ||
| fi | ||
|
|
There was a problem hiding this comment.
This logic was only applied during a CI test as DOCKER_USERNAME was declared required in action.yml. I removed this section in favor of making the CI test explicitly set DOCKER_USERNAME to ${{ github.actor }} instead as discussed in a comment above.
|
LGTM |
Co-authored-by: Simon Li <orpheus+devel@gmail.com>
|
Congrats on your first merged pull request in this project! 🎉 |
|
Thanks a lot, @consideRatio and @manics!!! |
| # Set image name to username/repo_name if not provided | ||
| if [ -z "$INPUT_IMAGE_NAME" ]; then | ||
| if [[ -z "$INPUT_DOCKER_USERNAME" ]]; then |
There was a problem hiding this comment.
I think this is causing #74. If IMAGE_NAME is not set but NO_PUSH is set, this is all ok...
The idea is that by making these fields optional, we support letting the
user of the action pre-configure credentials or use already configured
credentials instead.
Closes #70