resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute in VpcConfig #11442

seddarj · 2019-12-31T08:56:17Z

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

First time contributing to this project, let me know if anything needs changing.

~~This will only work once #11375 gets merged.~~ #11375 has been merged, this PR has been rebased on master.

Release note for CHANGELOG:

service/eks: Add support for public_access_cidrs attribute in vpc_config

Output from acceptance testing:

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

… in VpcConfig

…ttribute

seddarj · 2020-01-03T13:57:58Z

Rebased on master following merge of #11375, ready for review ;-)

bflad

Hi @seddarj 👋 Thank you for submitting this. The implementation looks great, just one testing change to ensure we cover updates of this attribute as well as ensuring the data source will work properly.

aws/resource_aws_eks_cluster.go

aws/resource_aws_eks_cluster_test.go

seddarj · 2020-01-03T17:11:58Z

Thank you for the very detailed review @bflad. I've tried to accommodate all your remarks. Let me know if anything needs changing.

…n vpc_config

… to test for update case

bflad

Hi again 👋 When running the other acceptance tests for this resource, it looks like we need a slight tweak to how we send this parameter to prevent API errors. Hopefully last change needed. 🤞

aws/resource_aws_eks_cluster.go

…f empty

seddarj · 2020-01-03T21:51:31Z

Hi again @bflad, I've committed the changes regarding the case where the list is empty. Thanks for your patience and guidance!

bflad

Thanks for the continued updates, @seddarj! The latest acceptance testing was still showing failures (noted below). Since this was close to completion and testing is lengthy, we went ahead and made the minor adjustments noted below so we can release this tomorrow. 🚀

Output from acceptance testing (after Computed: true additions):

--- PASS: TestAccAWSEksCluster_basic (1221.74s)
--- PASS: TestAccAWSEksCluster_Tags (1277.77s)
--- PASS: TestAccAWSEksClusterDataSource_basic (1277.90s)
--- PASS: TestAccAWSEksCluster_Logging (1278.03s)
--- PASS: TestAccAWSEksCluster_VpcConfig_SecurityGroupIds (1295.59s)
--- PASS: TestAccAWSEksCluster_VpcConfig_PublicAccessCidrs (1422.58s)
--- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPublicAccess (1905.76s)
--- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPrivateAccess (2322.50s)
--- PASS: TestAccAWSEksCluster_Version (2450.83s)

bflad · 2020-01-09T03:28:24Z

aws/data_source_aws_eks_cluster.go

@@ -114,6 +114,11 @@ func dataSourceAwsEksCluster() *schema.Resource {
 							Computed: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
 						},
+						"public_access_cidrs": {
+							Type:     schema.TypeSet,
+							Optional: true,


This is not a configurable argument in the data source, so we should use Computed: true instead.

Suggested change

Optional: true,

Computed: true,

bflad · 2020-01-09T03:35:54Z

aws/resource_aws_eks_cluster.go

@@ -145,6 +145,14 @@ func resourceAwsEksCluster() *schema.Resource {
 							MinItems: 1,
 							Elem:     &schema.Schema{Type: schema.TypeString},
 						},
+						"public_access_cidrs": {
+							Type:     schema.TypeSet,
+							Optional: true,


It appears that EKS provides a default 0.0.0.0/0 entry when this argument is not configured:

--- FAIL: TestAccAWSEksCluster_basic (1307.89s) testing.go:640: Step 0 error: After applying this step, the plan was not empty: DIFF: UPDATE: aws_eks_cluster.test ... omitted for clarity ... vpc_config.0.public_access_cidrs.#: "1" => "0" vpc_config.0.public_access_cidrs.0: "0.0.0.0/0" => "" ... omitted for clarity ...

Given that the Terraform Plugin SDK currently does not support a way to configure a Default for TypeList/TypeSet, our only recourse here is to add Computed: true to this for now.

Suggested change

Optional: true,

Optional: true,

Computed: true,

Reference: #11397 Reference: #11442 Output from acceptance testing: ``` --- PASS: TestAccAWSEksCluster_basic (1221.74s) --- PASS: TestAccAWSEksCluster_Tags (1277.77s) --- PASS: TestAccAWSEksClusterDataSource_basic (1277.90s) --- PASS: TestAccAWSEksCluster_Logging (1278.03s) --- PASS: TestAccAWSEksCluster_VpcConfig_SecurityGroupIds (1295.59s) --- PASS: TestAccAWSEksCluster_VpcConfig_PublicAccessCidrs (1422.58s) --- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPublicAccess (1905.76s) --- PASS: TestAccAWSEksCluster_VpcConfig_EndpointPrivateAccess (2322.50s) --- PASS: TestAccAWSEksCluster_Version (2450.83s) ```

ghost · 2020-01-10T16:57:26Z

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2020-03-27T17:54:51Z

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!

seddarj requested a review from a team December 31, 2019 08:56

seddarj force-pushed the eks-public-access-cidrs branch from 7e7be47 to c0780b5 Compare December 31, 2019 09:54

ghost added size/XXL Managed by automation to categorize the size of a PR. dependencies Used to indicate dependency changes. and removed size/M Managed by automation to categorize the size of a PR. labels Dec 31, 2019

seddarj force-pushed the eks-public-access-cidrs branch 2 times, most recently from 2de76fb to a67f062 Compare December 31, 2019 12:39

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/XXL Managed by automation to categorize the size of a PR. labels Dec 31, 2019

seddarj mentioned this pull request Dec 31, 2019

Amazon EKS cluster public endpoint network access restriction via IPv4 address ranges #11397

Closed

ghost added the documentation Introduces or discusses updates to documentation. label Dec 31, 2019

bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed dependencies Used to indicate dependency changes. needs-triage Waiting for first response or review from a maintainer. labels Jan 3, 2020

Jawad Seddar added 3 commits January 3, 2020 14:29

resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute…

97074fb

… in VpcConfig

resource/aws_eks_cluster: Add test for PublicAccessCidrs attribute

e005254

resource/aws_eks_cluster: Add documentation for public_access_cidrs a…

9f365c7

…ttribute

seddarj force-pushed the eks-public-access-cidrs branch from e6fbf01 to 9f365c7 Compare January 3, 2020 13:29

bflad suggested changes Jan 3, 2020

View reviewed changes

aws/resource_aws_eks_cluster.go Show resolved Hide resolved

aws/resource_aws_eks_cluster_test.go Outdated Show resolved Hide resolved

bflad self-assigned this Jan 3, 2020

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

bflad added this to the v2.44.0 milestone Jan 3, 2020

seddarj requested a review from bflad January 3, 2020 17:11

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

seddarj force-pushed the eks-public-access-cidrs branch from f55b444 to cf8e3cb Compare January 3, 2020 17:29

Jawad Seddar added 2 commits January 3, 2020 18:39

data/aws_eks_cluster: Add support for public_access_cidrs attribute i…

a6e1a9a

…n vpc_config

resource/aws_eks_cluster: Update test for PublicAccessCidrs attribute…

45077b1

… to test for update case

seddarj force-pushed the eks-public-access-cidrs branch from cf8e3cb to 45077b1 Compare January 3, 2020 17:39

bflad suggested changes Jan 3, 2020

View reviewed changes

aws/resource_aws_eks_cluster.go Outdated Show resolved Hide resolved

aws/resource_aws_eks_cluster.go Outdated Show resolved Hide resolved

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

resource/aws_eks_cluster: Don't specify PublicAccessCidrs attribute i…

1b11a03

…f empty

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

seddarj requested a review from bflad January 3, 2020 21:51

bflad approved these changes Jan 9, 2020

View reviewed changes

bflad merged commit 1b11a03 into hashicorp:master Jan 9, 2020

bflad added a commit that referenced this pull request Jan 9, 2020

Update CHANGELOG for #11442

246a597

seddarj deleted the eks-public-access-cidrs branch January 10, 2020 13:43

ghost locked and limited conversation to collaborators Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute in VpcConfig #11442

resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute in VpcConfig #11442

seddarj commented Dec 31, 2019 •

edited

Loading

seddarj commented Jan 3, 2020

bflad left a comment

seddarj commented Jan 3, 2020

bflad left a comment

seddarj commented Jan 3, 2020

bflad left a comment

bflad Jan 9, 2020

bflad Jan 9, 2020

ghost commented Jan 10, 2020

ghost commented Mar 27, 2020

resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute in VpcConfig #11442

resource/aws_eks_cluster: Add support for PublicAccessCidrs attribute in VpcConfig #11442

Conversation

seddarj commented Dec 31, 2019 • edited Loading

Community Note

seddarj commented Jan 3, 2020

bflad left a comment

Choose a reason for hiding this comment

seddarj commented Jan 3, 2020

bflad left a comment

Choose a reason for hiding this comment

seddarj commented Jan 3, 2020

bflad left a comment

Choose a reason for hiding this comment

bflad Jan 9, 2020

Choose a reason for hiding this comment

bflad Jan 9, 2020

Choose a reason for hiding this comment

ghost commented Jan 10, 2020

ghost commented Mar 27, 2020

seddarj commented Dec 31, 2019 •

edited

Loading