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(globalaccelerator): stabilize AWS Global Accelerator module #13843

Merged
merged 10 commits into from Mar 30, 2021

Conversation

@rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Mar 29, 2021

There are a number of changes to this module, made in order to stabilize it. The changes are as follows:

  • Endpoints as constructs would only work in TypeScript; they have been moved out as integration classes into
    aws-globalaccelerator-endpoints in order to support languages like Java and C#.
  • The automatic naming algorithm has been changed to reduce chances of conflict.
  • There are now convenience methods, addListener() and addEndpointGroup() that will create
    the appropriate objects, as alternatives to new Listener() and new EndpointGroup().
  • EndpointGroups can take a list of endpoints in the constructor.
  • A Listener's toPort is optional (and defaults to fromPort if not supplied).
  • Support all the EndpointGroup properties.
  • An EndpointGroup's region is automatically determined from its configured endpoints, if possible.
  • The looked-up SecurityGroup is no longer accessible as a full Security Group, it can just
    be reference as a Peer (modifying the rules is not recommended by AGA and should not be allowed
    from the CDK).

Changes to other libraries made to support this:

  • core, elbv2: imported Load Balancers now are aware of the region and account they were actually imported from, in
    order to be able to make region implicit in the AGA API.

BREAKING CHANGE: automatic naming algorithm has been changed: if you have existing Accelerators you will need to pass an
explicit name to prevent them from being replaced. All endpoints are now added by calling addEndpoint() with a
target-specific class that can be found in @aws-cdk/aws-globalaccelerator-endpoints. The generated Security Group
is now looked up by calling endpointGroup.connectionsPeer().


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

rix0rrr added 5 commits Mar 26, 2021
BREAKING CHANGE: the automatic name of the `Accelerator` construct has
changed; explicitly pass an `acceleratorName` property if you need your
accelerator to not be replaced.
@rix0rrr rix0rrr requested a review from aws/aws-cdk-team Mar 29, 2021
@rix0rrr rix0rrr self-assigned this Mar 29, 2021
@gitpod-io
Copy link

@gitpod-io gitpod-io bot commented Mar 29, 2021

@@ -0,0 +1,18 @@
# Event Targets for Amazon EventBridge

This comment has been minimized.

@nija-at

nija-at Mar 29, 2021
Contributor

fix

user traffic coming in on the Listener is ultimately sent. The Endpoint port
used is the same as the traffic came in on at the Listener, unless overridden.

## Types of Endpoints

This comment has been minimized.

@nija-at

nija-at Mar 29, 2021
Contributor

Curious: Why you've chosen this way, instead of a brief summary here and then documenting the actual list in the endpoints package?

This comment has been minimized.

@rix0rrr

rix0rrr Mar 30, 2021
Author Contributor

Yeah, it wasn't an easy choice. Ultimately I decided that looking in 1 place is better for users than forcing them to look in 2 places, and the list wasn't impressive enough that we'd gain clarity from splitting it up.

This comment has been minimized.

@nija-at

nija-at Mar 30, 2021
Contributor

Sounds good.

I was curious because I've done the opposite with APIGWv2.

@nija-at
Copy link
Contributor

@nija-at nija-at commented Mar 29, 2021

Obviously, I've not gone through all of the code. just the configuration and skimmed through the README.

I trust 'ya.

@mergify
Copy link
Contributor

@mergify mergify bot commented Mar 30, 2021

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

@aws-cdk-automation
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation commented Mar 30, 2021

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 3f56dc5
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

@mergify mergify bot commented Mar 30, 2021

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

@mergify mergify bot merged commit 8571008 into master Mar 30, 2021
7 checks passed
7 checks passed
@github-actions
auto-approve
Details
@github-actions
validate-pr
Details
AWS CodeBuild us-east-1 (AutoBuildProject89A8053A-LhjRyN9kxr8o) Build succeeded for project AutoBuildProject89A8053A-LhjRyN9kxr8o
Details
@gitpod-io
Gitpod Open an online workspace in Gitpod
Details
@mergify
Rule: automatic merge (merge) The pull request has been merged automatically
Details
@semantic-pull-requests
Semantic Pull Request ready to be squashed
Details
@mergify
Summary 1 rule matches and 5 potential rules
Details
@mergify mergify bot deleted the huijbers/globalacc branch Mar 30, 2021
hollanddd added a commit to hollanddd/aws-cdk that referenced this pull request Mar 31, 2021
…#13843)

There are a number of changes to this module, made in order to stabilize it. The changes are as follows:

* Endpoints as constructs would only work in TypeScript; they have been moved out as integration classes into
  `aws-globalaccelerator-endpoints` in order to support languages like Java and C#.
* The automatic naming algorithm has been changed to reduce chances of conflict.
* There are now convenience methods, `addListener()` and `addEndpointGroup()` that will create
  the appropriate objects, as alternatives to `new Listener()` and `new EndpointGroup()`.
* EndpointGroups can take a list of `endpoints` in the constructor.
* A Listener's `toPort` is optional (and defaults to `fromPort` if not supplied).
* Support all the EndpointGroup properties.
* An EndpointGroup's `region` is automatically determined from its configured endpoints, if possible.
* The looked-up SecurityGroup is no longer accessible as a full Security Group, it can just
  be reference as a Peer (modifying the rules is not recommended by AGA and should not be allowed
  from the CDK).


Changes to other libraries made to support this:

* core, elbv2: imported Load Balancers now are aware of the region and account they were actually imported from, in
  order to be able to make `region` implicit in the AGA API.

BREAKING CHANGE: automatic naming algorithm has been changed: if you have existing Accelerators you will need to pass an
explicit name to prevent them from being replaced. All endpoints are now added by calling `addEndpoint()` with a
target-specific class that can be found in `@aws-cdk/aws-globalaccelerator-endpoints`. The generated Security Group
is now looked up by calling `endpointGroup.connectionsPeer()`.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants