search

berenddeboer / cdk-rds-sql

A CDK construct that allows creating roles and databases an on Aurora Serverless Postgresql cluster.
Apache License 2.0
23 stars 11 forks source link

Provider Depends On Is Too Broad #29

Open timothymathison opened 3 months ago

timothymathison commented 3 months ago

There is a call to addDependency(props.cluster) which I think is too broad. https://github.com/berenddeboer/cdk-rds-sql/blob/aa111d209931165b4457934a68c226bef0cc93c3/src/provider.ts#L105

The resulting SAM template has every single child resource of the DatabaseCluster instance in the dependsOn array of the provider resources. Because the DatabaseCluster construct I'm using is a L2/L3 construct, there are quite a few child resources which aren't required for the Provider to work. In particular any DatabaseProxy instances created with cluster.addProxy() are now in the dependency array of the provider resources even though they don't interact with the proxy. This is a problem because I actually want to use the cdk-rds-sql provider to create roles which are then passed to the proxy via the secrets prop. But this leads to a circular dependency because the provider will depend on the proxy. Furthermore, I'm working on an L3 DatabaseCluster construct which has the provider built into it - making it a child resource - which leads to the provider resources depending on themselves.

Does the provider actually need to depend on anything besides the CfnDBCluster and writer instance resources?

berenddeboer commented 3 months ago

Haven't tried it, but dependencies are superhard :-(

We only connect to the writer, but we also need the root secret. So I think if you have both as dependencies you probably are good. Happy to have a PR for this, although it needs some good testing.

timothymathison commented 3 months ago

Haven't tried it, but dependencies are superhard :-(

We only connect to the writer, but we also need the root secret. So I think if you have both as dependencies you probably are good. Happy to have a PR for this, although it needs some good testing.

I think the cluster/writer resources already depend on the secret. So I think its just the writer that we need to directly depend on for the provider. Yeah, full disclosure I'm still trying to figure out the best way to reference the writer dependency since its not directly exposed.