Support empty username and password for docker login.

[mauritsvanrees]

For background, see issue #11, but that has a bigger picture than you need to review this PR.

In my testing, the check for an empty USERNAME by doing -z "${USERNAME+x}" always fails: it is never empty. The +x seems wrong to me. A trick like that can be needed when comparing two strings, but not when checking if a single string is empty.

I have tested in a shell. First without the USERNAME variable existing at all:

$ env | grep USERNAME
$ if [ -z "${USERNAME+x}" ] ; then echo "empty"; fi
empty
$ if [ -z "${USERNAME}" ] ; then echo "empty"; fi
empty

Fine.

Now with USERNAME variable defined, and not empty:

$ export USERNAME=me
$ if [ -z "${USERNAME+x}" ] ; then echo "empty"; fi
$ if [ -z "${USERNAME}" ] ; then echo "empty"; fi

Also fine.

Now with USERNAME variable defined, but empty:

$ export USERNAME=
$ if [ -z "${USERNAME+x}" ] ; then echo "empty"; fi
$ if [ -z "${USERNAME}" ] ; then echo "empty"; fi
empty

The check with +x is wrong here.

I have tested my fix on a different branch, where I did some extra changes so I could make my own docker image available. Usage is then like this:

      - name: Deploy to cluster
        uses: mauritsvanrees/docker-stack-deploy@maurits-empty-username-password
        with:
          registry: "ghcr.io"
          # These no longer need to be passed:
          # username: ${{ github.actor }}
          # password: ${{ secrets.DEPLOY_GHCR_READ_TOKEN }}

[davisagli]

I guess it is ok, but I don't really understand what the +x was attempting to do. @ericof?

[mauritsvanrees]

Ah, I should have searched for that in shell documentation. Now I found this:
https://tldp.org/LDP/abs/html/parameter-substitution.html#PARAMALTV

First, x could also be written 123 or anything else, as long as it is a non-empty string.

And then you have either + or :+. And in our case the last one is better.

• "${USERNAME+x}" means: if USERNAME is defined, use x as value. So this is an empty string when USERNAME is not defined at all, otherwise it is x.
• "${USERNAME:+x}" means: if USERNAME is filled, use x as value. So this is an empty string when USERNAME is not defined or if USERNAME is empty.

So with the -z test, which returns true for an empty string:

• [ -z "${USERNAME+x}" ] is true only when USERNAME is not defined at all.
• [ -z "${USERNAME:+x}" ] is true only when USERNAME is not defined or is empty.
• [ -z "${USERNAME}" ] is also true only when USERNAME is not defined or is empty.

And USERNAME is always defined, because action.yml puts USERNAME: ${{ inputs.username }} in the environment, even if inputs.username is empty.

So in our case "${USERNAME+x}" is wrong, but "${USERNAME}" and "${USERNAME:+x}" have the same effect. But let's use the simpler one.

BTW, if we use :+x, the quotes are not even needed, but with the simpler case they are. Otherwise you get an error when USERNAME is defined and has spaces:

$ export USERNAME="Maurits van Rees"
$ if [ -z ${USERNAME:+x} ] ; then echo "empty"; fi
$ if [ -z ${USERNAME} ] ; then echo "empty"; fi
-bash: [: too many arguments