Skip to content

Commit a73f881

Browse files
jglickjglick
authored
Merge pull request #351 from jglick/FilePathPickle-JENKINS-75679
[JENKINS-75679] `FilePathPickle` deprecated
2 parents d0fdd5b + 2a3a161 commit a73f881

File tree

4 files changed

+171
-17
lines changed

4 files changed

+171
-17
lines changed

‎pom.xml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@
5757
<scope>import</scope>
5858
<type>pom</type>
5959
</dependency>
60+
<!-- TODO until in BOM: -->
61+
<dependency>
62+
<groupId>org.jenkins-ci.plugins</groupId>
63+
<artifactId>docker-commons</artifactId>
64+
<version>457.v0f62a_94f11a_3</version>
65+
</dependency>
6066
</dependencies>
6167
</dependencyManagement>
6268
<dependencies>
@@ -150,6 +156,17 @@
150156
<version>2.2</version>
151157
<scope>test</scope>
152158
</dependency>
159+
<dependency>
160+
<groupId>org.jenkins-ci.plugins</groupId>
161+
<artifactId>ssh-slaves</artifactId>
162+
<scope>test</scope>
163+
</dependency>
164+
<dependency>
165+
<groupId>org.testcontainers</groupId>
166+
<artifactId>testcontainers</artifactId>
167+
<version>1.21.0</version>
168+
<scope>test</scope>
169+
</dependency>
153170
<!-- Required because git plugin optionally depends on a recent version of matrix-project, and if we do not depend
154171
on matrix-project here, an older version is installed automatically as a detached plugin, causing git plugin to fail to load. -->
155172
<dependency>

‎src/main/java/org/jenkinsci/plugins/docker/workflow/AbstractEndpointStepExecution2.java‎

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,14 @@
2323
*/
2424
package org.jenkinsci.plugins.docker.workflow;
2525

26+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2627
import hudson.EnvVars;
28+
import hudson.FilePath;
2729
import java.io.IOException;
2830
import java.util.logging.Level;
2931
import java.util.logging.Logger;
3032
import org.jenkinsci.plugins.docker.commons.credentials.KeyMaterial;
33+
import org.jenkinsci.plugins.docker.commons.credentials.KeyMaterial2;
3134
import org.jenkinsci.plugins.docker.commons.credentials.KeyMaterialFactory;
3235
import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
3336
import org.jenkinsci.plugins.workflow.steps.GeneralNonBlockingStepExecution;
@@ -50,19 +53,19 @@ protected AbstractEndpointStepExecution2(StepContext context) {
5053

5154
private void doStart() throws Exception {
5255
KeyMaterialFactory keyMaterialFactory = newKeyMaterialFactory();
53-
KeyMaterial material = keyMaterialFactory.materialize();
56+
KeyMaterial2 material = keyMaterialFactory.materialize2();
5457
getContext().newBodyInvoker().
55-
withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), new Expander(material))).
56-
withCallback(new Callback(material)).
58+
withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), new Expander2(material))).
59+
withCallback(new Callback2(material)).
5760
start();
5861
}
5962

60-
private static class Expander extends EnvironmentExpander {
63+
private static class Expander2 extends EnvironmentExpander {
6164

6265
private static final long serialVersionUID = 1;
63-
private final KeyMaterial material;
66+
private final KeyMaterial2 material;
6467

65-
Expander(KeyMaterial material) {
68+
Expander2(KeyMaterial2 material) {
6669
this.material = material;
6770
}
6871

@@ -72,15 +75,53 @@ private static class Expander extends EnvironmentExpander {
7275

7376
}
7477

75-
private class Callback extends TailCall {
78+
private class Callback2 extends TailCall {
7679

7780
private static final long serialVersionUID = 1;
78-
private final KeyMaterial material;
81+
private final KeyMaterial2 material;
7982

80-
Callback(KeyMaterial material) {
83+
Callback2(KeyMaterial2 material) {
8184
this.material = material;
8285
}
8386

87+
@Override protected void finished(StepContext context) throws Exception {
88+
try {
89+
material.close(context.get(FilePath.class).getChannel());
90+
} catch (IOException | InterruptedException x) {
91+
Logger.getLogger(AbstractEndpointStepExecution2.class.getName()).log(Level.WARNING, null, x);
92+
}
93+
}
94+
95+
}
96+
97+
@SuppressFBWarnings(value = {"NP_UNWRITTEN_FIELD", "UWF_NULL_FIELD"})
98+
@Deprecated
99+
private static class Expander extends EnvironmentExpander {
100+
101+
private static final long serialVersionUID = 1;
102+
private final KeyMaterial material = null;
103+
104+
private Expander() {
105+
assert false : "only deserialized";
106+
}
107+
108+
@Override public void expand(EnvVars env) throws IOException, InterruptedException {
109+
env.putAll(material.env());
110+
}
111+
112+
}
113+
114+
@SuppressFBWarnings(value = {"NP_UNWRITTEN_FIELD", "UWF_NULL_FIELD"})
115+
@Deprecated
116+
private class Callback extends TailCall {
117+
118+
private static final long serialVersionUID = 1;
119+
private final KeyMaterial material = null;
120+
121+
private Callback() {
122+
assert false : "only deserialized";
123+
}
124+
84125
@Override protected void finished(StepContext context) throws Exception {
85126
try {
86127
material.close();

‎src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@
4646
import org.jvnet.hudson.test.RestartableJenkinsRule;
4747

4848
import java.io.File;
49+
import java.io.FileNotFoundException;
4950
import java.io.IOException;
51+
import java.nio.charset.StandardCharsets;
52+
import java.nio.file.NoSuchFileException;
5053
import java.util.Collections;
5154
import java.util.Set;
5255
import java.util.TreeSet;
@@ -100,8 +103,14 @@ private static void grep(File dir, String text, String prefix, Set<String> match
100103
String qualifiedName = prefix + kid.getName();
101104
if (kid.isDirectory()) {
102105
grep(kid, text, qualifiedName + "/", matches);
103-
} else if (kid.isFile() && FileUtils.readFileToString(kid).contains(text)) {
104-
matches.add(qualifiedName);
106+
} else {
107+
try {
108+
if (FileUtils.readFileToString(kid, StandardCharsets.UTF_8).contains(text)) {
109+
matches.add(qualifiedName);
110+
}
111+
} catch (FileNotFoundException | NoSuchFileException x) {
112+
// ignore, e.g. tmp file
113+
}
105114
}
106115
}
107116
}

‎src/test/java/org/jenkinsci/plugins/docker/workflow/RegistryEndpointStepTest.java‎

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*/
2424
package org.jenkinsci.plugins.docker.workflow;
2525

26+
import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey;
2627
import com.cloudbees.plugins.credentials.CredentialsProvider;
2728
import com.cloudbees.plugins.credentials.CredentialsScope;
2829
import com.cloudbees.plugins.credentials.common.IdCredentials;
@@ -37,6 +38,9 @@
3738
import hudson.model.Node;
3839
import hudson.model.Result;
3940
import hudson.model.User;
41+
import hudson.plugins.sshslaves.SSHLauncher;
42+
import hudson.slaves.DumbSlave;
43+
import java.io.ByteArrayOutputStream;
4044
import jenkins.model.Jenkins;
4145
import jenkins.security.QueueItemAuthenticatorConfiguration;
4246
import org.jenkinsci.plugins.docker.commons.credentials.DockerRegistryEndpoint;
@@ -59,28 +63,43 @@
5963
import org.junit.Rule;
6064
import org.junit.Test;
6165
import org.jvnet.hudson.test.Issue;
62-
import org.jvnet.hudson.test.JenkinsRule;
6366
import org.jvnet.hudson.test.MockAuthorizationStrategy;
6467
import org.jvnet.hudson.test.MockQueueItemAuthenticator;
6568
import org.jvnet.hudson.test.TestExtension;
6669
import org.kohsuke.stapler.DataBoundConstructor;
6770

6871
import java.io.Serializable;
72+
import java.nio.charset.StandardCharsets;
73+
import java.nio.file.Files;
6974
import java.util.Set;
7075
import java.util.logging.Level;
76+
import org.apache.commons.text.StringEscapeUtils;
77+
import org.apache.sshd.common.config.keys.KeyUtils;
78+
import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
79+
import org.apache.sshd.common.keyprovider.KeyPairProvider;
80+
import static org.hamcrest.MatcherAssert.assertThat;
81+
import static org.hamcrest.Matchers.containsString;
82+
import static org.hamcrest.Matchers.not;
83+
import static org.jenkinsci.plugins.docker.workflow.DockerTestUtil.assumeDocker;
7184
import org.jenkinsci.plugins.structs.describable.DescribableModel;
85+
import org.jenkinsci.plugins.workflow.support.pickles.FilePathPickle;
86+
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
87+
import org.junit.ClassRule;
7288
import org.jvnet.hudson.test.BuildWatcher;
89+
import org.jvnet.hudson.test.JenkinsSessionRule;
7390
import org.jvnet.hudson.test.LoggerRule;
91+
import org.testcontainers.containers.GenericContainer;
7492

7593
public class RegistryEndpointStepTest {
7694

77-
@Rule public JenkinsRule r = new JenkinsRule();
95+
@Rule public final JenkinsSessionRule rr = new JenkinsSessionRule();
7896
@Rule public LoggerRule logging = new LoggerRule();
79-
@Rule public BuildWatcher bw = new BuildWatcher();
97+
@ClassRule public static BuildWatcher bw = new BuildWatcher();
8098

8199
@Issue("JENKINS-51395")
82-
@Test public void configRoundTrip() throws Exception {
100+
@Test public void configRoundTrip() throws Throwable {
83101
logging.record(DescribableModel.class, Level.FINE);
102+
rr.then(r -> {
84103
{ // Recommended syntax.
85104
SnippetizerTester st = new SnippetizerTester(r);
86105
RegistryEndpointStep step = new RegistryEndpointStep(new DockerRegistryEndpoint("https://myreg/", null));
@@ -116,12 +135,14 @@ public class RegistryEndpointStepTest {
116135
assertEquals("registryCreds", registry.getCredentialsId());
117136
// TODO check toolName
118137
}
138+
});
119139
}
120140

121141
@Test
122-
public void stepExecutionWithCredentials() throws Exception {
142+
public void stepExecutionWithCredentials() throws Throwable {
123143
assumeNotWindows();
124144

145+
rr.then(r -> {
125146
IdCredentials registryCredentials = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "registryCreds", null, "me", "s3cr3t");
126147
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), registryCredentials);
127148

@@ -137,12 +158,14 @@ public void stepExecutionWithCredentials() throws Exception {
137158
r.assertBuildStatusSuccess(r.waitForCompletion(b));
138159
r.assertLogContains("docker login -u me -p ******** https://my-reg:1234", b);
139160
r.assertLogNotContains("s3cr3t", b);
161+
});
140162
}
141163

142164
@Test
143-
public void stepExecutionWithCredentialsAndQueueItemAuthenticator() throws Exception {
165+
public void stepExecutionWithCredentialsAndQueueItemAuthenticator() throws Throwable {
144166
assumeNotWindows();
145167

168+
rr.then(r -> {
146169
r.getInstance().setSecurityRealm(r.createDummySecurityRealm());
147170
MockAuthorizationStrategy auth = new MockAuthorizationStrategy()
148171
.grant(Jenkins.READ).everywhere().to("alice", "bob")
@@ -181,6 +204,70 @@ public void stepExecutionWithCredentialsAndQueueItemAuthenticator() throws Excep
181204

182205
// Bob does not have Credentials.USE_ITEM permission and should not be able to use the credential.
183206
r.assertBuildStatus(Result.FAILURE, p2.scheduleBuild2(0));
207+
});
208+
}
209+
210+
@Issue("JENKINS-75679")
211+
@Test public void noFilePathPickle() throws Throwable {
212+
assumeDocker();
213+
try (var agent = new SSHAgentContainer()) {
214+
agent.start();
215+
rr.then(r -> {
216+
agent.register("remote");
217+
var registryCredentials = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "registryCreds", null, "me", "s3cr3t");
218+
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), registryCredentials);
219+
var p = r.createProject(WorkflowJob.class, "p");
220+
p.setDefinition(new CpsFlowDefinition(
221+
"""
222+
node('remote') {
223+
mockDockerLogin {
224+
withDockerRegistry(url: 'https://my-reg:1234', credentialsId: 'registryCreds') {
225+
semaphore 'wait'
226+
}
227+
}
228+
}
229+
""", true));
230+
var b = p.scheduleBuild2(0).waitForStart();
231+
SemaphoreStep.waitForStart("wait/1", b);
232+
});
233+
@SuppressWarnings("deprecation")
234+
var verboten = FilePathPickle.class.getName();
235+
assertThat(StringEscapeUtils.escapeJava(Files.readString(rr.getHome().toPath().resolve("jobs/p/builds/1/program.dat"), StandardCharsets.ISO_8859_1)), not(containsString(verboten)));
236+
rr.then(r -> {
237+
var b = r.jenkins.getItemByFullName("p", WorkflowJob.class).getBuildByNumber(1);
238+
SemaphoreStep.success("wait/1", null);
239+
r.assertBuildStatusSuccess(r.waitForCompletion(b));
240+
r.assertLogContains("docker login -u me -p ******** https://my-reg:1234", b);
241+
r.assertLogNotContains("s3cr3t", b);
242+
});
243+
}
244+
}
245+
246+
private static final class SSHAgentContainer extends GenericContainer<SSHAgentContainer> {
247+
private final String priv;
248+
SSHAgentContainer() {
249+
super("jenkins/ssh-agent:6.17.0");
250+
try {
251+
var kp = KeyUtils.generateKeyPair(KeyPairProvider.SSH_RSA, 2048);
252+
var kprw = new OpenSSHKeyPairResourceWriter();
253+
var baos = new ByteArrayOutputStream();
254+
kprw.writePublicKey(kp, null, baos);
255+
var pub = baos.toString(StandardCharsets.US_ASCII);
256+
baos.reset();
257+
kprw.writePrivateKey(kp, null, null, baos);
258+
priv = baos.toString(StandardCharsets.US_ASCII);
259+
withEnv("JENKINS_AGENT_SSH_PUBKEY", pub);
260+
withExposedPorts(22);
261+
} catch (Exception x) {
262+
throw new AssertionError(x);
263+
}
264+
}
265+
void register(String name) throws Exception{
266+
var creds = new BasicSSHUserPrivateKey(CredentialsScope.GLOBAL, null, "jenkins", new BasicSSHUserPrivateKey.DirectEntryPrivateKeySource(priv), null, null);
267+
CredentialsProvider.lookupStores(Jenkins.get()).iterator().next().addCredentials(Domain.global(), creds);
268+
var port = getMappedPort(22);
269+
Jenkins.get().addNode(new DumbSlave(name, "/home/jenkins/agent", new SSHLauncher(getHost(), port, creds.getId())));
270+
}
184271
}
185272

186273
public static class MockLauncherStep extends Step {

0 commit comments

Comments
 (0)