Skip to content

add Vue service to DNS load balancer config#1425

Open
serglom21 wants to merge 1 commit intomasterfrom
dns-vue-service
Open

add Vue service to DNS load balancer config#1425
serglom21 wants to merge 1 commit intomasterfrom
dns-vue-service

Conversation

@serglom21
Copy link
Copy Markdown
Collaborator

@serglom21 serglom21 commented Apr 10, 2026

Description

Adding Vue domain to load balancer config as part of Vue Demo Refresh (#994 )

@serglom21 serglom21 requested a review from maxkosty April 10, 2026 00:38
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
empower Ignored Ignored Apr 10, 2026 0:39am

Request Review

@serglom21 serglom21 requested a review from ndmanvar April 10, 2026 00:39
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

Comment on lines +52 to +53
- host: "${VUE_SUBDOMAIN}.${DOMAIN}"
gae_service: ${VUE_APP_ENGINE_SERVICE}
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.

Bug: The DNS configuration template references the variable VUE_SUBDOMAIN, which is not defined in any environment files, causing the deployment script to fail.
Severity: CRITICAL

Suggested Fix

Define the VUE_SUBDOMAIN variable in the relevant environment configuration files, such as production.env and staging.env. Follow the existing pattern for other services, for example, by adding VUE_SUBDOMAIN=vue to production.env and VUE_SUBDOMAIN=staging-vue to staging.env.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: _dns/config.yaml.template#L52-L53

Potential issue: The new DNS configuration in `_dns/config.yaml.template` references the
variable `${VUE_SUBDOMAIN}`. However, this variable is not defined in any environment
configuration files (e.g., `production.env`, `staging.env`). The deployment script
utilizes `envsubst.sh` with a strict mode (`--strict-allow-empty`) that explicitly fails
when a variable is undefined. As a result, attempting to deploy the `_dns` project will
cause the deployment script to terminate with an error, preventing the entire DNS load
balancer configuration from being applied. This affects all services that rely on this
DNS configuration, not just the new Vue service.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Collaborator

@maxkosty maxkosty left a comment

Choose a reason for hiding this comment

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

please update staging.env, production.env and .local.env.base
#994 only adds vars to staging.env
make sure to follow the current pattern with _SUBDOMAIN e.g.

empower/production.env

Lines 107 to 112 in 991204b

ANGULAR_DSN=${__GCP_SECRET__(ANGULAR_DSN_SANDBOX_MIRROR)}
ANGULAR_SENTRY_PROJECT=frontend-${ANGULAR_SUBDOMAIN}
ANGULAR_APP_ENGINE_SERVICE=${ANGULAR_SUBDOMAIN}
ANGULAR_SENTRY_ENVIRONMENT=production
ANGULAR_RELEASE=empower.${ANGULAR_SUBDOMAIN}@${__DYNAMIC_VERSION__}
ANGULAR_SUBDOMAIN=angular
(what Seer comment below is referring to).

otherwise looks good!

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.

2 participants