From 5dd9a1e34689ec5bec21b4254a2dc8a302a99df0 Mon Sep 17 00:00:00 2001 From: Zakaria Date: Sun, 25 Oct 2026 17:25:15 -0400 Subject: [PATCH] fix(efs): place mount targets in their real subnet CreateMountTarget now looks the subnet up in EC2 and takes the mount target's VPC, Availability Zone and IP address from it, instead of reporting vpc-12345678 and the region's first zone for every mount target. An unknown subnet is refused with SubnetNotFound, and, as in AWS, a file system gets one mount target per zone, all in one VPC (MountTargetConflict otherwise). Lambda checks a function's VPC against its file system's mount targets, so the made-up VPC kept consoles from finding the security group to use. --- .../floci/services/efs/EfsException.java | 4 + .../floci/services/efs/EfsService.java | 56 ++++++++++-- .../services/efs/EfsIntegrationTest.java | 7 +- .../efs/EfsMountTargetSubnetTest.java | 91 +++++++++++++++++++ 4 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 src/test/java/io/github/hectorvent/floci/services/efs/EfsMountTargetSubnetTest.java diff --git a/src/main/java/io/github/hectorvent/floci/services/efs/EfsException.java b/src/main/java/io/github/hectorvent/floci/services/efs/EfsException.java index d849107..b64e10a 100644 --- a/src/main/java/io/github/hectorvent/floci/services/efs/EfsException.java +++ b/src/main/java/io/github/hectorvent/floci/services/efs/EfsException.java @@ -29,6 +29,10 @@ public class EfsException extends AwsException { return new EfsException(Status.NOT_FOUND, "MountTargetNotFound", "Mount target " + mountTargetId + " does not exist."); } + public static EfsException subnetNotFound(String subnetId) { + return new EfsException(Status.BAD_REQUEST, "SubnetNotFound", "The subnet ID '" + subnetId + "' does not exist"); + } + public static EfsException mountTargetConflict(String message) { return new EfsException(Status.CONFLICT, "MountTargetConflict", message); } diff --git a/src/main/java/io/github/hectorvent/floci/services/efs/EfsService.java b/src/main/java/io/github/hectorvent/floci/services/efs/EfsService.java index 3baefa2..f2bf95c 100644 --- a/src/main/java/io/github/hectorvent/floci/services/efs/EfsService.java +++ b/src/main/java/io/github/hectorvent/floci/services/efs/EfsService.java @@ -2,7 +2,10 @@ package io.github.hectorvent.floci.services.efs; import io.github.hectorvent.floci.core.storage.StorageBackend; import io.github.hectorvent.floci.core.storage.StorageFactory; +import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.core.common.Resettable; +import io.github.hectorvent.floci.services.ec2.Ec2Service; +import io.github.hectorvent.floci.services.ec2.model.Subnet; import io.github.hectorvent.floci.services.efs.model.*; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -33,13 +36,16 @@ public class EfsService implements Resettable { private final ConcurrentHashMap syncLocks = new ConcurrentHashMap<>(); private final RegionResolver regionResolver; + private final Ec2Service ec2Service; + private Object lockFor(String key) { return syncLocks.computeIfAbsent(key, k -> new Object()); } @Inject - public EfsService(StorageFactory storageFactory, RegionResolver regionResolver) { + public EfsService(StorageFactory storageFactory, RegionResolver regionResolver, Ec2Service ec2Service) { this.regionResolver = regionResolver; + this.ec2Service = ec2Service; this.fileSystemStore = storageFactory.create("efs", "efs-filesystems.json", new com.fasterxml.jackson.core.type.TypeReference>() {}); this.mountTargetStore = storageFactory.create("efs", "efs-mounttargets.json", @@ -361,14 +367,24 @@ public class EfsService implements Resettable { public MountTarget createMountTarget(CreateMountTargetRequest request, String region) { synchronized (lockFor(regionKey(region, request.getFileSystemId()))) { FileSystem fs = getFileSystem(region, request.getFileSystemId()); - - for (MountTarget existing : mountTargetStore.scan(k -> k.startsWith(region + "::"))) { + Subnet subnet = requireSubnet(region, request.getSubnetId()); + + List regionMountTargets = mountTargetStore.scan(k -> k.startsWith(region + "::")); + for (MountTarget existing : regionMountTargets) { if (existing.getFileSystemId().equals(request.getFileSystemId())) { if (existing.getSubnetId().equals(request.getSubnetId())) { throw EfsException.mountTargetConflict("Mount target already exists in this subnet"); } + // AWS allows one mount target per Availability Zone, all in one VPC. + if (subnet.getAvailabilityZone().equals(existing.getAvailabilityZoneName())) { + throw EfsException.mountTargetConflict("Mount target already exists in this AZ"); + } + if (!subnet.getVpcId().equals(existing.getVpcId())) { + throw EfsException.mountTargetConflict("Mount targets of a file system must be in the same VPC"); + } } } + long inSubnet = regionMountTargets.stream().filter(existing -> request.getSubnetId().equals(existing.getSubnetId())).count(); String mtId = "fsmt-" + UUID.randomUUID().toString().replace("-", "").substring(0, 17); @@ -376,11 +392,11 @@ public class EfsService implements Resettable { mt.setMountTargetId(mtId); mt.setFileSystemId(request.getFileSystemId()); mt.setSubnetId(request.getSubnetId()); - mt.setIpAddress(request.getIpAddress() != null ? request.getIpAddress() : "10.0.0.10"); + mt.setIpAddress(request.getIpAddress() != null ? request.getIpAddress() : hostAddress(subnet.getCidrBlock(), inSubnet)); mt.setLifeCycleState(LifeCycleState.available); - mt.setVpcId("vpc-12345678"); - mt.setAvailabilityZoneName(region + "a"); - mt.setAvailabilityZoneId(generateAzId(region + "a")); + mt.setVpcId(subnet.getVpcId()); + mt.setAvailabilityZoneName(subnet.getAvailabilityZone()); + mt.setAvailabilityZoneId(generateAzId(subnet.getAvailabilityZone())); mt.setNetworkInterfaceId("eni-" + UUID.randomUUID().toString().replace("-", "").substring(0, 17)); if (request.getSecurityGroups() != null) { mt.setSecurityGroups(new ArrayList<>(request.getSecurityGroups())); @@ -613,6 +629,32 @@ public class EfsService implements Resettable { return region + "::" + id; } + private Subnet requireSubnet(String region, String subnetId) { + if (subnetId == null || subnetId.isBlank()) { + throw EfsException.badRequest("SubnetId is required."); + } + try { + return ec2Service.requireSubnet(region, subnetId); + } catch (AwsException e) { + throw EfsException.subnetNotFound(subnetId); + } + } + + /** + * An address inside the subnet for its n-th mount target, skipping the first four + * addresses AWS reserves in every subnet. + */ + static String hostAddress(String cidrBlock, long index) { + String[] parts = cidrBlock.split("/"); + String[] octets = parts[0].split("\\."); + long base = 0; + for (String octet : octets) { + base = (base << 8) | Long.parseLong(octet); + } + long address = base + 4 + index; + return ((address >> 24) & 0xff) + "." + ((address >> 16) & 0xff) + "." + ((address >> 8) & 0xff) + "." + (address & 0xff); + } + private String generateAzId(String azName) { if (azName == null || azName.length() < 2) return azName + "-id"; String region = azName.substring(0, azName.length() - 1); diff --git a/src/test/java/io/github/hectorvent/floci/services/efs/EfsIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/efs/EfsIntegrationTest.java index 9a1e5d5..2a567f3 100644 --- a/src/test/java/io/github/hectorvent/floci/services/efs/EfsIntegrationTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/efs/EfsIntegrationTest.java @@ -90,7 +90,7 @@ class EfsIntegrationTest { .body(""" { "FileSystemId": "%s", - "SubnetId": "subnet-12345", + "SubnetId": "subnet-default-us-east-1-a", "SecurityGroups": ["sg-11111"] } """.formatted(fileSystemId)) @@ -100,7 +100,10 @@ class EfsIntegrationTest { .statusCode(200) .body("MountTargetId", startsWith("fsmt-")) .body("FileSystemId", equalTo(fileSystemId)) - .body("SubnetId", equalTo("subnet-12345")) + .body("SubnetId", equalTo("subnet-default-us-east-1-a")) + .body("VpcId", equalTo("vpc-default-us-east-1")) + .body("AvailabilityZoneName", equalTo("us-east-1a")) + .body("IpAddress", equalTo("172.31.0.4")) .extract().jsonPath().getString("MountTargetId"); } diff --git a/src/test/java/io/github/hectorvent/floci/services/efs/EfsMountTargetSubnetTest.java b/src/test/java/io/github/hectorvent/floci/services/efs/EfsMountTargetSubnetTest.java new file mode 100644 index 0000000..d7ae399 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/efs/EfsMountTargetSubnetTest.java @@ -0,0 +1,91 @@ +package io.github.hectorvent.floci.services.efs; + +import io.github.hectorvent.floci.testing.RestAssuredJsonUtils; +import io.quarkus.test.junit.QuarkusTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** Mount targets take their VPC, zone and address from the EC2 subnet they are created in. */ +@QuarkusTest +class EfsMountTargetSubnetTest { + + @BeforeAll + static void configureRestAssured() { + RestAssuredJsonUtils.configureAwsContentTypes(); + } + + private String createFileSystem(String token) { + return given() + .contentType("application/json") + .body("{\"CreationToken\": \"" + token + "\"}") + .when() + .post("/2015-02-01/file-systems") + .then() + .statusCode(201) + .extract().jsonPath().getString("FileSystemId"); + } + + private io.restassured.response.ValidatableResponse createMountTarget(String fileSystemId, String subnetId) { + return given() + .contentType("application/json") + .body("{\"FileSystemId\": \"" + fileSystemId + "\", \"SubnetId\": \"" + subnetId + "\"}") + .when() + .post("/2015-02-01/mount-targets") + .then(); + } + + @Test + void usesTheZoneAndVpcOfEachSubnet() { + String fileSystemId = createFileSystem("subnet-zones"); + + createMountTarget(fileSystemId, "subnet-default-us-east-1-b") + .statusCode(200) + .body("VpcId", equalTo("vpc-default-us-east-1")) + .body("AvailabilityZoneName", equalTo("us-east-1b")) + .body("IpAddress", equalTo("172.31.16.4")); + createMountTarget(fileSystemId, "subnet-default-us-east-1-c") + .statusCode(200) + .body("AvailabilityZoneName", equalTo("us-east-1c")); + } + + @Test + void refusesASubnetThatDoesNotExist() { + String fileSystemId = createFileSystem("subnet-missing"); + + createMountTarget(fileSystemId, "subnet-nope") + .statusCode(400) + .body("__type", equalTo("SubnetNotFound")); + } + + @Test + void allowsOneMountTargetPerZone() { + String fileSystemId = createFileSystem("subnet-one-per-zone"); + String vpcId = "vpc-default-us-east-1"; + String secondSubnet = given() + .formParam("Action", "CreateSubnet") + .formParam("VpcId", vpcId) + .formParam("CidrBlock", "172.31.200.0/24") + .formParam("AvailabilityZone", "us-east-1a") + .header("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/ec2/aws4_request") + .when() + .post("/") + .then() + .statusCode(200) + .extract().path("CreateSubnetResponse.subnet.subnetId"); + + createMountTarget(fileSystemId, "subnet-default-us-east-1-a").statusCode(200); + createMountTarget(fileSystemId, secondSubnet) + .statusCode(409) + .body("__type", equalTo("MountTargetConflict")); + } + + @Test + void picksAddressesAfterTheFourAwsReserves() { + assertEquals("10.0.1.4", EfsService.hostAddress("10.0.1.0/24", 0)); + assertEquals("10.0.2.1", EfsService.hostAddress("10.0.1.0/24", 253)); + } +}