-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
docs(ec2): update aws-ec2
README to include workaround for using a service principal in VPCEService allowedPrincipals
#29512
Conversation
…ncipal in VPCEService `allowedPrincipals` `VpcEndpointService` has the member `allowedPrincipals` which is of type `ArnPrincipal[]`. However, `ServicePrincipal` is also valid and works in the AWS console. This documentation update includes a workaround for including service principals in the `allowedPrincipals`.
There was a problem hiding this 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.
aws-ec2
README to include workaround for using a service pri…aws-ec2
README to include workaround for using a service principal in VPCEService allowedPrincipals
(#29478)
aws-ec2
README to include workaround for using a service principal in VPCEService allowedPrincipals
(#29478)aws-ec2
README to include workaround for using a service principal in VPCEService allowedPrincipals
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
Can you please add test coverage for this scenario for both unit and integration tests?
To include a service principal in the `allowedPrincipals`, there is a workaround where you can use a service principal string as input to the `ArnPrincipal` type. The resulting VPC endpoint will have an allowlisted principal of type `Service`, instead of `Arn` for that item in the list. | ||
```ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To include a service principal in the `allowedPrincipals`, there is a workaround where you can use a service principal string as input to the `ArnPrincipal` type. The resulting VPC endpoint will have an allowlisted principal of type `Service`, instead of `Arn` for that item in the list. | |
```ts | |
You can also include a service principal in the `allowedPrincipals` property by specifying it as a parameter to the `ArnPrincipal` constructor. | |
The resulting VPC endpoint will have an allowlisted principal of type `Service`, instead of `Arn` for that item in the list. | |
```ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make this change in the revision that includes the tests.
Will do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same feedback on the test coverage. Otherwise happy to approve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
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). |
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). |
@Mergifyio update |
❌ Mergify doesn't have permission to updateFor security reasons, Mergify can't update this pull request. Try updating locally. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
…service principal in VPCEService `allowedPrincipals` (aws#29512) `VpcEndpointService` has the member `allowedPrincipals` which is of type `ArnPrincipal[]`. However, `ServicePrincipal` is also valid and works in the AWS console. This documentation update includes a workaround for including service principals in the `allowedPrincipals`. ### Issue aws#29478 Closes aws#29478 ### Reason for this change `VpcEndpointService` has the member `allowedPrincipals` which is of type `ArnPrincipal[]`. However, if you use the AWS console, allowlisting a service principal is supported as well. Users are not able to use the type `ServicePrincipal` in `allowedPrincipals` in CDK. This is a feature gap. I brought this up in aws#29478, and was told that the type couldn't be changed, but the workaround I was using could be added to the documentation. ### Description of changes Documentation update for the `aws-ec2` module which includes a workaround for including service principals in the `allowedPrincipals`. ### Description of how you validated changes N/A - minor documentation changes only ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…service principal in VPCEService `allowedPrincipals` (aws#29512) `VpcEndpointService` has the member `allowedPrincipals` which is of type `ArnPrincipal[]`. However, `ServicePrincipal` is also valid and works in the AWS console. This documentation update includes a workaround for including service principals in the `allowedPrincipals`. ### Issue aws#29478 Closes aws#29478 ### Reason for this change `VpcEndpointService` has the member `allowedPrincipals` which is of type `ArnPrincipal[]`. However, if you use the AWS console, allowlisting a service principal is supported as well. Users are not able to use the type `ServicePrincipal` in `allowedPrincipals` in CDK. This is a feature gap. I brought this up in aws#29478, and was told that the type couldn't be changed, but the workaround I was using could be added to the documentation. ### Description of changes Documentation update for the `aws-ec2` module which includes a workaround for including service principals in the `allowedPrincipals`. ### Description of how you validated changes N/A - minor documentation changes only ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
VpcEndpointService
has the memberallowedPrincipals
which is of typeArnPrincipal[]
. However,ServicePrincipal
is also valid and works in the AWS console. This documentation update includes a workaround for including service principals in theallowedPrincipals
.Issue #29478
Closes #29478
Reason for this change
VpcEndpointService
has the memberallowedPrincipals
which is of typeArnPrincipal[]
. However, if you use the AWS console, allowlisting a service principal is supported as well. Users are not able to use the typeServicePrincipal
inallowedPrincipals
in CDK. This is a feature gap.I brought this up in #29478, and was told that the type couldn't be changed, but the workaround I was using could be added to the documentation.
Description of changes
Documentation update for the
aws-ec2
module which includes a workaround for including service principals in theallowedPrincipals
.Description of how you validated changes
N/A - minor documentation changes only
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license