Skip to content
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

The bash script in webhook-github-app ignores the -ws and -we parameters and always attempts to fetch from TF state #3556

Closed
rgrama opened this issue Oct 23, 2023 · 1 comment · Fixed by #3625
Labels
good first issue Good for newcomers

Comments

@rgrama
Copy link

rgrama commented Oct 23, 2023

https://github.com/philips-labs/terraform-aws-github-runner/blob/main/modules/webhook-github-app/bin/update-app.sh#L33-L43

while getopts a:f:k:ws:we:h flag
do
    case "${flag}" in
        a) APP_ID=${OPTARG};;
        f) APP_PRIVATE_KEY_FILE=${OPTARG};;
        k) APP_PRIVATE_KEY_BASE64=${OPTARG};;
        we) WEBHOOK_ENDPOINT=${OPTARG};;
        ws) WEBHOOK_SECRET=${OPTARG};;
        h) usages ;;
    esac
done

If you echo $flag in that while loop you'll see that each of the letters are printed individually which means we and ws will not be set.

an alternative to that while loop:

while [[ "$#" -gt 0 ]]; do
    case "$1" in
        -a) APP_ID="$2"; shift ;;
        -f) APP_PRIVATE_KEY_FILE="$2"; shift ;;
        -k) APP_PRIVATE_KEY_BASE64="$2"; shift ;;
        -ws) WEBHOOK_SECRET="$2"; shift ;;
        -we) WEBHOOK_ENDPOINT="$2"; shift ;;
        -h) usages ;;
        *) echo "Unknown option passed: $1"; exit 1 ;;
    esac
    shift
done
@npalm
Copy link
Member

npalm commented Oct 27, 2023

Thanks for taking time to create an issue, feel free to suggest the change in a PR.

@npalm npalm added the good first issue Good for newcomers label Oct 27, 2023
aniespica pushed a commit to Veevarts/terraform-aws-github-runner that referenced this issue Nov 23, 2023
…bs#3625)

## What
* Change to `webhook-github-app` to use single character options

## Why

The module fails silently, but bash `getopts` does not support
multicharacter options this changes that to use single character options
instead.

## References
* fixes: philips-labs#3556
*
https://stackoverflow.com/questions/3975004/how-to-make-a-multi-character-parameter-in-unix-using-getopt
*
https://unix.stackexchange.com/questions/684717/how-to-use-getopts-in-bash
*
https://stackoverflow.com/questions/402377/using-getopts-to-process-long-and-short-command-line-options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants