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

[ecs-patterns; ApplicationLoadBalancedFargateService] When attaching certificate, target group protocol set to that of listener protocol #4983

Closed
kevin-lindsay-1 opened this issue Nov 12, 2019 · 3 comments · Fixed by #4988
Assignees
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. in-progress This issue is being actively worked on. p0

Comments

@kevin-lindsay-1
Copy link

As of aws-cdk/aws-ecs-patterns@1.16.0, specifying an ApplicationLoadBalancedFargateService like so:

...
const lbfs = new ApplicationLoadBalancedFargateService(this, 'LBFS', {
    serviceName: ccFqAppName,
    cluster,
    taskImageOptions: {
      image,
      environment: {
        STAGE: appStage,
      },
    },
    domainZone,
    domainName,
    certificate,
  });
  lbfs.targetGroup.configureHealthCheck({
    path: '/.well-known/apollo/server-health',
  });

outputs:

...
"ServiceLBFSLBPublicListenerECSGroup46113E88": {
      "Type": "AWS::ElasticLoadBalancingV2::TargetGroup",
      "Properties": {
        "HealthCheckPath": "/.well-known/apollo/server-health",
        "Port": 80,
        "Protocol": "HTTPS",
        "TargetType": "ip",
        "VpcId": {
          "Ref": "ServiceVpc4872DC6E"
        }
      },
      "Metadata": {
        "aws:cdk:path": "standalone/Service/LBFS/LB/PublicListener/ECSGroup/Resource"
      }
    },
...

whereas on 1.15.x, it outputs:

...
"ServiceLBFSLBPublicListenerECSGroup46113E88": {
      "Type": "AWS::ElasticLoadBalancingV2::TargetGroup",
      "Properties": {
        "HealthCheckPath": "/.well-known/apollo/server-health",
        "Port": 80,
        "Protocol": "HTTP",
        "TargetType": "ip",
        "VpcId": {
          "Ref": "ServiceVpc4872DC6E"
        }
      },
      "Metadata": {
        "aws:cdk:path": "standalone/Service/LBFS/LB/PublicListener/ECSGroup/Resource"
      }
    },
...

This appears to be causing 502s on the ECS service, and I don't see a way to manually override it in 1.16.x

Reproduction Steps

Error Log

Environment

  • CLI Version :
  • Framework Version:
  • OS :
  • Language :

Other


This is 🐛 Bug Report

@kevin-lindsay-1 kevin-lindsay-1 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 12, 2019
@kevin-lindsay-1 kevin-lindsay-1 changed the title [ecs-patterns] When attaching certificate, target group protocol set to that of listener protocol [ecs-patterns; ApplicationLoadBalancedFargateService] When attaching certificate, target group protocol set to that of listener protocol Nov 12, 2019
@kevin-lindsay-1
Copy link
Author

kevin-lindsay-1 commented Nov 12, 2019

Update: confirmed that this issue caused a 502 on my service, which listens on HTTP/80.

@wcauchois
Copy link
Contributor

I'm experiencing the same issue, thanks for reporting this.

@SomayaB SomayaB added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Nov 12, 2019
@piradeepk piradeepk added in-progress This issue is being actively worked on. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 12, 2019
@piradeepk
Copy link
Contributor

Issue looks to be related to this commit: 20b8e5d

@NGL321 NGL321 added the p0 label Nov 13, 2019
@mergify mergify bot closed this as completed in #4988 Nov 13, 2019
RomainMuller added a commit that referenced this issue Nov 13, 2019
1. Upgrades `jsii` to `0.20.5`(fixes #4989, fixes #4966, fixes #1904, fixes #1845)
2. Fixes the `ecs-patterns` library bug #4983
3. Fixes the CLI's "legacy" mode of operation bug #4998
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. in-progress This issue is being actively worked on. p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants