Skip to content

docs(s3,kms): replace object-literal IResourceWithPolicyV2 example with named class - #38461

Open
AmariahAK wants to merge 1 commit into
aws:mainfrom
AmariahAK:docs/s3-kms-replace-object-literal
Open

docs(s3,kms): replace object-literal IResourceWithPolicyV2 example with named class#38461
AmariahAK wants to merge 1 commit into
aws:mainfrom
AmariahAK:docs/s3-kms-replace-object-literal

Conversation

@AmariahAK

@AmariahAK AmariahAK commented Jul 31, 2026

Copy link
Copy Markdown

Description

Closes #38452

Reason for this change

The IResourcePolicyFactory / IResourceWithPolicyV2 usage example in the S3
and KMS READMEs used a TypeScript object-literal-with-inline-method pattern
for the return value of forResource():

return {
  env: resource.env,
  addToResourcePolicy(statement) { ... }
};

While this satisfies the structural interface in TypeScript, jsii/Rosetta
cannot translate anonymous objects with methods to other languages. The
generated Ruby docs showed an inline def inside a hash, Python had raw
function bodies inside dicts, and Java produced Map.of() with a broken
method body. See the full breakdown in #38452.

Description of changes

Updated the nofixture code blocks in both READMEs to use a proper named
class MyResourceWithPolicy that explicitly implements IResourceWithPolicyV2:

  • packages/aws-cdk-lib/aws-s3/README.md — replaced object-literal with
    MyResourceWithPolicy class
  • packages/aws-cdk-lib/aws-kms/README.md — identical fix, adapted for
    AWS::KMS::Key

Both now follow the same class-based pattern as CfnBucketWithPolicy and
CfnKeyWithPolicy in their respective default-traits.ts source files.

Additional imports added: AddToResourcePolicyResult (from
aws-cdk-lib/aws-iam) and ResourceEnvironment (from
aws-cdk-lib/interfaces). Removed unused IConstruct import.

Note: this is NOT the full fix for #38452. This PR only addresses the
README examples. The Rosetta transpiler itself still has bugs in the C#,
Golang, and Java targets (e.g. resource = resource; self-assignment in C#,
incorrect pointer usage in Go, missing getEnv() in Java) that will need a
separate PR in the Rosetta codebase. See @omarqureshi's analysis in the issue
for details on those remaining transpiler issues.

Describe any new or updated permissions being added

None — documentation-only change.

Description of how you validated changes

  • Verified the new class structure matches the existing CfnBucketWithPolicy
    and CfnKeyWithPolicy implementations in default-traits.ts exactly
  • Confirmed all imported types (AddToResourcePolicyResult,
    ResourceEnvironment, IResourceWithPolicyV2, IResourcePolicyFactory,
    PolicyStatement, ResourceWithPolicies) are exported from their
    respective modules
  • Confirmed the issue author's Rosetta translation output shows clean,
    idiomatic Ruby, Python, and Java for the class-based pattern
  • Full Rosetta extraction (yarn rosetta:extract --strict) should be run
    on a machine with ≥16GB RAM to verify cross-language translation before
    merge

Checklist


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

@aws-cdk-automation
aws-cdk-automation requested a review from a team July 31, 2026 12:57
@github-actions github-actions Bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2 labels Jul 31, 2026

@aws-cdk-automation aws-cdk-automation left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The pull request linter fails with the following errors:

❌ The title prefix of this pull request must be one of "feat|fix|build|chore|ci|docs|style|refactor|perf|test|revert"

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

@omarqureshi

Copy link
Copy Markdown

@AmariahAK - for cleanliness, instead of a merge from main, could you rebase instead and push (force with lease) a single squashed commit?

…th named class

The object-literal pattern with inline methods in the forResource()
return value does not translate across jsii/rosetta to non-TypeScript
languages (Ruby, Python, Java, C#, Go).

Replace it with a proper MyResourceWithPolicy class that implements
IResourceWithPolicyV2, matching the existing CfnBucketWithPolicy and
CfnKeyWithPolicy patterns in default-traits.ts.

closes aws#38452

Co-authored-by: atlarix-agent <agent@atlarix.dev>
@AmariahAK
AmariahAK force-pushed the docs/s3-kms-replace-object-literal branch from 79fe45c to ac26af6 Compare August 2, 2026 12:10
@AmariahAK

Copy link
Copy Markdown
Author

@omarqureshi should be good now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

s3/kms: IAM IResourcePolicyFactory, function in object

3 participants