Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add env subst command [CPE-1773] #910

Merged
merged 2 commits into from
Apr 19, 2023
Merged

feat: add env subst command [CPE-1773] #910

merged 2 commits into from
Apr 19, 2023

Conversation

KyleTryon
Copy link
Contributor

Checklist

=========

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked for similar issues and haven't found anything relevant.
  • This is not a security issue (which should be reported here: https://circleci.com/security/)
  • I have read Contribution Guidelines.

Internal Checklist

  • I am requesting a review from my own team as well as the owning team
  • I have a plan in place for the monitoring of the changes that I am making (this can include new monitors, logs to be aware of, etc...)

Changes

=======

This PR adds a new command env, with a sub-command subst which implements the envsubst library (~2MB) to provide a universal method for CircleCI jobs to safely substitute variables within strings.

Example:

$ export MYVAR=world
$ paragraph="Hello ${MYVAR}"
$ circleci env subst $paragraph
> Hello world

Rationale

=========

CircleCI's orbs rely on a number of "standard" dependencies in order to work, which we require of our users to have installed for the majority of orbs to work, such as curl and JQ. Recently we have seen that an additional tool is necessary to properly evaluate strings containing environment variables on all platforms properly.

The way we develop orbs has changed in the last year to change patterns in a way that is more maintainable but has had unintended consequences.

Old orb method:

version: 2.1

commands:
  sayhello:
    description: "A very simple command for demonstration purposes"
    parameters:
      to:
        type: string
        default: "World"
    steps:
      - run: echo Hello << parameters.to >>


Notice here, there is bash inside the yaml where the run statement is found. Inside this bash, we have yet another syntax for CircleCI's parameters.

To make orbs more maintainable, we now move the bash script out of the orb source so that the user can execute it locally and get proper syntax highlighting in their IDE for the bash code. To accommodate parameters, we switched to something like this:

version: 2.1
commands:
  sayhello:
    description: A very simple command for demonstration purposes
    parameters:
      to:
        type: string
        default: World
    steps:
      - run:
          env:
            PARAM_TO: <<parameters.to>>
          command: <<include(script/source.sh)>>


Here, the script is imported at the time the orb is packed and it relies on the value of the parameter being within an environment variable

The problem


The problem with this new way of building orbs, is we now have environment variables with environment variables within them being interpreted as strings. We originally tried using the eval linux command, but this has many unintended consequences, such as treating = literally within a string where it should not.

Solution


Using this tool, users can 'sanitize' environment variables like so.

MYVAR=$(circleci env subst $MYVAR)

Why not require users to install envsubst?

We have taken this approach for all other tools such as BASH (vs shell), CURL, and JQ. However, those tools are not inherent to the function of orbs the product, instead these tools are necessary by individual orbs to accomplish tasks, and it may be reasonable that to use an orb which parses JSON, to require JQ. For this tool, this would be used by all orbs as part of a "standard library" to properly interpret parameters, not accomplish any specific task as described by the orb. Additionally we have seen that many tools such as awk and sed, which add a lot of friction for orb developers, also behave differently on different operating systems.

Considerations

==============

How the command name was chosen


In the local CircleCI CLI we have a command for contexts but contexts are not the only type of environment variable that can be updated via API. It might make sense eventually to move all "secret management" tools within our CLIs to a command such as env so that we may group secret management commands together. So theoretically, maybe the public CLI would eventually change from circleci contexts to circleci env contexts. This would give a home for all secrets tools, including the proposed subst command in this PR. I would eventually like this command to be found in both CLIs so that scripts can be run locally.

Screenshots

============

image

@KyleTryon KyleTryon requested a review from a team as a code owner April 18, 2023 18:32
Copy link
Contributor

@EricRibeiro EricRibeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@JalexChen JalexChen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than lint and coverage jobs in the build, LGTM

Copy link
Contributor

@JulesFaucherre JulesFaucherre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just need to fix the CI
I remember you proposed putting circleci env context as an alias to circleci context. Is there any reason you finally decided not to do it?

@KyleTryon
Copy link
Contributor Author

@JulesFaucherre While that suggestion was part of the justification for the potential to add the env command namespace, the context change was just not part of the goals of this PR.

@KyleTryon KyleTryon merged commit 5b836ba into develop Apr 19, 2023
@KyleTryon KyleTryon deleted the feat/env-subst branch April 19, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants