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

Allow nested_type terraform attributes #101

Merged
merged 6 commits into from
Nov 17, 2023

Conversation

justinc1
Copy link
Contributor

@justinc1 justinc1 commented Nov 4, 2023

SUMMARY

terraform providers schema -json attributes can be nested. In this case, there is no type field, but we have nested_type.

The collection uses schema to sanitize diff in terraform module output.
If attribute is sensitive, its value will be hidden.

In case of nested attribute it might be possible to have top level attribute with sensitive=False containing a child attribute with sensitive=True. But I was not able to find terraform provider with such schema. I looked only at 35 official providers, and ignored remaining ~3000 partner and community providers.

The only official provider with nested_type attributes and also sensitive=True child was waypoint. A schema fragment is included into unit tests. git_auth_basic has non-sensitive username and sensitive password. Also git_auth_basic is marked as sensitive. Thus it seems it is not needed to modify logic for diff sanitization - whole git_auth_basic will be hidden.

Fixes #93

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cloud.terraform.terraform

ADDITIONAL INFORMATION

Terraform provider awscc schema also contains nested attributes, an integration test is added to check schema is parsed.

EDIT:

terraform providers schema -json | jq '.provider_schemas["registry.terraform.io/terraform-redhat/rhcs"].resource_schemas.rhcs_cluster_rosa_classic.block.attributes.admin_credentials'

This is example of nested_type attribute that is not marked as sensitive, but it does contain sensitive sub-attribute.
Maybe a follow up PR will be need to ensure admin_credentials.password is hidden in ansible diff output.

Copy link

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8c52bf2) 73.00% compared to head (45dc3b4) 74.10%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   73.00%   74.10%   +1.09%     
==========================================
  Files          16       17       +1     
  Lines        1015     1058      +43     
  Branches      182      186       +4     
==========================================
+ Hits          741      784      +43     
  Misses        243      243              
  Partials       31       31              
Flag Coverage Δ
sanity ?
units 74.10% <ø> (+1.49%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@abikouo abikouo closed this Nov 16, 2023
@abikouo abikouo reopened this Nov 16, 2023
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/ansible-collections/cloud.terraform for 101,14a2d5c27d883c76d710ce0ca060d7facfa3277c

@abikouo
Copy link
Contributor

abikouo commented Nov 16, 2023

@justinc1 you need to rebase the PR to fix linters issue (especially the isort one on tests/unit/plugins/modules/test_terraform.py)

Copy link

Signed-off-by: Justin Cinkelj <[email protected]>
Copy link

Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @justinc1

Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@justinc1 Thank you for working on this! LGTM!

@alinabuzachis alinabuzachis merged commit a0306d7 into ansible-collections:main Nov 17, 2023
40 of 41 checks passed
@justinc1 justinc1 deleted the no-type branch November 17, 2023 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeyError: 'type' with valid Ansible and Terrform data
3 participants