diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index bd11b3d13..430dd3360 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -381,7 +381,7 @@ ENIs are managed via `spinifex-vpc-enis` KV bucket with CAS-based IPAM allocatio | `describe-network-interfaces` | `--network-interface-ids`, `--filters` (subnet-id, vpc-id, attachment.instance-id) | `--max-results`, `--dry-run` | None | NATS `ec2.DescribeNetworkInterfaces` → daemon lists all keys from `spinifex-vpc-enis` KV → applies filters (subnet-id, vpc-id, attachment.instance-id) → filters by ENI IDs if specified → returns error for non-existent requested IDs | 1. List all ENIs
2. Filter by subnet-id
3. Filter by vpc-id
4. Filter by attachment.instance-id
5. Non-existent ENI returns error | **DONE** | | `attach-network-interface` | — | `--network-interface-id`, `--instance-id`, `--device-index` | ENI and instance must exist | Internal operation used by `run-instances --subnet-id`. Marks ENI as "in-use", sets instance attachment, generates attachment ID. No standalone API endpoint yet. | 1. Attach ENI to instance
2. Already attached (error)
3. Missing device index (error) | **NOT STARTED** (internal only) | | `detach-network-interface` | — | `--attachment-id`, `--force` | ENI must be attached | Internal operation used by terminate-instances. Marks ENI as "available", clears attachment info. No standalone API endpoint yet. | 1. Detach ENI
2. Force detach
3. Not attached (error) | **NOT STARTED** (internal only) | -| `modify-network-interface-attribute` | — | `--network-interface-id`, `--description`, `--source-dest-check`, `--groups` | ENI must exist | NATS `ec2.ModifyNetworkInterfaceAttribute` → daemon updates ENI attributes in KV → return success | 1. Modify description
2. Toggle source/dest check
3. Update security groups | **NOT STARTED** | +| `modify-network-interface-attribute` | — | `--network-interface-id`, `--description`, `--groups` | ENI must exist; at least one attribute required | NATS `ec2.ModifyNetworkInterfaceAttribute` → daemon validates ENI exists → updates description and/or security groups in `spinifex-vpc-enis` KV (optimistic locking) → return success | 1. Modify description
2. Update security groups
3. No attributes (InvalidParameterValue)
4. Non-existent ENI (error) | **DONE** | | `assign-private-ip-addresses` | — | `--network-interface-id`, `--private-ip-addresses` or `--secondary-private-ip-address-count` | ENI must exist | NATS `ec2.AssignPrivateIpAddresses` → daemon allocates IPs from subnet pool → assign to ENI → return assigned IPs | 1. Assign specific IPs
2. Assign by count
3. Cannot specify both (error) | **NOT STARTED** | | `unassign-private-ip-addresses` | — | `--network-interface-id`, `--private-ip-addresses` | IPs must be assigned to ENI | NATS `ec2.UnassignPrivateIpAddresses` → daemon releases IPs → return success | 1. Unassign secondary IP
2. Unassign primary IP (error) | **NOT STARTED** | diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index fb8b3ff85..0240ebc4f 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -425,6 +425,7 @@ func (d *Daemon) subscribeAll() error { {"ec2.CreateNetworkInterface", d.handleEC2CreateNetworkInterface, "spinifex-workers"}, {"ec2.DeleteNetworkInterface", d.handleEC2DeleteNetworkInterface, "spinifex-workers"}, {"ec2.DescribeNetworkInterfaces", d.handleEC2DescribeNetworkInterfaces, "spinifex-workers"}, + {"ec2.ModifyNetworkInterfaceAttribute", d.handleEC2ModifyNetworkInterfaceAttribute, "spinifex-workers"}, {"ec2.CreateSecurityGroup", d.handleEC2CreateSecurityGroup, "spinifex-workers"}, {"ec2.DeleteSecurityGroup", d.handleEC2DeleteSecurityGroup, "spinifex-workers"}, {"ec2.DescribeSecurityGroups", d.handleEC2DescribeSecurityGroups, "spinifex-workers"}, diff --git a/spinifex/daemon/daemon_handlers_vpc.go b/spinifex/daemon/daemon_handlers_vpc.go index 7ba84e0e9..9aa3d61ab 100644 --- a/spinifex/daemon/daemon_handlers_vpc.go +++ b/spinifex/daemon/daemon_handlers_vpc.go @@ -54,6 +54,10 @@ func (d *Daemon) handleEC2DeleteNetworkInterface(msg *nats.Msg) { handleNATSRequest(msg, d.vpcService.DeleteNetworkInterface) } +func (d *Daemon) handleEC2ModifyNetworkInterfaceAttribute(msg *nats.Msg) { + handleNATSRequest(msg, d.vpcService.ModifyNetworkInterfaceAttribute) +} + func (d *Daemon) handleEC2DescribeNetworkInterfaces(msg *nats.Msg) { handleNATSRequest(msg, d.vpcService.DescribeNetworkInterfaces) } diff --git a/spinifex/gateway/ec2.go b/spinifex/gateway/ec2.go index f1e156888..720368e62 100644 --- a/spinifex/gateway/ec2.go +++ b/spinifex/gateway/ec2.go @@ -278,6 +278,9 @@ var ec2Actions = map[string]EC2Handler{ "DescribeNetworkInterfaces": ec2Handler(func(input *ec2.DescribeNetworkInterfacesInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_vpc.DescribeNetworkInterfaces(input, gw.NATSConn, accountID) }), + "ModifyNetworkInterfaceAttribute": ec2Handler(func(input *ec2.ModifyNetworkInterfaceAttributeInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_ec2_vpc.ModifyNetworkInterfaceAttribute(input, gw.NATSConn, accountID) + }), "CreateSecurityGroup": ec2Handler(func(input *ec2.CreateSecurityGroupInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_vpc.CreateSecurityGroup(input, gw.NATSConn, accountID) }), diff --git a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go new file mode 100644 index 000000000..0d24c49e0 --- /dev/null +++ b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go @@ -0,0 +1,30 @@ +package gateway_ec2_vpc + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" + "github.com/nats-io/nats.go" +) + +// ModifyNetworkInterfaceAttribute handles the EC2 ModifyNetworkInterfaceAttribute API call +func ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput, natsConn *nats.Conn, accountID string) (ec2.ModifyNetworkInterfaceAttributeOutput, error) { + var output ec2.ModifyNetworkInterfaceAttributeOutput + + if input == nil { + return output, errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { + return output, errors.New(awserrors.ErrorMissingParameter) + } + + svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) + result, err := svc.ModifyNetworkInterfaceAttribute(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/gateway_test.go b/spinifex/gateway/gateway_test.go index 56075386a..0b45ada58 100644 --- a/spinifex/gateway/gateway_test.go +++ b/spinifex/gateway/gateway_test.go @@ -1023,7 +1023,7 @@ func TestEC2ActionMapCompleteness(t *testing.T) { "CreatePlacementGroup", "DeletePlacementGroup", "DescribePlacementGroups", "CreateVpc", "DeleteVpc", "DescribeVpcs", "ModifyVpcAttribute", "DescribeVpcAttribute", "CreateSubnet", "DeleteSubnet", "DescribeSubnets", "ModifySubnetAttribute", - "CreateNetworkInterface", "DeleteNetworkInterface", "DescribeNetworkInterfaces", + "CreateNetworkInterface", "DeleteNetworkInterface", "DescribeNetworkInterfaces", "ModifyNetworkInterfaceAttribute", "CreateSecurityGroup", "DeleteSecurityGroup", "DescribeSecurityGroups", "AuthorizeSecurityGroupIngress", "AuthorizeSecurityGroupEgress", "RevokeSecurityGroupIngress", "RevokeSecurityGroupEgress", diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index 741ab0951..a74c69fa3 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -35,6 +35,7 @@ type ENIRecord struct { DeviceIndex int64 `json:"device_index"` PublicIpAddress string `json:"public_ip_address,omitempty"` // Auto-assigned or EIP PublicIpPool string `json:"public_ip_pool,omitempty"` // Pool name the public IP came from + SecurityGroupIds []string `json:"security_group_ids,omitempty"` Tags map[string]string `json:"tags"` CreatedAt time.Time `json:"created_at"` } @@ -81,6 +82,13 @@ func (s *VPCServiceImpl) CreateNetworkInterface(input *ec2.CreateNetworkInterfac description = *input.Description } + var sgIds []string + for _, id := range input.Groups { + if id != nil { + sgIds = append(sgIds, *id) + } + } + record := ENIRecord{ NetworkInterfaceId: eniId, SubnetId: subnetId, @@ -90,6 +98,7 @@ func (s *VPCServiceImpl) CreateNetworkInterface(input *ec2.CreateNetworkInterfac MacAddress: macAddr, Description: description, Status: "available", + SecurityGroupIds: sgIds, Tags: make(map[string]string), CreatedAt: time.Now(), } @@ -164,6 +173,58 @@ func (s *VPCServiceImpl) DeleteNetworkInterface(input *ec2.DeleteNetworkInterfac return &ec2.DeleteNetworkInterfaceOutput{}, nil } +// ModifyNetworkInterfaceAttribute modifies ENI attributes (security groups, description). +func (s *VPCServiceImpl) ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) { + if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + if len(input.Groups) == 0 && input.Description == nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + + eniId := *input.NetworkInterfaceId + key := utils.AccountKey(accountID, eniId) + + entry, err := s.eniKV.Get(key) + if err != nil { + return nil, errors.New(awserrors.ErrorInvalidNetworkInterfaceIDNotFound) + } + + var record ENIRecord + if err := json.Unmarshal(entry.Value(), &record); err != nil { + slog.Error("ModifyNetworkInterfaceAttribute: corrupted ENI record", "eniId", eniId, "accountID", accountID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + if len(input.Groups) > 0 { + sgIds := make([]string, 0, len(input.Groups)) + for _, id := range input.Groups { + if id != nil { + sgIds = append(sgIds, *id) + } + } + record.SecurityGroupIds = sgIds + } + + if input.Description != nil && input.Description.Value != nil { + record.Description = *input.Description.Value + } + + data, err := json.Marshal(record) + if err != nil { + slog.Error("ModifyNetworkInterfaceAttribute: failed to marshal ENI record", "eniId", eniId, "accountID", accountID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if _, err := s.eniKV.Update(key, data, entry.Revision()); err != nil { + slog.Error("ModifyNetworkInterfaceAttribute: KV update failed", "eniId", eniId, "accountID", accountID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + slog.Info("ModifyNetworkInterfaceAttribute completed", "eniId", eniId, "accountID", accountID) + + return &ec2.ModifyNetworkInterfaceAttributeOutput{}, nil +} + // DescribeNetworkInterfaces lists ENIs with optional filters func (s *VPCServiceImpl) DescribeNetworkInterfaces(input *ec2.DescribeNetworkInterfacesInput, accountID string) (*ec2.DescribeNetworkInterfacesOutput, error) { var enis []*ec2.NetworkInterface @@ -376,6 +437,16 @@ func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string) *ec Groups: []*ec2.GroupIdentifier{}, } + if len(record.SecurityGroupIds) > 0 { + groups := make([]*ec2.GroupIdentifier, 0, len(record.SecurityGroupIds)) + for _, sgId := range record.SecurityGroupIds { + groups = append(groups, &ec2.GroupIdentifier{ + GroupId: aws.String(sgId), + }) + } + eni.Groups = groups + } + if record.PublicIpAddress != "" { eni.Association = &ec2.NetworkInterfaceAssociation{ PublicIp: aws.String(record.PublicIpAddress), diff --git a/spinifex/handlers/ec2/vpc/eni_test.go b/spinifex/handlers/ec2/vpc/eni_test.go index 49660dfa6..5e1b314ae 100644 --- a/spinifex/handlers/ec2/vpc/eni_test.go +++ b/spinifex/handlers/ec2/vpc/eni_test.go @@ -407,3 +407,91 @@ func TestDeleteNetworkInterface_PublishesEvent(t *testing.T) { t.Fatal("timed out waiting for vpc.delete-port event") } } + +func TestModifyNetworkInterfaceAttribute_SecurityGroups(t *testing.T) { + svc := setupTestVPCService(t) + vpcId := createTestVPC(t, svc, "10.0.0.0/16") + subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") + eniId := createTestENI(t, svc, subnetId) + + _, err := svc.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String(eniId), + Groups: []*string{aws.String("sg-111"), aws.String("sg-222")}, + }, testAccountID) + require.NoError(t, err) + + desc, err := svc.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{ + NetworkInterfaceIds: []*string{aws.String(eniId)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.NetworkInterfaces, 1) + require.Len(t, desc.NetworkInterfaces[0].Groups, 2) + assert.Equal(t, "sg-111", *desc.NetworkInterfaces[0].Groups[0].GroupId) + assert.Equal(t, "sg-222", *desc.NetworkInterfaces[0].Groups[1].GroupId) +} + +func TestModifyNetworkInterfaceAttribute_Description(t *testing.T) { + svc := setupTestVPCService(t) + vpcId := createTestVPC(t, svc, "10.0.0.0/16") + subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") + eniId := createTestENI(t, svc, subnetId) + + _, err := svc.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String(eniId), + Description: &ec2.AttributeValue{Value: aws.String("updated description")}, + }, testAccountID) + require.NoError(t, err) + + desc, err := svc.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{ + NetworkInterfaceIds: []*string{aws.String(eniId)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.NetworkInterfaces, 1) + assert.Equal(t, "updated description", *desc.NetworkInterfaces[0].Description) +} + +func TestModifyNetworkInterfaceAttribute_NoAttributes(t *testing.T) { + svc := setupTestVPCService(t) + vpcId := createTestVPC(t, svc, "10.0.0.0/16") + subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") + eniId := createTestENI(t, svc, subnetId) + + _, err := svc.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String(eniId), + }, testAccountID) + assert.ErrorContains(t, err, "InvalidParameterValue") +} + +func TestModifyNetworkInterfaceAttribute_NotFound(t *testing.T) { + svc := setupTestVPCService(t) + + _, err := svc.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String("eni-nonexistent"), + Groups: []*string{aws.String("sg-111")}, + }, testAccountID) + assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound") +} + +func TestModifyNetworkInterfaceAttribute_MissingID(t *testing.T) { + svc := setupTestVPCService(t) + + _, err := svc.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + Groups: []*string{aws.String("sg-111")}, + }, testAccountID) + assert.ErrorContains(t, err, "MissingParameter") +} + +func TestCreateNetworkInterface_WithSecurityGroups(t *testing.T) { + svc := setupTestVPCService(t) + vpcId := createTestVPC(t, svc, "10.0.0.0/16") + subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") + + out, err := svc.CreateNetworkInterface(&ec2.CreateNetworkInterfaceInput{ + SubnetId: aws.String(subnetId), + Groups: []*string{aws.String("sg-aaa"), aws.String("sg-bbb")}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.NetworkInterface.Groups, 2) + assert.Equal(t, "sg-aaa", *out.NetworkInterface.Groups[0].GroupId) + assert.Equal(t, "sg-bbb", *out.NetworkInterface.Groups[1].GroupId) +} diff --git a/spinifex/handlers/ec2/vpc/service.go b/spinifex/handlers/ec2/vpc/service.go index bb7b6cf38..029a01254 100644 --- a/spinifex/handlers/ec2/vpc/service.go +++ b/spinifex/handlers/ec2/vpc/service.go @@ -16,6 +16,7 @@ type VPCService interface { CreateNetworkInterface(input *ec2.CreateNetworkInterfaceInput, accountID string) (*ec2.CreateNetworkInterfaceOutput, error) DeleteNetworkInterface(input *ec2.DeleteNetworkInterfaceInput, accountID string) (*ec2.DeleteNetworkInterfaceOutput, error) DescribeNetworkInterfaces(input *ec2.DescribeNetworkInterfacesInput, accountID string) (*ec2.DescribeNetworkInterfacesOutput, error) + ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) CreateSecurityGroup(input *ec2.CreateSecurityGroupInput, accountID string) (*ec2.CreateSecurityGroupOutput, error) DeleteSecurityGroup(input *ec2.DeleteSecurityGroupInput, accountID string) (*ec2.DeleteSecurityGroupOutput, error) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput, accountID string) (*ec2.DescribeSecurityGroupsOutput, error) diff --git a/spinifex/handlers/ec2/vpc/service_nats.go b/spinifex/handlers/ec2/vpc/service_nats.go index 5c278a15b..f04c55a84 100644 --- a/spinifex/handlers/ec2/vpc/service_nats.go +++ b/spinifex/handlers/ec2/vpc/service_nats.go @@ -66,6 +66,10 @@ func (s *NATSVPCService) DescribeNetworkInterfaces(input *ec2.DescribeNetworkInt return utils.NATSRequest[ec2.DescribeNetworkInterfacesOutput](s.natsConn, "ec2.DescribeNetworkInterfaces", input, 30*time.Second, accountID) } +func (s *NATSVPCService) ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) { + return utils.NATSRequest[ec2.ModifyNetworkInterfaceAttributeOutput](s.natsConn, "ec2.ModifyNetworkInterfaceAttribute", input, 30*time.Second, accountID) +} + func (s *NATSVPCService) CreateSecurityGroup(input *ec2.CreateSecurityGroupInput, accountID string) (*ec2.CreateSecurityGroupOutput, error) { return utils.NATSRequest[ec2.CreateSecurityGroupOutput](s.natsConn, "ec2.CreateSecurityGroup", input, 30*time.Second, accountID) }