Skip to content

Commit dd6b3e7

Browse files
committed
Fix remote flag handling and update job options tests for extends resolution
1 parent 185d58d commit dd6b3e7

File tree

3 files changed

+35
-28
lines changed

3 files changed

+35
-28
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@noxify/gitlab-ci-builder": patch
3+
---
4+
5+
Fixed remote flag handling - internal properties are now stripped after extends resolution to ensure remote jobs/templates are correctly excluded from merging while preserving their references.

src/config.ts

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -545,10 +545,7 @@ export class Config {
545545
// - resolveTemplatesOnly: extendKey starts with .
546546
// - else: merge all
547547
// - never merge if extendJob?.remote === true
548-
if (
549-
(!resolveTemplatesOnly || extendKey.startsWith(".")) &&
550-
!(extendJob.remote === true)
551-
) {
548+
if ((!resolveTemplatesOnly || extendKey.startsWith(".")) && extendJob.remote !== true) {
552549
result = merge(result, extendJob)
553550
}
554551
}
@@ -571,19 +568,20 @@ export class Config {
571568
* @param pipeline - The pipeline object being prepared for output.
572569
*/
573570
private clear(pipeline: GitLabCi) {
574-
// Finally, remove all existing `extends`
571+
// Remove local extends entries, keep only remote/external ones, preserve extends if only remote remains
575572
if (!pipeline.jobs) return
576573

577574
const jobIds = Object.keys(pipeline.jobs)
578575
for (const key of jobIds) {
579576
const job = pipeline.jobs[key] as JobDefinitionWithRemote
580-
581577
const jobOpts = this.jobOptionsMap[key]
582578
const mergeExtends = jobOpts?.mergeExtends ?? this.globalOptionsValue.mergeExtends
579+
580+
// Always delete needsExtends
581+
delete job.needsExtends
582+
583583
if (mergeExtends === false) {
584-
// Always delete needsExtends
585-
delete job.needsExtends
586-
// Normalize single-element array to string for cleaner output
584+
// If mergeExtends is false, just optimize extends to string if only one remains
587585
if (Array.isArray(job.extends) && job.extends.length === 1) {
588586
job.extends = job.extends[0]
589587
}
@@ -593,20 +591,21 @@ export class Config {
593591
if (job.extends) {
594592
// Normalize to array for filtering
595593
const extendsArray = Array.isArray(job.extends) ? job.extends : [job.extends]
596-
const filtered = extendsArray.filter((extendName: string) => !jobIds.includes(extendName))
597-
594+
// Remove all local extends entries (i.e., those that exist in pipeline.jobs and are not remote)
595+
const filtered = extendsArray.filter((extendName: string) => {
596+
const extJob = pipeline.jobs?.[extendName]
597+
// Keep if not a local job/template, or if remote
598+
return !extJob || (extJob as JobDefinitionWithRemote).remote === true
599+
})
600+
// If all remaining extends are remote, preserve them
598601
if (filtered.length === 0) {
599602
delete job.extends
600603
} else if (filtered.length === 1) {
601-
// Keep as string if only one extends remains (preserves original format)
602604
job.extends = filtered[0]
603605
} else {
604606
job.extends = filtered
605607
}
606608
}
607-
608-
// Always delete needsExtends as it's internal metadata
609-
delete job.needsExtends
610609
}
611610
}
612611

@@ -635,13 +634,13 @@ export class Config {
635634
public getPlainObject() {
636635
// Create a deep copy to avoid mutating internal state
637636
// Merge templates and jobs (templates first, then jobs can override)
638-
// Prepare jobs/templates for output, removing internal-only props
639-
const jobsOut: Record<string, JobDefinition> = {}
637+
// Keep remote flag during resolution, strip it later
638+
const jobsWithRemote: Record<string, JobDefinitionWithRemote> = {}
640639
for (const [name, job] of Object.entries(this.templatesValue)) {
641-
jobsOut[name] = Config.stripInternalProps(job)
640+
jobsWithRemote[name] = job
642641
}
643642
for (const [name, job] of Object.entries(this.jobsValue)) {
644-
jobsOut[name] = Config.stripInternalProps(job)
643+
jobsWithRemote[name] = job
645644
}
646645

647646
const copy: GitLabCi = JSON.parse(
@@ -657,14 +656,21 @@ export class Config {
657656
default: this.defaultValue,
658657
variables: Object.keys(this.variablesValue).length ? { ...this.variablesValue } : undefined,
659658
include: this.includeValue.length ? [...this.includeValue] : undefined,
660-
jobs: jobsOut,
659+
jobs: jobsWithRemote,
661660
}),
662661
) as GitLabCi
663662

664-
// Resolve extends
663+
// Resolve extends (needs remote flag to be present)
665664
this.resolveExtends(copy)
666665
this.clear(copy)
667666

667+
// Strip internal properties after resolution
668+
if (copy.jobs) {
669+
for (const [name, job] of Object.entries(copy.jobs)) {
670+
copy.jobs[name] = Config.stripInternalProps(job as JobDefinitionWithRemote)
671+
}
672+
}
673+
668674
// Apply patchers (for macros and imports)
669675
for (const patcher of this.patchers) {
670676
patcher(copy)

tests/job-options.test.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,18 @@ import { Config } from "../src/config"
44

55
describe("Job Options", () => {
66
describe("remote option", () => {
7-
it("should ignore remote jobs when merging extends", () => {
7+
it("should ignore remote jobs when merging extends, but keep reference", () => {
88
const config = new Config()
99
config.template(".base", { script: ["template"] })
1010
config.job("remotejob", { script: ["remote"] }, { remote: true })
1111
config.job("child", { extends: [".base", "remotejob"], stage: "test" })
1212

1313
const result = config.getPlainObject()
14-
// Nur .base wird gemerged, remotejob wird ignoriert
1514
expect(result.jobs?.child).toMatchObject({
1615
script: ["template"],
1716
stage: "test",
17+
extends: "remotejob",
1818
})
19-
expect(result.jobs?.child?.extends).toBeUndefined()
2019
})
2120

2221
it("should ignore remote templates when merging", () => {
@@ -26,12 +25,11 @@ describe("Job Options", () => {
2625
config.job("child", { extends: [".remote", ".base"], stage: "test" })
2726

2827
const result = config.getPlainObject()
29-
// Nur .base wird gemerged, .remote wird ignoriert
3028
expect(result.jobs?.child).toMatchObject({
3129
script: ["base"],
3230
stage: "test",
31+
extends: ".remote",
3332
})
34-
expect(result.jobs?.child?.extends).toBeUndefined()
3533
})
3634
})
3735
describe("resolveTemplatesOnly option", () => {
@@ -42,7 +40,6 @@ describe("Job Options", () => {
4240
config.job("child", { extends: [".base", "basejob"], stage: "test" })
4341

4442
const result = config.getPlainObject()
45-
// Nur .base wird gemerged, basejob als lokaler Job wird entfernt
4643
expect(result.jobs?.child).toMatchObject({
4744
script: ["template"],
4845
stage: "test",
@@ -61,7 +58,6 @@ describe("Job Options", () => {
6158
)
6259

6360
const result = config.getPlainObject()
64-
// Beide werden gemerged, Reihenfolge: basejob, dann .base
6561
expect(result.jobs?.child).toMatchObject({
6662
script: ["job", "template"],
6763
stage: "test",

0 commit comments

Comments
 (0)