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

[FIXED JENKINS-43872, JENKINS-43910] Yet more env tweaks #155

Merged
merged 6 commits into from May 2, 2017

Conversation

Projects
None yet
4 participants
@abayer
Copy link
Member

abayer commented Apr 26, 2017

  • JENKINS issue(s):
  • Description:
    • Hey look, another dumb environment/script.evaluate bug! I still should probably rewrite all this yet again, but for now...this "escapes" dollar signs in literal values and credentials values before evaluateing non-literal env vars that may contain references to these literal/credential vars, and then "unescapes" the dollar signs back into place after all the pesky evaluateing is done.
    • Yet more circuitous processing to make sure we have credentials (including file credentials) processed before we try to process the environment, and definitely making agent get processed before either credentials or environment, along with an ignored test for JENKINS-43911, which existed prior to this but is more obvious now.
  • Documentation changes:
    • n/a
  • Users/aliases to notify:

@abayer abayer added this to the 1.1.4 milestone Apr 26, 2017

@abayer abayer requested a review from rsandell Apr 26, 2017

@reviewbybees

This comment has been minimized.

Copy link

reviewbybees commented Apr 26, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@@ -43,6 +43,8 @@ import org.jenkinsci.plugins.workflow.cps.EnvActionImpl
*/
@SuppressFBWarnings(value="SE_NO_SERIALVERSIONID")
public class Environment implements Serializable {
public static final String DOLLAR_PLACEHOLDER = "___DOLLAR_SIGN___"

This comment has been minimized.

@rsandell

rsandell Apr 27, 2017

Member

or __MONEY_MONEY_MONEY__ :-p

This comment has been minimized.

@bitwiseman

bitwiseman Apr 27, 2017

Contributor

or __DOLLAR_DOLLAR_BILLS__.

@rsandell
Copy link
Member

rsandell left a comment

sigh 🐝

abayer added some commits Apr 27, 2017

[FIXED JENKINS-43910] File credentials should be usable in env
More testing needed, and need to think about per-stage agent stuff.
Fix things, definitively put agent first.
Also add an ignored test for JENKINS-43911, since that did pre-exist
this change but is more obvious as a result of it.

@abayer abayer changed the title [FIXED JENKINS-43872] Properly escape dollar signs in env evaluation [FIXED JENKINS-43872, JENKINS-43910] Yet more env tweaks Apr 27, 2017

abayer added some commits Apr 27, 2017

@rsandell
Copy link
Member

rsandell left a comment

🐝 🏇

@abayer abayer merged commit 2a227fd into jenkinsci:master May 2, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.