scottwinkler / terraform-provider-shell

Terraform provider for executing shell commands and saving output to state file
Mozilla Public License 2.0
279 stars 60 forks source link

Provider configuration #49

Closed derhally closed 4 years ago

derhally commented 4 years ago

Thanks for your time and effort for creating this provider. It works great. I was wondering if it would make sense to have the provider take a map and have it in turn set those as environment variable when the scripts execute. This would allow certain values be transient and saved in state, for example an api token which you don't want stored in state and needs to be refreshed in every execution.

right now if an api token is passed in via the sensitive_environment section, it gets stored in state, but the token can expire and that would cause errors when the resource needs to be deleted because the token would be read from state and not be refreshed.

scottwinkler commented 4 years ago

Hi derhally,

All attributes of Terraform resources are stored in state, no matter if they are marked as sensitive or not. There is no sense of "transient" attributes, as you suggest. That being said, what you are asking for is already technically possible. The shell_script resource will pick up any environment variables set on the machine running Terraform, not just the ones explicitly passed in through the environment block. If you want to change the api token without it triggering an update, then I would just set an environment variable on the machine. It wouldn't be marked as sensitive, and there would be no way of tracking changes, but as long as you don't print out the value, and just use it for something like API tokens, then it would be perfectly fine.

Another possibility is that you can implement an update() method. Currently, if you change any variable in the environment block, it will trigger an update(). If you did not implement update() yourself, then what happens is the same as a ForceNew resource - delete() is called, then create() is called again. This is probably why you are getting the resource to be deleted. On the other hand, if you implement update() then you could simply do nothing when the api token is changed. Here is an example of using create, read, update, and delete for a simple resource: https://github.com/scottwinkler/terraform-provider-shell/tree/master/examples/complete-with-update

derhally commented 4 years ago

Thanks scott.

Correct me if I'm wrong, but even implementing the update script, the issue still exists. When a resource needs to be deleted, terraform will use what is in state and the env variable is stale and can't be used.

You are correct that I could set an environment variable on the machine, but would it not be cleaner if the provider could take a set of values, those would always be current and that would allow the delete to happen.

Would you be open to accepting a PR that allows setting a configuration section on the provider?

scottwinkler commented 4 years ago

You are incorrect, and this is really a problem in the documentation, or lack thereof. I created an example of using this provider to manage a Github repository and uploaded it just now. it can be found here: https://github.com/scottwinkler/terraform-provider-shell/blob/master/examples/github-repo/main.tf

In this example, I am using an OAUTH_TOKEN in the sensitive environment block to make requests. If this variable were to be changed, then it would trigger an update on the resource, but that wouldn't actually delete anything. I would still prefer to using an environment variable set on the machine for this, but it is still doable this way.

derhally commented 4 years ago

The OAUTH_TOKEN changing would trigger an update, but not a delete. What I'm talking about is when the resource needs to be deleted, OAUTH_TOKEN from state is used and that token could be stale/expired which would make the delete script fail.

Imagine the token expires after 1 hour, and someone deletes the resource, the delete script would get called and it would fail because OATH_TOKEN is expired and not updated.

The objective is to be able to always pass in environment variables to the shell that are guaranteed to be current.

scottwinkler commented 4 years ago

Released as part of version 1.4.0