Bugfix: Removed @RequirePost and changed ssh key migration behavior

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

src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly

@@ -37,7 +37,7 @@ THE SOFTWARE.
     <f:select/>
   </f:entry>
   <f:entry field="sshKeysCredentialsId" title="${%EC2 Key Pair's Private Key}">
-    <c:select includeUser="false"/>
+    <c:select includeUser="false" checkMethod="post"/>
   </f:entry>
   <f:advanced>
     <f:entry title="${%Instance Cap}" field="instanceCapStr">

src/main/resources/hudson/plugins/ec2/Eucalyptus/config-entries.jelly

@@ -32,7 +32,7 @@ THE SOFTWARE.
     <f:checkbox />
   </f:entry>
   <f:entry field="sshKeysCredentialsId" title="${%EC2 Key Pair's Private Key}">
-    <c:select includeUser="false"/>
+    <c:select includeUser="false" checkMethod="post"/>
   </f:entry>
   <f:advanced>
     <f:entry title="${%Instance Cap}" field="instanceCapStr">

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);

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());

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<String> 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");
+                }
+            }
+
+        }
+    }
+
+}