Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for storing subnet id in ec2instance and report vpc for stored subnet id in describeInstances #58

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.tlswe.awsmock.ec2.control;

import java.io.IOException;
import java.sql.Date;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -20,7 +19,6 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.joda.time.DateTime;
import org.omg.CORBA.SystemException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -90,7 +88,6 @@
import com.tlswe.awsmock.ec2.cxf_generated.SecurityGroupItemType;
import com.tlswe.awsmock.ec2.cxf_generated.SecurityGroupSetType;
import com.tlswe.awsmock.ec2.cxf_generated.StartInstancesResponseType;
import com.tlswe.awsmock.ec2.cxf_generated.StateReasonType;
import com.tlswe.awsmock.ec2.cxf_generated.StopInstancesResponseType;
import com.tlswe.awsmock.ec2.cxf_generated.SubnetSetType;
import com.tlswe.awsmock.ec2.cxf_generated.SubnetType;
Expand Down Expand Up @@ -501,8 +498,14 @@ public void handle(final Map<String, String[]> queryParams,
int minCount = Integer.parseInt(queryParams.get("MinCount")[0]);
int maxCount = Integer.parseInt(queryParams.get("MaxCount")[0]);

final String[] subnetIdArray = queryParams.get("SubnetId");
String subnetId = null;
if (subnetIdArray != null) {
subnetId = subnetIdArray[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if SubnetId is not provided? Can we allocate a random uuid as the subnetId for an instance?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possibilty would be, that we use the MOCK_SUBNET_ID as default.
What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we use MOCK_SUBNET_ID as default.

}

responseXml = JAXBUtil.marshall(
runInstances(imageID, instanceType, minCount, maxCount),
runInstances(imageID, instanceType, minCount, maxCount, subnetId),
"RunInstancesResponse", version);

} else if ("DescribeImages".equals(action)) {
Expand Down Expand Up @@ -1139,9 +1142,9 @@ private DescribeInstancesResponseType describeInstances(final Set<String> instan
instItem.setDnsName(instance.getPubDns());

// set network information
instItem.setVpcId(MOCK_VPC_ID);
instItem.setVpcId(getVpcForSubnetId(instance.getSubnetId()));
instItem.setPrivateIpAddress(MOCK_PRIVATE_IP_ADDRESS);
instItem.setSubnetId(MOCK_SUBNET_ID);
instItem.setSubnetId(instance.getSubnetId());

instsSet.getItem().add(instItem);

Expand Down Expand Up @@ -1180,7 +1183,8 @@ protected String generateToken() {
}

/**
* Handles "runInstances" request, with only simplified filters of imageId, instanceType, minCount and maxCount.
* Handles "runInstances" request, with only simplified filters of imageId, instanceType, minCount, maxCount
* and subnetId.
*
* @param imageId
* AMI of new mock ec2 instance(s)
Expand All @@ -1190,10 +1194,12 @@ protected String generateToken() {
* max count of instances to run
* @param maxCount
* min count of instances to run
* @param subnetId
* subnet ID of new mock ec2 instance(s).
* @return a RunInstancesResponse that includes all information for the started new mock ec2 instances
*/
private RunInstancesResponseType runInstances(final String imageId, final String instanceType,
final int minCount, final int maxCount) {
final int minCount, final int maxCount, final String subnetId) {

RunInstancesResponseType ret = new RunInstancesResponseType();

Expand All @@ -1215,7 +1221,7 @@ private RunInstancesResponseType runInstances(final String imageId, final String
List<AbstractMockEc2Instance> newInstances = null;

newInstances = mockEc2Controller
.runInstances(clazzOfMockEc2Instance, imageId, instanceType, minCount, maxCount);
.runInstances(clazzOfMockEc2Instance, imageId, instanceType, minCount, maxCount, subnetId);

for (AbstractMockEc2Instance i : newInstances) {
RunningInstancesItemType instItem = new RunningInstancesItemType();
Expand All @@ -1230,9 +1236,9 @@ private RunInstancesResponseType runInstances(final String imageId, final String
instItem.setPlacement(DEFAULT_MOCK_PLACEMENT);

// set network information
instItem.setVpcId(MOCK_VPC_ID);
instItem.setSubnetId(subnetId);
instItem.setVpcId(getVpcForSubnetId(subnetId));
instItem.setPrivateIpAddress(MOCK_PRIVATE_IP_ADDRESS);
instItem.setSubnetId(MOCK_SUBNET_ID);

instSet.getItem().add(instItem);

Expand Down Expand Up @@ -2048,4 +2054,19 @@ private String getBlankResponseXml() {
}
return ret;
}

/**
* Gets the VPC id for a given subnetId.
*
* @param subnetId The subnet id.
* @return The VPC id. Returns null, if no matching subnet is found.
*/
private String getVpcForSubnetId(final String subnetId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be more meaningful if we change the name to getVpcIdForSubnet

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

for (MockSubnet subnet : mockSubnetController.describeSubnets()) {
if (subnet.getSubnetId().equals(subnetId)) {
return subnet.getVpcId();
}
}
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's always expected that a VPC is tied to a subnet - it's better if we throw an Exception than returning null. It will also be difficult to catch if for some reason we do return null.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use a default value for not provided subnetIds, then we need here either a check for MOCK_SUBNET_ID where we return MOCK_VPC_ID. Or we just return MOCK_VPC_ID by default.

if (MOCK_SUBNET_ID.equals(subnetId)) { return MOCK_VPC_ID; }
Or
private String getVpcIdForSubnet(final String subnetId) { for (MockSubnet subnet : mockSubnetController.describeSubnets()) { if (subnet.getSubnetId().equals(subnetId)) { return subnet.getVpcId(); } } return MOCK_VPC_ID; }
What would be your prefered solution?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we can return MOCK_VPC_ID if no matching vpcId found.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,15 @@ public List<String> listInstanceIDs(final Set<String> instanceIDs) {
* max count of instances to run (but limited to {@link #MAX_RUN_INSTANCE_COUNT_AT_A_TIME})
* @param maxCount
* min count of instances to run (should larger than 0)
* @param subnetId
* The subnet id of new mock ec2 instance(s). May be null.
*
* @return a list of objects of clazz as started new mock ec2 instances
*
*/
public <T extends AbstractMockEc2Instance> List<T> runInstances(final Class<? extends T> clazz,
final String imageId, final String instanceTypeName,
final int minCount, final int maxCount) {
final int minCount, final int maxCount, final String subnetId) {

// EC2 Query Request action name
final String action = "runInstances";
Expand Down Expand Up @@ -202,6 +205,7 @@ public <T extends AbstractMockEc2Instance> List<T> runInstances(final Class<? ex
inst.setImageId(imageId);
inst.setInstanceType(instanceType);
// inst.setSecurityGroups(securityGroups);
inst.setSubnetId(subnetId);

inst.start();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ private static long getMsFromProperty(final String propertyName,
*/
private Set<String> securityGroups = new TreeSet<String>();

/**
* Subnet id for this ec2 instance.
*/
private String subnetId = null;

/**
* Flag that indicates whether internal timer of this mock ec2 instance has been started (on instance start()).
*/
Expand Down Expand Up @@ -800,6 +805,24 @@ public final InstanceState getInstanceState() {
: InstanceState.STOPPED)));
}

/**
* Get subnet ID of this mock ec2 instance.
*
* @return subnet ID of this mock ec2 instance
*/
public final String getSubnetId() {
return subnetId;
}

/**
* Set the subnet ID of this mock ec2 instance.
*
* @param subnetId subnet ID of this mock ec2 instance
*/
public void setSubnetId(final String subnetId) {
this.subnetId = subnetId;
}

/**
* Get the AMI this mock ec2 instance started from.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class DefaultMockEc2Instance extends AbstractMockEc2Instance {
*
* @see Serializable
*/
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why the version is incremented for this class?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, the base model would be a better place for the increment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah not necessary a change here.


@Override
public void onStarted() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -18,6 +19,7 @@

import javax.servlet.http.HttpServletResponse;

import com.tlswe.awsmock.ec2.model.MockSubnet;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -57,12 +59,10 @@
import com.tlswe.awsmock.ec2.cxf_generated.DescribeVolumesResponseType;
import com.tlswe.awsmock.ec2.cxf_generated.DescribeVpcsResponseType;
import com.tlswe.awsmock.ec2.cxf_generated.InternetGatewayType;
import com.tlswe.awsmock.ec2.cxf_generated.IpPermissionType;
import com.tlswe.awsmock.ec2.cxf_generated.ReservationInfoType;
import com.tlswe.awsmock.ec2.cxf_generated.RunInstancesResponseType;
import com.tlswe.awsmock.ec2.cxf_generated.RunningInstancesItemType;
import com.tlswe.awsmock.ec2.cxf_generated.RunningInstancesSetType;
import com.tlswe.awsmock.ec2.cxf_generated.SecurityGroupItemType;
import com.tlswe.awsmock.ec2.cxf_generated.VpcType;
import com.tlswe.awsmock.ec2.exception.BadEc2RequestException;
import com.tlswe.awsmock.ec2.model.AbstractMockEc2Instance;
Expand All @@ -86,6 +86,8 @@ public class MockEC2QueryHandlerTest {
private static final String ACTION_KEY = "Action";
private static final String VERSION_KEY = "Version";
private static final String VERSION_1 = "version1";
private static final String SUBNET_ID = "subnetId";
private static final String VPC_ID = "vpcId";

static {
InputStream inputStream = null;
Expand Down Expand Up @@ -510,17 +512,18 @@ public void Test_runInstances() throws Exception {
MockEc2Controller controller = Mockito.spy(MockEc2Controller.class);
Whitebox.setInternalState(handler, "mockEc2Controller", controller);

createAndInjectMockSubnetController(handler);

RunInstancesResponseType ret = Whitebox.invokeMethod(handler, "runInstances", "ami-1",
InstanceType.C1_MEDIUM.getName(), 1, 1);
InstanceType.C1_MEDIUM.getName(), 1, 1, SUBNET_ID);

Assert.assertTrue(ret != null);
Assert.assertTrue(ret.getInstancesSet().getItem().size() == 1);

RunningInstancesItemType instItem = ret.getInstancesSet().getItem().get(0);
Assert.assertTrue(instItem.getVpcId().equals(properties.get(Constants.PROP_NAME_VPC_ID))); // from
// aws.mock-default.properties
Assert.assertTrue(instItem.getVpcId().equals(VPC_ID));
Assert.assertTrue(
instItem.getSubnetId().equals(properties.get(Constants.PROP_NAME_SUBNET_ID)));
instItem.getSubnetId().equals(SUBNET_ID));
Assert.assertTrue(instItem.getPrivateIpAddress()
.equals(properties.get(Constants.PROP_NAME_PRIVATE_IP_ADDRESS)));
Assert.assertTrue(instItem.getImageId().equals("ami-1"));
Expand Down Expand Up @@ -713,9 +716,11 @@ public void Test_describeInstances() throws Exception {

CustomMockEc2Instance ec2Mocked1 = new CustomMockEc2Instance();
ec2Mocked1.setInstanceType(InstanceType.C1_MEDIUM);
ec2Mocked1.setSubnetId(SUBNET_ID);

CustomMockEc2Instance ec2Mocked2 = new CustomMockEc2Instance();
ec2Mocked2.setInstanceType(InstanceType.C3_8XLARGE);
ec2Mocked2.setSubnetId(SUBNET_ID);

MockEc2Controller controller = Mockito.spy(MockEc2Controller.class);

Expand All @@ -731,6 +736,8 @@ public void Test_describeInstances() throws Exception {
allMockEc2Instances);
Whitebox.setInternalState(handler, "mockEc2Controller", controller);

createAndInjectMockSubnetController(handler);

Set<String> instanceStateSet = new HashSet<String>();
instanceStateSet.add(InstanceState.STOPPED.getName());

Expand Down Expand Up @@ -758,27 +765,31 @@ public void Test_describeInstances() throws Exception {

String instanceId1 = runningSetType.getItem().get(0).getInstanceId();

// check if default params were applied
// check if network params were applied
Assert.assertTrue(runningSetType.getItem().get(0).getVpcId()
.equals(properties.get(Constants.PROP_NAME_VPC_ID)));
.equals(VPC_ID));
Assert.assertTrue(runningSetType.getItem().get(0).getSubnetId()
.equals(SUBNET_ID));

// check if default params were applied
Assert.assertTrue(runningSetType.getItem().get(0).getPrivateIpAddress()
.equals(properties.get(Constants.PROP_NAME_PRIVATE_IP_ADDRESS)));
Assert.assertTrue(runningSetType.getItem().get(0).getSubnetId()
.equals(properties.get(Constants.PROP_NAME_SUBNET_ID)));
Assert.assertTrue(runningSetType.getItem().get(0).getInstanceState().getName()
.equals(InstanceState.STOPPED.getName()));

runningSetType = ret.getReservationSet().getItem().get(1).getInstancesSet();

String instanceId2 = runningSetType.getItem().get(0).getInstanceId();

// check if default params were applied
// check if network params were applied
Assert.assertTrue(runningSetType.getItem().get(0).getVpcId()
.equals(properties.get(Constants.PROP_NAME_VPC_ID)));
.equals(VPC_ID));
Assert.assertTrue(runningSetType.getItem().get(0).getSubnetId()
.equals(SUBNET_ID));

// check if default params were applied
Assert.assertTrue(runningSetType.getItem().get(0).getPrivateIpAddress()
.equals(properties.get(Constants.PROP_NAME_PRIVATE_IP_ADDRESS)));
Assert.assertTrue(runningSetType.getItem().get(0).getSubnetId()
.equals(properties.get(Constants.PROP_NAME_SUBNET_ID)));
Assert.assertTrue(runningSetType.getItem().get(0).getInstanceState().getName()
.equals(InstanceState.STOPPED.getName()));

Expand Down Expand Up @@ -1106,6 +1117,35 @@ public void Test_handleRunInstances() throws IOException {
Assert.assertTrue(responseString.equals(DUMMY_XML_RESPONSE));
}

@Test
public void Test_handleRunInstancesWithSubnetId() throws IOException {

HttpServletResponse response = Mockito.spy(HttpServletResponse.class);
MockEC2QueryHandler handler = MockEC2QueryHandler.getInstance();

StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw);

Mockito.when(response.getWriter()).thenReturn(pw);
Mockito.when(JAXBUtil.marshall(Mockito.any(), Mockito.eq("RunInstancesResponse"),
Mockito.eq(VERSION_1)))
.thenReturn(DUMMY_XML_RESPONSE);

Map<String, String[]> queryParams = new HashMap<String, String[]>();

queryParams.put(VERSION_KEY, new String[] { VERSION_1 });
queryParams.put(ACTION_KEY, new String[] { "RunInstances" });
queryParams.put("ImageId", new String[] { "img-1" });
queryParams.put("MinCount", new String[] { "2" });
queryParams.put("MaxCount", new String[] { "5" });
queryParams.put("InstanceType", new String[] { "m1.small" });
queryParams.put("SubnetId", new String[] { "subnetId" });
handler.handle(queryParams, null, response);

String responseString = sw.toString();
Assert.assertTrue(responseString.equals(DUMMY_XML_RESPONSE));
}

@Test
public void Test_handleStartInstances() throws IOException {

Expand Down Expand Up @@ -2315,4 +2355,14 @@ public void Test_handleNonSafeAPI_DescribeVolumes() throws IOException {
String responseString = sw.toString();
Assert.assertTrue(responseString.contains("Response"));
}

private static void createAndInjectMockSubnetController(final MockEC2QueryHandler handler) {
final MockSubnet subnet = new MockSubnet();
subnet.setSubnetId(SUBNET_ID);
subnet.setVpcId(VPC_ID);

MockSubnetController subnetControllerMock = Mockito.spy(MockSubnetController.class);
Mockito.when(subnetControllerMock.describeSubnets()).thenReturn(Collections.singletonList(subnet));
Whitebox.setInternalState(handler, "mockSubnetController", subnetControllerMock);
}
}
Loading