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

#199 add CLI feature to use command line parameter '--env-var' secret=… #202

Merged

Conversation

mirkogolze
Copy link
Contributor

#199 add CLI feature to use command line parameter '--env-var secret=xzy123'

}
if (processVars && Array.isArray(processVars)) {
processVars.forEach((value) => {
let parts = value.split('=');
Copy link
Contributor

Choose a reason for hiding this comment

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

@mirkogolze This way of doing it will fail if the value has a = sign which is not uncommon in access tokens

Can you update this code to handle the case.
Something like

processVars.forEach((value) => {
  // split the string at the first equals sign
  const match = value.match(/^([^=]+)=(.*)$/);
  if (!match) {
    console.error(
      chalk.red(`Overridable environment variable not correct: use name=value - presented: `) +
        chalk.dim(`${value}`)
    );
    return;
  }
  const name = match[1];
  const val = match[2];
  envVars[name] = val;
});

@helloanoop
Copy link
Contributor

@mirkogolze I have one comment on the PR. Everything else looks good.

@helloanoop helloanoop merged commit 7a1b448 into usebruno:feature/env-secrets Sep 22, 2023
@helloanoop
Copy link
Contributor

helloanoop commented Sep 22, 2023

@mirkogolze The feature/env-secrets branch might take time to get it completed and merged.

Can you raise a new PR to the main branch with the fixes per my comment ?
With the changes in this PR, we can merge and cut a release for @bruno/cli in the main branch.
Thanks.

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.

None yet

2 participants