feat(core): add shared execution role + executionRole getter

[Simone319]

Problem

Multi-compute needs a single IAM identity that every Building Block grants to, reachable from any block, so that (in later work) multiple compute types can share one permission model. Today each Building Block grants to the auto-generated role of the one NodejsFunction.

This is the first foundational, non-breaking step (Multi-Compute A1).

Issue #, if available: #1017

Changes

• setupBlocksInfra (shared by BlocksStack + BlocksBackend) now provisions one explicit iam.Role (BlocksRole) with AWSLambdaBasicExecutionRole attached, and the handler assumes it via role: executionRole instead of an auto-generated role.
• Exposed executionRole on BlocksStack and BlocksBackend.
• Added a Scope.executionRole getter that resolves the role from any Building Block via the same construct-tree walk as get handler() (with the globalThis.CURRENT_BLOCKS_STACK fallback).

Non-breaking: the same handler function is created, now backed by an explicit role. Block grants land on the role's default (inline) policy exactly as they did on the auto-role, so synth output and runtime behavior are unchanged. No block references executionRole yet.

Note: AWSLambdaBasicExecutionRole is attached explicitly because the auto-generated NodejsFunction role included it by default — omitting it would silently break CloudWatch Logs. We deliberately did not add a shared managed policy: a single managed policy caps at 6,144 chars (smaller than the inline budget) and CDK's grant*() route to the role's inline policy anyway. Grant-accumulation limits are handled later by per-compute roles, with a synth-time statement packer as the fallback (tracked separately).

Validation

• npm run build (full monorepo) green.
• npm test in @aws-blocks/core: all pass, including 4 new tests in blocks-backend.test.ts — surface (executionRole exposed), synth shape (Lambda-assumable role with basic execution, handler references it), and a nested-block resolution test proving Scope.executionRole walks up to the owner and its grant lands on the role's inline policy.
• npm run lint, npm run lint:deps, npm run check:api clean (no public-surface drift).
• npm run test:e2e:local green (comprehensive app unaffected).

Checklist

• PR description included
• Tests are changed or added
• Relevant documentation is changed or added (and PR referenced)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[changeset-bot]

🦋 Changeset detected

Latest commit: 87c39ef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@aws-blocks/core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[osama-rizk]

Reviewed the A1 shared-role step. Scope is tight and the intent is clean: one explicit role, block grants land where they always did, no block consumes it yet. I verified the load-bearing claims rather than trusting them — the getter is a faithful copy of handler (identical walk + fallback), the nested-resolution test genuinely proves a grant from two levels deep lands on the shared role's inline policy, and re-adding AWSLambdaBasicExecutionRole correctly preserves CloudWatch Logs after dropping the auto-role. Good instinct catching that last one.

My findings cluster at two altitudes: a couple of documentation-vs-reality mismatches (the getter docstring contradicts the PR body on managed-vs-inline; "synth output unchanged" glosses a role logical-ID change), and some maintainability/coverage items (duplicated tree-walk, untested BlocksStack + fallback paths). None block the merge — the code does what it intends. The doc mismatches are worth fixing before this lands because they'll mislead the next person building on A1.

I intentionally stayed light on line-level correctness of the getter — it's a verbatim copy of an already-shipped, already-trusted getter, so re-deriving it would be low-yield. The higher-leverage questions for a foundational-role change are "what actually changes in the deployed stack" and "will the next author be misled," which is where the comments sit.

[osama-rizk]

This docstring says grants "accumulate in the stack's shared managed policy" — but that directly contradicts the PR description, which says you deliberately did not add a shared managed policy and that CDK's grant*() route to the role's inline (default) policy. Your own nested-resolution test confirms the inline reality: it asserts the marker action lands on an AWS::IAM::Policy (the role's default policy), not a ManagedPolicy.

So the docstring is wrong in the one place a future block author will read to understand where their grants go. Since this is foundational for the whole multi-compute line, an incorrect mental model here compounds. Suggest: "grants accumulate on the role's default (inline) policy" — and if a shared managed policy is the eventual A-series design, say "(a shared managed policy may replace this later; see #1017)" so the intent is explicit rather than prematurely stated as fact.

[Simone319]

Fixed. Reworded the getter docstring to state that grants land on the role default (inline) policy — matching the impl and the nested-resolution test. Did not add the "managed policy may replace this later" phrasing: we specifically decided against a shared managed policy (a single managed policy caps at 6,144 chars, smaller than the inline budget, and grant*() route to the inline policy anyway), so inline is the intended design, not a placeholder.

[osama-rizk]

"Non-breaking / synth output unchanged" is true at the runtime level but not at the CloudFormation level, and the gap matters for anyone upgrading a live stack. On base, the handler used NodejsFunction's auto-generated role (logical id ...HandlerServiceRole...). I confirmed base has no explicit role: on the function. Introducing new iam.Role(scope, 'BlocksRole', ...) and passing role: executionRole means the auto-role logical id disappears and a new BlocksRole* logical id appears.

Consequence: on an existing deployed stack, cdk deploy replaces the execution role — the old role is deleted and a new one created. Harmless for a stateless Lambda exec role (no data, grants re-attach identically), and there's no downtime concern here, but it is a resource replacement, not a no-op diff. The claim "synth output ... unchanged" will surprise an operator who diffs the change set and sees a role delete+create. Worth a one-line migration note in the changeset: existing stacks replace the Lambda execution role on upgrade; runtime-equivalent, no action needed. (Also worth confirming nothing external references the old role ARN — e.g. a resource policy or trust relationship outside this stack.)

[Simone319]

Good catch — you are right that this is a CloudFormation-level role replacement, not a true no-op. Added a migration note to the changeset: upgrading an existing stack deletes the auto-generated role and creates BlocksRole; runtime-equivalent (grants re-attach identically) and no action needed, but a change-set diff shows a role delete+create. On the external-reference point: nothing in this repo references the old auto-role ARN (it was an unnamed, auto-generated logical id, never exported or surfaced), so there is no trust-relationship or resource-policy dependency to break.

[osama-rizk]

Duplication: this getter's tree-walk is now a verbatim copy of get handler() right above it — same while (current.node.scope) loop, same instanceof BlocksStack || BlocksBackend check, same globalThis fallback, even the same (repo-atypical) 4-space body indentation carried over. Two copies of a construct-tree walk will drift: the day someone adds a third owner type or fixes the fallback, they'll update one and miss the other.

Extract the shared traversal, e.g. private owner(): BlocksStack | BlocksBackend | undefined (or a walkToOwner() returning the matched construct), and have both getters read this.owner()?.handler / ?.executionRole with the shared fallback. Small refactor, removes the drift risk, and makes the "resolves the same way as handler" comment true by construction instead of by copy-paste.

[Simone319]

Deliberately leaving these as two copies. get handler() is on its way out — once the event/observability blocks migrate off this.handler to this.compute/this.executionRole, the handler getter is removed entirely. Extracting a shared owner() helper now would create an abstraction whose two callers collapse to one shortly after, so the drift risk you describe has a short lifetime and the refactor would just be undone. Keeping executionRole as a standalone copy means its removal-partner (handler) can be deleted without untangling a shared helper. Will revisit if handler outlives expectations.

[osama-rizk]

Test-coverage gaps (non-blocking, but this is the foundation the whole A-series builds on, so worth locking down now):

1. BlocksStack path is untested. All four new tests exercise BlocksBackend. But index.ts BlocksStack gained the identical executionRole surface + wiring, and it has its own blocks-stack.test.ts. setupBlocksInfra is shared so behavior should match, but "should" is exactly what a test pins — a mirror test in blocks-stack.test.ts (or a note explaining why the shared-infra test covers both) closes it.
2. The getter's globalThis fallback branch is untested. The nested-resolution test hits the tree-walk branch; the return ((globalThis as any).CURRENT_BLOCKS_STACK...).executionRole fallback — the branch that fires for a parent-less Scope with no owner in the tree — is never exercised. That's the branch most likely to throw (undefined deref) in real misuse, so it's the one most worth a test.

[Simone319]

Both gaps now covered in blocks-stack.test.ts: (1) a BlocksStack mirror test asserting the executionRole surface, the handler assuming BlocksRole, and nested-block resolution landing a grant on the role inline policy; (2) a test for the getter globalThis fallback branch — a Scope parented under a plain cdk.Stack (no Blocks owner in the tree) resolves via globalThis.CURRENT_BLOCKS_STACK.

[osama-rizk]

Minor: new iam.CompositePrincipal(new iam.ServicePrincipal('lambda.amazonaws.com')) wrapping a single principal is functionally identical to just new iam.ServicePrincipal('lambda.amazonaws.com') — the CompositePrincipal adds nothing until there's a second principal. I assume this is deliberate scaffolding for later multi-compute (ECS/other services assuming the same role), which would be a good reason to keep it — but as written it reads as accidental over-construction. Either add a short comment (// CompositePrincipal so later compute types can assume the same role — A-series) so the intent is legible, or drop the wrapper until the second principal actually arrives. Given the PR is explicitly the 'foundational' step, I'd lean toward the comment.

[Simone319]

Intentional scaffolding — went with your lean and added a comment. It is there so additional compute types can assume the same shared role by adding principals (e.g. ecs-tasks.amazonaws.com) rather than restructuring the role later. Comment now makes that legible at the call site.