Skip to content

DOC: Update terraform_state.py #152

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DOC: Update terraform_state.py #152

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions plugins/inventory/terraform_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
- Aubin Bikouo (@abikouo)
short_description: Builds an inventory from resources created by cloud providers.
description:
- This plugin works with an existing state file to create an inventory from resources created by cloud providers.
- The plugin accepts a Terraform backend config to an existing state file or a path to an existing state file.
- Uses a YAML configuration file that ends with terraform_state.(yml|yaml).
- To read the state file command ``Terraform show`` is used.
- This plugin works by making a temporary directory, writing a C(main.tf) file with the configured backend, then running C(terraform init),
followed by C(terraform show) in JSON format to read state.
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

These are implementation details and not something I think should appear in the module docs. There's currently a PR (#153) that changes this.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I obviously disagree. The current way it works should be stated somewhere (perhaps in NOTES?) I think considering the potential for danger in running something like init, it should be quite clear what it currently does. It currently states "terraform state is used", so I felt it needed fleshing out.

There's currently a PR (#153) that changes this.

If the PR changes this, then the doco can change at that point?

The parameters section documents things that implement changes, so 🤷

- The plugin accepts a Terraform backend config to an existing backend, or path(s) to files containing backend config.
- Uses a YAML configuration file that ends with C(terraform_state.yml) or C(terraform_state.yaml). For example, C(aws_terraform_state.yaml).
- Currently only supports the following Terraform providers - C(aws), C(azurerm), C(google).
Copy link
Member

Choose a reason for hiding this comment

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

#146 would add the ability to use any provider.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, nice, but again, until such time, the text I've suggested above would have saved me a few hours to realise the module does nothing for me when using a provider not in that currently supported group.

- Does not support caching.
extends_documentation_fragment:
- constructed
Expand Down
Loading