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

docs: add storage configuration block for eks_cluster resource #41267

Conversation

stefanfreitag
Copy link
Contributor

@stefanfreitag stefanfreitag commented Feb 6, 2025

Description

  • The resource aws_eks_cluster contains a storage_config block, see e.g. example code available here.
  • The provider documentation is missing details on the storage_config.

The pull requests

  • adds the missing documentation for storage_config
  • adds links from "config block" argument to the corresponding description.

Relations

Closes #41226

References

Output from Acceptance Testing

Not applicable. Only documentation is updated.

Copy link

github-actions bot commented Feb 6, 2025

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. service/eks Issues and PRs that pertain to the eks service. needs-triage Waiting for first response or review from a maintainer. labels Feb 6, 2025
@stefanfreitag stefanfreitag marked this pull request as ready for review February 7, 2025 19:44
@stefanfreitag stefanfreitag requested a review from a team as a code owner February 7, 2025 19:44
justinretzolk
justinretzolk previously approved these changes Feb 11, 2025
Copy link
Member

@justinretzolk justinretzolk left a comment

Choose a reason for hiding this comment

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

Had a minor suggestion, but otherwise looks good to me! Thank you for taking the time to raise this @stefanfreitag! 🎉

@@ -477,7 +489,7 @@ This resource exports the following attributes in addition to the arguments abov
* `endpoint` - Endpoint for your Kubernetes API server.
* `id` - Name of the cluster.
* `identity` - Attribute block containing identity provider information for your cluster. Only available on Kubernetes version 1.13 and 1.14 clusters created or upgraded on or after September 3, 2019. Detailed below.
* `kubernetes_network_config` - Attribute block containing Kubernetes network configuration for the cluster. Detailed below.
* `kubernetes_network_config` - Attribute block containing Kubernetes network configuration for the cluster. [Detailed](#kubernetes_network_config-1) below.
Copy link
Member

Choose a reason for hiding this comment

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

Since this is an argument, I think we should remove it from the attributes section entirely to be more consistent with other areas of the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @justinretzolk,

we can do so. One question I have before going that route:

As part of kubernetes_network_config there is service_ipv6_cidr which is Computed: true.
Is this maybe the reason why kubernetes_network_config block is listed under the Attributes section?

(rephrased: How would we need to document service_ipv6_cidr then without mentioning the block under Attributes? - sorry for this possibly stupid question)

Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting one, and I think a great question @stefanfreitag!

From my perspective, the right approach would be to combine the two kubernetes_network_config block definitions (the ones starting at lines 416 and 506 respectively) and then note service_ipv6_cidr as (Read-Only) or (Computed) similar to the way we do for (Optional). I'm not aware of another place in the documentation that we do this, so I'll spend some time looking around to see if we have a precedent here.

Copy link
Member

Choose a reason for hiding this comment

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

In looking, it seems the (Computed) note is an established pattern:

Another time where I wish we were generating all of the docs, but alas 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced (Computed) and applied our approach on kubernetes_network_config also on vpc_config to be consistent in the document.

Merge the block definitions existing in arguments and attributes section for vpc_config and kubernetes_network_config.
Copy link
Member

@justinretzolk justinretzolk left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks again for taking care of this! I'll get it merged once the checks go green 🚀

@justinretzolk justinretzolk merged commit a244b2d into hashicorp:main Feb 12, 2025
22 checks passed
@github-actions github-actions bot removed the needs-triage Waiting for first response or review from a maintainer. label Feb 12, 2025
@github-actions github-actions bot added this to the v5.87.0 milestone Feb 12, 2025
Copy link

This functionality has been released in v5.87.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. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Introduces or discusses updates to documentation. service/eks Issues and PRs that pertain to the eks service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Docs]: aws_eks_cluster is missing details for storage_config
2 participants