Skip to content

fix(cloudformation): resolve IAM assume role policy intrinsics - #2630

Open
AbarnaaSree wants to merge 1 commit into
floci-io:mainfrom
AbarnaaSree:fix/iam-role-assume-policy-intrinsics-2538
Open

fix(cloudformation): resolve IAM assume role policy intrinsics#2630
AbarnaaSree wants to merge 1 commit into
floci-io:mainfrom
AbarnaaSree:fix/iam-role-assume-policy-intrinsics-2538

Conversation

@AbarnaaSree

Copy link
Copy Markdown
Contributor

Summary

Closes #2538

Resolve AssumeRolePolicyDocument through CloudFormationTemplateEngine.resolveJsonAttribute before storing it in AWS::IAM::Role.

Previously, the raw template node was serialized directly, causing intrinsics such as Fn::Sub and Ref to be stored as unresolved CloudFormation syntax.

Added a regression test to verify that the resolved trust policy is passed to IamService.createRole without unresolved intrinsic keys.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

The IAM role AssumeRolePolicyDocument was previously stored with unresolved CloudFormation intrinsic functions such as Fn::Sub.

The fix resolves JSON-valued attributes using the existing resolveJsonAttribute helper before passing the policy to the IAM service.

Checklist

  • mvn -Dtest=IamRoleCfnProvisionerTest test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@github-actions

Copy link
Copy Markdown

🎉 Thanks for your first pull request to Floci!

Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime.

While you wait, a couple of things that make review faster:

  • Link the issue this fixes with Closes #N in the description
  • Commits follow Conventional Commits (feat(s3): ..., fix(dynamodb): ...)
  • Behaviour changes come with a test — see CONTRIBUTING.md

Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it.

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

The PR resolves IAM role trust-policy JSON through the CloudFormation intrinsic engine before creating or updating a role.

  • Replaces raw AssumeRolePolicyDocument serialization with resolveJsonAttribute and retains the empty-policy fallback.
  • Adds regression coverage for a nested Fn::Sub principal.
  • Non-scalar Fn::If branches remain corrupted by the resolver path, and the added test has inconsistent indentation.

Confidence Score: 4/5

The PR should not merge until JSON-valued Fn::If branches in IAM trust policies are preserved instead of being stored as empty strings.

Routing the full trust policy through resolveJsonAttribute fixes scalar intrinsics but exposes the resolver's scalar coercion for object-valued Fn::If branches, producing malformed stored policies.

Files Needing Attention: src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/IamRoleCfnProvisioner.java; src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/IamRoleCfnProvisionerTest.java

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/IamRoleCfnProvisioner.java Resolves trust-policy intrinsics before persistence, but object- or array-valued Fn::If branches can collapse to empty strings.
src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/IamRoleCfnProvisionerTest.java Adds scalar Fn::Sub regression coverage, with inconsistent class-member indentation and no coverage for non-scalar intrinsic results.

Reviews (1): Last reviewed commit: "fix(cloudformation): resolve IAM assume ..." | Re-trigger Greptile

: "{\"Version\":\"2012-10-17\",\"Statement\":[]}";
String assumeDoc = props == null
? null
: ctx.engine().resolveJsonAttribute(props.path("AssumeRolePolicyDocument"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Fn::If corrupts policy objects

When an AssumeRolePolicyDocument contains an Fn::If whose selected branch is an object or array, resolveJsonAttribute routes that branch through scalar resolution and replaces it with an empty string, causing the role to store a malformed trust policy.

Comment on lines +372 to +373
@Test
void assumeRolePolicyIntrinsicsAreResolved() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test indentation breaks convention

The new test annotation and method declaration are placed at column one rather than using the class-member indentation established throughout this file, while the adjacent ctx() helper also receives unrelated inconsistent indentation. Aligning these lines with the existing test structure keeps this focused fix readable and avoids formatting churn.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@hectorvent hectorvent added bug Something isn't working cloudformation AWS CloudFormation labels Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cloudformation AWS CloudFormation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CFN: IamRoleCfnProvisioner stores AssumeRolePolicyDocument without resolving intrinsics

2 participants