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

WIP: Tighter security group rules #4

Closed
wants to merge 1 commit into from

Conversation

tanmng
Copy link

@tanmng tanmng commented Jun 7, 2018

Improvement on security group

Description

As per AWS documentation, you can set up the Security groups for the control plane and worker node to have either minimum rules, or have more access according to recommendation (ie. more relaxed rules and our instances more access).

I added 3 more variables to let the user specify which scenario they would want to use:

  • worker_node_allow_all_egress (default true)
  • cp_to_wn_from_port (default 1025)
  • cp_to_wn_to_port (default 65355)

The default values of these correspond to the recommended case by AWS, but for any users who wish to set up a strict set of rules, they can provide different values to those variables, such as

  • worker_node_allow_all_egress = false
  • cp_to_wn_from_port = 10250
  • cp_to_wn_from_port = 10250

Beside this, we also added the ability to provide a list of custom security groups that we would like to set onto the worker node. That will allow users to use other Terraform modules to set up the groups in the way that they like and then assign them onto the instance.

Since it's possible that user can set worker_node_allow_all_egress = false, we have to add one more security group rule resource to allow worker node egress to allow communication within the worker nodes themselves.

Checklist

  • terraform fmt and terraform validate both work from the root and examples/eks_test_fixture directories (look in CI for an example)
  • Tests for the changes have been added and passing (for bug fixes/features)
  • Test results are pasted in this PR (in lieu of CI)
  • Docs have been added/updated (for bug fixes/features)
  • Any breaking changes are noted in the description above

@brandonjbjelland
Copy link
Contributor

I'd like this to be more flexible than just being able to control one or two specific variables of a rule or two. Giving the option for a module consumer to bring their own pre-defined security group to attach to either set of resources feels like a better approach. I'm working on this currently.

Thanks again on the idea even if the implementation isn't exactly what I think will be best for module consumers. The input is incredibly valuable! ⭐

@tanmng
Copy link
Author

tanmng commented Jun 10, 2018

@brandoconnor thanks for the feedback. I'll close this then

@tanmng tanmng closed this Jun 10, 2018
@brandonjbjelland
Copy link
Contributor

Thanks again @tanmng . I'll give you credit (because it's due!) in the changelog.

max-rocket-internet pushed a commit that referenced this pull request Jan 27, 2020
…701)

* Configurable local exec command for waiting until cluster is healthy

* readme

* line feeds

* format

* fix readme

* fix readme

* Configurable local exec command for waiting until cluster is healthy (#1)

* Configurable local exec command for waiting until cluster is healthy

* readme

* line feeds

* format

* fix readme

* fix readme

* change log

* Configurable local exec wait 4 cluster op (#2)

* Configurable local exec command for waiting until cluster is healthy

* readme

* line feeds

* format

* fix readme

* fix readme

* change log

* changelog (#3)

* Changelog (#4)

* changelog

* changelog

* simplify wait_for_cluster command

* readme

* no op for manage auth false

* formatting

* docs? not sure

* linter

* specify dependency to wait for cluster more accurately
@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 26, 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