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

Add shellchecking of the shell scripts #179

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

yarikoptic
Copy link
Member

This would fail now and fixes are needed (assigning to @asmacdo)

❯ shellcheckit doit
cleanup.sh:#!/bin/bash
install.sh:#!/bin/bash
scripts/account-enforcer.sh:#!/bin/bash
scripts/ensure-vars.sh:#!/bin/bash
gh-yarikoptic

In cleanup.sh line 5:
#!/bin/bash
^-- SC1128 (error): The shebang must be on the first line. Delete blanks and move comments.


In cleanup.sh line 34:
terraform workspace select $ENV
                           ^--^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
terraform workspace select "$ENV"


In cleanup.sh line 56:
    kubectl get namespace $ns -o json | sed 's/"kubernetes"//' | kubectl replace --raw "/api/v1/namespaces/$ns/finalize" -f -
                          ^-^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
    kubectl get namespace "$ns" -o json | sed 's/"kubernetes"//' | kubectl replace --raw "/api/v1/namespaces/$ns/finalize" -f -


In install.sh line 21:
./scripts/account-enforcer.sh $ENV
                              ^--^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
./scripts/account-enforcer.sh "$ENV"


In install.sh line 27:
BACKEND_FILE="$ENV_DIR/backend.tf"
^----------^ SC2034 (warning): BACKEND_FILE appears unused. Verify use (or export if used externally).


In install.sh line 41:
./scripts/merge_config.py $BASE_CONFIG $ENV_OVERRIDE $OUTPUT
                                       ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                     ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
./scripts/merge_config.py $BASE_CONFIG "$ENV_OVERRIDE" "$OUTPUT"


In install.sh line 44:
if [ $? -ne 0 ]; then
     ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.


In install.sh line 64:
terraform workspace select -or-create $ENV
                                      ^--^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
terraform workspace select -or-create "$ENV"


In install.sh line 99:
$(terraform output -raw configure_kubectl)
^-- SC2091 (warning): Remove surrounding $() to avoid executing output (or use eval if intentional).


In scripts/account-enforcer.sh line 12:
  echo "Environments: ${!ENV_TO_PROFILE[@]}"
                      ^-------------------^ SC2145 (error): Argument mixes string and array. Use * or separate argument.


In scripts/account-enforcer.sh line 21:
  echo "Valid environments: ${!ENV_TO_PROFILE[@]}"
                            ^-------------------^ SC2145 (error): Argument mixes string and array. Use * or separate argument.

For more information:
  https://www.shellcheck.net/wiki/SC1128 -- The shebang must be on the first ...
  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
  https://www.shellcheck.net/wiki/SC2034 -- BACKEND_FILE appears unused. Veri...

@kabilar
Copy link
Member

kabilar commented Mar 3, 2025

Hi @yarikoptic, is this workflow still a priority? Thanks.

@asmacdo
Copy link
Member

asmacdo commented Mar 3, 2025

Out to be pretty simple (and helpful)

@yarikoptic
Copy link
Member Author

yes, having a second look at it - seems to be trivial, like adding "" around most of them etc. I think less than an hour if even that

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "shellcheckit autopatch",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Member Author

ok, spent ... 7 minutes, rebased, addressed most. Here is some left:

❯ shellcheckit doit
.github/scripts/cleanup-ec2.sh:#!/usr/bin/env bash
.github/scripts/launch-ec2.sh:#!/usr/bin/env bash
cleanup.sh:#!/bin/bash
install.sh:#!/bin/bash
scripts/account-enforcer.sh:#!/bin/bash
scripts/ensure-vars.sh:#!/bin/bash

In .github/scripts/cleanup-ec2.sh line 9:
  source "$ENV_FILE"
         ^---------^ SC1090 (warning): ShellCheck can't follow non-constant source. Use a directive to specify location.


In .github/scripts/launch-ec2.sh line 39:
export INSTANCE_ID=$(aws ec2 run-instances \
       ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In .github/scripts/launch-ec2.sh line 63:
export ALLOC_ID=$(aws ec2 allocate-address \
       ^------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In .github/scripts/launch-ec2.sh line 77:
export EIP_ASSOC=$(aws ec2 associate-address \
       ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In .github/scripts/launch-ec2.sh line 89:
export PUBLIC_IP=$(aws ec2 describe-addresses \
       ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.

For more information:
  https://www.shellcheck.net/wiki/SC1090 -- ShellCheck can't follow non-const...
  https://www.shellcheck.net/wiki/SC2155 -- Declare and assign separately to ...

so all uniform but one to likely add pragma to ignore. Please finish it up @asmacdo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants