feat(agentcore): agentcore gateway L2 construct

[dineshSajwan]

Issue # (if applicable)

Related to aws/aws-cdk-rfcs#785

Reason for this change

Adding bedrock agent core gateway and gateway target

Description of changes

• Added a new L2 construct for agentcore gateway
• Added a new L2 construct for agentcore gateway target
• Added validation helper methods, convenience method like addLambdaTarget, addOpenApiTarget, and other static methods.
• Added integ and unit test cases

Describe any new or updated permissions being added

The gateway creates a role with permission to bedrock agentcore , s3

Description of how you validated changes

Unit tests, integration tests, manual tests

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[mazyu36]

I received a review request in another place, so I've reviewed part of it.
I left one nit comment. I'll review the rest when I have time.

[mazyu36]

I think this should be IUserPool (Interface).

[kumvprat]

Added inline comments, overall adding more test cases in integration test can increase visibility on how the gateway and target combinations can be used.

[kumvprat]

Since we are assigning undefined values and for protocolConfiguration and authorizerConfig we are just type casting properties but not assigning any values, do we need to do this ?

[dineshSajwan]

These properties are already typecast in base class hence in child we are initializing all the optional fields. This is as per a previous code review comment for runtime construct ( avoid duplicate type cast wherever possible). The protocolConfiguration and authorizerConfig are required fields and cannot be assigned to undefined hence they are cast in child class too.

[kumvprat]

Since the role in the original resource can be a customer provided role, hardcoding it to ManagedPolicy is not correct in a referenced version of Gateway resource.

[dineshSajwan]

Agreed , wrong commit. we actually use fromGatewayAttributes to import an existing resource . we don't need fromGatewayArn because it ultimately use fromGatewayAttributes. Removing fromGatewayArn.

[kumvprat]

I am confused here
Why not restrict the permissions here to take in a resource field ? That way we can scope it down to a specific resource inside the source account ?
Or why do we need to scope it down like this we already have limited via a service principal.

Just curious around the ideology behind scoping down the permissions via the Conditions field

On the same lines : Is it possible for gateway in one account to invoke a target in another account. If so will this role create any issues, since we are tying it to a SourceAccount?

[dineshSajwan]

The trust policy conditions follow the official AWS Bedrock AgentCore Gateway documentation.

The documentation explicitly requires these conditions in the trust policy:
StringEquals: 'aws:SourceAccount' - Ensures only gateways from this account can assume the role
ArnLike: 'aws:SourceArn' - Scopes to specific gateway resources.
I think this is for to prevent the 'confused deputy' security issue where a malicious actor could potentially trick the bedrock-agentcore service into using our role via their gateway in a different account.

Cross account access - No, this will not create any issue.
This condition only check trust policy ( who can assume the role) meaning only gateways from Account A can assume this role.
The cross account permission check (WHAT the role can do once assumed) happen at invoke time , example - //

// Gateway role (in account A) needs permissions to cross-account target
gateway.role.addToPolicy(new iam.PolicyStatement({
  actions: ['lambda:InvokeFunction'],
  resources: ['arn:aws:lambda:us-east-1:222222222222:function/TargetFunc']
}));

// Lambda (in account B) must allow Account A's gateway role
targetFunction.grantInvoke(
  iam.Role.fromRoleArn(this, 'GatewayRole', 
    'arn:aws:iam::111111111111:role/GatewayRole'
  )
);

[kumvprat]

Since this creating an authorizer config can we change the name to createDefaultCognitoAuthorizerConfig or something similar ?

[dineshSajwan]

Ack.

[kumvprat]

I dont think we need to redirect call from one public static function to another. We can directly use the this one and shift the logic here to ensure consistency in code design

[dineshSajwan]

Ack. Didn't updated the old code. removed fromCognito and directly using usingCognito

[kumvprat]

Again if it is a provider itself we should remove configuration from the class name

[dineshSajwan]

Updated ICredentialProvider to ICredentialProviderConfig

[kumvprat]

Why are we not using GatewayTargetProps as the base class for each of the target types ?

[dineshSajwan]

We follow this structure

McpGatewayTargetCommonProps       // ← Common base for both generic and specific targets
└──GatewayTargetProps                  // ← For generic use
└── GatewayTargetLambdaProps   // ← For Lambda Target-specific use
            GatewayTargetOpenApiProps // ← For open API target-specific use
            GatewayTargetSmithyProps    //← For Smithy target-specific use

we have convenience methods to create specific target like lambda, openapi and smithy. These specific target have fixed targetconfiguration while GatewayTargetProps can be used to create any target with generic constructor with targetconfiguration.
The McpGatewayTargetCommonProps help us avoiding duplication for common properties.

[kumvprat]

Why not just keep : GatewayTargetProps

And provide 3 helper functions inside the above class that allows anyone to create 3 different types of GatewayProps

Basically moving the stuff here : Code link to config classes and that way we can keep GatewayTarget simple and all the abstraction resides on the configuration class.

[kumvprat]

Example :

export interface LambdaGatewayTargetProps extends GatewayTargetProps {
...
constructor(
    gatewayTargetName: string,
    credentialProviderConfigurations: ICredentialProviderConfig,
    lambdaFunction: IFunction,
    toolSchema: IToolSchema,
    description: string
  ) {
    super(
      gatewayTargetName,
      description,
      credentialProviderConfigurations,
      targetConfiguration: LambdaTargetConfiguration.create(
        lambdaFunction,
        toolSchema,
      ),
    });
  }
}

Now in GatewayTarget :

export class GatewayTarget extends GatewayTargetBase implements IMcpGatewayTarget {
...
public static forLambda(
    scope: Construct,
    id: string,
    props: LambdaGatewayTargetProps,
  ): GatewayTarget {
    return new GatewayTarget(scope, id,  props);
  }

}

[kumvprat]

This function seems too bloated.

I assume all the validations here are necessary but clubbing them together doesnot really allow for maintainable code.

Can we break into smaller function chunks ? We can encapsulate the overall logic into smaller functions that handle a specific part of validation. (I have not read the whole function details and assumption behind this comment is that all the validation are mutually-exclusive )

[dineshSajwan]

Ack.

[dineshSajwan]

yes they are mutually exclusive and don't interfere with each other

[kumvprat]

Since this is a cross-service integration we should have assertion logic that check that the gateway is properly tied to the Lambda function and is able to invoke the lambda properly when a user request comes in.

[kumvprat]

Some examples here : https://github.com/aws/aws-cdk/blob/76e0fa0ecf60b1d3167ed4f6421fe1395be73fd0/INTEGRATION_TESTS.md#assertions

[dineshSajwan]

Ack

[kumvprat]

This and the integ test above seem to be duplicate : packages/@aws-cdk/aws-bedrock-agentcore-alpha/test/agentcore/gateway/integ.gateway.ts

[kumvprat]

Added a comment for GatewayTargetProps structure

[kumvprat]

Why not just keep : GatewayTargetProps

And provide 3 helper functions inside the above class that allows anyone to create 3 different types of GatewayProps

Basically moving the stuff here : Code link to config classes and that way we can keep GatewayTarget simple and all the abstraction resides on the configuration class.

[kumvprat]

Example :

export interface LambdaGatewayTargetProps extends GatewayTargetProps {
...
constructor(
    gatewayTargetName: string,
    credentialProviderConfigurations: ICredentialProviderConfig,
    lambdaFunction: IFunction,
    toolSchema: IToolSchema,
    description: string
  ) {
    super(
      gatewayTargetName,
      description,
      credentialProviderConfigurations,
      targetConfiguration: LambdaTargetConfiguration.create(
        lambdaFunction,
        toolSchema,
      ),
    });
  }
}

Now in GatewayTarget :

export class GatewayTarget extends GatewayTargetBase implements IMcpGatewayTarget {
...
public static forLambda(
    scope: Construct,
    id: string,
    props: LambdaGatewayTargetProps,
  ): GatewayTarget {
    return new GatewayTarget(scope, id,  props);
  }

}