Skip to content

feat(deploy): unify backend and UI deployment under deploy.sh - updated README#106

Draft
christophervoelpel wants to merge 17 commits into
google-marketing-solutions:mainfrom
christophervoelpel:combined-deployment-script
Draft

feat(deploy): unify backend and UI deployment under deploy.sh - updated README#106
christophervoelpel wants to merge 17 commits into
google-marketing-solutions:mainfrom
christophervoelpel:combined-deployment-script

Conversation

@christophervoelpel

Copy link
Copy Markdown
Contributor
  1. Refactored argument parsing and added targeted deployment flags ( --backend-only , --ui-only ).
  2. Optimized API enablement with an API cache comparison to speed up subsequent runs.
  3. Made pre-flight tool requirements component-aware so they only check for what is actually needed.
  4. Deprecated deploy-ui.sh and redirected all UI flows through the unified deploy.sh .
  5. Added startup pre-flight checks to ensure the Firebase CLI session is actively authenticated.
  6. Optimized GCS/Firebase linkage checks to avoid timeouts on accounts with 1,500+ active projects.
  7. Restored direct Firestore API curl loops for database configurations upload.
  8. Added environment override capability for CLOUD_RUN_URL .
  9. Enforced OAuth Consent Screen setup via a blocking 15-second polling loop.
  10. Enforced App Engine IAP configuration via a blocking 15-second polling loop.

@gps-readability-bot

Copy link
Copy Markdown

Readability review is not required.

Comment thread deploy-ui.sh Outdated
echo
echo "════════════════════════════════════════════════════════════════════════"
echo
echo "WARNING: deploy-ui.sh is deprecated. Redirecting to './deploy.sh --ui-only'..."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove deploy-ui.sh, no need to support it in the future

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1. This seems more confusing than helpful to new users.

Comment thread deploy.sh Outdated
echo " the setup dialog. User Type is set under 'Audience' — pick"
echo " 'Internal' if you have a Workspace org; otherwise 'External' and"
echo " add yourself as a test user."
echo " the setup dialog. Set User Type to 'Internal' for @google.com logins."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why @google.com?
Proposed: "In the OAuth setup dialog, set User Type to 'Internal' if you only want users within your own Google Workspace organization to access the app (recommended for testing), or 'External' if you want anyone to be able to log in."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd just say that internal is recommended for security purposes when possible.

Comment thread README.md Outdated
* **Deploy UI Only**: `./deploy.sh --ui-only`
* **Local Development Build**: `./deploy.sh --ui-only --local` (or `./deploy.sh --ui-only local`) to build the Angular application files locally without triggering an App Engine deployment.

To run the script in a non-interactive/headless environment (such as an AI agent like Jetski or Claude), use the `-y` / `--yes` / `--non-interactive` flag to auto-confirm target configuration prompts:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a bit confusing to have too many flags with the same functionality, I would keep only standard -y & --yes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as above, I would go with --non-interactive, as it's the most clear.

Comment thread deploy.sh Outdated
AUTO_CONFIRM=true
shift
;;
--backend-only|--be-only)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's leave only one --be-only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1. I'd say --backend-only is better, as it's more descriptive and clear.

Comment thread deploy.sh
TARGETED=false
DEPLOY_MODE=""

while [[ $# -gt 0 ]]; do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's have flags -h & --help for help (to output all the available options)?

@adamread adamread left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have some comments on the flags and some minor concerns about the polling during a non-interactive deployment.

Comment thread deploy-ui.sh Outdated
echo
echo "════════════════════════════════════════════════════════════════════════"
echo
echo "WARNING: deploy-ui.sh is deprecated. Redirecting to './deploy.sh --ui-only'..."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1. This seems more confusing than helpful to new users.

Comment thread deploy.sh Outdated
AUTO_CONFIRM=true
shift
;;
--backend-only|--be-only)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1. I'd say --backend-only is better, as it's more descriptive and clear.

Comment thread deploy.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Google tools generally use lower_snake_case for flags. Should we do this for consistency?

Comment thread deploy.sh Outdated

while [[ $# -gt 0 ]]; do
case "$1" in
-y|--yes|--non-interactive)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think just having --non_interactive would suffice, as it's more descriptive and clear.

Comment thread deploy.sh
TARGETED=true
shift
;;
local|--local)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would stick to just the flag version (i.e. --local) for consistency.

Comment thread deploy.sh
echo "Enabling Firebase Management API..."
gcloud services enable firebase.googleapis.com --project=$PROJECT
# Refresh cache
ENABLED_APIS=$(gcloud services list --enabled --project="$PROJECT" --format="value(config.name)" 2>/dev/null || true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Comment thread deploy.sh Outdated
echo " the setup dialog. User Type is set under 'Audience' — pick"
echo " 'Internal' if you have a Workspace org; otherwise 'External' and"
echo " add yourself as a test user."
echo " the setup dialog. Set User Type to 'Internal' for @google.com logins."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd just say that internal is recommended for security purposes when possible.

Comment thread deploy.sh
else
echo "Enabling Identity Toolkit API (needed for Auth config)..."
gcloud services enable identitytoolkit.googleapis.com --project=$PROJECT
ENABLED_APIS=$(gcloud services list --enabled --project="$PROJECT" --format="value(config.name)" 2>/dev/null || true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also not required. It's not used again in the script.

Comment thread deploy.sh
echo "[>] Checking if bucket ${GCS_BUCKET} is linked to Firebase Storage..."
BUCKET_WAIT_ATTEMPTS=0
BUCKET_WAIT_MAX=120 # 120 × 15s = 30 min total
while true; do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about this (and the polling further down) from the automated deployment perspective. If this isn't being run in an interactive session, someone may eventually see this in the logs after the deployment fails.

I think fast fail when AUTO_CONFIRM is true might be a good idea. That way we don't block CI pipelines for 30 min if someone messed up the initial deployment.

We should also add a note in the readme that an automated deployment cannot be done for the first time.

Comment thread README.md Outdated
* **Deploy UI Only**: `./deploy.sh --ui-only`
* **Local Development Build**: `./deploy.sh --ui-only --local` (or `./deploy.sh --ui-only local`) to build the Angular application files locally without triggering an App Engine deployment.

To run the script in a non-interactive/headless environment (such as an AI agent like Jetski or Claude), use the `-y` / `--yes` / `--non-interactive` flag to auto-confirm target configuration prompts:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as above, I would go with --non-interactive, as it's the most clear.

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