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

feat(cdk): expose authorizer id and authorization type #31622

Merged
merged 13 commits into from
Oct 8, 2024

Conversation

JonWallsten
Copy link
Contributor

@JonWallsten JonWallsten commented Oct 2, 2024

Issue # (if applicable)

Closes #31605.

Reason for this change

I need to access the authorizer id and type to be able to import the authorizer in another CDK project

Description of changes

I have added a public readonly variable for the authorizationType that was previously a string in the return object from the bind function.
I have also added a getter that will return the authorizerId after the first binding. If not bound yet it will throw an error.

Description of how you validated changes

I have added unit tests for all changed files.

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 October 2, 2024 14:01
@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Oct 2, 2024
@JonWallsten
Copy link
Contributor Author

@pahud

Copy link
Collaborator

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

Choose a reason for hiding this comment

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

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.

@pahud
Copy link
Contributor

pahud commented Oct 2, 2024

Thank you for your PR.

This PR will need at least one community review/approval before the flow can bump it to pending for maintainer review queue.

Please check CDK Community Reviews for more details
https://github.com/aws/aws-cdk/wiki/CDK-Community-PR-Reviews

@JonWallsten
Copy link
Contributor Author

JonWallsten commented Oct 3, 2024

Edit: Contributing to AWS CDK using Windows is a horrible experience. Switched to Mac.

@JonWallsten JonWallsten force-pushed the expose-authorizer-id-and-type branch from ef489a3 to f0f94fd Compare October 3, 2024 12:01
@JonWallsten JonWallsten changed the title feat(cdk): Exposes authorizer id and authorization type feat(cdk):exposes authorizer id and authorization type Oct 3, 2024
@JonWallsten JonWallsten force-pushed the expose-authorizer-id-and-type branch from f0f94fd to 1613fe0 Compare October 4, 2024 08:20
@JonWallsten JonWallsten force-pushed the expose-authorizer-id-and-type branch from e16afd9 to 6b75152 Compare October 4, 2024 09:45
@JonWallsten JonWallsten changed the title feat(cdk):exposes authorizer id and authorization type feat(cdk): expose authorizer id and authorization type Oct 4, 2024
@pahud
Copy link
Contributor

pahud commented Oct 4, 2024

@JonWallsten My personal favorite is CodeCatalyst DevEnv.

You may find this article helpful FYI
https://community.aws/content/2jhmIXdVWZJy3uJnUfEzEmp652j/contributing-to-aws-cdk

@JonWallsten JonWallsten force-pushed the expose-authorizer-id-and-type branch 2 times, most recently from 513012e to e430118 Compare October 5, 2024 15:31
@JonWallsten JonWallsten force-pushed the expose-authorizer-id-and-type branch from e430118 to 04e35f7 Compare October 5, 2024 18:03
@JonWallsten
Copy link
Contributor Author

JonWallsten commented Oct 6, 2024

@pahud: I'm trying to understand how the integration tests works. I have added som code to the test and built the project. But I can't run the test due to:

Unable to resolve AWS account to use. It must be either configured when you define your CDK Stack, or through the environment
  FAILED     aws-apigatewayv2-authorizers/test/http/integ.lambda-integ.lambda (undefined/us-east-1) 2.932s
      Integration test failed: Error: Command exited with status 1

Since I had to move the development to my personal computer where I don't have an account, I can't seem to run tests. Is there any workaround for this?

Edit: I'm trying out the Free tier CodeCatalyst DevEnv now to see it it will solve my issues. I must say the threshold to get going is pretty steep and high.

Edit2: This is a never ending path of brick walls. This was not an issue on either my Mac nor running the DevContainer outside of CodeCatalyst on my PC.
image

Edit3: Tried to add --verbose flag. Still no clue about what's wrong.
image

Final: Turns out free tier does not have enough memory to build the project...

@GavinZZ
Copy link
Contributor

GavinZZ commented Oct 7, 2024

@JonWallsten Did you have a chance to figure out the integration test issue? Given the previous comments, it seems that you don't have an AWS account to run the test? Is my understanding correct?

