aaronstillwell / terraform-provider-dokku

A terraform provider for provisioning applications on the Dokku PaaS
Mozilla Public License 2.0
54 stars 4 forks source link

Config variables don't have quotes escaped when being set #7

Closed aaronstillwell closed 1 month ago

aaronstillwell commented 2 years ago

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

jyoost commented 2 years ago

You can assign this to me @aaronstillwell

jyoost commented 2 years ago

One question. Is there a way to put terraform in a verbose mode to spit out the command it is about to perform? @aaronstillwell ?

aaronstillwell commented 2 years ago

@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

aaronstillwell commented 2 years ago

@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

jyoost commented 2 years ago

@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 commented 2 years ago

Nice job @jyoost. London UK, UTC

jyoost commented 2 years ago

@aaronstillwell I want to verify we are looking to have it handle embedded quotes like "hell"o" dokku" or "hello" dokku"

aaronstillwell commented 2 years ago

Correct.

aaronstillwell commented 2 years ago

Sorry double quotes as per your PR - misread your last comment here.

jyoost commented 2 years ago

@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.

aaronstillwell commented 2 years ago

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?

aaronstillwell commented 2 years ago

@jyoost I've just updated the vagrantfile & readme to make it easier to run these tests locally using a vagrant VM.

jyoost commented 2 years ago

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

aaronstillwell commented 2 years ago

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.

jyoost commented 2 years ago

@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.

jyoost commented 2 years ago

Hey @aaronstillwell ,

Could we have a short zoom meeting some time?