Skip to content

Commit

Permalink
fix: predefinedVariables should have the lowest precedence and discou…
Browse files Browse the repository at this point in the history 
…rage overriding of predefined variables (#1466)

* fix: predefinedVariables should have the lowest precedence

* feat: discourage overriding of predefined variables
  • Loading branch information
@ANGkeith
ANGkeith authored Jan 13, 2025
1 parent fa11a3e commit dd044e2
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 48 deletions.
102 changes: 65 additions & 37 deletions src/job.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as yaml from "js-yaml";
import {Parser} from "./parser.js";
import globby from "globby";
import {validateIncludeLocal} from "./parser-includes.js";
import terminalLink from "terminal-link";

const CI_PROJECT_DIR = "/gcl-builds";
const GCL_SHELL_PROMPT_PLACEHOLDER = "<gclShellPromptPlaceholder>";
Expand Down Expand Up @@ -135,14 +136,11 @@ export class Job {

constructor (opt: JobOptions) {
const jobData = opt.data;
const gitData = opt.gitData;
const jobVariables = jobData.variables ?? {};
const variablesFromFiles = opt.variablesFromFiles;
const argv = opt.argv;
const cwd = argv.cwd;
const stateDir = argv.stateDir;
const argvVariables = argv.variable;
const predefinedVariables = opt.predefinedVariables;
const expandVariables = opt.expandVariables ?? true;

this.argv = argv;
Expand All @@ -167,39 +165,8 @@ export class Job {

const matrixVariables = opt.matrixVariables ?? {};
const fileVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir);
this._variables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...fileVariables, ...argvVariables};

let ciProjectDir = `${cwd}`;
if (this.jobData["image"]) {
ciProjectDir = CI_PROJECT_DIR;
} else if (argv.shellIsolation) {
ciProjectDir = `${cwd}/${stateDir}/builds/${this.safeJobName}`;
}

predefinedVariables["CI_JOB_ID"] = `${this.jobId}`;
predefinedVariables["CI_PIPELINE_ID"] = `${this.pipelineIid + 1000}`;
predefinedVariables["CI_PIPELINE_IID"] = `${this.pipelineIid}`;
predefinedVariables["CI_JOB_NAME"] = `${this.name}`;
predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
predefinedVariables["CI_JOB_STAGE"] = `${this.stage}`;
predefinedVariables["CI_PROJECT_DIR"] = ciProjectDir;
predefinedVariables["CI_JOB_URL"] = `${this._variables["CI_SERVER_URL"]}/${gitData.remote.group}/${gitData.remote.project}/-/jobs/${this.jobId}`; // Changes on rerun.
predefinedVariables["CI_PIPELINE_URL"] = `${this._variables["CI_SERVER_URL"]}/${gitData.remote.group}/${gitData.remote.project}/pipelines/${this.pipelineIid}`;
predefinedVariables["CI_ENVIRONMENT_NAME"] = this.environment?.name ?? "";
predefinedVariables["CI_ENVIRONMENT_SLUG"] = this.environment?.name?.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 23).replace(/-$/, "").toLowerCase() ?? "";
predefinedVariables["CI_ENVIRONMENT_URL"] = this.environment?.url ?? "";
predefinedVariables["CI_ENVIRONMENT_TIER"] = this.environment?.deployment_tier ?? "";
predefinedVariables["CI_ENVIRONMENT_ACTION"] = this.environment?.action ?? "";

if (opt.nodeIndex !== null) {
predefinedVariables["CI_NODE_INDEX"] = `${opt.nodeIndex}`;
}
predefinedVariables["CI_NODE_TOTAL"] = `${opt.nodesTotal}`;
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};
const predefinedVariables = this._predefinedVariables(opt);
this._variables = {...predefinedVariables, ...this.globalVariables, ...jobVariables, ...matrixVariables, ...fileVariables, ...argvVariables};

// Expand variables in rules:changes
if (this.rules && expandVariables) {
Expand Down Expand Up @@ -235,8 +202,11 @@ export class Job {
}
const envMatchedVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir, this.environment);

const userDefinedVariables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...ruleVariables, ...envMatchedVariables, ...argvVariables};
this.discourageOverridingOfPredefinedVariables(predefinedVariables, userDefinedVariables);

// Merge and expand after finding env matched variables
this._variables = {...this.globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...ruleVariables, ...envMatchedVariables, ...argvVariables};
this._variables = {...predefinedVariables, ...userDefinedVariables};
// Delete variables the user intentionally wants unset
for (const unsetVariable of argv.unsetVariables) {
delete this._variables[unsetVariable];
Expand Down Expand Up @@ -280,6 +250,64 @@ export class Job {
}
}

/**
* Warn when overriding of predefined variables is detected
*/
private discourageOverridingOfPredefinedVariables (predefinedVariables: {[name: string]: string}, userDefinedVariables: {[name: string]: string}) {
const predefinedVariablesKeys = Object.keys(predefinedVariables);
const userDefinedVariablesKeys = Object.keys(userDefinedVariables);

const overridingOfPredefinedVariables = userDefinedVariablesKeys.filter(ele => predefinedVariablesKeys.includes(ele));
if (overridingOfPredefinedVariables.length == 0) {
return;
}

const linkToGitlab = "https://gitlab.com/gitlab-org/gitlab/-/blob/v17.7.1-ee/doc/ci/variables/predefined_variables.md?plain=1&ref_type=tags#L15-16";
this.writeStreams.memoStdout(chalk`{bgYellowBright WARN } ${terminalLink("Avoid overriding predefined variables", linkToGitlab)} [{bold ${overridingOfPredefinedVariables}}] as it can cause the pipeline to behave unexpectedly.\n`);
}

/**
* Get the predefinedVariables that's enriched with the additional info that's only available when constructing
*/
private _predefinedVariables (opt: JobOptions) {
const argv = this.argv;
const cwd = argv.cwd;
const stateDir = this.argv.stateDir;
const gitData = this.gitData;

let ciProjectDir = `${cwd}`;
if (this.jobData["image"]) {
ciProjectDir = CI_PROJECT_DIR;
} else if (argv.shellIsolation) {
ciProjectDir = `${cwd}/${stateDir}/builds/${this.safeJobName}`;
}

const predefinedVariables = opt.predefinedVariables;

predefinedVariables["CI_JOB_ID"] = `${this.jobId}`;
predefinedVariables["CI_PIPELINE_ID"] = `${this.pipelineIid + 1000}`;
predefinedVariables["CI_PIPELINE_IID"] = `${this.pipelineIid}`;
predefinedVariables["CI_JOB_NAME"] = `${this.name}`;
predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
predefinedVariables["CI_JOB_STAGE"] = `${this.stage}`;
predefinedVariables["CI_PROJECT_DIR"] = ciProjectDir;
predefinedVariables["CI_JOB_URL"] = `${predefinedVariables["CI_SERVER_URL"]}/${gitData.remote.group}/${gitData.remote.project}/-/jobs/${this.jobId}`; // Changes on rerun.
predefinedVariables["CI_PIPELINE_URL"] = `${predefinedVariables["CI_SERVER_URL"]}/${gitData.remote.group}/${gitData.remote.project}/pipelines/${this.pipelineIid}`;
predefinedVariables["CI_ENVIRONMENT_NAME"] = this.environment?.name ?? "";
predefinedVariables["CI_ENVIRONMENT_SLUG"] = this.environment?.name?.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 23).replace(/-$/, "").toLowerCase() ?? "";
predefinedVariables["CI_ENVIRONMENT_URL"] = this.environment?.url ?? "";
predefinedVariables["CI_ENVIRONMENT_TIER"] = this.environment?.deployment_tier ?? "";
predefinedVariables["CI_ENVIRONMENT_ACTION"] = this.environment?.action ?? "";

if (opt.nodeIndex !== null) {
predefinedVariables["CI_NODE_INDEX"] = `${opt.nodeIndex}`;
}
predefinedVariables["CI_NODE_TOTAL"] = `${opt.nodesTotal}`;
predefinedVariables["CI_REGISTRY"] = `local-registry.${this.gitData.remote.host}`;
predefinedVariables["CI_REGISTRY_IMAGE"] = `$CI_REGISTRY/${predefinedVariables["CI_PROJECT_PATH"].toLowerCase()}`;
return predefinedVariables;
}

get jobStatus () {
if (this.preScriptsExitCode == null) return "pending";
if (this.preScriptsExitCode == 0) return "success";
Expand Down
27 changes: 16 additions & 11 deletions tests/test-cases/variable-order/integration.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {WriteStreamsMock} from "../../../src/write-streams.js";
import {handler} from "../../../src/handler.js";
import chalk from "chalk";
import {initSpawnSpy} from "../../mocks/utils.mock.js";
import {WhenStatics} from "../../mocks/when-statics.js";

Expand All @@ -15,16 +14,22 @@ test("variable-order <test-job> --needs", async () => {
job: ["test-job"],
variable: ["PROJECT_VARIABLE=project-value"],
home: "tests/test-cases/variable-order/.home",
noColor: true,
}, writeStreams);

const expected = [
chalk`{blueBright test-job} {greenBright >} PIPELINE_VARIABLE=pipeline-value`,
chalk`{blueBright test-job} {greenBright >} JOB_VARIABLE=job-value`,
chalk`{blueBright test-job} {greenBright >} OVERRIDDEN_BY_JOB=job-value`,
chalk`{blueBright test-job} {greenBright >} CI_PIPELINE_ID=1000`,
chalk`{blueBright test-job} {greenBright >} HOME_VARIABLE=home-value`,
chalk`{blueBright test-job} {greenBright >} PROJECT_VARIABLE=project-value`,
chalk`{blueBright test-job} {greenBright >} PROD_ONLY_VARIABLE=notprod`,
];
expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
const expected = `
test-job > PIPELINE_VARIABLE=pipeline-value
test-job > JOB_VARIABLE=job-value
test-job > OVERRIDDEN_BY_JOB=job-value
test-job > CI_PIPELINE_ID=pipeline-value
test-job > CI_JOB_ID=job-value
test-job > HOME_VARIABLE=home-value
test-job > PROJECT_VARIABLE=project-value
test-job > PROD_ONLY_VARIABLE=notprod
`.trim();

const filteredStdout = writeStreams.stdoutLines.filter(f => f.startsWith("test-job >")).join("\n");
expect(filteredStdout).toEqual(expected);

expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining([" WARN Avoid overriding predefined variables (​https://gitlab.com/gitlab-org/gitlab/-/blob/v17.7.1-ee/doc/ci/variables/predefined_variables.md?plain=1&ref_type=tags#L15-16​) [CI_PIPELINE_ID,CI_JOB_ID] as it can cause the pipeline to behave unexpectedly."]));
});

0 comments on commit dd044e2

Please sign in to comment.