feat(agentcore): add L2 constructs for policy and policy engine #37238
feat(agentcore): add L2 constructs for policy and policy engine #37238dineshSajwan wants to merge 22 commits intoaws:mainfrom
Conversation
|
|
||||||||||||||
|
|
||||||||||||||
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| /** | ||
| * IAM permissions for PolicyEngine runtime operations (data plane) | ||
| */ | ||
| export namespace PolicyEnginePerms { |
There was a problem hiding this comment.
I think this file will no longer be required, due to the new way of setting grants: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#grants
But if still needed, then lets drop namespace and use flat exports instead (like in runtime perms, or tools perms)
There was a problem hiding this comment.
Namespace - removed.
Autogenerated grant.json - Checked this, the auto-generated grants pattern (grants.json + spec2cdk) doesn't apply to alpha packages, only stable L1 modules get that pipeline.
| * @param grantee - The IAM principal to grant permissions to | ||
| * @param actions - The actions to grant | ||
| */ | ||
| grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant; |
There was a problem hiding this comment.
We are using a new pattern for grants: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#grants
They are now autogenerated (could be wrong for alpha modules tho, please double check) and the permission are based on the grants.json that needs to be added. Please review the documentation for this new pattern, and let us know if something cannot be applied (due to alpha module, but I am 99% sure it should work)
| * Minimal reference interface for Policy resources. | ||
| * Used for resource identification and ARN construction. | ||
| */ | ||
| export interface IPolicyRef { |
There was a problem hiding this comment.
These IxxxRef classes are autogenerated, there is no need to implement them, just consume them in the IPolicy one. Check this example: https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-mediapackagev2-alpha/lib/channel.ts#L36
Same applies for all other IxxRef I saw in the PR
There was a problem hiding this comment.
Done. IPolicy now extends the auto-generated IPolicyRef and IPolicyEngine extends IPolicyEngineRef, both imported from aws-cdk-lib/aws-bedrockagentcore.
| /** | ||
| * The ARN of the policy | ||
| */ | ||
| readonly policyArn: string; |
There was a problem hiding this comment.
Added @Attribute tags on the interface properties and the concrete class properties.
| * Contains methods and attributes valid for Policies either created with CDK or imported. | ||
| */ | ||
| export abstract class PolicyBase extends Resource implements IPolicy { | ||
| public abstract readonly policyArn: string; |
There was a problem hiding this comment.
Missing @attribute tag here too? review all attributes are correctly tagged please
There was a problem hiding this comment.
Added @Attribute tags on the interface properties and the concrete class properties. Abstract base class properties intentionally don't carry them since CDK tooling resolves them via the interface and concrete implementation.
| */ | ||
| public grantEvaluateForGateway(grantee: iam.IGrantable, gateway: IGateway): iam.Grant { | ||
| const getPolicyEngineGrant = this.grant(grantee, 'bedrock-agentcore:GetPolicyEngine'); | ||
| const gatewayResourceName = Token.isUnresolved(gateway.name) ? '*' : `${gateway.name}-*`; |
There was a problem hiding this comment.
Can't this use the gateway.gatewayArn directly?
There was a problem hiding this comment.
Yes , gateway.name is used intentionally to to avoid a circular dependency between PolicyEngine and Gateway. The gateway needs policyengine and the policyengine needs gateway arn. The gateway.gatewayArnis synthesized as a CloudFormation Fn::GetAtt token that references the Gateway resource, and if PolicyEngine tries to reference that ARN directly, it creates a circular dependency that CloudFormation cannot resolve at deploy time.
By using gateway.name with a wildcard suffix, we can construct the necessary ARN pattern without directly referencing the unresolved gateway ARN, thus avoiding the circular dependency while still granting permissions scoped to the correct gateway.
Added JS doc.
| * @param scope - The construct scope for error reporting (optional) | ||
| * @returns Array of validation error messages, empty if valid | ||
| */ | ||
| export function validatePolicyEngineName(name: string, scope?: IConstruct): string[] { |
There was a problem hiding this comment.
For this file in general, any string that is validated needs to also check if its a unresolved token first
| public addPolicy(id: string, options: AddPolicyOptions): any { | ||
| // Import Policy here to avoid circular dependency | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| const { Policy } = require('./policy'); |
There was a problem hiding this comment.
Not the correct way to add imports. What is the circular dependency?
There was a problem hiding this comment.
Bad check in. I used it to fix a false circular dependency . Now removed the bad import created a policy-types.ts file and the false circular dependency was also removed.
| * Effect of the policy statement - whether to permit or forbid the action. | ||
| * @internal | ||
| */ | ||
| enum PolicyEffect { |
There was a problem hiding this comment.
I think all this enums here potentially can be extended to support new values. Correct me if I am wrong on this. We need to use enum-like instead, to provide the escape hatch if a new value is added but not yet supported in CDK
There was a problem hiding this comment.
These are all @internal enumns and the user will not use these. Also we support multiple ways to create the cedar policy, for example PolicyStatement.fromCedar('any raw cedar string you want'); . The user can always use the fromCedar for raw string to support any value. IMHO , I think this should be fine.
Issue # (if applicable)
Add L2 constructs for Policy and PolicyEngine with type-safe Cedar policy builder
Closes # 37219 .
Reason for this change
This PR adds support for Amazon Bedrock AgentCore Policy and PolicyEngine constructs to enable fine-grained authorization control for Bedrock agents using Cedar policy language. Previously, there was no CDK construct support for creating and managing AgentCore policies, requiring users to manually configure Cedar authorization rules through the console or API calls.
Description of changes
New Constructs Added:
Policy-engine.ts
L2 construct with base class pattern (PolicyEngineBase)
Support for KMS encryption, tags, and descriptions
Import method: PolicyEngine.fromPolicyEngineAttributes()
Convenience method: addPolicy() for attaching policies
Policy.ts -
L2 construct with base class pattern (PolicyBase)
Two ways to define policies:
Raw Cedar: definition property for advanced users
Type-safe builder: PolicyStatement class for guided policy creation
Validation modes: FAIL_ON_ANY_FINDINGS (default) or IGNORE_ALL_FINDINGS
Import method: Policy.fromPolicyAttributes()
PolicyStatement Builder - Type-safe Cedar policy builder
This is required because cedar policy use cedar language . The user has to be well aware of this language when creating raw cedar policy. This statement builder mimic the AWS console Form capability to create a cedar policy which makes it easy for user who are not well versed with cedar language to create cedar policy.
Describe any new or updated permissions being added
PolicyEngine.grantRead()
Actions Granted: bedrock-agentcore:GetPolicyEngine
Use Case: Read policy engine configuration at runtime for monitoring/audit
PolicyEngine.grantEvaluate()
Actions Granted: bedrock-agentcore:GetPolicyEnginebedrock-agentcore:AuthorizeActionbedrock-agentcore:PartiallyAuthorizeActions
Use Case: Evaluate authorization decisions during agent requests
Policy.grantRead()
Actions Granted: bedrock-agentcore:GetPolicy
Use Case: Read individual policy configuration and Cedar statements
Description of how you validated changes
Unit test, Integration test and with a sample CDK APP deployed the policies.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license