chore: add quotes to variables in shell scripts #340
Conversation
moorec-aws
force-pushed
the
moorec/unqouted-variables
branch
from
3e8d9b7 to
21b72de
Compare
moorec-aws
changed the title
scripts/add_queue_env.sh
Outdated
| @@ -1,7 +1,7 @@ | |||
| #!/bin/bash | |||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
|
|
|||
| export ENV_JSON=$(dirname $0)/queue-env.json | |||
| export ENV_JSON=$(dirname "$0")/queue-env.jso | |||
There was a problem hiding this comment.
| export ENV_JSON=$(dirname "$0")/queue-env.jso | |
| export ENV_JSON=$(dirname "$0")/queue-env.json |
scripts/run_posix_docker.sh
Outdated
| -v "$(pwd)":/code:ro \ | ||
| -v "${HOME}"/.aws:/aws \ | ||
| --env-file "${TMP_ENV_FILE}" \ | ||
| "${ARGS}" \ |
There was a problem hiding this comment.
| "${ARGS}" \ | |
| ${ARGS} \ |
I don't think that this one should be quotes. ARGS is built-up in this script and contains multiple command-line arguments appended together. Quoting this turns that list of args into a single argument.
| sudo -u agentuser -i "${ENVS}" /home/agentuser/run_agent.sh |
There was a problem hiding this comment.
| sudo -u agentuser -i "${ENVS}" /home/agentuser/run_agent.sh | |
| sudo -u agentuser -i ${ENVS} /home/agentuser/run_agent.sh |
ENVS is built-up in this script by appending multiple env var definitions together. This needs to be interpreted as multiple args to the shell, so it cannot be quoted.
moorec-aws
force-pushed
the
moorec/unqouted-variables
branch
2 times, most recently
from
e5a5694 to
e184024
Compare
scripts/run_posix_docker.sh
Outdated
| -v $(pwd):/code:ro \ | ||
| -v ${HOME}/.aws:/aws \ | ||
| --env-file ${TMP_ENV_FILE} \ | ||
| --name test_worker_agentss \ |
There was a problem hiding this comment.
This looks like an unintended change, right?
This container name is referenced in a few other places:
deadline-cloud-worker-agent/DEVELOPMENT.md
Line 128 in 9367c10
There was a problem hiding this comment.
Yes, that was unintended. Thanks.
There was a problem hiding this comment.
Looks like there is still an unintended change from test_worker_agent → test_worker_agents. We should restore the original name (test_worker_agent)
moorec-aws
force-pushed
the
moorec/unqouted-variables
branch
from
e184024 to
70d90b1
Compare
Signed-off-by: Charles Moore <[email protected]>
moorec-aws
force-pushed
the
moorec/unqouted-variables
branch
from
70d90b1 to
6e0550b
Compare
| -v $(pwd):/code:ro \ | ||
| -v ${HOME}/.aws:/aws \ | ||
| --env-file ${TMP_ENV_FILE} \ | ||
| --name test_worker_agents \ |
There was a problem hiding this comment.
This is still breaks the docker workflows I think. This should now have changed and should actually be test_worker_agent
scripts/run_posix_docker.sh
Outdated
| -v $(pwd):/code:ro \ | ||
| -v ${HOME}/.aws:/aws \ | ||
| --env-file ${TMP_ENV_FILE} \ | ||
| --name test_worker_agentss \ |
There was a problem hiding this comment.
Looks like there is still an unintended change from test_worker_agent → test_worker_agents. We should restore the original name (test_worker_agent)
|
What was the problem/requirement? (What/Why)
We have a bunch of unquoted variables in our shell scripts
What was the solution? (How)
Add the quotes
What is the impact of this change?
n/a
How was this change tested?
hatch run testWas this change documented?
No
Is this a breaking change?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.