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.

Sign up for GitHub

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

Jump to bottom

feat(elbv2): Supports new types of listener rule conditions #7848

Merged
merged 24 commits into from
May 21, 2020
Merged

feat(elbv2): Supports new types of listener rule conditions #7848

merged 24 commits into from
May 21, 2020

Conversation

karupanerura
Copy link
Contributor

@karupanerura karupanerura commented May 7, 2020 •
edited
Loading

Commit Message

feat(elbv2): Supports new types of listener rule conditions

Fixes #3888

End Commit Message

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

karupanerura added 14 commits May 7, 2020 23:57
@karupanerura
test(elbv2): expect to render new style listener rule condition
7247360
@karupanerura
feat(elbv2): supports new style listener rule conditions
b325bbc
@karupanerura
test(elbv2): add test cases for new style listener rule condition tes…
0351d81 
…t cases
@karupanerura
refactor(elbv2): replate setCondition to addCondition
8b4f53e
@karupanerura
fix(elbv2): remove meaningless/broken validation
9ac59e5
@karupanerura
test(elbv2): fix aws-ecs-patterns test case
5c73487
@karupanerura
doc(elbv2): add example to use addCondition method
8de2fd7
@karupanerura
Revert "test(elbv2): expect to render new style listener rule condition"
c67387e 
This reverts commit d3a057d.
@karupanerura
Revert "test(elbv2): fix aws-ecs-patterns test case"
cc5841b 
This reverts commit 051cc91.
@karupanerura
refactor(elbv2): reduce exported implements for simple
74cfa70
@karupanerura
fix(elbv2): keep legacy cfn template format for deprecated interfaces
318d82b
@karupanerura
refactor(elbv2): remove unnecessary removeCondition method
07c4ae3
@karupanerura
refactor(elbv2): add conditions property for supports new style lis…
76a939d 
…tenr rule conditon interface
@karupanerura
doc(elbv2): use conditions in example
f5f397c
@karupanerura karupanerura mentioned this pull request May 7, 2020
Closed
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: f5f397c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 5bfd267
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

rix0rrr
rix0rrr previously requested changes May 14, 2020
View reviewed changes
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Almost there!

packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts Outdated
/**
* Properties for the key/value pair of the query string
*/
export interface QueryStringConditionProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

QueryStringCondition please.

packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts Outdated
/**
* Interface for a listener rule condition
*/
export abstract class ListenerRuleCondition {
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel like this could be ListenerCondition, not as precise but I like shorter names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please put the conditions in a separate file conditions.ts.

packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts Outdated
/**
* Add a non-standard condition to this rule
*
* If the condition conflicts with an already set condition, it will be overwritten by the one you specified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that still true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops! this is wrong. thank you!

packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts Outdated
* Host header config of the listener rule condition
*/
class HostHeaderListenerRuleCondition extends ListenerRuleCondition {
public readonly field = 'host-header';
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe field is ever used outside this class anymore, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree! removed it

@mergify mergify bot dismissed rix0rrr’s stale review May 15, 2020 11:00

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 0d4b73a
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@karupanerura karupanerura requested a review from rix0rrr May 15, 2020 13:43
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 173e89b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@karupanerura
Copy link
Contributor Author

fixed the conflict at 173e89b

@mergify
Copy link
Contributor

mergify bot commented May 21, 2020

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 CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: d0df699
  • 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 bot commented May 21, 2020

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 CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 679dd32
  • 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 bot commented May 21, 2020

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 CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 9c43d31
  • 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 bot commented May 21, 2020

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 3d30ffa into aws:master May 21, 2020
@karupanerura karupanerura deleted the feature/supports-new-alb-listener-rule2 branch May 21, 2020 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rix0rrr rix0rrr rix0rrr approved these changes

Assignees

@rix0rrr rix0rrr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ALB: New types of listener rule conditions
3 participants
@karupanerura @aws-cdk-automation @rix0rrr