Skip to content

Add dispatch-only CDK deployment and weekly drift check workflows - #2070

Open
KyleOps wants to merge 2 commits into
mainfrom
cdk-deploy-and-drift-workflows
Open

Add dispatch-only CDK deployment and weekly drift check workflows#2070
KyleOps wants to merge 2 commits into
mainfrom
cdk-deploy-and-drift-workflows

Conversation

@KyleOps

@KyleOps KyleOps commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Why

The CDK stacks have no automation: deploying means building the packages locally and running cdk deploy by hand, and nothing compares main against what is actually running. Both stacks sat un-deployed from 2025-08 until 2026-07 without anyone noticing.

What

cdk_deploy.yml is workflow_dispatch only, since deploying replaces the HAPI tasks and briefly takes the FHIR servers offline. Inputs are the stack (both by default) and the action (diff or deploy, defaulting to diff), so an accidental run changes nothing. The diff job always runs first and writes the pending changes to the run summary, so the approver sees what they are approving. After deploying, it polls both FHIR endpoints until they serve 200.

cdk_drift_check.yml runs weekly and on demand, reports whether either stack differs from main, and opens, updates, or closes an issue labelled cdk-drift accordingly.

Setup common to both lives in a cdk-setup composite action, and .github/scripts/cdk-stacks.sh maps the stack input to stack names and app directories.

Two decisions worth reviewing

Access control is the aws-production GitHub environment with required reviewers, not an actor check, which is trivially bypassable. Until the environment is configured the deploy job cannot run, while diff still works.

The two jobs diff differently on purpose. Deployment uses the change set method so CloudFormation validates the resource graph; a template-only comparison is what let the circular dependency fixed in #2069 reach a deploy attempt undetected. The drift check only asks whether the stacks differ at all, which --method=template answers without permission to create change sets.

Not included

The IAM role the workflows assume (repository variable AWS_CDK_DEPLOY_ROLE_ARN) does not exist yet and should be created deliberately by someone with account ownership. The trust policy and permissions it needs are documented in deployment/README.md.

The CDK stacks had no automation at all. Deploying meant someone building the
packages locally and running cdk deploy by hand, and because nothing compared
main against the running infrastructure, the two drifted apart from 2025-08 to
2026-07 without anyone noticing.

Add two workflows. Deployment is dispatch only rather than on push: these stacks
back demo and evaluation environments, and deploying replaces the HAPI tasks,
which briefly takes the FHIR servers offline. It takes a stack and an action,
both defaulting to the safe option, and the diff job always runs first so
whoever approves a deploy can see the pending changes in the run summary.

Access control is the aws-production GitHub environment rather than an actor
check in the workflow, because an actor check is trivially bypassable and is not
a security boundary. Until that environment is configured the deploy job cannot
run, while diff still works.

The drift check runs weekly, reports whether either stack differs from main, and
opens or updates an issue labelled cdk-drift, closing it once they match again.

The two jobs deliberately diff differently. Deployment uses the default change
set method, because that asks CloudFormation to validate the resource graph; a
template-only comparison is what let a circular dependency between the HAPI
service and its database reach a deploy attempt undetected. The drift check only
asks whether main and the deployed stack differ, which a template comparison
answers without needing permission to create change sets.

Both need a repository variable AWS_CDK_DEPLOY_ROLE_ARN. The account already has
the GitHub OIDC provider and the cdk-hnb659fds-* bootstrap roles, but no role for
running CDK from CI; SmartFormsReactAppDeployment is scoped to the React app's S3
deployment and widening it would be worse than adding a dedicated role. The
required trust policy and permissions are documented in deployment/README.md.
@KyleOps

KyleOps commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Blocked on the IAM role, tracked in aehrc/smart-forms-infrastructure#1.

That issue carries everything needed to create it: trust policy, the exact bootstrap role ARNs to allow, a working reference implementation, and the reasoning for putting it in Terraform there rather than as CDK in this repo (in short: the workflow should not be able to deploy the role that grants the workflow its permissions).

One detail worth surfacing here, because it will otherwise cost someone an afternoon. The aehrc org has GitHub's immutable-ID OIDC subject claims enabled, so the trust policy must match both forms:

repo:aehrc/smart-forms:*
repo:aehrc@3783873/smart-forms@477486769:*

Matching only the documented first form fails to assume with an unhelpful STS error. Note the existing SmartFormsReactAppDeployment role lists only the plain form, so it is not a safe template.

This PR is safe to leave open in the meantime. Both workflows check for AWS_CDK_DEPLOY_ROLE_ARN and fail immediately with an explicit message while it is unset, rather than failing obscurely at the AWS call.

@KyleOps

KyleOps commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Note for review: this repo is public, so these workflow runs are too

Worth a conscious decision before merging rather than discovering it later. aehrc/smart-forms is a public repository, and for public repos anyone can read Actions run logs and job summaries without signing in. The weekly drift check therefore publishes a cdk diff of the production infrastructure on a schedule, and the deploy workflow publishes one every time it runs.

What that does expose

  • CloudFormation logical ids and CDK construct paths (EhrProxyHapi/EhrProxyHapiDatabase/...)
  • Resource types being added, changed or removed
  • The shape of IAM statement and security group changes
  • The AWS account id, which is already public in this repo: it appears in forms-server-app-stack.ts, deploy_app.yml and deploy_docs.yml today

What it does not expose

I checked this against real cdk diff output for both stacks rather than assuming.

  • No credentials. Keyword matches on password and secret in the output are all structural: the env var name SPRING_DATASOURCE_PASSWORD, the :password:: JSON key selector, IAM action names, logical ids. No values. The database credentials are ECS task secrets resolved from Secrets Manager at task start, so they never enter a CloudFormation template in the first place.
  • No resolvable infrastructure ids. Zero vpc-, subnet-, sg- or eni- ids in any diff output. CDK works in logical ids and Fn::GetAtt, so nothing in the diff can be used to address a real resource.
  • No database endpoint. The datasource URL is an Fn::GetAtt in the template, never a literal. Both instances are PubliclyAccessible: false in private subnets regardless.

Recommendation

I would leave it as is. The CDK source is already public, so a diff of it reveals nothing the code does not, and having the detail in the run summary is most of the value: it is what lets a reviewer see what they are approving before the deploy job runs.

If we would rather not publish it, the change is small and I am happy to make it: have the drift check report only which stacks drifted and drop the per-resource detail from $GITHUB_STEP_SUMMARY and the issue body, leaving the full diff visible only in the collapsed job log. That reduces but does not eliminate the exposure, since the raw log is still public. Fully avoiding it would mean moving the drift check somewhere non-public, which seems disproportionate for architecture that is already open source.

Related

The trust policy in aehrc/smart-forms-infrastructure#1 has been tightened since I first wrote it, for the same reason. It no longer uses repo:aehrc/smart-forms:*, which in a public repo would let any ref assume the deployment role and bypass the aws-production environment gate. It is now scoped to environment:aws-production for deploys and ref:refs/heads/main for the read-only diff and drift jobs, which makes the environment gate load bearing rather than advisory.

@KyleOps

KyleOps commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Discussed with team and they are happy with weekly drift check that pings me

Extract the Node setup, deployment role check, npm install, package
builds, and AWS credential configuration shared by all three CDK jobs
into a cdk-setup composite action. The deploy job previously lacked the
role check; it now inherits it from the shared action.

Remove the diff job's has_changes output, which nothing consumed.

Derive the drift check's stack list from cdk-stacks.sh instead of
hardcoding it in two places.

Shrink the workflow header rationale comments to one line each and
point to deployment/README.md, which keeps the full rationale.
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.

1 participant