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

[aws-elasticloadbalancingv2] Imported listener conditions attribute is ignored #9643

Closed
danilo-valente opened this issue Aug 12, 2020 · 1 comment · Fixed by #9939
Closed
Assignees
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. duplicate This issue is a duplicate. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@danilo-valente
Copy link

This bug is related to #8328, but instead occurs only to existing (imported) listeners.

When attempting to add a rule to an importer listener on an application load balancer, it is not possible to specify multiple conditions with the addTargetGroups helper, because it ignores the conditions array of the input.

Workarounds:

  • Use deprecated attributes like hostHeader
  • Construct the new ApplicationListenerRule directly

Reproduction Steps

Check file application-listener.ts on release v1.57.0. Look for method addTargetGroups of class ImportedApplicationListener and you will note that a conditions attribute is not passed to new ApplicationListenerRule:

  /**
   * Load balance incoming requests to the given target groups.
   *
   * It's possible to add conditions to the TargetGroups added in this way.
   * At least one TargetGroup must be added without conditions.
   */
  public addTargetGroups(id: string, props: AddApplicationTargetGroupsProps): void {
    checkAddRuleProps(props);

    if (props.priority !== undefined) {
      // New rule
      new ApplicationListenerRule(this, id, {
        listener: this,
        hostHeader: props.hostHeader,
        pathPattern: props.pathPattern,
        pathPatterns: props.pathPatterns,
        priority: props.priority,
        targetGroups: props.targetGroups,
      });
    } else {
      throw new Error('Cannot add default Target Groups to imported ApplicationListener');
    }
  }

What did you expect to happen?

To properly use conditions attribute of a target group on existing listeners.

What actually happened?

conditions is ignored and if no other attribute (like hostHeader) is provided it throws an error: At least one of 'conditions', 'hostHeader', 'pathPattern' or 'pathPatterns' is required when defining a load balancing rule..

Environment

  • CLI Version : 1.57.0
  • Framework Version: 1.57.0
  • Node.js Version: 14.4.0
  • OS : 10.14.5
  • Language (Version): 3.9.6

Other

This bug is related to #8328 and was probably left behind by #8385.


This is 🐛 Bug Report

@danilo-valente danilo-valente added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2020
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Aug 12, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 17, 2020

Duplicate of #9262

@rix0rrr rix0rrr marked this as a duplicate of #9262 Aug 17, 2020
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 labels Aug 17, 2020
@SomayaB SomayaB added duplicate This issue is a duplicate. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 18, 2020
@SomayaB SomayaB closed this as completed Aug 18, 2020
njlynch added a commit that referenced this issue Aug 24, 2020
…ribute

New conditions attribute usage was fixed in #8385 for owned listeners, but
missed imported listeners.

fixes #9262
fixes #9320
fixes #9643
mergify bot pushed a commit that referenced this issue Aug 24, 2020
…ribute (#9939)

New conditions attribute usage was fixed in #8385 for owned listeners, but
missed imported listeners.

fixes #9262
fixes #9320
fixes #9643


----

*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
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. duplicate This issue is a duplicate. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants