Fix parameter expansion on check-env-var-param command

[joaopluigi]

Checklist

• [x] All new jobs, commands, executors, parameters have descriptions
• [x] Examples have been added for any significant new features
• [x] README has been updated, if necessary

Motivation, issues

The check-env-var-param command was not working as expected.

Description

In order to evaluate whether a string is defined as a variable, you'll need to use an indirect shell parameter expansion:

The basic form of parameter expansion is ${parameter}. The value of parameter is substituted.

If the first character of parameter is an exclamation point (!), it introduces a level of variable indirection. Bash uses the value of the variable formed from the rest of parameter as the name of the variable; this variable is then expanded and that value is used in the rest of the substitution, rather than the value of parameter itself.

[KyleTryon]

Indirect Parameter Expansion is new to me: https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

Can you help us (or maybe just me 😅) understand, what is it you expect to happen? What is currently happening? What is the output of your modification?

Thank you, much appreciated!

[joaopluigi]

Oops, I'm sorry for the poor description. I have this problem of not being clear on what I'm trying to explain, but I'm working on that hahaha.

The way check-env-var-param.yml is implemented is not checking if the variable on the string inside params is defined. It actually expands ${i} to the STRING itself; consequently, the condition will always be true. I'll give you an example:

IFS="," read -ra PARAMS <<< "VAR_1,VAR_2,VAR_3"

set -x
for i in "${PARAMS[@]}"; do
    if [[ -z "${i}" ]]; then
        echo "ERROR: Missing environment variable {i}" >&2
    else
        echo "Yes, ${i} is defined!"
    fi
done

If you run this script (note that I'm not defining any variable), you can see that the if..else is expanding to [[ -z VAR_1 ]], which will make every condition be true, and not to [[ -z ${VAR_1}]].

+ IFS=,
+ read -ra PARAMS
+ for i in '"${PARAMS[@]}"'
+ [[ -z VAR_1 ]]
+ echo 'Yes, VAR_1 is defined!'
Yes, VAR_1 is defined!
+ for i in '"${PARAMS[@]}"'
+ [[ -z VAR_2 ]]
+ echo 'Yes, VAR_2 is defined!'
Yes, VAR_2 is defined!
+ for i in '"${PARAMS[@]}"'
+ [[ -z VAR_3 ]]
+ echo 'Yes, VAR_3 is defined!'
Yes, VAR_3 is defined!

Now, lets add the exclamation point (!).

IFS="," read -ra PARAMS <<< "VAR_1,VAR_2,VAR_3"

set -x
for i in "${PARAMS[@]}"; do
    if [[ -z "${!i}" ]]; then
        echo "ERROR: Missing environment variable {i}" >&2
    else
        echo "Yes, ${i} is defined!"
    fi
done

Note that ${VAR_X} is correctly expanded to '', because it's not defined.

+ for i in '"${PARAMS[@]}"'
+ [[ -z '' ]]
+ echo 'ERROR: Missing environment variable {i}'
ERROR: Missing environment variable {i}
+ for i in '"${PARAMS[@]}"'
+ [[ -z '' ]]
+ echo 'ERROR: Missing environment variable {i}'
ERROR: Missing environment variable {i}
+ for i in '"${PARAMS[@]}"'
+ [[ -z '' ]]
+ echo 'ERROR: Missing environment variable {i}'
ERROR: Missing environment variable {i}

I'll now define ${VAR_1} and run again:

+ VAR_1=test
+ for i in '"${PARAMS[@]}"'
+ [[ -z test ]]
+ echo 'Yes, VAR_1 is defined!'
Yes, VAR_1 is defined!
+ for i in '"${PARAMS[@]}"'
+ [[ -z '' ]]
+ echo 'ERROR: Missing environment variable {i}'
ERROR: Missing environment variable {i}
+ for i in '"${PARAMS[@]}"'
+ [[ -z '' ]]
+ echo 'ERROR: Missing environment variable {i}'
ERROR: Missing environment variable {i}

Note that the first if...else was expanded to [[ -z test ]] with the correct value of ${VAR_1}

ANOTHER POINT

This should probably be changed from:

echo "ERROR: Missing environment variable {i}" >&2

To:

echo "ERROR: Missing environment variable ${i}" >&2

[joaopluigi]

@KyleTryon

[KyleTryon]

@KyleTryon

Hey thank you for that amazing explanation. Sorry it took so long to get enough time to dive in on this. Fantastic work you have here and thank you for it :pray:. I wish this was stack overflow so I could give you some form of points.