Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Can't use "$" (dollar-sign) char in snap task files #1691

Open
michep opened this issue Jul 18, 2017 · 9 comments
Open

Can't use "$" (dollar-sign) char in snap task files #1691

michep opened this issue Jul 18, 2017 · 9 comments

Comments

@michep
Copy link
Contributor

michep commented Jul 18, 2017

Due to calling os.ExpandEnv() in snap\core\task.go:331 [func UnmarshalBody()] and in snap\cmd\snaptel\task.go:355 [func createTaskUsingTaskManifest()] it is not possible to use "$" char in snap task files, it will be unconditionally removed, and there is no way to escape this symbol.

Example here.

Now Im creating a processor plugin in which one can specify command line to execute, and if it contains "$" char in it, like cat /etc/fstab |grep ext[2,3,4]| awk '{print($2);}', it will be broken.

@kjlyon
Copy link
Contributor

kjlyon commented Jul 18, 2017

Good catch @michep, we'll add this to our backlog. Though if you have a fix in mind, go ahead and submit a PR. We appreciate all contributions.

@michep
Copy link
Contributor Author

michep commented Jul 18, 2017

Approach to resolve this issue is depend on how current functionality supposed to work. Why, in the first place, it was implemented. Is it really necessary to use environment variables expansion in tasks?

One way is to use regexp \$\{{0,1}[A-Z_]+(\}{0,1}|s+).
Take a look here: https://play.golang.org/p/l__RnjmqnE
This will be signicantly slower and more resource intensive solution, but this is not so frequently used, actually.

@kjlyon
Copy link
Contributor

kjlyon commented Jul 24, 2017

@michep, do you mind sharing your task file with me? I am curious to see where exactly you are using the command cat /etc/fstab |grep ext[2,3,4]| awk '{print($2);}'

@michep
Copy link
Contributor Author

michep commented Jul 24, 2017

This is all about my plugin - https://github.com/michep/snap-plugin-processor-maptag
Dollar-sign can be used in regex and in cmd&args in task.

@IzabellaRaulin
Copy link
Contributor

Hello @michep, I see your point that you want to directly pass e.g. cat /etc/fstab |grep ext[2,3,4]| awk '{print($2);}' as a value of one of config params in your processor plugin. However, there is functionality in Snap that in task manifest you can point to the value of environment variable by its name - for example, you can control where your data are published by setting an appropriate value of $INFLUXDB_HOST as below:

[...]
      publish:
        - plugin_name: "influxdb"
          config:
             host: "${INFLUXDB_HOST}"
             port: 8086
             database: "test"
[...]

See full example here

In your case, you need to pass a value of config param (which includes a dollar char) in a different way, not directly. @michep, do you need help in that?

@michep
Copy link
Contributor Author

michep commented Jul 25, 2017

Hi, @IzabellaRaulin. So, what is that other, indirect way?

@IzabellaRaulin
Copy link
Contributor

@michep - one way is providing in task manifest a path to file in which you can have the dollar sign. What is more, you as an author of plugin can decide what this structure of the file should look like - it might be just one line file with a command or some json/yaml when user can declare more params, depending on needs.

Example 1 - where command.txt has one line containing the command cat /etc/fstab |grep ext[2,3,4]| awk '{print($2);}':

plugin_name: "my_plugin"
          config:
             commandfile: "/home/user/command.txt"
             param2: "something"
             param3: 100  

As a value of config you receive path to command file, so in plugin needs to be handled opening this file and interpreting its contents.

Example 2 - where setfile.yaml contains all possible parameters with determined its value (notice that there are no param2, neither param3. All config parameters might be determined in setfile.yaml together with command.

plugin_name: "my_plugin"
          config:
             setfile: "/home/user/setfile.yaml"         

Please, let me know if this has sense in your use case and what do you think about it .

@michep
Copy link
Contributor Author

michep commented Jul 28, 2017

Following the Occam's razor principle one should not create new entities until this is truly necessary.
I believe that solution here is not "you must move your dollar-sign-contained texts into separate files", but "fix how environment variables expansion works".
I think unconditional removing of all dollar-sign symbols in tasks and configs is a huge issue.
I think something like python's os.path.expandvars() should be implemented, using regexp $(\w+|{[^}]*}): os.path.expandvars() source code

@IzabellaRaulin
Copy link
Contributor

Hello @michep, sorry for delayed response. I think your proposition is valuable. I look on python's os.path.expandvars() and I agree that similar approach should be considered in Snap. I will create RFC and describe the proposition, just to summarize our discussion and continue design discussion under new GH issue. It would be great if you would like to contribute to that. Thank You.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants