-
Notifications
You must be signed in to change notification settings - Fork 57.3k
fix(Jenkins Node): Properly load job parameters #28631
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
Changes from all commits
f899be1
b53a5a7
85d9ade
32ee15a
e3c5bb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -452,15 +452,25 @@ export class Jenkins implements INodeType { | |
| async getJobParameters(this: ILoadOptionsFunctions): Promise<INodePropertyOptions[]> { | ||
| const job = this.getCurrentNodeParameter('job') as string; | ||
| const returnData: INodePropertyOptions[] = []; | ||
| const endpoint = `/job/${job}/api/json?tree=actions[parameterDefinitions[*]]`; | ||
| const { actions } = await jenkinsApiRequest.call(this, 'GET', endpoint); | ||
| for (const { _class, parameterDefinitions } of actions) { | ||
| if (_class?.includes('ParametersDefinitionProperty')) { | ||
| for (const { name, type } of parameterDefinitions) { | ||
| const endpoint = `/job/${job}/api/json?tree=actions[parameterDefinitions[*]],property[parameterDefinitions[*]]`; | ||
| const result = await jenkinsApiRequest.call(this, 'GET', endpoint); | ||
| const allParameters = [...(result.actions ?? []), ...(result.property ?? [])]; | ||
| const seenParameterNames = new Set<string>(); | ||
| for (const { _class, parameterDefinitions } of allParameters) { | ||
| if ( | ||
| !_class?.includes('ParametersDefinitionProperty') || | ||
| !Array.isArray(parameterDefinitions) | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| for (const { name, type } of parameterDefinitions) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if Jenkins API may allow having null here, but maybe worth adding guard against null/undefined here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| if (!seenParameterNames.has(name)) { | ||
| returnData.push({ | ||
| name: `${name} - (${type})`, | ||
| value: name, | ||
| }); | ||
| seenParameterNames.add(name); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| import { mockDeep } from 'jest-mock-extended'; | ||
| import type { ILoadOptionsFunctions, INodePropertyOptions } from 'n8n-workflow'; | ||
|
|
||
| import * as GenericFunctions from '../GenericFunctions'; | ||
| import { Jenkins } from '../Jenkins.node'; | ||
|
|
||
| describe('Jenkins node', () => { | ||
| let node: Jenkins; | ||
| let loadOptionsFunctions: jest.Mocked<ILoadOptionsFunctions>; | ||
| const jenkinsApiRequestSpy = jest.spyOn(GenericFunctions, 'jenkinsApiRequest'); | ||
|
|
||
| beforeEach(() => { | ||
| node = new Jenkins(); | ||
| loadOptionsFunctions = mockDeep<ILoadOptionsFunctions>(); | ||
| loadOptionsFunctions.getCurrentNodeParameter.mockReturnValue('demo-job'); | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.resetAllMocks(); | ||
| }); | ||
|
|
||
| describe('loadOptions.getJobParameters', () => { | ||
| it('loads parameters from actions', async () => { | ||
| jenkinsApiRequestSpy.mockResolvedValue({ | ||
| actions: [ | ||
| { | ||
| _class: 'hudson.model.ParametersDefinitionProperty', | ||
| parameterDefinitions: [ | ||
| { name: 'BRANCH', type: 'StringParameterDefinition' }, | ||
| { name: 'DRY_RUN', type: 'BooleanParameterDefinition' }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await node.methods.loadOptions.getJobParameters.call(loadOptionsFunctions); | ||
|
|
||
| expect(result).toEqual<INodePropertyOptions[]>([ | ||
| { name: 'BRANCH - (StringParameterDefinition)', value: 'BRANCH' }, | ||
| { name: 'DRY_RUN - (BooleanParameterDefinition)', value: 'DRY_RUN' }, | ||
| ]); | ||
| expect(jenkinsApiRequestSpy).toHaveBeenCalledWith( | ||
| 'GET', | ||
| '/job/demo-job/api/json?tree=actions[parameterDefinitions[*]],property[parameterDefinitions[*]]', | ||
| ); | ||
| }); | ||
|
|
||
| it('loads parameters from property', async () => { | ||
| jenkinsApiRequestSpy.mockResolvedValue({ | ||
| property: [ | ||
| { | ||
| _class: | ||
| 'org.jenkinsci.plugins.workflow.job.properties.PipelineTriggersJobProperty ParametersDefinitionProperty', | ||
| parameterDefinitions: [{ name: 'VERSION', type: 'StringParameterDefinition' }], | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await node.methods.loadOptions.getJobParameters.call(loadOptionsFunctions); | ||
|
|
||
| expect(result).toEqual<INodePropertyOptions[]>([ | ||
| { name: 'VERSION - (StringParameterDefinition)', value: 'VERSION' }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('merges actions and property results and deduplicates parameter names', async () => { | ||
| jenkinsApiRequestSpy.mockResolvedValue({ | ||
| actions: [ | ||
| { | ||
| _class: 'hudson.model.ParametersDefinitionProperty', | ||
| parameterDefinitions: [{ name: 'ENV', type: 'StringParameterDefinition' }], | ||
| }, | ||
| ], | ||
| property: [ | ||
| { | ||
| _class: 'hudson.model.ParametersDefinitionProperty', | ||
| parameterDefinitions: [ | ||
| { name: 'ENV', type: 'StringParameterDefinition' }, | ||
| { name: 'REGION', type: 'ChoiceParameterDefinition' }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await node.methods.loadOptions.getJobParameters.call(loadOptionsFunctions); | ||
|
|
||
| expect(result).toEqual<INodePropertyOptions[]>([ | ||
| { name: 'ENV - (StringParameterDefinition)', value: 'ENV' }, | ||
| { name: 'REGION - (ChoiceParameterDefinition)', value: 'REGION' }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('filters non parameter classes and sorts by display name', async () => { | ||
| jenkinsApiRequestSpy.mockResolvedValue({ | ||
| actions: [ | ||
| { | ||
| _class: 'hudson.model.ScmProperty', | ||
| parameterDefinitions: [ | ||
| { name: 'SHOULD_NOT_APPEAR', type: 'StringParameterDefinition' }, | ||
| ], | ||
| }, | ||
| { | ||
| _class: 'hudson.model.ParametersDefinitionProperty', | ||
| parameterDefinitions: [ | ||
| { name: 'ZZZ', type: 'StringParameterDefinition' }, | ||
| { name: 'AAA', type: 'StringParameterDefinition' }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await node.methods.loadOptions.getJobParameters.call(loadOptionsFunctions); | ||
|
|
||
| expect(result).toEqual<INodePropertyOptions[]>([ | ||
| { name: 'AAA - (StringParameterDefinition)', value: 'AAA' }, | ||
| { name: 'ZZZ - (StringParameterDefinition)', value: 'ZZZ' }, | ||
| ]); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can Jenkins return both
result.actionsandresult.propertyleading to duplicate parameters?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, here's one of the API responses from actual Jenkins service, as you can see both

propertyandactionhave a parameterstr_freestyle