@JonWallsten
Copy link
Contributor Author

JonWallsten commented Oct 8, 2024

@JonWallsten Did you have a chance to figure out the integration test issue? Given the previous comments, it seems that you don't have an AWS account to run the test? Is my understanding correct?

@GavinZZ Long story short. So far I've spent 30 minutes on the code and the tests, and 4 days trying to get the project to build and test.
First try: Windows as-is. Did not work at all. (Work computer)
Second try: Windows DevContainer using Windows Subsystem for Linux. Unbearable slow. (Work computer)
Third try: Mac as-is. Everything worked fine with build and test, until I got to integration tests since I don't have a personal AWS account.
Forth try: Windows Using AWS CodeCatalyst DevContainer. Got issues with memory, upped my billing plan and choose a better machine, and it mostly works besided running unit tests in VSCode, and some projects fails to build without any error message. But I could then build them one by one manually. So now I'm back to error message about the integration tests. Will try to add the credentials for my account today.

Edit: I have added the credentials to the credential files and the account to the config file as the default profile. But it does not seem to work. Do I have to configure the integration tests to pick up the credentials/config?

I even tried to add the profile to manually through the cli:
yarn integ test/aws-apigatewayv2-authorizers/test/http/integ.lambda.js --update-on-failed --profiles test

Edit 2: Got it to work by calling aws sso login --profile test

Edit 3: I still have a problem that the build fails after a while.
image
And I can't run ./scripts/run-rosetta.sh without all projects being built. So now I have to build them one by one manually.

@aws-cdk-automation aws-cdk-automation dismissed their stale review October 8, 2024 10:01

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

@mergify mergify bot dismissed GavinZZ’s stale review October 8, 2024 12:45

Pull request has been modified.

@JonWallsten JonWallsten requested a review from GavinZZ October 8, 2024 13:16
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 8, 2024
Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

Copy link
Contributor

mergify bot commented Oct 8, 2024

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 aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 8, 2024
@GavinZZ
Copy link
Contributor

GavinZZ commented Oct 8, 2024

@JonWallsten Did you have a chance to figure out the integration test issue? Given the previous comments, it seems that you don't have an AWS account to run the test? Is my understanding correct?

@GavinZZ Long story short. So far I've spent 30 minutes on the code and the tests, and 4 days trying to get the project to build and test. First try: Windows as-is. Did not work at all. (Work computer) Second try: Windows DevContainer using Windows Subsystem for Linux. Unbearable slow. (Work computer) Third try: Mac as-is. Everything worked fine with build and test, until I got to integration tests since I don't have a personal AWS account. Forth try: Windows Using AWS CodeCatalyst DevContainer. Got issues with memory, upped my billing plan and choose a better machine, and it mostly works besided running unit tests in VSCode, and some projects fails to build without any error message. But I could then build them one by one manually. So now I'm back to error message about the integration tests. Will try to add the credentials for my account today.

Edit: I have added the credentials to the credential files and the account to the config file as the default profile. But it does not seem to work. Do I have to configure the integration tests to pick up the credentials/config?

I even tried to add the profile to manually through the cli: yarn integ test/aws-apigatewayv2-authorizers/test/http/integ.lambda.js --update-on-failed --profiles test

Edit 2: Got it to work by calling aws sso login --profile test

Edit 3: I still have a problem that the build fails after a while. image And I can't run ./scripts/run-rosetta.sh without all projects being built. So now I have to build them one by one manually.

Wow, glad that you're finally able to make it build and work. Windows are known to have issues with CDK development unfortunately, and we really need to improve it for better experiences. I'll reach out internally to see if there's any items we could do to improve Windows user experience.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@mergify mergify bot merged commit daaf0aa into aws:main Oct 8, 2024
15 checks passed
Copy link
Contributor

mergify bot commented Oct 8, 2024

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).

Copy link

github-actions bot commented Oct 8, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-apigatewayv2-authorizers): Expose Authorizer ID to make it reusable from other projects
4 participants