Support EKS cluster private access #8024

ewbankkit · 2019-03-20T14:06:57Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8022.
WIP as I need this for evaluation; Happy to close if there's a more official PR coming otherwise I will continue to work on this.
Includes AWS SDK update from #8021.
Right now only the public/private access flags can be updated in vpc_config although the API suggests that security groups and subnets can be updated in future.

EKS Private Endpoint Support aws/containers-roadmap#22

TODO

Update aws_eks_cluster data source
More acceptance tests

Acceptance tests so far:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSEksCluster_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSEksCluster_basic -timeout 120m
=== RUN   TestAccAWSEksCluster_basic
=== PAUSE TestAccAWSEksCluster_basic
=== CONT  TestAccAWSEksCluster_basic
--- PASS: TestAccAWSEksCluster_basic (1400.91s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1400.960s

Updated via: go get github.com/aws/aws-sdk-go@v1.18.5 go mod tidy go mod vendor

bflad · 2019-03-20T14:27:07Z

@ewbankkit would you prefer a review or me to add the required testing and data source changes in a separate commit after yours? Already have them written.

ewbankkit · 2019-03-20T14:35:36Z

@bflad I'll keep going as long as I can keep focus on this, if that's OK.
Slowest will be running the acceptance tests 😄.

bflad · 2019-03-20T14:42:37Z

aws/data_source_aws_eks_cluster.go

@@ -70,6 +70,14 @@ func dataSourceAwsEksCluster() *schema.Resource {
 							Computed: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
 						},
+						"endpoint_private_access": {


Please add acceptance testing for these two attributes, e.g.

resource.TestCheckResourceAttrPair(resourceName, "vpc_config.0.endpoint_private_access", dataSourceResourceName, "vpc_config.0.endpoint_private_access"), resource.TestCheckResourceAttrPair(resourceName, "vpc_config.0.endpoint_public_access", dataSourceResourceName, "vpc_config.0.endpoint_public_access"),

Nit: Attribute alphabetical ordering

bflad · 2019-03-20T14:42:43Z

aws/resource_aws_eks_cluster.go

@@ -98,6 +97,16 @@ func resourceAwsEksCluster() *schema.Resource {
 							MinItems: 1,
 							Elem:     &schema.Schema{Type: schema.TypeString},
 						},
+						"endpoint_private_access": {


Please add acceptance testing for these new arguments. Example below should get you close.

Nit: Attribute alphabetical ordering

func TestAccAWSEksCluster_VpcConfig_EndpointPrivateAccess(t *testing.T) { var cluster1, cluster2, cluster3 eks.Cluster rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5)) resourceName := "aws_eks_cluster.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSEksClusterDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPrivateAccess(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster1), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_private_access", "true"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPrivateAccess(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster2), testAccCheckAWSEksClusterNotRecreated(&cluster1, &cluster2), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_private_access", "false"), ), }, { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPrivateAccess(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster3), testAccCheckAWSEksClusterNotRecreated(&cluster2, &cluster3), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_private_access", "true"), ), }, }, }) } func TestAccAWSEksCluster_VpcConfig_EndpointPublicAccess(t *testing.T) { var cluster1, cluster2, cluster3 eks.Cluster rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5)) resourceName := "aws_eks_cluster.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSEksClusterDestroy, Steps: []resource.TestStep{ { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPublicAccess(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster1), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_public_access", "false"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPublicAccess(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster2), testAccCheckAWSEksClusterNotRecreated(&cluster1, &cluster2), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_public_access", "true"), ), }, { Config: testAccAWSEksClusterConfig_VpcConfig_EndpointPublicAccess(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEksClusterExists(resourceName, &cluster3), testAccCheckAWSEksClusterNotRecreated(&cluster2, &cluster3), resource.TestCheckResourceAttr(resourceName, "vpc.config.#", "1"), resource.TestCheckResourceAttr(resourceName, "vpc.config.0.endpoint_public_access", "false"), ), }, }, }) } func testAccAWSEksClusterConfig_VpcConfig_EndpointPrivateAccess(rName string, endpointPrivateAccess bool) string { return fmt.Sprintf(` %[1]s resource "aws_eks_cluster" "test" { name = %[2]q role_arn = "${aws_iam_role.test.arn}" vpc_config { endpoint_private_access = %[3]t endpoint_public_access = true subnet_ids = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] } depends_on = ["aws_iam_role_policy_attachment.test-AmazonEKSClusterPolicy", "aws_iam_role_policy_attachment.test-AmazonEKSServicePolicy"] } `, testAccAWSEksClusterConfig_Base(rName), rName, endpointPrivateAccess) } func testAccAWSEksClusterConfig_VpcConfig_EndpointPublicAccess(rName string, endpointPublicAccess bool) string { return fmt.Sprintf(` %[1]s resource "aws_eks_cluster" "test" { name = %[2]q role_arn = "${aws_iam_role.test.arn}" vpc_config { endpoint_private_access = true endpoint_public_access = %[3]t subnet_ids = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] } depends_on = ["aws_iam_role_policy_attachment.test-AmazonEKSClusterPolicy", "aws_iam_role_policy_attachment.test-AmazonEKSServicePolicy"] } `, testAccAWSEksClusterConfig_Base(rName), rName, endpointPublicAccess) }

