fix: variable expansion in environment (#1443)

firecow · Dec 2, 2024 · 2329081 · 2329081
1 parent bde98f6
commit 2329081
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 10 deletions.
diff --git a/src/job.ts b/src/job.ts
@@ -198,6 +198,9 @@ export class Job {
         predefinedVariables["CI_REGISTRY"] = `local-registry.${this.gitData.remote.host}`;
         predefinedVariables["CI_REGISTRY_IMAGE"] = `$CI_REGISTRY/${this._variables["CI_PROJECT_PATH"].toLowerCase()}`;
 
+        // NOTE: Update `this._variables` with the patched predefinedVariables
+        this._variables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...fileVariables, ...argvVariables};
+
         // Expand variables in rules:changes
         if (this.rules && expandVariables) {
             const expanded = Utils.expandVariables(this._variables);
@@ -214,6 +217,16 @@ export class Job {
             });
         }
 
+        let ruleVariables: {[name: string]: string} | undefined;
+        // Set {when, allowFailure} based on rules result
+        if (this.rules) {
+            const ruleResult = Utils.getRulesResult({argv, cwd, rules: this.rules, variables: this._variables}, this.gitData, this.when, this.allowFailure);
+            this.when = ruleResult.when;
+            this.allowFailure = ruleResult.allowFailure;
+            ruleVariables = ruleResult.variables;
+            this._variables = {...this._variables, ...ruleVariables, ...argvVariables};
+        }
+
         // Find environment matched variables
         if (this.environment && expandVariables) {
             const expanded = Utils.expandVariables(this._variables);
@@ -223,15 +236,7 @@ export class Job {
         const envMatchedVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir, this.environment);
 
         // Merge and expand after finding env matched variables
-        this._variables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...envMatchedVariables, ...argvVariables};
-
-        // Set {when, allowFailure} based on rules result
-        if (this.rules) {
-            const ruleResult = Utils.getRulesResult({argv, cwd, rules: this.rules, variables: this._variables}, this.gitData, this.when, this.allowFailure);
-            this.when = ruleResult.when;
-            this.allowFailure = ruleResult.allowFailure;
-            this._variables = {...this.globalVariables, ...jobVariables, ...ruleResult.variables, ...matrixVariables, ...predefinedVariables, ...envMatchedVariables, ...argvVariables};
-        }
+        this._variables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...ruleVariables, ...envMatchedVariables, ...argvVariables};
         // Delete variables the user intentionally wants unset
         for (const unsetVariable of argv.unsetVariables) {
             delete this._variables[unsetVariable];

diff --git a/src/parser.ts b/src/parser.ts
@@ -184,7 +184,7 @@ export class Parser {
                     baseName: jobName,
                     globalVariables: gitlabData.variables,
                     pipelineIid: pipelineIid,
-                    predefinedVariables,
+                    predefinedVariables: {...predefinedVariables}, // NOTE: pass by value because predefinedVariables is mutated in the constructor
                     gitData,
                     variablesFromFiles,
                     matrixVariables: parallelMatrixVariables,

diff --git a/tests/test-cases/variable-expansion/.gitlab-ci-1.yml b/tests/test-cases/variable-expansion/.gitlab-ci-1.yml
@@ -0,0 +1,6 @@
+---
+job:
+  environment:
+    name: review/$CI_JOB_STAGE/deploy     # =>  review/job/deploy
+  script:
+    - echo 'deploy staging'
diff --git a/tests/test-cases/variable-expansion/.gitlab-ci-2.yml b/tests/test-cases/variable-expansion/.gitlab-ci-2.yml
@@ -0,0 +1,10 @@
+---
+job:
+  rules:
+    - variables:
+        PIPELINE_ENVIRONMENT: test
+  image: alpine
+  environment:
+    name: $PIPELINE_ENVIRONMENT  # expects this to be expanded to `test`
+  script:
+    - echo $PIPELINE_ENVIRONMENT
diff --git a/tests/test-cases/variable-expansion/integration.test.ts b/tests/test-cases/variable-expansion/integration.test.ts
@@ -63,3 +63,29 @@ test(`variable-expansion <${test_job_4}>`, async () => {
 
     expect(writeStreams.stdoutLines.join("\n")).toContain(expected);
 });
+
+test("should expand predefined variables in environment", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/variable-expansion",
+        file: ".gitlab-ci-1.yml",
+        noColor: true,
+    }, writeStreams);
+
+    const expected = "job environment: { name: review/test/deploy }";
+    const filteredStdout = writeStreams.stdoutLines.filter(f => f.startsWith("job environment")).join("\n");
+    expect(filteredStdout).toEqual(expected);
+});
+
+test("should expand rule variables in environment", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/variable-expansion",
+        file: ".gitlab-ci-2.yml",
+        noColor: true,
+    }, writeStreams);
+
+    const expected = "job environment: { name: test }";
+    const filteredStdout = writeStreams.stdoutLines.filter(f => f.startsWith("job environment")).join("\n");
+    expect(filteredStdout).toEqual(expected);
+});