Skip to content

fix(ecs-patterns): Revert listener protocol and port being passed to target group #4988

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
Merged
merged 1 commit into from
Nov 13, 2019
Merged

fix(ecs-patterns): Revert listener protocol and port being passed to target group #4988

merged 1 commit into from
Nov 13, 2019

Conversation

piradeepk
Copy link
Contributor

@piradeepk piradeepk commented Nov 12, 2019 •
edited
Loading

Fix issue related to protocol being passed to target group that's causing 502 when a cert is created using protocol HTTPS, but the target group should be using HTTP.

Reverted the change made to target groups as part of: 20b8e5d

Fixes: #4983

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

@piradeepk piradeepk added @aws-cdk/aws-ecs Related to Amazon Elastic Container @aws-cdk/aws-ecs-patterns Related to ecs-patterns library labels Nov 12, 2019
@piradeepk piradeepk requested a review from SoManyHs November 12, 2019 19:47
@mergify
Copy link
Contributor

mergify bot commented Nov 12, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

2 similar comments
@mergify
Copy link
Contributor

mergify bot commented Nov 12, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@mergify
Copy link
Contributor

mergify bot commented Nov 12, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@SomayaB SomayaB added the contribution/core This is a PR that came from AWS. label Nov 12, 2019
@piradeepk piradeepk changed the title fix(ecs-patterns): Fix issue related to protocol being passed to target group fix(ecs-patterns): Fix issue related to protocol and port being passed to target group Nov 13, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@piradeepk piradeepk added the pr/do-not-merge This PR should not be merged at this time. label Nov 13, 2019
@piradeepk piradeepk requested a review from rix0rrr November 13, 2019 00:44
@piradeepk piradeepk changed the title fix(ecs-patterns): Fix issue related to protocol and port being passed to target group fix(ecs-patterns): Fix issue related to listener protocol and port being passed to target group Nov 13, 2019
@piradeepk piradeepk changed the title fix(ecs-patterns): Fix issue related to listener protocol and port being passed to target group fix(ecs-patterns): Revert listener protocol and port being passed to target group Nov 13, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • 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

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@piradeepk piradeepk requested a review from kohidave November 13, 2019 01:19
NGL321
NGL321 approved these changes Nov 13, 2019
View reviewed changes
Copy link
Contributor

@NGL321 NGL321 left a comment

Choose a reason for hiding this comment

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

Seems like a straightforward rollback, and the added tests make sense to me!

😸

@piradeepk piradeepk removed the pr/do-not-merge This PR should not be merged at this time. label Nov 13, 2019
@mergify
Copy link
Contributor

mergify bot commented Nov 13, 2019

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit a257d4d into aws:master Nov 13, 2019
RomainMuller pushed a commit that referenced this pull request Nov 13, 2019
@piradeepk @RomainMuller
fix(ecs-patterns): Fix issue related to protocol being passed to targ…
6bb29b5 
…et group (#4988)
arhea pushed a commit to arhea/aws-cdk that referenced this pull request Nov 14, 2019
Merge branch '4295-windows-ecs-support' of github.com:arhea/aws-cdk i…
6258122 
…nto 4295-windows-ecs-support

* '4295-windows-ecs-support' of github.com:arhea/aws-cdk:
  chore(deps-dev): bump @types/lodash from 4.14.146 to 4.14.147 (aws#5021)
  Revert "fix(assets): support exceptions to exclude patterns (aws#4473)" (aws#5022)
  chore(deps): bump jsii-pacmak from 0.20.3 to 0.20.5 (aws#5003)
  chore(deps): bump codemaker from 0.20.3 to 0.20.5 (aws#5007)
  chore(deps-dev): bump @types/jest from 24.0.22 to 24.0.23 (aws#4993)
  chore(deps): bump jsii from 0.20.3 to 0.20.5 (aws#5006)
  chore(deps-dev): bump jsii-diff from 0.20.3 to 0.20.5 (aws#5005)
  chore(deps): bump jsii-spec from 0.20.3 to 0.20.5 (aws#5008)
  chore(core): resolve tokens before publishing tree.json (aws#4984)
  feat(cli): adding new option to `cdk deploy` to indicate whether ChangeSet should be executed (aws#4852)
  chore: move semantic.yaml to .github/
  fix(core): unable to find stack by name using the cli in legacy mode (aws#4998)
  fix(ecs-patterns): Fix issue related to protocol being passed to target group (aws#4988)
arhea pushed a commit to arhea/aws-cdk that referenced this pull request Nov 19, 2019
Merge branch '4955-eks-patterns' of github.com:arhea/aws-cdk into 495…
0e5ec13 
…5-eks-patterns

* '4955-eks-patterns' of github.com:arhea/aws-cdk:
  feat(cli): adding new option to `cdk deploy` to indicate whether ChangeSet should be executed (aws#4852)
  chore: move semantic.yaml to .github/
  fix(core): unable to find stack by name using the cli in legacy mode (aws#4998)
  fix(ecs-patterns): Fix issue related to protocol being passed to target group (aws#4988)
  chore: Backport CHANGELOG entries from v1.16.2 (aws#4980)
  feat(custom-resources): python handler skelaton in readme (aws#4977)
  fix(logs): cannot use same Lambda for multiple SubscriptionFilters (aws#4975)
  release: v1.16.1 (aws#4965)
  chore: resolve inaccurate label line in depbot config (aws#4729)
  chore(deps): bump aws-sdk from 2.568.0 to 2.569.0 (aws#4958)
@LukvonStrom LukvonStrom mentioned this pull request Jan 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SoManyHs SoManyHs Awaiting requested review from SoManyHs

@rix0rrr rix0rrr Awaiting requested review from rix0rrr

2 more reviewers

@kohidave kohidave kohidave approved these changes

@NGL321 NGL321 NGL321 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@SoManyHs SoManyHs

Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container @aws-cdk/aws-ecs-patterns Related to ecs-patterns library contribution/core This is a PR that came from AWS.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ecs-patterns; ApplicationLoadBalancedFargateService] When attaching certificate, target group protocol set to that of listener protocol
6 participants
@piradeepk @aws-cdk-automation @kohidave @NGL321 @SomayaB @SoManyHs