From d8a6879b7b183858aae3690b94db6b8d56972e1b Mon Sep 17 00:00:00 2001 From: Joyce Yee Date: Wed, 1 Apr 2020 22:11:45 +0000 Subject: [PATCH 1/2] Conditionally set AWS network configuration directly on EC2 request if associate public ip is true --- .../hudson/plugins/ec2/SlaveTemplate.java | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java index 1e32dfe92..f0c7621c3 100644 --- a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java +++ b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java @@ -733,7 +733,11 @@ private List provisionOndemand(int number, EnumSet provisionOndemand(int number, EnumSet groupIds = getEc2SecurityGroups(ec2); if (!groupIds.isEmpty()) { - net.setGroups(groupIds); + if (getAssociatePublicIp()) { + net.setGroups(groupIds); + } else { + riRequest.setSecurityGroupIds(groupIds); + } + diFilters.add(new Filter("instance.group-id").withValues(groupIds)); } } } else { + riRequest.setSecurityGroups(securityGroupSet); List groupIds = getSecurityGroupsBy("group-name", securityGroupSet, ec2) .getSecurityGroups() .stream().map(SecurityGroup::getGroupId) @@ -761,7 +771,10 @@ private List provisionOndemand(int number, EnumSet instTags = buildTags(EC2Cloud.EC2_SLAVE_TYPE_DEMAND); for (Tag tag : instTags) { From 629d668bf3a6415eb93bd918fe0f7d69f8af3c80 Mon Sep 17 00:00:00 2001 From: Joyce Yee Date: Mon, 6 Apr 2020 19:36:42 +0000 Subject: [PATCH 2/2] Add SlaveTemplate tests to assert that ec2 networking config is set accordingly based on associatePublicIp value --- .../hudson/plugins/ec2/SlaveTemplateTest.java | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java b/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java index fc4d996f0..e8a5d7c1e 100644 --- a/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java +++ b/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java @@ -23,9 +23,32 @@ */ package hudson.plugins.ec2; +import com.amazonaws.services.ec2.AmazonEC2; +import com.amazonaws.services.ec2.model.DescribeImagesRequest; +import com.amazonaws.services.ec2.model.DescribeImagesResult; +import com.amazonaws.services.ec2.model.DescribeInstancesRequest; +import com.amazonaws.services.ec2.model.DescribeInstancesResult; +import com.amazonaws.services.ec2.model.DescribeSecurityGroupsRequest; +import com.amazonaws.services.ec2.model.DescribeSecurityGroupsResult; +import com.amazonaws.services.ec2.model.DescribeSubnetsRequest; +import com.amazonaws.services.ec2.model.DescribeSubnetsResult; +import com.amazonaws.services.ec2.model.IamInstanceProfile; +import com.amazonaws.services.ec2.model.Image; +import com.amazonaws.services.ec2.model.Instance; +import com.amazonaws.services.ec2.model.InstanceNetworkInterfaceSpecification; +import com.amazonaws.services.ec2.model.InstanceState; import com.amazonaws.services.ec2.model.InstanceType; +import com.amazonaws.services.ec2.model.KeyPair; +import com.amazonaws.services.ec2.model.RunInstancesRequest; +import com.amazonaws.services.ec2.model.RunInstancesResult; +import com.amazonaws.services.ec2.model.SecurityGroup; +import com.amazonaws.services.ec2.model.Subnet; +import com.amazonaws.services.identitymanagement.model.InstanceProfile; + import hudson.model.Node; +import hudson.plugins.ec2.SlaveTemplate.ProvisionOptions; import hudson.plugins.ec2.util.MinimumNumberOfInstancesTimeRangeConfig; +import com.amazonaws.services.ec2.model.Reservation; import jenkins.model.Jenkins; import net.sf.json.JSONObject; @@ -35,14 +58,23 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.ArgumentCaptor; + import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; +import static org.mockito.Matchers.any; /** * Basic test to validate SlaveTemplate. @@ -387,4 +419,126 @@ public void testMinimumNumberOfInstancesActiveRangeConfig() throws Exception { Assert.assertEquals(false, stored.getMinimumNoInstancesActiveTimeRangeDays().get("monday")); Assert.assertEquals(true, stored.getMinimumNoInstancesActiveTimeRangeDays().get("tuesday")); } + + @Test + public void provisionOndemandSetsAwsNetworkingOnEc2Request() throws Exception { + boolean associatePublicIp = false; + String ami = "ami1"; + String description = "foo ami"; + String subnetId = "some-subnet"; + String securityGroups = "some security group"; + String iamInstanceProfile = "some instance profile"; + + EC2Tag tag1 = new EC2Tag("name1", "value1"); + EC2Tag tag2 = new EC2Tag("name2", "value2"); + List tags = new ArrayList(); + tags.add(tag1); + tags.add(tag2); + + SlaveTemplate orig = new SlaveTemplate(ami, EC2AbstractSlave.TEST_ZONE, null, securityGroups, "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, subnetId, tags, null, false, null, iamInstanceProfile, true, false, "", associatePublicIp, ""); + + List templates = new ArrayList(); + templates.add(orig); + AmazonEC2 mockedEC2 = setupTestForProvisioning(orig); + + ArgumentCaptor riRequestCaptor = ArgumentCaptor.forClass(RunInstancesRequest.class); + + orig.provision(2, EnumSet.noneOf(ProvisionOptions.class)); + verify(mockedEC2).runInstances(riRequestCaptor.capture()); + + RunInstancesRequest actualRequest = riRequestCaptor.getValue(); + List actualNets = actualRequest.getNetworkInterfaces(); + + assertEquals(actualNets.size(), 0); + assertEquals(actualRequest.getSubnetId(), subnetId); + assertEquals(actualRequest.getSecurityGroupIds(), Stream.of("some-group-id").collect(Collectors.toList())); + } + + @Test + public void provisionOndemandSetsAwsNetworkingOnNetworkInterface() throws Exception { + boolean associatePublicIp = true; + String ami = "ami1"; + String description = "foo ami"; + String subnetId = "some-subnet"; + String securityGroups = "some security group"; + String iamInstanceProfile = "some instance profile"; + + EC2Tag tag1 = new EC2Tag("name1", "value1"); + EC2Tag tag2 = new EC2Tag("name2", "value2"); + List tags = new ArrayList(); + tags.add(tag1); + tags.add(tag2); + + SlaveTemplate orig = new SlaveTemplate(ami, EC2AbstractSlave.TEST_ZONE, null, securityGroups, "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, subnetId, tags, null, false, null, iamInstanceProfile, true, false, "", associatePublicIp, ""); + + List templates = new ArrayList(); + templates.add(orig); + AmazonEC2 mockedEC2 = setupTestForProvisioning(orig); + + ArgumentCaptor riRequestCaptor = ArgumentCaptor.forClass(RunInstancesRequest.class); + + orig.provision(2, EnumSet.noneOf(ProvisionOptions.class)); + verify(mockedEC2).runInstances(riRequestCaptor.capture()); + + RunInstancesRequest actualRequest = riRequestCaptor.getValue(); + InstanceNetworkInterfaceSpecification actualNet = actualRequest.getNetworkInterfaces().get(0); + + assertEquals(actualNet.getSubnetId(), subnetId); + assertEquals(actualNet.getGroups(), Stream.of("some-group-id").collect(Collectors.toList())); + assertEquals(actualRequest.getSubnetId(), null); + assertEquals(actualRequest.getSecurityGroupIds(), Collections.emptyList()); + } + + private AmazonEC2 setupTestForProvisioning(SlaveTemplate template) throws Exception { + AmazonEC2Cloud mockedCloud = mock(AmazonEC2Cloud.class); + AmazonEC2 mockedEC2 = mock(AmazonEC2.class); + EC2PrivateKey mockedPrivateKey = mock(EC2PrivateKey.class); + KeyPair mockedKeyPair = new KeyPair(); + mockedKeyPair.setKeyName("some-key-name"); + when(mockedPrivateKey.find(mockedEC2)).thenReturn(mockedKeyPair); + when(mockedCloud.connect()).thenReturn(mockedEC2); + when(mockedCloud.getPrivateKey()).thenReturn(mockedPrivateKey); + template.parent = mockedCloud; + + DescribeImagesResult mockedImagesResult = mock(DescribeImagesResult.class); + Image mockedImage = new Image(); + mockedImage.setImageId(template.getAmi()); + when(mockedImagesResult.getImages()).thenReturn(Stream.of(mockedImage).collect(Collectors.toList())); + when(mockedEC2.describeImages(any(DescribeImagesRequest.class))).thenReturn(mockedImagesResult); + + DescribeSecurityGroupsResult mockedSecurityGroupsResult = mock(DescribeSecurityGroupsResult.class); + SecurityGroup mockedSecurityGroup = new SecurityGroup(); + mockedSecurityGroup.setVpcId("some-vpc-id"); + mockedSecurityGroup.setGroupId("some-group-id"); + + List mockedSecurityGroups = Stream.of(mockedSecurityGroup).collect(Collectors.toList()); + when(mockedSecurityGroupsResult.getSecurityGroups()).thenReturn(mockedSecurityGroups); + when(mockedEC2.describeSecurityGroups(any(DescribeSecurityGroupsRequest.class))).thenReturn(mockedSecurityGroupsResult); + + DescribeSubnetsResult mockedDescribeSubnetsResult = mock(DescribeSubnetsResult.class); + Subnet mockedSubnet = new Subnet(); + List mockedSubnets = Stream.of(mockedSubnet).collect(Collectors.toList()); + when(mockedDescribeSubnetsResult.getSubnets()).thenReturn(mockedSubnets); + when(mockedEC2.describeSubnets(any(DescribeSubnetsRequest.class))).thenReturn(mockedDescribeSubnetsResult); + + IamInstanceProfile mockedInstanceProfile = new IamInstanceProfile(); + mockedInstanceProfile.setArn(template.getIamInstanceProfile()); + InstanceState mockInstanceState = new InstanceState(); + mockInstanceState.setName("not terminated"); + Instance mockedInstance = new Instance(); + mockedInstance.setState(mockInstanceState); + mockedInstance.setIamInstanceProfile(mockedInstanceProfile); + Reservation mockedReservation = new Reservation(); + mockedReservation.setInstances(Stream.of(mockedInstance).collect(Collectors.toList())); + List mockedReservations = Stream.of(mockedReservation).collect(Collectors.toList()); + DescribeInstancesResult mockedDescribedInstancesResult = mock(DescribeInstancesResult.class); + when(mockedDescribedInstancesResult.getReservations()).thenReturn(mockedReservations); + when(mockedEC2.describeInstances(any(DescribeInstancesRequest.class))).thenReturn(mockedDescribedInstancesResult); + + RunInstancesResult mockedResult = mock(RunInstancesResult.class); + when(mockedResult.getReservation()).thenReturn(mockedReservation); + when(mockedEC2.runInstances(any(RunInstancesRequest.class))).thenReturn(mockedResult); + + return mockedEC2; + } }