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

Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version #668

Merged
merged 2 commits into from
Jan 13, 2020

Conversation

barryib
Copy link
Member

@barryib barryib commented Jan 8, 2020

  1. Re-geneate docs with the latest version of terraform-docs 0.7.0 to escape underscores (variable\_with\_underscore).
    Should resolves docs generation issues for Don't fail on destroy, when provider resource was removed #667 and Correcting name of module #666

  2. Update pre-commit-terraform version

@barryib barryib changed the title Re-generate docs with terraform-docs 0.7.0 Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version Jan 8, 2020
@dpiddockcmp
Copy link
Contributor

They've really buggered this up. If you have backticks in the description then the newly added \ get rendered directly.

Input: description = "Valid if: `var.a_thing = false`"

Old output: Valid if: var.a_thing = false

New output: Valid if: var.a\_thing = false

Module's variables and outputs documentation is inconsistent in formatting anyway. Remove backticks in descriptions? Use quotes instead?

@barryib
Copy link
Member Author

barryib commented Jan 9, 2020

Ohh nice catch. I didn't try to render the generated markdown page. I'll try to fixe this.

@dpiddockcmp
Copy link
Contributor

There's a --no-escape option to the markdown subcommand. Not sure if that would make things better or worse. Not that it matters as pre-commit doesn't support passing options to the subcommand.

@barryib
Copy link
Member Author

barryib commented Jan 9, 2020

Yes. I was testing that option manually. If it works, I'll send a PR to pre-commit-terraform.

@barryib barryib changed the title Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version WIP: Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version Jan 9, 2020
@barryib
Copy link
Member Author

barryib commented Jan 9, 2020

I would prefer to use backticks if needed because itThe --no-escape seems to work (at least on github). But I don't know if the right choice. There is a discussion about escaping underscores in markdown terraform-docs/terraform-docs#111.

Opening PR on pre-commit-terraform to add the --no-escape option.

@barryib
Copy link
Member Author

barryib commented Jan 9, 2020

I opened the following PR antonbabenko/pre-commit-terraform#83 to add (or let us add) --no-escape option.

@barryib barryib changed the title WIP: Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version Jan 13, 2020
@barryib
Copy link
Member Author

barryib commented Jan 13, 2020

@max-rocket-internet @dpiddockcmp could you review please ?

Copy link
Contributor

@dpiddockcmp dpiddockcmp left a comment

Choose a reason for hiding this comment

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

Rendered output looks much better. Luckily github's markdown engine isn't fussy about needing to escape _ 😄

Copy link
Contributor

@max-rocket-internet max-rocket-internet left a comment

Choose a reason for hiding this comment

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

Love it.

@barryib barryib merged commit c5f50d5 into terraform-aws-modules:master Jan 13, 2020
@barryib barryib deleted the tba-fix-docs branch January 13, 2020 13:40
barryib added a commit that referenced this pull request Jan 15, 2020
* Don't fail on destroy, when provider resource was removed

* Update Changelog

* Node groups submodule (#650)

* WIP Move node_groups to a submodule

* Split the old node_groups file up

* Start moving locals

* Simplify IAM creation logic

* depends_on from the TF docs

* Wire in the variables

* Call module from parent

* Allow to customize the role name. As per workers

* aws_auth ConfigMap for node_groups

* Get the managed_node_groups example to plan

* Get the basic example to plan too

* create_eks = false works

"The true and false result expressions must have consistent types. The
given expressions are object and object, respectively."
Well, that's useful. But apparently set(string) and set() are ok. So
everything else is more complicated. Thanks.

* Update Changelog

* Update README

* Wire in node_groups_defaults

* Remove node_groups from workers_defaults_defaults

* Synchronize random and node_group defaults

* Error: "name_prefix" cannot be longer than 32

* Update READMEs again

* Fix double destroy

Was producing index errors when running destroy on an empty state.

* Remove duplicate iam_role in node_group

I think this logic works. Needs some testing with an externally created
role.

* Fix index fail if node group manually deleted

* Keep aws_auth template in top module

Downside: count causes issues as usual: can't use distinct() in the
child module so there's a template render for every node_group even if
only one role is really in use. Hopefully just output noise instead of
technical issue

* Hack to have node_groups depend on aws_auth etc

The AWS Node Groups create or edit the aws-auth ConfigMap so that nodes
can join the cluster. This breaks the kubernetes resource which cannot
do a force create. Remove the race condition with explicit depend.

Can't pull the IAM role out of the node_group any more.

* Pull variables via the random_pet to cut logic

No point having the same logic in two different places

* Pass all ForceNew variables through the pet

* Do a deep merge of NG labels and tags

* Update README.. again

* Additional managed node outputs #644

Add change from @TBeijin from PR #644

* Remove unused local

* Use more for_each

* Remove the change when create_eks = false

* Make documentation less confusing

* node_group version user configurable

* Pass through raw output from aws_eks_node_groups

* Merge workers defaults in the locals

This simplifies the random_pet and aws_eks_node_group logic. Which was
causing much consernation on the PR.

* Fix typo

Co-authored-by: Max Williams <max.williams@deliveryhero.com>

* Update Changelog

* Add public access endpoint CIDRs option (terraform-aws-eks#647) (#673)

* Add public access endpoint CIDRs option (terraform-aws-eks#647)

* Update required provider version to 2.44.0

* Fix formatting in docs

* Re-generate docs with terraform-docs 0.7.0 and bump pre-commit-terraform version (#668)

* re-generate docs with terraform-docs 0.7.0

* bump pre-commit-terraform version

* Release 8.0.0 (#662)

* Release 8.0.0

* Update changelog

* remove 'defauls' node group

* Make curl silent

* Update Changelog

Co-authored-by: Daniel Piddock <33028589+dpiddockcmp@users.noreply.github.com>
Co-authored-by: Max Williams <max.williams@deliveryhero.com>
Co-authored-by: Siddarth Prakash <1428486+sidprak@users.noreply.github.com>
Co-authored-by: Thierno IB. BARRY <ibrahima.br@gmail.com>
@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 20, 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.

None yet

3 participants