buildkite-plugins / docker-buildkite-plugin

🐳📦 Run any build step in a Docker container
MIT License
112 stars 106 forks source link

How to pass environment variable to command? #248

Closed benwen-instacart closed 1 year ago

benwen-instacart commented 1 year ago

Similar to what we are having for volumes: expand-volume-vars, we would like to pass environment variable to command too. Have tried many possible ways, like single dollar sign $VARIABLE_NAME, double dollar sign $$VARIABLE_NAME, etc, but none works. Could you add expand-command-vars? Or what is the best practice to pass environment variable to command?

code snippet:

steps:
  - label: "My step"
    plugins:
      - my/plugin#v0.0.1: # This is where env var MY_COMMAND_VARIABLE is exported
      - docker#5.6.0:
          image: "my-image"
          expand-volume-vars: true
          volumes:
            - "$$MY_VOLUME_VARIABLE_SRC:$$MY_VOLUME_VARIABLE_DST"
         environment:
           - "MY_COMMAND_VARIABLE"
         command: ["./bin/my-app", "$$MY_COMMAND_VARIABLE"]
benwen-instacart commented 1 year ago

I am able to update ./bin/my-app to read env var, instead of command line, as a workaround. But it will be great if passing env var to command is supported too, in case that the entry point(./bin/my-app in the example) reads parameter from command only and we are not able to update the entry point to read from env var.

toote commented 1 year ago

Hi @benwen-instacart while it would be possible to add the ability to expand variables on commands themselves, the only way to do so would be quite insecure (allowing a malformed or malicious variable value to inject any command). Just as you have figured out, the correct way to achieve what you want is to pass the variable name in the environment plugin configuration and then have your command read the value off the environment. Even in the worst-case scenario in which you are not able to modify the command itself, you can always create a wrapper script in your repository and execute that one instead so I don't think that the modification to the plugin is necessary.

If you feel otherwise, feel free to re-open this issue and we'll continue the discussion