Add support for shared VPC configuration in ECS frontend preview deployment - #59
Conversation
…p ID in ECS deployment
…valid top-level flags
There was a problem hiding this comment.
Pull request overview
Adds an optional use-shared-vpc flag to preview deployments so ECS Express previews can attach to a shared preview VPC by fetching network config from SSM.
Changes:
- Adds
use-shared-vpcas an optionalworkflow_callinput to the preview workflows and passes it through to the ECS Express deploy action. - Extends
deploy-ecs-express-serviceto (optionally) fetch VPC security group + subnet IDs from SSM and pass--network-configurationto ECS create/update. - Improves ECS Express readiness polling by requiring both
ACTIVEstatus and a resolvable public ingress URL before succeeding.
Changed files:
.github/workflows/deployment.preview.yml.github/workflows/deployment.preview.on-comment.yml.github/actions/deployment/preview/deploy-ecs-express-service/action.yml
Per-file validation
.github/workflows/deployment.preview.yml— Adds new optional input and forwards it; ❌ Error: references an internal action via a feature-branch ref instead of@v2..github/workflows/deployment.preview.on-comment.yml— Adds new optional input and forwards it; ❌ Error: references an internal workflow via a feature-branch ref instead of@v2..github/actions/deployment/preview/deploy-ecs-express-service/action.yml— Adds new optional input and shared-VPC SSM lookup;⚠️ Warning: introduces SSM lookup paths under/config/..., which is inconsistent with existing/__deployment__/...patterns used elsewhere in this repo.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/deployment.preview.yml | Adds use-shared-vpc input and passes it to ECS Express preview deploy step. |
| .github/workflows/deployment.preview.on-comment.yml | Adds use-shared-vpc input and forwards it into the reusable preview workflow. |
| .github/actions/deployment/preview/deploy-ecs-express-service/action.yml | Optionally fetches VPC config from SSM and attaches ECS Express service to that VPC; tightens readiness check for public URL availability. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/actions/deployment/preview/deploy-ecs-express-service/action.yml:69
- The new shared-VPC SSM lookups use a new
/config/...parameter root, but this repo’s established deployment registry convention uses/__deployment__/...(e.g..github/actions/deployment/get-application-deployment-info/action.yml:66-71and/__deployment__/applications/...at lines 150-165). Introducing a new root increases the risk that preview deployments fail due to missing/misnamed parameters and makes the service registry inconsistent across actions. Consider sourcing these values from the existing/__deployment__/...registry (or plumbing them throughget-application-deployment-info) rather than adding a new/confignamespace.
- name: Get shared VPC config
id: vpc-config
if: ${{ inputs.use-shared-vpc == 'true' }}
shell: bash
run: |
echo "sg_id=$(aws ssm get-parameter \
--name /config/${{ inputs.service-name }}/frontend_preview_security_group_id \
--query Parameter.Value --output text)" >> $GITHUB_OUTPUT
echo "subnet_ids=$(aws ssm get-parameter \
--name /config/shared/frontend_preview_subnet_ids \
--query Parameter.Value --output text)" >> $GITHUB_OUTPUT
…action.yml Co-authored-by: Fredrik Førde Lindhagen <fredrik@lindhagen.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/actions/deployment/preview/deploy-ecs-express-service/action.yml:69
- The subnet ID SSM lookup should also pin the region to
inputs.aws-regionto avoid relying on the runner’s default AWS region.
echo "subnet_ids=$(aws ssm get-parameter \
--name /__deployment__/shared/frontend_preview_subnet_ids \
--query Parameter.Value --output text)" >> $GITHUB_OUTPUT
.github/actions/deployment/preview/deploy-ecs-express-service/action.yml:66
- The SSM lookups for shared VPC config don’t specify a region, while the action otherwise supports an explicit
aws-regioninput. If the runner’s default region differs frominputs.aws-region, these reads can fail or pull values from the wrong region. Pass--regionexplicitly to keep behavior consistent.
This issue also appears on line 67 of the same file.
echo "security_group_id=$(aws ssm get-parameter \
--name /__deployment__/${{ inputs.service-name }}/frontend_preview_security_group_id \
--query Parameter.Value --output text)" >> $GITHUB_OUTPUT
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/actions/deployment/preview/deploy-ecs-express-service/action.yml:64
- The
Get shared VPC configstep writes the security group output assecurity_group_id, but later the deploy/update steps read${{ steps.vpc-config.outputs.sg_id }}. This makessg_idempty and will produce an invalid--network-configurationwhenuse-shared-vpcis enabled.
echo "security_group_id=$(aws ssm get-parameter \
…lows" This reverts commit 38bf513.
…:nsbno/platform-actions into support-shared-vpc-for-frontend-previews
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/deployment.preview.yml:173
- The workflow is referencing a feature-branch ref in
uses:(@support-shared-vpc-for-frontend-previews). After merge this ref likely won’t exist, and it also bypasses the repo’s documented@v2floating tag strategy, which can break all consumers of this reusable workflow.
uses: nsbno/platform-actions/.github/actions/deployment/preview/deploy-ecs-express-service@support-shared-vpc-for-frontend-previews
.github/workflows/deployment.preview.on-comment.yml:74
- This workflow calls
deployment.preview.ymlusing a feature-branch ref (@support-shared-vpc-for-frontend-previews). That ref is not stable and will likely disappear after merge; callers should use the repo’s supported@v2floating tag.
uses: nsbno/platform-actions/.github/workflows/deployment.preview.yml@support-shared-vpc-for-frontend-previews
.github/actions/deployment/preview/deploy-ecs-express-service/action.yml:66
- The SSM parameter path for the security group deviates from the established
/__deployment__/applications/<service>/...convention used elsewhere (e.g..github/actions/deployment/get-application-deployment-info/action.yml:150and.github/actions/deployment/preview-spa/get-spa-deployment-info/action.yml:25). Using/__deployment__/<service>/...increases the chance of missing parameters and makes the service registry inconsistent.
echo "security_group_id=$(aws ssm get-parameter \
--name /__deployment__/${{ inputs.service-name }}/frontend_preview_security_group_id \
--query Parameter.Value --output text)" >> $GITHUB_OUTPUT
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
.github/workflows/deployment.preview.on-comment.yml:74
- This called-workflow ref is also pinned to the temporary feature branch rather than the stable
@v2ref used by the other internal workflow callers. After the branch is removed,.previewcomment deployments will fail when GitHub tries to loaddeployment.preview.yml; switch this back to@v2once the new input is released.
uses: nsbno/platform-actions/.github/workflows/deployment.preview.yml@support-shared-vpc-for-frontend-previews
.github/workflows/deployment.preview.yml:173
- This leaves the shared workflow calling a temporary feature-branch ref instead of the repository's
@v2ref documented inREADME.md:34-42. Once this branch is deleted (or is no longer updated), every ECS Express preview deployment that reaches this step will fail to resolve the action. Promote the action and reference the stable@v2ref before merging.
uses: nsbno/platform-actions/.github/actions/deployment/preview/deploy-ecs-express-service@support-shared-vpc-for-frontend-previews
No description provided.