Fix behavior regression when no decryption needed #12
Conversation
configure
Outdated
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/bash | |||
|
|
|||
| MAGE_KEY= | |||
There was a problem hiding this comment.
This is needed due to set nounset below making the script fail if there is no definition of MAGE_KEY
There was a problem hiding this comment.
umm this is going to create the MAGE_KEY variable as always empty unless I'm missing something. Either we define the default like MAGE_KEY=${MAGE_KEY:-} to preserve possible env variables values or we remove the nounset options which is quite weird to use nowadays in bash due to some controversy: https://mywiki.wooledge.org/BashFAQ/112
There was a problem hiding this comment.
I'm happy to remove it, WDYT @nuclearsandwich since that has been there since the first creation of the script.
There was a problem hiding this comment.
These scripts get enough attention that I don't think we need it. If we get the POSIX sh PRs in I think it will make even more sense to remove since a lot of the techniques for mitigating nounset usage are Bash-specific or harder in POSIX sh.
configure
Outdated
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/bash | |||
|
|
|||
| MAGE_KEY= | |||
There was a problem hiding this comment.
umm this is going to create the MAGE_KEY variable as always empty unless I'm missing something. Either we define the default like MAGE_KEY=${MAGE_KEY:-} to preserve possible env variables values or we remove the nounset options which is quite weird to use nowadays in bash due to some controversy: https://mywiki.wooledge.org/BashFAQ/112
Co-authored-by: Steven! Ragnarök <[email protected]>
configure
Outdated
| @@ -77,15 +76,16 @@ freshen_chef_repo | |||
| pushd chef_repo | |||
| wd="$(pwd)" | |||
|
|
|||
| if test \( -n "$CHEF_MAGE_PROFILE" -a test -z "$MAGE_KEY" \) -o \( test -z "$CHEF_MAGE_PROFILE" -a test -n "$MAGE_KEY" \); then | |||
There was a problem hiding this comment.
| if test \( -n "$CHEF_MAGE_PROFILE" -a test -z "$MAGE_KEY" \) -o \( test -z "$CHEF_MAGE_PROFILE" -a test -n "$MAGE_KEY" \); then | |
| if test \( -n "$CHEF_MAGE_PROFILE" -a -z "$MAGE_KEY" \) -o \( -z "$CHEF_MAGE_PROFILE" -a -n "$MAGE_KEY" \); then |
I don't recall if these were there in my original suggestion but they are spurious.
This should be the thing which might also work in bash where the other thing doesn't.
If not, just switch to [[ ]] and we'll port it when we finish the posix switch.
There was a problem hiding this comment.
This indeed work. Tested the script with these changes and see that behavior is restored if no MAGE variable is enabled and faults if MAGE is present
Co-authored-by: Steven! Ragnarök <[email protected]>
By adding the integration to Mage tool for decryption a behavior regression was introduced where it would fail if no MAGE related variables where configured.
This was first addressed by @j-rivero on #11 by adding a flag to bypass this.
This PR removes that option and restores the behavior to be: