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

Required attributes are deleted when empty #137

Closed
pengux opened this issue Jul 11, 2019 · 4 comments
Closed

Required attributes are deleted when empty #137

pengux opened this issue Jul 11, 2019 · 4 comments

Comments

@pengux
Copy link
Contributor

pengux commented Jul 11, 2019

While trying to add support for a new provider, running import doesn't include some required attributes for the resource in the generated TF file. This is because in the ConvertTFstate function of the resource, it delete keys from attributes which are empty. There is an AllowEmptyValue argument which can be used to allow attributes with empty values, but not for those of type array (List).

Instead of deleting empty attributes from HCL generation, wouldn't it be better if it was checked against the schema for the resources and only remove the attributes which don't have Required set to true?

@sergeylanzman
Copy link
Collaborator

  1. AllowEmptyValue - it's regex pattern on tfstate attributes. You can use for list pattern like this ^YOU_LIST_NAME.* or ^YOU_LIST_NAME.[0-9].*
  2. About Required=true - Maybe your right but I think it's need more code changes. Maybe part of this task Start use schema.Schema instead terraform.ResourceProvider #25. If you can change for with small code changes why not? or wait for finish 0.12 support here(master...meshuga:tf_12) and after this we can change provider wrapper code

@meshuga
Copy link
Contributor

meshuga commented Jul 11, 2019

It'll take me a while to move to 0.12. There are internal changes in TF, so want to make sure everything is working as expected.

@pengux
Copy link
Contributor Author

pengux commented Jul 12, 2019

  1. AllowEmptyValue - it's regex pattern on tfstate attributes. You can use for list pattern like this ^YOU_LIST_NAME.* or ^YOU_LIST_NAME.[0-9].*

Yes, but it doesn't apply for arrays it seem. It's trivial to add the check for AllowEmptyValue for arrays too of course but I still think checking the schema from the provider for Required set to true is still the best approach.

  1. About Required=true - Maybe your right but I think it's need more code changes. Maybe part of this task Start use schema.Schema instead terraform.ResourceProvider #25. If you can change for with small code changes why not? or wait for finish 0.12 support here(master...meshuga:tf_12) and after this we can change provider wrapper code

OK, I guess it's better to wait after your 0.12 rewrite, but will you accept a PR to add AllowEmptyValue check for arrays with the current code?

@sergeylanzman
Copy link
Collaborator

You right, removing empty array it's before AllowEmptyValue checking.
Yes I'll accept PR with support array for AllowEmptyValue.

islomar added a commit to form3tech-oss/terraformer that referenced this issue Jul 12, 2019
Currently, the `ConvertTFstate` is deleting keys of type arrays which
are empty, without checking the `allowEmptyValues` argument.

This commit is intended to fix that.

GoogleCloudPlatform#137
islomar added a commit to form3tech-oss/terraformer that referenced this issue Jul 12, 2019
Currently, the `ConvertTFstate` is deleting keys of type arrays which
are empty, without checking the `allowEmptyValues` argument.

This commit is intended to fix that.

GoogleCloudPlatform#137
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants