[Tools] Add scripts to facilitate 1-click deployment of PCUI in personal environment #291
Conversation
scripts/deploy.sh
Outdated
| # Usage: ./scripts/deploy.sh [ENVIRONMENT] | ||
| # Example: ./scripts/deploy.sh demo | ||
|
|
||
| CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" |
There was a problem hiding this comment.
minor: extra space after $( and before )
|
|
||
| info "Selected environment: $ENVIRONMENT" | ||
|
|
||
| source "$INFRASTRUCTURE_DIR/environments/$ENVIRONMENT-variables.sh" |
There was a problem hiding this comment.
minor: it's useful to use {} when variables are used to compose a string
There was a problem hiding this comment.
AFAIK it's useful only when you're putting variables isde by side, e.g. "$VAR_1$VAR_2" => "${VAR_1}${VAR_2}". In this case what's the advantage?
There was a problem hiding this comment.
In your case there are no differences because after $ENVIRONMENT you have a -, but in general it's worth using curly braces. Let's think to this simple example:
$ test=one
$ echo "$test1"
$ echo "${test}1"
one1
The result of the first echo is an empty string, because $test1 would expand the variable identified by test1 rather than test. This kind of issues are hard to discover, so as a best practice is better to use curly braces everywhere.
scripts/deploy.sh
Outdated
| --logical-resource-id InfrastructureBucket \ | ||
| --output json \ | ||
| --query 'StackResources[0].PhysicalResourceId'\ | ||
| | tr -d '"' ) |
There was a problem hiding this comment.
info: tr -d '"' can be replaced with xargs
There was a problem hiding this comment.
is there any advantage of using xargs instead of tr?
There was a problem hiding this comment.
you know what:i think in this case none of the two are required, but simply use "--output text"
| if [[ -n $INFRA_BUCKET_NAME ]]; then | ||
| BUCKET=$INFRA_BUCKET_NAME | ||
| else | ||
| BUCKET=$(aws cloudformation describe-stack-resources \ |
There was a problem hiding this comment.
useful to add region parameter in every aws call.
|
|
||
| CLI_INPUT_YAML=$(sed "s#BUCKET_URL_PLACEHOLDER#$BUCKET_URL#g" "$CFN_CLI_INPUT_YAML_FILE") | ||
|
|
||
| AWS_PAGER="cat" aws cloudformation $CFN_DEPLOY_COMMAND \ |
There was a problem hiding this comment.
do we need to pass cat or can we use AWS_PAGER=""?
There was a problem hiding this comment.
I suggest to keep cat at least for the time being because I took that from a script used to update the demo environment from github.
The idea is to have a unique 1click deployment script that works for both personal env and demo env, so I suggest to keep it as is for the time being.
…nd script 'setup-env.sh' to create config files for personal environments.
gmarciani
force-pushed
the
wip/mgiacomo/2023120/scripts-one-click-deploy
branch
from
f2e7ba5
to
7a2c21f
Compare
Changes
Command to create files for the personal environment (this must be done just the first time):
Command to deploy (create or update) PCUI into the environment:
Example of execution:
How Has This Been Tested?
Used both script to deploy PCUI in personal account.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.