Remove the circular dependency between the HAPI service and its database - #2069
Merged
Conversation
Deploying either stack failed during change set creation: Circular dependency between resources: [EhrProxyHapiEhrProxyHapiServiceSecurityGroupfrom...LoadBalancer..., EhrProxyLoadBalancerSecurityGroupto...HapiServiceSecurityGroup..., EhrProxyHapiEhrProxyHapiService..., EhrProxyHapiEhrProxyHapiServiceSecurityGroup..., EhrProxyHapiEhrProxyHapiDatabaseSecurityGroupfrom...IndirectPort...] The cause was `service.node.addDependency(database)`. It reads like a harmless ordering hint, but `node.addDependency` applies across whole construct subtrees rather than between the two resources named. It therefore made the service's security group depend on the database's ingress rule, and that rule already references the service's security group, which closes the loop. The ordering that line was meant to guarantee already exists without it. SPRING_DATASOURCE_URL resolves the database endpoint through Fn::GetAtt and the credentials resolve through the generated secret, so the task definition cannot be created before the database, and the service cannot be created before its task definition. Verified by creating a real change set against both deployed stacks: each now succeeds and reports the same six additions and no deletions. The earlier `cdk diff` runs had been falling back to a template-only comparison with "Could not create a change set", which hid the problem.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cdk deployfails on both stacks during change set creation, so #2065 cannot actually be deployed:The cause is a single line introduced in #2065:
It reads like a harmless ordering hint. The catch is that
node.addDependencyapplies across whole construct subtrees, not between the two resources named. So it made the HAPI service's security group depend on the database's ingress rule, and that ingress rule already references the service's security group. That closes the loop, and CloudFormation rejects it.Fix
Remove the line. The ordering it was meant to guarantee already exists implicitly:
SPRING_DATASOURCE_URLresolves the database endpoint throughFn::GetAtt, and the credentials resolve through the generated secret, so the task definition cannot be created before the database.Verified in the synthesized template: the task definition does reference the database via
Fn::GetAtt, and the service SG'sDependsOnis back to just its task role.A comment now records why the line must not come back, since re-adding it looks like an obvious improvement.
Verification
The earlier
cdk diffruns on #2065 had silently fallen back to a template-only comparison, printing "Could not create a change set, will base the diff on template differences". That message was CloudFormation rejecting the cycle, and it is why this was not caught before the deploy attempt. Template-only diffs do not validate the resource graph.With this change,
cdk diffcreates a real change set against both deployed stacks, which is CloudFormation validating the graph:EhrProxyAppStackFormsServerAppStackThe intended change is otherwise unaffected: the same RDS instance, subnet group, security group, ingress rule, secret and secret attachment, plus the expected ECS task definition revision.
Blast radius of the failed deploy
None. The deployment failed at change set creation, before any resource was touched: