Skip to content

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

Merged
mergify[bot] merged 1 commit intoaws:masterfrom
piradeepk:fixProtocolIssue
Nov 13, 2019
Merged

fix(ecs-patterns): Revert listener protocol and port being passed to target group#4988
mergify[bot] merged 1 commit intoaws:masterfrom
piradeepk:fixProtocolIssue

Conversation

@piradeepk
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

aws-cdk-automation commented Nov 12, 2019

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
Copy Markdown
Collaborator

aws-cdk-automation commented Nov 13, 2019

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
Copy Markdown
Collaborator

aws-cdk-automation commented Nov 13, 2019

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
Copy Markdown
Collaborator

aws-cdk-automation commented Nov 13, 2019

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
Copy Markdown
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
Copy Markdown
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