-
Notifications
You must be signed in to change notification settings - Fork 29
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
Make DOCKER_USERNAME and _PASSWORD optional #73
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,5 +39,5 @@ jobs: | |
uses: ./ | ||
with: | ||
NO_PUSH: true | ||
DOCKER_USERNAME: ${{ github.actor }} | ||
MYBINDERORG_TAG: ${{ github.sha }} | ||
|
||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,22 +18,20 @@ if [ "$INPUT_APPENDIX_FILE" ]; then | |
echo "Appendix read from $INPUT_APPENDIX_FILE:\n$APPENDIX" | ||
fi | ||
|
||
if [ -z "$INPUT_NO_PUSH" ]; then | ||
check_env "INPUT_DOCKER_USERNAME" | ||
check_env "INPUT_DOCKER_PASSWORD" | ||
# Login to Docker registry | ||
# Login to Docker registry if about to push and credentials are passed | ||
if [[ -z "$INPUT_NO_PUSH" && -n "$INPUT_DOCKER_PASSWORD" && -n "$INPUT_DOCKER_USERNAME" ]]; then | ||
echo ${INPUT_DOCKER_PASSWORD} | docker login $INPUT_DOCKER_REGISTRY -u ${INPUT_DOCKER_USERNAME} --password-stdin | ||
fi | ||
|
||
REPO_NAME=`echo $GITHUB_REPOSITORY | cut -d "/" -f 2` | ||
|
||
# Set Docker username to the actor name not provided | ||
if [ -z "$INPUT_DOCKER_USERNAME" ]; then | ||
INPUT_DOCKER_USERNAME="$GITHUB_ACTOR" | ||
fi | ||
|
||
Comment on lines
-30
to
-34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
# 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is causing #74. If IMAGE_NAME is not set but |
||
echo "IMAGE_NAME must be explicitly set when DOCKER_USERNAME isn't set. Exiting..." | ||
exit 1 | ||
fi | ||
|
||
INPUT_IMAGE_NAME="$INPUT_DOCKER_USERNAME/$REPO_NAME" | ||
# Lower-case | ||
INPUT_IMAGE_NAME="${INPUT_IMAGE_NAME,,}" | ||
|
@@ -45,7 +43,6 @@ if [ "$INPUT_DOCKER_REGISTRY" ]; then | |
fi | ||
|
||
# Set username | ||
|
||
if [ -z "$INPUT_NOTEBOOK_USER" ] || [ "$INPUT_MYBINDERORG_TAG" ] || [ "$INPUT_BINDER_CACHE" ]; | ||
then | ||
NB_USER="jovyan" | ||
|
@@ -55,7 +52,6 @@ if [ -z "$INPUT_NOTEBOOK_USER" ] || [ "$INPUT_MYBINDERORG_TAG" ] || [ "$INPUT_BI | |
fi | ||
|
||
# Set REPO_DIR | ||
|
||
if [ -z "$INPUT_REPO_DIR" ]; | ||
then | ||
REPO_DIR="/home/${NB_USER}" | ||
|
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.
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.