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

feat: Add ability to use security groups for private access #1274

Conversation

marc-slingshot
Copy link
Contributor

@marc-slingshot marc-slingshot commented Mar 13, 2021

PR o'clock

Description

Add the ability to use security groups sources as opposed to CIDR sources for private access rules

Resolves #1275

Checklist

@aidan-melen
Copy link

This is an excellent contribution. I would like to see this merged.

@aidan-melen
Copy link

aidan-melen commented Mar 14, 2021

I still think it would be nice for the module to support source sg's in addition to CIDR. That being said, I did find a workaround. We can get the additional security group from the module output and append it with a new security group rule with our source sg.

module "eks" {
  source          = "terraform-aws-modules/eks/aws"
  version         = "~> 14.0.0"

  cluster_name    = "my-cluster"
  cluster_version = "1.18"

  cluster_endpoint_private_access = true
  cluster_endpoint_public_access  = false
  
  ...
}

resource "aws_security_group_rule" "data-vpn" {
  description              = "Allow K8s API from my source security group."
  type                     = "ingress"
  from_port                = 443
  to_port                  = 443
  protocol                 = "tcp"
  source_security_group_id = "sg-???????????"
  security_group_id        = module.eks.cluster_security_group_id
}

hope that helps.

@marc-slingshot
Copy link
Contributor Author

The problem I am experiencing currently is I have setup the EKS cluster as only having private access and I am running terraform remotely through a VPN. This module gets stuck on the null_resource wait trying to contact the EKS cluster, which I cannot since the ACL to get to it has not been added.

I am a bit new to terraform, but if the SG resource relies on an output from the EKS module, wouldn't the EKS module need to finish first before getting that output and running that resource stanza?

@marc-slingshot
Copy link
Contributor Author

also when adding this PR the CI linter complained about a lot of missing portions in the README that this PR did not modify. Not sure why the linter requires those now, but I added them in this PR as well.

@aidan-melen
Copy link

aidan-melen commented May 21, 2021

This will give you complete control of the security before the cluster is created and deleted after the health check (aka null resource in older versions of the module).

module "eks" {
  source          = "terraform-aws-modules/eks/aws"
  version         = "16.1.0"

  cluster_name    = "my-cluster"
  cluster_version = "1.20"

  cluster_endpoint_private_access = true
  cluster_endpoint_public_access  = false
  
  cluster_create_security_group   = false
  cluster_security_group_id       = aws_security_group.cluster.id

  ...
}

resource "aws_security_group" "cluster" {
  description = "EKS cluster security group."
  vpc_id      = var.vpc_id

  tags = { 
    "Name" = "eks_cluster_sg"
  }
}

resource "aws_security_group_rule" "data-vpn" {
  description              = "Allow K8s API from my source security group."
  type                     = "ingress"
  from_port                = 443
  to_port                  = 443
  protocol                 = "tcp"
  source_security_group_id = "sg-???????????"
  security_group_id        = aws_security_group.cluster.id
}

@barryib barryib self-assigned this May 27, 2021
@barryib barryib changed the title feat: add ability to use security groups for private access feat: Add ability to use security groups for private access May 28, 2021
@barryib
Copy link
Member

barryib commented May 28, 2021

@marc-slingshot Thanks for opening this and sorry for the delay. Can you please update your branch and resolve conflict ? I want to ship this (during the next couple of days) in the next release with other breaking changes.

@barryib barryib force-pushed the add-private-access-sg-sources branch from ff8140c to 91f640d Compare May 28, 2021 12:35
@barryib barryib merged commit 796cbea into terraform-aws-modules:master May 28, 2021
@barryib
Copy link
Member

barryib commented May 28, 2021

Thanks @marc-slingshot for your contribution.

barryib added a commit that referenced this pull request May 28, 2021
…point (#1412)

NOTES: In this bug fix, we remove a duplicated security rule introduced during a merge conflict resolution in [#1274](#1274)
ArchiFleKs pushed a commit to ArchiFleKs/terraform-aws-eks that referenced this pull request Jun 1, 2021
…nt access (terraform-aws-modules#1274)

BREAKING CHANGES: The private endpoint security group rule has been renamed to allow the use of CIDR blocks and Security Groups as source. This will delete the `cluster_private_access` Security Group Rule for existing cluster. Please rename by `aws_security_group_rule.cluster_private_access[0]` into `aws_security_group_rule.cluster_private_access_cidrs_source[0]`.

Co-authored-by: Thierno IB. BARRY <ibrahima.br@gmail.com>
ArchiFleKs pushed a commit to ArchiFleKs/terraform-aws-eks that referenced this pull request Jun 1, 2021
…point (terraform-aws-modules#1412)

NOTES: In this bug fix, we remove a duplicated security rule introduced during a merge conflict resolution in [terraform-aws-modules#1274](terraform-aws-modules#1274)
@aidan-melen
Copy link

wahoo! thanks @marc-slingshot and @barryib

SergK pushed a commit to SergK/terraform-aws-eks that referenced this pull request Aug 16, 2021
…point (terraform-aws-modules#1412)

NOTES: In this bug fix, we remove a duplicated security rule introduced during a merge conflict resolution in [terraform-aws-modules#1274](terraform-aws-modules#1274)
baibailiha added a commit to baibailiha/terraform-aws-eks that referenced this pull request Sep 13, 2022
…point (#1412)

NOTES: In this bug fix, we remove a duplicated security rule introduced during a merge conflict resolution in [#1274](terraform-aws-modules/terraform-aws-eks#1274)
@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 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to use security groups for private access rules
3 participants