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

Control plane security group always whitelist worker security group and revert #186 #631

Merged
merged 6 commits into from
Dec 20, 2019

Conversation

ryanooi
Copy link
Contributor

@ryanooi ryanooi commented Dec 12, 2019

PR o'clock

Description

  1. Main change on cluster.tf aws_security_group_rule.cluster_https_worker_ingress intended to whitelist port 443 for newly created security group or self attach security group from worker's security group. Self attach control plane security group will not able to whitelist worker security group before it get created therefore additional step will required.
  2. Revert PR Add ability to pass computed values to cluster_security_group_id and worker_security_group_id #186 as Terraform v0.12.x fixed "count cannot be computed" issue
    2.1 Remove cluster_create_security_group and worker_create_security_group to improve consistency. There is no guarantee/enforcement when you set cluster_create_security_group or worker_create_security_group to false, user will also provide cluster_security_group_id or worker_security_group_id.

Checklist

@ryanooi ryanooi changed the title Updates and revert #186 Control plane security group always whitelist worker security group and revert #186 Dec 12, 2019
@ryanooi
Copy link
Contributor Author

ryanooi commented Dec 17, 2019

Need some explanation? or?

CHANGELOG.md Outdated Show resolved Hide resolved
@max-rocket-internet
Copy link
Contributor

Actually, this is a breaking change as you are removing some vars. So also please write details under Important notes that clearly explain how to migrate after this change 🙂

Copy link
Contributor

@max-rocket-internet max-rocket-internet left a comment

Choose a reason for hiding this comment

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

OK nice. Great to remove 2 variables 🚀

@max-rocket-internet max-rocket-internet merged commit b7ffc1b into terraform-aws-modules:master Dec 20, 2019
@barryib barryib mentioned this pull request Jan 10, 2020
4 tasks
devgrok added a commit to devgrok/terraform-aws-eks that referenced this pull request Jan 23, 2020
ryanooi added a commit to ryanooi/terraform-aws-eks that referenced this pull request Jan 30, 2020
max-rocket-internet pushed a commit that referenced this pull request Feb 27, 2020
* Revert #631

* fix README lint

* fix README lint for bool
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants