chore(lambda): resolve unable to reference AuthType from FunctionUrl by watany-dev · Pull Request #31590 · aws/aws-cdk

watany-dev · 2024-09-28T12:28:21Z

Reason for this change

I will resolve this issue: #31339 (comment). In the current FunctionURL implementation, there is no way to access AuthType, and therefore, when writing logic that depends on AuthType, there is no method to reference it.

Description of changes

I will fix the construct to allow access to functionurl.AuthType

Description of how you validated changes

adding unittest and integ-test re-run.

Reason for Exemption:

The fix introduces no changes to the resources being created.

Clarification Request:

This fix only makes the internal property authType of the L2 Construct publicly accessible, and does not introduce any differences in the created resources.

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. 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.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

badmintoncryer

Thank you for your PR! I've added only a small comment.

badmintoncryer · 2024-09-30T03:29:34Z

packages/aws-cdk-lib/aws-lambda/test/function-url.test.ts

+    if (fnUrl.authType === lambda.FunctionUrlAuthType.NONE) {
+      ; // ok
+    } else {
+      throw new Error('AuthType must be NONE');
+    }


I think it is better to use jest expect().

Suggested change

if (fnUrl.authType === lambda.FunctionUrlAuthType.NONE) {

; // ok

} else {

throw new Error('AuthType must be NONE');

}

expect(fnUrl.authType).toBe(lambda.FunctionUrlAuthType.NONE);

Agreed. Also, I believe this PR should be a fix not a chore

Since there is no change in the resource being created, I don't think this fix needs an integ test, but if the maintainer thinks it does, I'll change it to a fix.

badmintoncryer

Thanks!

godwingrs22

Thanks @watany-dev for the contribution. Overall LGTM.

Just to make sure this change doesn't break anything, could you please rerun the existing integ test and try to deploy the stack manually and make sure the template is deploying successfully. Also validate if there is any change in the snapshot required. I hope it doesn't.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. 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.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

watany-dev · 2024-10-03T15:18:57Z

I reran the integration test and confirmed it was successful. Since the test contents haven't changed, I reverted the fix back to chore.

godwingrs22 · 2024-10-03T15:39:21Z

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

Not sure why the difference in this feature_flag file is shown here. Could we remove this as i hope it is not related to this change?

sorry. reverted.

godwingrs22 · 2024-10-03T15:45:57Z

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. 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.

Please add a comment to request for exemption of integ test with justification.

godwingrs22 · 2024-10-03T15:46:55Z

I reran the integration test and confirmed it was successful. Since the test contents haven't changed, I reverted the fix back to chore.

Thanks for confirming. Also please update in the Description of how you validated changes section about the testing of integration test you performed.

watany-dev · 2024-10-03T16:05:29Z

@godwingrs22
Thanks for the prompt review. Is this latest text ok?
#31590 (comment)

godwingrs22

Thanks @watany-dev for the contribution.

mergify · 2024-10-03T16:28:04Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-10-03T16:59:38Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: c17f0c1
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-10-03T17:00:08Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-10-03T17:00:26Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

fix(lambda): Resolve unable to reference AuthType from FunctionUrl

799e13b

aws-cdk-automation requested a review from a team September 28, 2024 12:28

github-actions bot added p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Sep 28, 2024

aws-cdk-automation previously requested changes Sep 28, 2024

View reviewed changes

watany-dev changed the title ~~fix(lambda): Resolve unable to reference AuthType from FunctionUrl~~ chore(lambda): resolve unable to reference AuthType from FunctionUrl Sep 28, 2024

Merge branch 'main' into functionUrl.auth

8219cf5

adding docstring

7c6cc23

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 28, 2024

badmintoncryer suggested changes Sep 30, 2024

View reviewed changes

refactor propaty check

e36e09b

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 30, 2024

badmintoncryer approved these changes Sep 30, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Sep 30, 2024

watany-dev mentioned this pull request Oct 1, 2024

feat(cloudfront): function URL origin access control L2 construct #31339

Merged

2 tasks

godwingrs22 self-assigned this Oct 2, 2024

godwingrs22 reviewed Oct 2, 2024

View reviewed changes

success integ

cb9347b

watany-dev changed the title ~~chore(lambda): resolve unable to reference AuthType from FunctionUrl~~ fix(lambda): resolve unable to reference AuthType from FunctionUrl Oct 3, 2024

aws-cdk-automation previously requested changes Oct 3, 2024

View reviewed changes

watany-dev changed the title ~~fix(lambda): resolve unable to reference AuthType from FunctionUrl~~ chore(lambda): resolve unable to reference AuthType from FunctionUrl Oct 3, 2024

godwingrs22 reviewed Oct 3, 2024

View reviewed changes

aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Oct 3, 2024

revert

3dcef93

godwingrs22 approved these changes Oct 3, 2024

View reviewed changes

godwingrs22 added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Oct 3, 2024

Merge branch 'main' into functionUrl.auth

c17f0c1

mergify bot merged commit b9e7855 into aws:main Oct 3, 2024

github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2024

Conversation

watany-dev commented Sep 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this change

Description of changes

Description of how you validated changes

Reason for Exemption:

Clarification Request:

Checklist

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

badmintoncryer left a comment

Choose a reason for hiding this comment

Uh oh!

badmintoncryer Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

sumupitchayan Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

watany-dev Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

badmintoncryer left a comment

Choose a reason for hiding this comment

Uh oh!

godwingrs22 left a comment

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

watany-dev commented Oct 3, 2024

Uh oh!

godwingrs22 Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

watany-dev Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

godwingrs22 commented Oct 3, 2024

Uh oh!

godwingrs22 commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

watany-dev commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

godwingrs22 left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Oct 3, 2024

Uh oh!

aws-cdk-automation commented Oct 3, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Oct 3, 2024

Uh oh!

github-actions bot commented Oct 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

watany-dev commented Sep 28, 2024 •

edited

Loading

godwingrs22 commented Oct 3, 2024 •

edited

Loading

watany-dev commented Oct 3, 2024 •

edited

Loading