aws/resource_aws_eks_cluster.go

bflad · 2019-03-20T14:43:06Z

aws/resource_aws_eks_cluster.go

+		"security_group_ids":      schema.NewSet(schema.HashString, flattenStringList(vpcConfig.SecurityGroupIds)),
+		"subnet_ids":              schema.NewSet(schema.HashString, flattenStringList(vpcConfig.SubnetIds)),
+		"vpc_id":                  aws.StringValue(vpcConfig.VpcId),
+		"endpoint_private_access": aws.BoolValue(vpcConfig.EndpointPrivateAccess),


Nit: Struct alphabetical ordering

bflad · 2019-03-20T14:43:19Z

aws/resource_aws_eks_cluster_test.go

@@ -97,6 +97,8 @@ func TestAccAWSEksCluster_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "vpc_config.0.security_group_ids.#", "0"),
 					resource.TestCheckResourceAttr(resourceName, "vpc_config.0.subnet_ids.#", "2"),
 					resource.TestMatchResourceAttr(resourceName, "vpc_config.0.vpc_id", regexp.MustCompile(`^vpc-.+`)),
+					resource.TestCheckResourceAttr(resourceName, "vpc_config.0.endpoint_private_access", "false"),


Nit: Attribute alphabetical ordering

aws/resource_aws_eks_cluster.go

@bflad

Add private access support to aws_eks_cluster data source. Add 'expandEksVpcConfigUpdateRequest'. - Additional aws_eks_cluster resource acceptance tests from @bflad. - Changes after initial review: - Attribute alphabetical ordering - Note on resource 'update' timeout

ewbankkit · 2019-03-20T15:13:21Z

Initial review issues addressed and additional acceptance tests added.
Removing WIP.
@bflad Can you please run acceptance tests? Thanks.

aws/resource_aws_eks_cluster_test.go

bflad

Thanks, @ewbankkit! 🚀

Output from acceptance testing:

--- PASS: TestAccAWSEksClusterDataSource_basic (1320.87s)

--- PASS: TestAccAWSEksCluster_basic (1295.89s)
--- PASS: TestAccAWSEksCluster_VpcConfig_SecurityGroupIds (1327.26s)
--- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPrivateAccess (1490.80s)
--- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPublicAccess (1526.63s)
--- PASS: TestAccAWSEksCluster_Version (2552.56s)

bflad · 2019-03-20T18:23:33Z

aws/resource_aws_eks_cluster.go

+		}
+	}
+
+	if d.HasChange("vpc_config") {


I'm not sure why this is not acting like it seems it should, be needed to change this to check the nested attributes. 🙁

if d.HasChange("vpc_config.0.endpoint_private_access") || d.HasChange("vpc_config.0.endpoint_public_access") {

Previous output from acceptance testing:

--- FAIL: TestAccAWSEksCluster_Version (2519.24s) testing.go:538: Step 2 error: Error applying: 1 error occurred: * aws_eks_cluster.test: 1 error occurred: * aws_eks_cluster.test: error updating EKS Cluster (tf-acc-test-zdu6h) config: InvalidParameterException: Cluster is already at the desired configuration with endpointPrivateAccess: false and endpointPublicAccess: true

Even though the acceptance test debug logs were correctly only showing:

UPDATE: aws_eks_cluster.test version: "1.10" => "1.11"

Output from acceptance testing:

--- PASS: TestAccAWSEksClusterDataSource_basic (1320.87s) --- PASS: TestAccAWSEksCluster_basic (1295.89s) --- PASS: TestAccAWSEksCluster_VpcConfig_SecurityGroupIds (1327.26s) --- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPrivateAccess (1490.80s) --- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPublicAccess (1526.63s) --- PASS: TestAccAWSEksCluster_Version (2552.56s)

I think it's a schema.TypeList vs. schema.TypeSet thing for the vpc_config attribute maybe?

bflad · 2019-03-21T20:51:16Z

This has been released in version 2.3.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-03-30T18:02:27Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

deps: github.com/aws/aws-sdk-go@v1.18.5

58ba15c

Updated via: go get github.com/aws/aws-sdk-go@v1.18.5 go mod tidy go mod vendor

bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Mar 20, 2019

bflad reviewed Mar 20, 2019

View reviewed changes

bflad mentioned this pull request Mar 20, 2019

aws eks private link support #8022

Closed

ewbankkit force-pushed the issue-8022 branch from 6348989 to 58a1d0a Compare March 20, 2019 15:11

ewbankkit changed the title ~~[WIP] Support EKS cluster private access~~ Support EKS cluster private access Mar 20, 2019

bflad reviewed Mar 20, 2019

View reviewed changes

aws/resource_aws_eks_cluster_test.go Outdated Show resolved Hide resolved

Fix acceptance test typo.

214b113

bflad added this to the v2.3.0 milestone Mar 20, 2019

bflad approved these changes Mar 20, 2019

View reviewed changes

bflad merged commit 214b113 into hashicorp:master Mar 20, 2019

bflad added a commit that referenced this pull request Mar 20, 2019

Update CHANGELOG for #8024

9077995

bflad added a commit that referenced this pull request Mar 20, 2019

Update CHANGELOG for #8024 data source change too

1cebd12

ewbankkit deleted the issue-8022 branch March 20, 2019 18:32

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support EKS cluster private access #8024

Support EKS cluster private access #8024

ewbankkit commented Mar 20, 2019 •

edited

Loading

bflad commented Mar 20, 2019 •

edited

Loading

ewbankkit commented Mar 20, 2019

bflad Mar 20, 2019

bflad Mar 20, 2019

bflad Mar 20, 2019

bflad Mar 20, 2019

ewbankkit commented Mar 20, 2019

bflad left a comment

bflad Mar 20, 2019

ewbankkit Mar 20, 2019

bflad commented Mar 21, 2019

ghost commented Mar 30, 2020

Support EKS cluster private access #8024

Support EKS cluster private access #8024

Conversation

ewbankkit commented Mar 20, 2019 • edited Loading

Community Note

bflad commented Mar 20, 2019 • edited Loading

ewbankkit commented Mar 20, 2019

bflad Mar 20, 2019

Choose a reason for hiding this comment

bflad Mar 20, 2019

Choose a reason for hiding this comment

bflad Mar 20, 2019

Choose a reason for hiding this comment

bflad Mar 20, 2019

Choose a reason for hiding this comment

ewbankkit commented Mar 20, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Mar 20, 2019

Choose a reason for hiding this comment

ewbankkit Mar 20, 2019

Choose a reason for hiding this comment

bflad commented Mar 21, 2019

ghost commented Mar 30, 2020

ewbankkit commented Mar 20, 2019 •

edited

Loading

bflad commented Mar 20, 2019 •

edited

Loading