Skip to content

feat(neptune)!: migrate cluster access to consumer-side grants - #420

Open
laazyj wants to merge 2 commits into
mainfrom
claude/neptune-consumer-grants-drynsz
Open

feat(neptune)!: migrate cluster access to consumer-side grants#420
laazyj wants to merge 2 commits into
mainfrom
claude/neptune-consumer-grants-drynsz

Conversation

@laazyj

@laazyj laazyj commented Aug 26, 2026

Copy link
Copy Markdown
Owner

What & why

Closes #372.

ADR-0013 deferred Neptune because allowAccessFrom coupled two things that belong on opposite sides of the same edge: an IAM grant and a security-group rule. This splits them, and each half now sits where its dependency already points.

The IAM half → clusterGrants.connect(...) (new packages/neptune/src/grants.ts). It delegates to the cluster's native grantConnect, is declared on the grantee builder, and is applied during that builder's own build() — so the edge runs consumer → cluster, like every other grant in the library.

The network half stays on the cluster, whose own security group the ingress rule is written into, renamed allowDefaultPortFrom(peer, description?) to match createInterfaceEndpointBuilder. It now takes a plain IConnectable and gained the optional rule description its sibling already had. Keeping the old name would have promised the IAM grant it no longer makes.

The old shape forced the cluster to depend on its own consumer — the exact reverse edge (and latent cycle) the ADR exists to prevent. The example stack shows the fix: graph now depends on bastionSg, and a new bastionRole component carries the grant and depends on graph.

Breaking change

allowAccessFrom and the exported ClusterAccessor type are removed. Migration, also documented in the package README:

// Before
graph: createClusterBuilder().allowAccessFrom(ref<InstanceBuilderResult>("bastion").get("instance")),
// { graph: ["network", "bastion"] }

// After
graph: createClusterBuilder().allowDefaultPortFrom(
  ref<SecurityGroupBuilderResult>("bastionSg").get("securityGroup"),
),
bastionRole: createServiceRoleBuilder("ec2.amazonaws.com").grant(
  clusterGrants.connect(ref<ClusterBuilderResult>("graph").get("cluster")),
),
// { graph: ["network", "bastionSg"], bastionRole: ["graph"] }

The grantee must be a builder that accepts grants, so where the old call granted a construct that owns a role (an EC2 instance), that construct now takes an explicit role component. The deployed permissions are unchanged: the bastion role still gets neptune-db:* plus AmazonSSMManagedInstanceCore, and the same ingress rule is emitted — only the logical IDs move, visible in the updated snapshot.

Also updated: ADR-0013's out-of-scope bullet records the resolution in place (the precedent its API Gateway entry set), the iamAuthentication default's doc comment, and the examples README row plus the smoke test's description of what it proves.

Notes for review

  • Only connect is exported. A variadic dataAccess(cluster, ...actions) was written and then dropped: no sibling namespace takes raw action strings, nothing in the repo used it, and it is permanent public API once published. A principal wanting less than neptune-db:* writes a narrower policy of its own; named capabilities can be added later without a break.
  • Not done here, worth a follow-up: ec2.Instance implements IGrantable and InstanceBuilder already accepts a role, so InstanceBuilder could become a grantee (GrantQueue + .grant(), ~15 lines) and the example's explicit bastionRole would collapse back into the instance. That is an @composurecdk/ec2 feature rather than this migration, so it stayed out.
  • No ADR added — this applies an existing decision to one package, per the AGENTS.md bar.

Checklist

  • Linked to an issue (or it's a small, obvious fix)
  • npm run verify passes locally
  • Tests added/updated for the change
  • If it adds an example stack: registered, listed in the examples README, and covered by a smoke test that exercises its runtime behaviour — n/a, no new stack; the existing Neptune example and its smoke test were updated

Generated by Claude Code

claude added 2 commits August 26, 2026 07:50
Split allowAccessFrom's two halves so each is declared where its dependency
points (ADR-0013). The IAM half becomes clusterGrants.connect/dataAccess,
declared on the grantee and applied during its own build; the security-group
rule stays on the cluster, whose own group it writes into, renamed
allowDefaultPortFrom to match createInterfaceEndpointBuilder and now taking a
plain IConnectable plus an optional rule description.

BREAKING CHANGE: allowAccessFrom is removed. Use allowDefaultPortFrom(peer)
for the network path and clusterGrants.connect(ref(...)) on the grantee for
the IAM half. The ClusterAccessor type is removed; allowDefaultPortFrom takes
an IConnectable.

Closes #372
Drop the speculative clusterGrants.dataAccess escape hatch — a variadic
action list no sibling capability namespace has, unused by the example and
the docs, and permanent public API once published. connect delegates to the
cluster's native grantConnect, which is what ADR-0013 named.

Hoist isolatedVpc into a shared test helper, fold the network-access tests
into one parameterised pair, scope the "no IAM grant here" assertion to the
policies themselves, and trim the duplicated rationale from the README and
the example's jsdoc.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

Overall line coverage: 99.40% across 24 package(s).

Package Statements Branches Functions Lines
acm 🟢 97.36% 🟢 94.73% 🟢 100.00% 🟢 97.22%
apigateway 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
budgets 🟢 99.20% 🟢 95.55% 🟢 100.00% 🟢 100.00%
cloudformation 🟢 98.57% 🟢 95.20% 🟢 100.00% 🟢 99.46%
cloudfront 🟢 99.40% 🟢 95.14% 🟢 100.00% 🟢 100.00%
cloudwatch 🟢 95.56% 🟡 89.55% 🟢 100.00% 🟢 98.23%
core 🟢 100.00% 🟢 97.43% 🟢 100.00% 🟢 100.00%
custom-resources 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
dynamodb 🟢 100.00% 🟢 93.10% 🟢 100.00% 🟢 100.00%
ec2 🟢 98.81% 🟢 97.72% 🟢 100.00% 🟢 99.58%
eslint-plugin 🟢 90.47% 🟡 83.87% 🟢 100.00% 🟢 98.19%
events 🟢 98.66% 🟢 94.59% 🟢 100.00% 🟢 100.00%
examples 🟢 97.77% 🟡 84.21% 🟢 100.00% 🟢 99.41%
iam 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
kms 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
lambda 🟢 98.00% 🟢 96.82% 🟢 100.00% 🟢 98.41%
logs 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
module-compat 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
neptune 🟢 97.18% 🟡 86.36% 🟢 100.00% 🟢 98.52%
route53 🟢 97.94% 🟢 95.68% 🟢 100.00% 🟢 99.18%
s3 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
ses 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
sns 🟢 100.00% 🟢 100.00% 🟢 100.00% 🟢 100.00%
sqs 🟢 100.00% 🟢 98.63% 🟢 100.00% 🟢 100.00%
Total 🟢 98.18% 🟢 94.80% 🟢 100.00% 🟢 99.40%

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.

feat(neptune): migrate to ADR 0013 Consumer Side Grants

2 participants