Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compute/api/startup-script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ wget $IMAGE_URL
convert * -pointsize 30 -fill white -stroke black -gravity center -annotate +10+40 "$TEXT" output.png

# Create a Google Cloud Storage bucket.
gsutil mb gs://$CS_BUCKET
gcloud storage buckets create gs://$CS_BUCKET
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The gcloud storage buckets create command will fail if the bucket already exists. This can happen if the startup script runs again on an instance that was restarted, or if the bucket was created through other means. To make the script more robust and idempotent, consider checking for the bucket's existence before attempting to create it. The suggestion also includes quoting the variable, which is a shell scripting best practice (as recommended by tools like ShellCheck and style guides like Google's) to prevent word splitting and globbing issues.

Suggested change
gcloud storage buckets create gs://$CS_BUCKET
gcloud storage ls "gs://$CS_BUCKET" &>/dev/null || gcloud storage buckets create "gs://$CS_BUCKET"


# Store the image in the Google Cloud Storage bucket and allow all users
# to read it.
gsutil cp -a public-read output.png gs://$CS_BUCKET/output.png
gcloud storage cp --predefined-acl=public-read output.png gs://$CS_BUCKET/output.png
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's a good practice to quote variables in shell scripts to prevent potential issues with word splitting or globbing. While GCS bucket names have a restricted character set that makes issues unlikely in this specific case, adopting this practice improves script robustness and is a good habit, as recommended by tools like ShellCheck and style guides like Google's.

Suggested change
gcloud storage cp --predefined-acl=public-read output.png gs://$CS_BUCKET/output.png
gcloud storage cp --predefined-acl=public-read output.png "gs://$CS_BUCKET/output.png"