terraform-provider-dokku
terraform-provider-dokku copied to clipboard
aaronstillwell
Config variables don't have quotes escaped when being set
Describe the bug
Config vars don't have quotes escaped when being set on an app.
To Reproduce
Try setting a config var on an app with quotes in it.
Expected behavior
Config vars with quotes should be supported without issue.
Additional context
This issue is being added retroactively. The above problem needs verifying before being worked on. The below snippet is where the suspected problem lies:
https://github.com/aaronstillwell/terraform-provider-dokku/blob/48f0f6827a5192e744556011c3bd7e6e4080c0c3/internal/provider/dokku.go#L96-L106
One question. Is there a way to put terraform in a verbose mode to spit out the command it is about to perform? @aaronstillwell ?
@jyoost yes see the TF_LOG environment variable. https://www.terraform.io/cli/config/environment-variables
The run command automatically logs all dokku commands sent to the server.
https://github.com/aaronstillwell/terraform-provider-dokku/blob/48f0f6827a5192e744556011c3bd7e6e4080c0c3/internal/provider/ssh.go#L25-L56
@jyoost a further good starting point for this issue would be to verify the problem exists by updating the following acceptance test to include a config var with a quote.
https://github.com/aaronstillwell/terraform-provider-dokku/blob/48f0f6827a5192e744556011c3bd7e6e4080c0c3/internal/provider/resource_app_test.go#L155-L200
@aaronstillwell just wanted you to know, I'm working on these Issues. I set up a development environment, forked the project. Was able to build and run local. I also was able to deploy to a new VPS set up with dokku. so i will be testing on a live remote server.
I am in Oregon and on PST. Where are you located.
@aaronstillwell I want to verify we are looking to have it handle embedded quotes like "hell"o" dokku" or "hello" dokku"
Sorry double quotes as per your PR - misread your last comment here.
@aaronstillwell I sent you an email so you would have my contact info outside this repo.
I got hung up on other projects this week and I also got STUCK on what to do to include a test for this issue.
I can run the test manually by plugging in a string with quotes and it passes, what I am stuck on is how to add a test case to the workflow to do that.
If you could make the change in a separate branch so I can see what you did, it would get me unstuck and I will know what to do on other tests.
No problem @jyoost.
Some background information on acceptance testing a Terraform provider can be found here.
We already have an acceptance test for setting config variables here:
https://github.com/aaronstillwell/terraform-provider-dokku/blob/48f0f6827a5192e744556011c3bd7e6e4080c0c3/internal/provider/resource_app_test.go#L155-L200
You can add a test step to the existing by coping the last step e.g
https://github.com/aaronstillwell/terraform-provider-dokku/blob/48f0f6827a5192e744556011c3bd7e6e4080c0c3/internal/provider/resource_app_test.go#L185-L197
but changing the value of the config var FOO3 in this example to something with a quote. You'd then need to change the assertion that calls testAccCheckDokkuAppConfigVar to match.
Hopefully that's enough guidance to get something working?
@jyoost I've just updated the vagrantfile & readme to make it easier to run these tests locally using a vagrant VM.
hey @aaronstillwell ,
After extensive testing on my live system, something is eating the quotes. since the debug messages print out the env vars as "******" it is very hard to determine what is happening.
HELP!!
@jyoost
Sorry slow reply @jyoost - yes the logging is filtered deliberately to prevent leaking secrets. Have you tried developing with an acceptance test? This might be the easiest way to verify your change vs testing against prod like that.
@aaronstillwell ,
No problem on being late , I have been busy on other projects.
I am using the acceptance test, but not vagrant. Vagrant requires virtualbox which I refuse to install. I run ubuntu Linux on my desktop. Virtualbox almost trashed my computer one time.
The test for this only checks the output. When I get back to it I think I have it solved.