[WiP] minor changes for Workflow support

[ndeloof]

Let user rely on snippet generator to use copyartifact build step

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[ndeloof]

@reviewbybees

[ndeloof]

not sure why CI fails a test since https://jenkins.ci.cloudbees.com/job/plugins/job/copyartifact-plugin/151/ as previous build https://jenkins.ci.cloudbees.com/job/plugins/job/copyartifact-plugin/150/ was on same commit (maven release)

[jglick]

I guess this is intended to follow up #55 and #59?

[jglick]

This would be a security regression. You need to retain @AncestorInPath Job job so you can still do a permission check on it.

[ndeloof]

This permission check applies to current job configuration, not the path to job the method is going to validate. This doesn't make much sense, can be arbitrary job on jenkins.
I've looked at git history to understand the intent but was imported with first commit.

[ikedam]

What you want really do is to make doCheckProjectName applicable to WorkflowJob.
You can do that simply by replacing AbstractProject with Job.

[jglick]

This permission check applies to current job configuration

You misunderstand completely. The intention is to avoid doing potentially sensitive computations unless the user is actually configuring this (downstream) job. Otherwise someone can fake up a …/checkProjectName request in cURL and get some information they should not have access to based on job permissions.

As @ikedam says, just replace AbstractProject with Job.

[jglick]

Suggestion: if you add

      <dependency>
        <groupId>org.jenkins-ci.plugins.workflow</groupId>
        <artifactId>workflow-step-api</artifactId>
        <version>${workflow.version}</version>
        <classifier>tests</classifier>
        <scope>test</scope>
      </dependency>

then it should be possible to use StepConfigTester on a new CoreStep(new CopyArtifact(…)) to actually reproduce the buildSelector and stableOnly problems and verify the fix (as well as defending against some future regressions).

[jglick]

Evaluate the CopyArtifactTest.testFieldValidation failure.

[ndeloof]

@reviewbybees ping

[ikedam]

Let's make it clear what this request does.

You can test the behavior of the snippet generator in a following way:

• Create a new workflow project
• Select "Groovy CPS DSL" in Definition for Workflow
• Check "Snippet generator"
• Select "General Build Step" for "Sample Step"
• Select "Copy artifacts from another project" for "Build Step"
• Configure copyartifact, and click "Generate Groovy"

Result before this fix:

• The validation for "Project name" fails with NPE.
• The generated result is

step <object of type hudson.plugins.copyartifact.CopyArtifact>

Result after this fix:

step([$class: 'CopyArtifact', excludes: '', filter: '**/*', fingerprintArtifacts: true, flatten: false, optional: false, projectName: 'test', selector: [$class: 'StatusBuildSelector', stable: false], target: ''])

What this request does:

• Makes CopyArtifact.DescriptorImpl#doCheckProjectName work with WorkflowJob which is a sub class not of AbstractProject but of Job.
• Make getter names match the names of parameters in the constructor (or maybe setters with DataBoundSetter).

[jglick]

What is the status of this? It is not currently mergeable.

[ikedam]

copyartifact-1.35.1 should work with workflow's snippet generator. (fixed with #62, #63).
It will be available in a day.
Please try that.

[jglick]

The substitution of AbstractProject with Job in a couple of places is still valid, right?

[ikedam]

This (and only this) change isn't applied in 1.35.1 as @jglick points.
JENKINS-28247 may be caused for that.