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-elb: Disabling cross zone load balancing is not respected in Java CDK #29866

Closed
codypenta opened this issue Apr 17, 2024 · 8 comments · Fixed by #29907
Closed

aws-elb: Disabling cross zone load balancing is not respected in Java CDK #29866

codypenta opened this issue Apr 17, 2024 · 8 comments · Fixed by #29907
Assignees
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug.

Comments

@codypenta
Copy link

Describe the bug

        ApplicationLoadBalancer loadBalancer = ApplicationLoadBalancer.Builder.create(this, "ApplicationLoadBalancer")
                .vpc(vpc)
                .crossZoneEnabled(false) // BUG: This is not respected and cross zone is enabled
                .desyncMitigationMode(DesyncMitigationMode.DEFENSIVE)
                .internetFacing(true)
                .build();

Expected Behavior

I expect cross zone to be disabled or a warning why it was enabled

Current Behavior

Cross zone load balancer is not respected

Reproduction Steps

See source

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.134.0

Framework Version

2

Node.js Version

v21.7.1

OS

MacOS

Language

Java

Language Version

Java (17)

Other information

No response

@codypenta codypenta added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2024
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Apr 17, 2024
@codypenta
Copy link
Author

codypenta commented Apr 17, 2024

Turn's out this is a feature request. While ALB does support cross zone load balancing, it's different than NLBs. You must enable it at the target group level with TargetAttributes. TargetAttributes are not available in the L2 construct for TargetGroups

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html

EDIT:

Workaround

        CfnTargetGroup cfnTargetGroup = (CfnTargetGroup) targetGroup.getNode().getDefaultChild();
        cfnTargetGroup.addPropertyOverride("TargetGroupAttributes", List.of(
                Map.of(
                        "Key", "load_balancing.cross_zone.enabled",
                        "Value", "false"
                )
        ));

@ashishdhingra ashishdhingra self-assigned this Apr 17, 2024
@ashishdhingra ashishdhingra added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2024
@ashishdhingra
Copy link
Contributor

ashishdhingra commented Apr 17, 2024

@codypenta Good morning. While I'm able to reproduce the issue with the following code (using TypeScript for comparison):

import { Stack, StackProps } from 'aws-cdk-lib';
import * as autoscaling from 'aws-cdk-lib/aws-autoscaling';
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2';
import { Construct } from 'constructs';

export class TypescriptStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const vpc = new ec2.Vpc(this, 'VPC');
    const asg = new autoscaling.AutoScalingGroup(this, 'ASG', {
      vpc: vpc,
      instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MICRO),
      machineImage: ec2.MachineImage.latestAmazonLinux2023({
        cpuType: ec2.AmazonLinuxCpuType.ARM_64
      })
    });

    const alb = new elbv2.ApplicationLoadBalancer(this, 'ALB',{
      vpc: vpc,
      crossZoneEnabled: false,
      desyncMitigationMode: elbv2.DesyncMitigationMode.DEFENSIVE,
      internetFacing: true,
    });

    const nlb = new elbv2.NetworkLoadBalancer(this, 'NLB',{
      vpc: vpc,
      crossZoneEnabled: true,
      internetFacing: true,
    });
   
    const listener = alb.addListener('Listener', {
      port: 80,
    });

    listener.addTargets('Target', {
      port: 80,
      targets: [asg]
    });

    listener.connections.allowDefaultPortFromAnyIpv4('Open to the world');

    asg.scaleOnRequestCount('AModestLoad', {
      targetRequestsPerMinute: 60,
    });
  }
}

Running cdk synth generates the below CF stack:

Resources:
...
...
  ALBAEE750D2:
    Type: AWS::ElasticLoadBalancingV2::LoadBalancer
    Properties:
      LoadBalancerAttributes:
        - Key: deletion_protection.enabled
          Value: "false"
        - Key: routing.http.desync_mitigation_mode
          Value: defensive
      Scheme: internet-facing
      SecurityGroups:
        - Fn::GetAtt:
            - ALBSecurityGroup8B8624F8
            - GroupId
      Subnets:
        - Ref: VPCPublicSubnet1SubnetB4246D30
        - Ref: VPCPublicSubnet2Subnet74179F39
      Type: application
    DependsOn:
      - VPCPublicSubnet1DefaultRoute91CEF279
      - VPCPublicSubnet1RouteTableAssociation0B0896DC
      - VPCPublicSubnet2DefaultRouteB7481BBA
      - VPCPublicSubnet2RouteTableAssociation5A808732
    Metadata:
      aws:cdk:path: TypescriptStack/ALB/Resource
...
...
  NLB55158F82:
    Type: AWS::ElasticLoadBalancingV2::LoadBalancer
    Properties:
      LoadBalancerAttributes:
        - Key: deletion_protection.enabled
          Value: "false"
        - Key: load_balancing.cross_zone.enabled
          Value: "true"
      Scheme: internet-facing
      Subnets:
        - Ref: VPCPublicSubnet1SubnetB4246D30
        - Ref: VPCPublicSubnet2Subnet74179F39
      Type: network
    DependsOn:
      - VPCPublicSubnet1DefaultRoute91CEF279
      - VPCPublicSubnet1RouteTableAssociation0B0896DC
      - VPCPublicSubnet2DefaultRouteB7481BBA
      - VPCPublicSubnet2RouteTableAssociation5A808732
    Metadata:
      aws:cdk:path: TypescriptStack/NLB/Resource
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Analytics: v2:deflate64:H4sIAAAAAAAA/31QTWvjQAz9Lb1PZpsU9u6GUgKlmDj0ushjxVUz1piRJqEY//fFdhq7lN2T9D6keZqNXT/8tvd3cJGVq04rT6XtCgV3MnCRPx26je3eWme2R37LtyZPpSdXpJJRB27u9iEpHqD0OPMzl4kER6AU+GYemqddPpRX0GdQvMCnySOdQXFevGPFyHgzTEmuKFMF994gqynQpUj6+RxDascM/yV2XEcUMS+Q2L0fsGk9TJl+Mt8Gn8a53kDSIA48cW27LGkoJnB77gd3gFijHiK4E3F9lfLgyY1XfSN6Q9DYbh+m//yqOxYFdpjHcCSPvUEPouR8gKoED+yI6/PGdlnbenLjh78EqB5HDeN43xIvfSSKfPV89Qt9in87bwlfUS8hnpab+97sUUKKDs02iYZmhkf+h5THcKYK4yMImkwEtVCoiethJocIDeoUcBu4oiFVbzhUaD/k13lzb9dru777EKJVTKzUoN1P9S9+Uqp86QIAAA==
    Metadata:
      aws:cdk:path: TypescriptStack/CDKMetadata/Default
    Condition: CDKMetadataAvailable
...
...

However, if you refer:

  • Cross-zone load balancing for target groups, it mentions that With Application Load Balancers, cross-zone load balancing is always turned on at the load balancer level, and cannot be turned off.. It also specifies, For target groups, the default is to use the load balancer setting, but you can override the default by explicitly turning cross-zone load balancing off at the target group level.
  • AWS::ElasticLoadBalancingV2::LoadBalancer LoadBalancerAttribute also specifies for load_balancing.cross_zone.enabled that Indicates whether cross-zone load balancing is enabled. The possible values are true and false. The default for Network Load Balancers and Gateway Load Balancers is false. The default for Application Load Balancers is true, and cannot be changed..

So for Application Load Balancer, cross-zone load balancing is always turned on at the load balancer level, and cannot be turned off. As rightly pointed by you, in #29866 (comment), you can override the default by explicitly turning cross-zone load balancing off at the target group level.

Thanks,
Ashish

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-reproduction This issue needs reproduction. labels Apr 17, 2024
@ashishdhingra
Copy link
Contributor

We would submit a PR to improve the documentation here.

@codypenta
Copy link
Author

That makes sense to me, a quick documentation PR or logging a warning for this (You tried disabling cross-zone on an ALB see xyz) would resolve this issue in my mind with a longer term fix being allowed to directly set these values on the ApplicationTargetGroup type vs having to do an escape hatch

@ashishdhingra
Copy link
Contributor

That makes sense to me, a quick documentation PR or logging a warning for this (You tried disabling cross-zone on an ALB see xyz) would resolve this issue in my mind with a longer term fix being allowed to directly set these values on the ApplicationTargetGroup type vs having to do an escape hatch

@codypenta It could be possible to set AWS::ElasticLoadBalancingV2::TargetGroup TargetGroupAttribute as below:

const tg = new elbv2.ApplicationTargetGroup(this, 'TargetGroup',{
});
tg.setAttribute('load_balancing.cross_zone.enabled', 'false');

Please check if it is working for you.

Thanks,
Ashish

@codypenta
Copy link
Author

Ahh yes that works, I was looking for a strongly typed option like they have in the ALB tg.crossZoneEnabled = false or something like that but the key/value map makes sense.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 18, 2024
@mergify mergify bot closed this as completed in #29907 Apr 19, 2024
mergify bot pushed a commit that referenced this issue Apr 19, 2024
…for ALB (#29907)

### Issue # (if applicable)

Closes #29866

### Reason for this change

`crossZoneEnabled` is not well handled when it's `false` with ALB.  Because:

1. When the L2 prop is set `false`, it will not pass down to the L1 and won't throw any error as ALB does not support being disabled. It just silently ignore it.
2. When the prop is `false` for NLB, the L1 attribute will be `undefined`, which is having the same result but it should be explicitly set as `false` in L1.


This PR covers the following cases:

1. When `crossZoneEnabled` is `true`, `load_balancing.cross_zone.enabled` should be `true`.
2. When `crossZoneEnabled` is `false`, `load_balancing.cross_zone.enabled` should be `false`, rather than `undefined`.
3. When `crossZoneEnabled` is `false` with ALB, cdk throws an error because ALB does not support disabling it per [doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-loadbalancer-loadbalancerattribute.html) description.
4. NLB supports either `true` or `false`.
5. This prop can be `undefined` for ALB or NLB.
6. Improve the doc string for the `crossZoneEnabled` prop.

### Description of changes



### Description of how you validated changes



### 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*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@aws-cdk-automation
Copy link
Collaborator

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.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug.
Projects
None yet
3 participants