Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behavior across AWS constructs #31080

Open
95Wictor opened this issue Aug 11, 2024 · 3 comments
Open

Inconsistent behavior across AWS constructs #31080

95Wictor opened this issue Aug 11, 2024 · 3 comments
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@95Wictor
Copy link

I opened another issue, #31059, describing a situation where the grantInvokeUrl method for Lambda functions raises an exception during synthesis, in a way that is inconsistent with other grant methods (of other constructs, and even of other grant methods of Lambda functions). This was closed, so I can't do anything there, and I believe closing it was a mistake. The person who closed the issue stated that there are source-code comments related to some decisions made by the developers. With the implication that this means it is not a bug. However, they did not provide any evidence that the developers were aware that their decisions would lead to inconsistent behavior across constructs. This inconsistency should be considered a bug, and I believe should be considered an issue even if the developers were aware of it.

AWS documentation at https://docs.aws.amazon.com/cdk/v2/guide/constructs.html states:

the AWS constructs that are included with the AWS Construct Library, such as s3.Bucket, follow guidelines and common patterns. This provides a consistent experience across all AWS resources.

Most AWS constructs have a set of grant methods that you can use to grant AWS Identity and Access Management (IAM) permissions on that construct to a principal.

Behaviors of AWS provided constructs should be consistent across all resource. It even mentions grant methods as an example. Thus any decision that leads to significant inconsistency is a bad decision.

@moelasmar moelasmar added the needs-triage This issue or PR still needs to be triaged. label Aug 28, 2024
@pahud pahud assigned pahud and ashishdhingra and unassigned pahud Aug 28, 2024
@ashishdhingra ashishdhingra added bug This issue is a bug. @aws-cdk/aws-lambda Related to AWS Lambda p2 labels Aug 28, 2024
@ashishdhingra
Copy link
Contributor

Needs input from team to review findings in comment #31059 (comment), reevaluate the design and advise if we could get away with memoizing the result in lambda.grantInvokeUrl().

@ashishdhingra ashishdhingra added effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2024
@ashishdhingra ashishdhingra removed their assignment Aug 29, 2024
@scorbiere
Copy link
Contributor

I think this is fair to call it a bug.

Comparing with other grant method in the function-base.ts file we could change the strategy that generate the identifier (use a hash).

I have a unit test and a code change that I used to validate the analysis. So I will re-use that to open a PR.

@scorbiere
Copy link
Contributor

Update

I tried to add an integration test to validate that the example provided could be successfully executed by CloudFormation, but it seems that this cannot work. My understanding is that we should not use ArnPrincipal with the function ARN.

ArnPrincipal doc

Specify a principal by the Amazon Resource Name (ARN).
You can specify AWS accounts, IAM users, Federated SAML users, IAM roles, and specific assumed-role sessions.
You cannot specify IAM groups or instance profiles as principals

Integration test

Test

class TestStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const granterFunction = new lambda.Function(this, 'GranterFunction', {
      runtime: STANDARD_NODEJS_RUNTIME,
      handler: 'index.handler',
      code: new lambda.InlineCode('foo'),
    });

    const receiverFunction = new lambda.Function(this, 'ReceiverFunction', {
      runtime: STANDARD_NODEJS_RUNTIME,
      handler: 'index.handler',
      code: new lambda.InlineCode('bar'),
    });

    const arnPrincipal = new iam.ArnPrincipal(receiverFunction.functionArn);
    granterFunction.grantInvokeUrl(arnPrincipal);
  }
}

const app = new cdk.App();
const stack = new TestStack(app, 'LambdaGrantInvokeUrlTest');

new IntegTest(app, 'LambdaGrantInvokeUrlIntegTest', {
  testCases: [stack],
});

Template generated

{
  "Resources": {
    "GranterFunctionServiceRole949BE0CB": {
      "Type": "AWS::IAM::Role",
      "Properties": {
        "AssumeRolePolicyDocument": {
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Effect": "Allow",
              "Principal": {
                "Service": "lambda.amazonaws.com"
              }
            }
          ],
          "Version": "2012-10-17"
        },
        "ManagedPolicyArns": [
          {
            "Fn::Join": [
              "",
              [
                "arn:",
                {
                  "Ref": "AWS::Partition"
                },
                ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
              ]
            ]
          }
        ]
      }
    },
    "GranterFunctionEED7FA05": {
      "Type": "AWS::Lambda::Function",
      "Properties": {
        "Code": {
          "ZipFile": "foo"
        },
        "Handler": "index.handler",
        "Role": {
          "Fn::GetAtt": [
            "GranterFunctionServiceRole949BE0CB",
            "Arn"
          ]
        },
        "Runtime": "nodejs18.x"
      },
      "DependsOn": [
        "GranterFunctionServiceRole949BE0CB"
      ]
    },
    "GranterFunctionInvokeFunctionUrl7LNpuRYF5DJkGhbBhmCh3ZbWBp5bIdFisYY4UJz0g73A65FFB": {
      "Type": "AWS::Lambda::Permission",
      "Properties": {
        "Action": "lambda:InvokeFunctionUrl",
        "FunctionName": {
          "Fn::GetAtt": [
            "GranterFunctionEED7FA05",
            "Arn"
          ]
        },
        "FunctionUrlAuthType": "AWS_IAM",
        "Principal": {
          "Fn::GetAtt": [
            "ReceiverFunctionAA322E73",
            "Arn"
          ]
        }
      }
    },
    "ReceiverFunctionServiceRole239E0A06": {
      "Type": "AWS::IAM::Role",
      "Properties": {
        "AssumeRolePolicyDocument": {
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Effect": "Allow",
              "Principal": {
                "Service": "lambda.amazonaws.com"
              }
            }
          ],
          "Version": "2012-10-17"
        },
        "ManagedPolicyArns": [
          {
            "Fn::Join": [
              "",
              [
                "arn:",
                {
                  "Ref": "AWS::Partition"
                },
                ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
              ]
            ]
          }
        ]
      }
    },
    "ReceiverFunctionAA322E73": {
      "Type": "AWS::Lambda::Function",
      "Properties": {
        "Code": {
          "ZipFile": "bar"
        },
        "Handler": "index.handler",
        "Role": {
          "Fn::GetAtt": [
            "ReceiverFunctionServiceRole239E0A06",
            "Arn"
          ]
        },
        "Runtime": "nodejs18.x"
      },
      "DependsOn": [
        "ReceiverFunctionServiceRole239E0A06"
      ]
    }
  },
  "Parameters": {
    "BootstrapVersion": {
      "Type": "AWS::SSM::Parameter::Value<String>",
      "Default": "/cdk-bootstrap/hnb659fds/version",
      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
    }
  },
  "Rules": {
    "CheckBootstrapVersion": {
      "Assertions": [
        {
          "Assert": {
            "Fn::Not": [
              {
                "Fn::Contains": [
                  [
                    "1",
                    "2",
                    "3",
                    "4",
                    "5"
                  ],
                  {
                    "Ref": "BootstrapVersion"
                  }
                ]
              }
            ]
          },
          "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
        }
      ]
    }
  }
}

Error

Failed resources:
LambdaGrantInvokeUrlTest | 2:18:08 p.m. | CREATE_FAILED | AWS::Lambda::Permission |
GranterFunction/InvokeFunctionUrl7LNpuRYF5DJk+GhbBhmCh3ZbWBp5bId+FisYY4UJz0g= (GranterFunctionInvokeFunctionUrl7LNpuRYF5DJkGhbBhmCh3ZbWBp5bIdFisYY4UJz0g73A65FFB) Resource handler returned message: "The provided input'arn:aws:lambda:us-east-1:<MY_ACCOUNT>:function:LambdaGrantInvokeUrlTest-ReceiverFunctionAA322E73-uXoCbILnM8VM'was invalid. Member must satisfy expression: [\w+=,.@-]* Please check your input and try again. (Service: Lambda, Status Code: 400, Request ID: 2cde7706-918a-4a2d-bf80-0e1d4449317d)" (RequestToken: cf884570-b634-d55b-21f9-4332c80ce029, HandlerErrorCode: InvalidRequest)

Conclusion

I can publish the fix that will handle the Token in the identifier, but it will not help you make your initial example work.

I think what you provided as a workaround is closer to what should be the correct way to setup the permission between the 2 lambda.

As a reminder:

		// To get around this issue, we can use a conditional principal
		const conditionalPrincipal = new ServicePrincipal('lambda.amazonaws.com').withConditions({
			// Creating the condition has its own problems, but these would be separate issues
			// 1) The AWS here MUST be lowercase, despite it showing uppercase in the AWS console.
			//    https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition.html (It should be case-insensitive.)
			// 2) We also cannot use StringEquals for Lambda, though we if the bucket was granting to this principal.
			"ArnLike": { "aws:SourceArn": lambdaThatWillInvoke.functionArn }
		});
		lambdaThatWillGrant.grantInvokeUrl(conditionalPrincipal); // works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

5 participants