From e772bfda271ebbf47d3d61363594c698c28c207f Mon Sep 17 00:00:00 2001 From: Damian Jesionek Date: Wed, 16 Sep 2020 14:54:53 +0200 Subject: [PATCH 1/3] Removed @RequirePost and changed ssh key migration behavior --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 998b15660..58725f710 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -171,12 +171,6 @@ protected EC2Cloud(String id, boolean useInstanceProfileForCredentials, String c this.credentialsId = credentialsId; this.sshKeysCredentialsId = sshKeysCredentialsId; - if (this.sshKeysCredentialsId == null && ( this.privateKey != null || privateKey != null)){ - migratePrivateSshKeyToCredential(this.privateKey != null ? this.privateKey.getPrivateKey() : privateKey); - } - this.privateKey = null; // This enforces it not to be persisted and that CasC will never output privateKey on export - - if (templates == null) { this.templates = Collections.emptyList(); } else { @@ -250,6 +244,11 @@ protected Object readResolve() { for (SlaveTemplate t : templates) t.parent = this; + if (this.sshKeysCredentialsId == null && this.privateKey != null ){ + migratePrivateSshKeyToCredential(this.privateKey.getPrivateKey()); + } + this.privateKey = null; // This enforces it not to be persisted and that CasC will never output privateKey on export + if (this.accessId != null && this.secretKey != null && credentialsId == null) { String secretKeyEncryptedValue = this.secretKey.getEncryptedValue(); // REPLACE this.accessId and this.secretId by a credential @@ -1066,7 +1065,6 @@ public ListBoxModel doFillSshKeysCredentialsIdItems(@QueryParameter String sshKe .includeCurrentValue(sshKeysCredentialsId); } - @RequirePOST public FormValidation doCheckSshKeysCredentialsId(@QueryParameter String value) throws IOException, ServletException { Jenkins.get().checkPermission(Jenkins.ADMINISTER); From 4e8e8da6c4fe7895e4902f32e9a922319f946061 Mon Sep 17 00:00:00 2001 From: Damian Jesionek Date: Wed, 16 Sep 2020 17:27:58 +0200 Subject: [PATCH 2/3] Added credential helper for creating private key credentials in tests where they are necessary --- .../plugins/ec2/EC2RetentionStrategyTest.java | 17 ++++--- .../plugins/ec2/EC2SlaveMonitorTest.java | 8 ++-- .../plugins/ec2/util/SSHCredentialHelper.java | 48 +++++++++++++++++++ 3 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 src/test/java/hudson/plugins/ec2/util/SSHCredentialHelper.java diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 18aefd8a8..94caaf6d0 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java @@ -7,6 +7,7 @@ import hudson.plugins.ec2.util.MinimumInstanceChecker; import hudson.plugins.ec2.util.MinimumNumberOfInstancesTimeRangeConfig; import hudson.plugins.ec2.util.PrivateKeyHelper; +import hudson.plugins.ec2.util.SSHCredentialHelper; import hudson.slaves.NodeProperty; import hudson.model.Executor; import hudson.model.Node; @@ -15,7 +16,9 @@ import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; +import org.testcontainers.shaded.org.bouncycastle.jce.provider.BouncyCastleProvider; +import java.security.Security; import java.time.Clock; import java.time.Instant; import java.time.LocalDateTime; @@ -210,7 +213,8 @@ public void testRetentionDespiteIdleWithMinimumInstances() throws Exception { InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false, true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList()); - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections .singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); @@ -288,8 +292,8 @@ public void testRetentionDespiteIdleWithMinimumInstanceActiveTimeRange() throws //Set fixed clock to be able to test properly MinimumInstanceChecker.clock = Clock.fixed(localDateTime.atZone(ZoneId.systemDefault()).toInstant(), ZoneId.systemDefault()); - - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections .singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); @@ -385,8 +389,8 @@ public void testRetentionDespiteIdleWithMinimumInstanceActiveTimeRangeAfterMidni //Set fixed clock to be able to test properly MinimumInstanceChecker.clock = Clock.fixed(localDateTime.atZone(ZoneId.systemDefault()).toInstant(), ZoneId.systemDefault()); - - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections .singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); @@ -440,7 +444,8 @@ public void testRetentionStopsAfterActiveRangeEnds() throws Exception { LocalDateTime localDateTime = LocalDateTime.of(2019, Month.SEPTEMBER, 24, 14, 0); //Tuesday MinimumInstanceChecker.clock = Clock.fixed(localDateTime.atZone(ZoneId.systemDefault()).toInstant(), ZoneId.systemDefault()); - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections .singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); diff --git a/src/test/java/hudson/plugins/ec2/EC2SlaveMonitorTest.java b/src/test/java/hudson/plugins/ec2/EC2SlaveMonitorTest.java index a8def2cc7..469b3a39b 100644 --- a/src/test/java/hudson/plugins/ec2/EC2SlaveMonitorTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2SlaveMonitorTest.java @@ -13,7 +13,7 @@ import com.amazonaws.services.ec2.model.InstanceType; import hudson.model.Node; -import hudson.plugins.ec2.util.PrivateKeyHelper; +import hudson.plugins.ec2.util.SSHCredentialHelper; import jenkins.model.Jenkins; public class EC2SlaveMonitorTest { @@ -30,7 +30,8 @@ public void init(){ @Test public void testMinimumNumberOfInstances() throws Exception { SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", null, null, 2, null, null, true, true, false, "", false, "", false, false, true, ConnectionStrategy.PRIVATE_IP, 0); - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", Collections.singletonList(template), "roleArn", "roleSessionName"); + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections.singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); r.configRoundtrip(); @@ -49,7 +50,8 @@ public void testMinimumNumberOfSpareInstances() throws Exception { "", false, false, true, ConnectionStrategy.PRIVATE_IP, 0, null); - AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3", Collections.singletonList(template), "roleArn", "roleSessionName"); + SSHCredentialHelper.assureSshCredentialAvailableThroughCredentialProviders("ghi"); + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections.singletonList(template), "roleArn", "roleSessionName"); r.jenkins.clouds.add(cloud); r.configRoundtrip(); Assert.assertEquals(2, Arrays.stream(Jenkins.get().getComputers()).filter(computer -> computer instanceof EC2Computer).count()); diff --git a/src/test/java/hudson/plugins/ec2/util/SSHCredentialHelper.java b/src/test/java/hudson/plugins/ec2/util/SSHCredentialHelper.java new file mode 100644 index 000000000..93bf6c821 --- /dev/null +++ b/src/test/java/hudson/plugins/ec2/util/SSHCredentialHelper.java @@ -0,0 +1,48 @@ +package hudson.plugins.ec2.util; + +import edu.umd.cs.findbugs.annotations.NonNull; +import jenkins.model.Jenkins; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey; +import com.cloudbees.plugins.credentials.Credentials; +import com.cloudbees.plugins.credentials.CredentialsProvider; +import com.cloudbees.plugins.credentials.CredentialsScope; +import com.cloudbees.plugins.credentials.CredentialsStore; +import com.cloudbees.plugins.credentials.SystemCredentialsProvider; +import com.cloudbees.plugins.credentials.domains.Domain; + +public class SSHCredentialHelper { + + public static void assureSshCredentialAvailableThroughCredentialProviders(String id){ + BasicSSHUserPrivateKey sshKeyCredentials = new BasicSSHUserPrivateKey(CredentialsScope.SYSTEM, id, "key", + new BasicSSHUserPrivateKey.PrivateKeySource() { + @NonNull + @Override + public List getPrivateKeys() { + return Collections.singletonList(PrivateKeyHelper.generate()); + } + }, "", "EC2 Testing Cloud Private Key"); + + addNewGlobalCredential(sshKeyCredentials); + } + + private static void addNewGlobalCredential(Credentials credentials){ + for (CredentialsStore credentialsStore: CredentialsProvider.lookupStores(Jenkins.get())) { + + if (credentialsStore instanceof SystemCredentialsProvider.StoreImpl) { + + try { + credentialsStore.addCredentials(Domain.global(), credentials); + } catch (IOException e) { + throw new IllegalStateException("Failed to add testing credential"); + } + } + + } + } + +} From 517b98bbec7372949bfa816c537e9bb4d375a6c9 Mon Sep 17 00:00:00 2001 From: Damian Jesionek Date: Wed, 16 Sep 2020 17:33:19 +0200 Subject: [PATCH 3/3] Re added @RequirePost together and aligned jelly templates for checking ssh keys --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 1 + .../hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly | 2 +- .../hudson/plugins/ec2/Eucalyptus/config-entries.jelly | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 58725f710..6adca05c1 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1065,6 +1065,7 @@ public ListBoxModel doFillSshKeysCredentialsIdItems(@QueryParameter String sshKe .includeCurrentValue(sshKeysCredentialsId); } + @RequirePOST public FormValidation doCheckSshKeysCredentialsId(@QueryParameter String value) throws IOException, ServletException { Jenkins.get().checkPermission(Jenkins.ADMINISTER); diff --git a/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly b/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly index b852445c4..668ad3e48 100644 --- a/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly +++ b/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly @@ -37,7 +37,7 @@ THE SOFTWARE. - + diff --git a/src/main/resources/hudson/plugins/ec2/Eucalyptus/config-entries.jelly b/src/main/resources/hudson/plugins/ec2/Eucalyptus/config-entries.jelly index 9e1b58a6e..ba6846464 100644 --- a/src/main/resources/hudson/plugins/ec2/Eucalyptus/config-entries.jelly +++ b/src/main/resources/hudson/plugins/ec2/Eucalyptus/config-entries.jelly @@ -32,7 +32,7 @@ THE SOFTWARE. - +