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

Add sensitive_content argument to local_file resource #9

Merged

Conversation

alewando
Copy link
Contributor

Diffs for the local_file resource include the file content. This can present a problem if the content contains sensitive data. This pull request adds a sensitive_content argument to the local_file resource, causing the file contents to not be included in diff output. The new argument is mutually exclusive with the existing content argument. Exactly one of the two must be present.

I alternatively considered adding a separate "sensitive_local_file" resource, but adding an attribute to the existing resource seemed more appropriate.

@apparentlymart
Copy link
Member

Thanks for working on this, @alewando!

This seems like a reasonable idea to me, and your implementation is good.

The only minor feedback I have here is that schema.Schema has a ConflictsWith field that can be used to implement the mutually-exclusive argument behavior you implemented here with create-time logic. As well as being a little simpler to write, this will also allow Terraform to detect the error during terraform plan rather than only during terraform apply.

Would you be able to make a revision here to use that ConflictsWith mechanism?

Thanks again for working on this!

@alewando
Copy link
Contributor Author

Thanks for the feedback. Using ConflictsWith does simplify things!
I noticed, though, that the validation message for ConflictsWith does expose sensitive attribute values.

Error: local_file.file: "content": conflicts with sensitive_content ("This is some sensitive content")

I'll open a separate issue for that.

@paultyng
Copy link
Contributor

Good catch on the error message as well, should definitely be fixed in core.

@apparentlymart
Copy link
Member

Ahh yes... including the value there is not really important for what this error is trying to communicate, so I think it'd be fine to just drop it and keep the rest of the error message.

We could potentially behave differently depending on whether Sensitive is set on the schema, but that seems like unnecessary complexity here and so I'd say let's just remove the value in all cases.

As Paul says, this'll be a change in the helper/schema package within the main Terraform repository. I don't think the core team at HashiCorp will be able to update this immediately since we're currently engaged in some other work, but definitely worth capturing as an issue!

@alewando
Copy link
Contributor Author

Submitted hashicorp/terraform#17738 on core to remove the attribute value from the ConflictsWith validation message

@rhodrid
Copy link

rhodrid commented May 25, 2018

Would be great if this could be released soon.

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

Successfully merging this pull request may close these issues.

None yet

4 participants