diff --git a/spinifex/daemon/daemon_handlers_vpc.go b/spinifex/daemon/daemon_handlers_vpc.go index 4922b29c3..7ba84e0e9 100644 --- a/spinifex/daemon/daemon_handlers_vpc.go +++ b/spinifex/daemon/daemon_handlers_vpc.go @@ -214,7 +214,11 @@ func (d *Daemon) ensureDefaultIGWRoute(accountID, vpcID string) { {Name: &mainFilter, Values: []*string{&trueVal}}, }, }, accountID) - if err != nil || len(descOut.RouteTables) == 0 { + if err != nil { + slog.Warn("Failed to query main route table for default IGW route", "vpcId", vpcID, "err", err) + return + } + if len(descOut.RouteTables) == 0 { slog.Debug("No main route table found for default IGW route", "vpcId", vpcID, "accountID", accountID) return } @@ -231,6 +235,7 @@ func (d *Daemon) ensureDefaultIGWRoute(accountID, vpcID string) { // Find the attached IGW for this VPC igwOut, err := d.igwService.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{}, accountID) if err != nil { + slog.Warn("Failed to query IGWs for default route", "vpcId", vpcID, "err", err) return } var igwID string diff --git a/spinifex/handlers/ec2/routetable/service_impl.go b/spinifex/handlers/ec2/routetable/service_impl.go index b5efe0819..f581152f1 100644 --- a/spinifex/handlers/ec2/routetable/service_impl.go +++ b/spinifex/handlers/ec2/routetable/service_impl.go @@ -78,10 +78,15 @@ func NewRouteTableServiceImplWithNATS(cfg *config.Config, natsConn *nats.Conn) ( func (s *RouteTableServiceImpl) getRouteTable(accountID, rtbID string) (*RouteTableRecord, error) { entry, err := s.rtbKV.Get(utils.AccountKey(accountID, rtbID)) if err != nil { - return nil, errors.New(awserrors.ErrorInvalidRouteTableIDNotFound) + if errors.Is(err, nats.ErrKeyNotFound) { + return nil, errors.New(awserrors.ErrorInvalidRouteTableIDNotFound) + } + slog.Error("Failed to read route table from KV", "routeTableId", rtbID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } var record RouteTableRecord if err := json.Unmarshal(entry.Value(), &record); err != nil { + slog.Error("Corrupt route table record in KV", "routeTableId", rtbID, "err", err) return nil, errors.New(awserrors.ErrorServerInternal) } return &record, nil @@ -91,9 +96,11 @@ func (s *RouteTableServiceImpl) getRouteTable(accountID, rtbID string) (*RouteTa func (s *RouteTableServiceImpl) putRouteTable(accountID string, record *RouteTableRecord) error { data, err := json.Marshal(record) if err != nil { + slog.Error("Failed to marshal route table record", "routeTableId", record.RouteTableId, "err", err) return errors.New(awserrors.ErrorServerInternal) } if _, err := s.rtbKV.Put(utils.AccountKey(accountID, record.RouteTableId), data); err != nil { + slog.Error("Failed to write route table to KV", "routeTableId", record.RouteTableId, "err", err) return errors.New(awserrors.ErrorServerInternal) } return nil @@ -130,11 +137,16 @@ func (s *RouteTableServiceImpl) allRouteTablesForVPC(accountID, vpcID string) ([ } entry, err := s.rtbKV.Get(key) if err != nil { - continue + if errors.Is(err, nats.ErrKeyNotFound) { + continue // deleted between Keys() and Get() + } + slog.Error("Failed to read route table during VPC scan", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } var record RouteTableRecord if err := json.Unmarshal(entry.Value(), &record); err != nil { - continue + slog.Error("Corrupt route table record", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } if record.VpcId == vpcID { results = append(results, record) @@ -274,12 +286,17 @@ func (s *RouteTableServiceImpl) DescribeRouteTables(input *ec2.DescribeRouteTabl entry, err := s.rtbKV.Get(key) if err != nil { - continue + if errors.Is(err, nats.ErrKeyNotFound) { + continue + } + slog.Error("Failed to read route table during describe", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } var record RouteTableRecord if err := json.Unmarshal(entry.Value(), &record); err != nil { - continue + slog.Error("Corrupt route table record", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } if len(rtbIDs) > 0 && !rtbIDs[record.RouteTableId] { @@ -554,7 +571,11 @@ func (s *RouteTableServiceImpl) DisassociateRouteTable(input *ec2.DisassociateRo prefix := accountID + "." keys, err := s.rtbKV.Keys() if err != nil { - return nil, errors.New(awserrors.ErrorInvalidAssociationIDNotFound) + if errors.Is(err, nats.ErrNoKeysFound) { + return nil, errors.New(awserrors.ErrorInvalidAssociationIDNotFound) + } + slog.Error("Failed to list route table keys", "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } for _, key := range keys { @@ -564,12 +585,17 @@ func (s *RouteTableServiceImpl) DisassociateRouteTable(input *ec2.DisassociateRo entry, err := s.rtbKV.Get(key) if err != nil { - continue + if errors.Is(err, nats.ErrKeyNotFound) { + continue + } + slog.Error("Failed to read route table during disassociate scan", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } var record RouteTableRecord if err := json.Unmarshal(entry.Value(), &record); err != nil { - continue + slog.Error("Corrupt route table record", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } for i, assoc := range record.Associations { @@ -613,7 +639,11 @@ func (s *RouteTableServiceImpl) ReplaceRouteTableAssociation(input *ec2.ReplaceR prefix := accountID + "." keys, err := s.rtbKV.Keys() if err != nil { - return nil, errors.New(awserrors.ErrorInvalidAssociationIDNotFound) + if errors.Is(err, nats.ErrNoKeysFound) { + return nil, errors.New(awserrors.ErrorInvalidAssociationIDNotFound) + } + slog.Error("Failed to list route table keys", "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } for _, key := range keys { @@ -623,12 +653,17 @@ func (s *RouteTableServiceImpl) ReplaceRouteTableAssociation(input *ec2.ReplaceR entry, err := s.rtbKV.Get(key) if err != nil { - continue + if errors.Is(err, nats.ErrKeyNotFound) { + continue + } + slog.Error("Failed to read route table during replace scan", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } var oldRecord RouteTableRecord if err := json.Unmarshal(entry.Value(), &oldRecord); err != nil { - continue + slog.Error("Corrupt route table record", "key", key, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) } for i, assoc := range oldRecord.Associations { @@ -657,6 +692,13 @@ func (s *RouteTableServiceImpl) ReplaceRouteTableAssociation(input *ec2.ReplaceR Main: assoc.Main, }) if err := s.putRouteTable(accountID, newRecord); err != nil { + // Compensate: restore association to old table to avoid data loss + oldRecord.Associations = append(oldRecord.Associations, assoc) + if restoreErr := s.putRouteTable(accountID, &oldRecord); restoreErr != nil { + slog.Error("CRITICAL: ReplaceRouteTableAssociation partial failure, association lost", + "associationId", assocID, "oldRouteTableId", oldRecord.RouteTableId, + "newRouteTableId", newRtbID, "restoreErr", restoreErr, "originalErr", err) + } return nil, err } diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index 01b7441cb..51fcc4f1c 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -248,7 +248,9 @@ func (s *VPCServiceImpl) CreateVpc(input *ec2.CreateVpcInput, accountID string) // Auto-create main route table with local route (matches AWS behavior) if s.rtbKV != nil { - s.createMainRouteTable(accountID, vpcID, record.CidrBlock) + if err := s.createMainRouteTable(accountID, vpcID, record.CidrBlock); err != nil { + slog.Error("Failed to create main route table for VPC", "vpcId", vpcID, "err", err) + } } return &ec2.CreateVpcOutput{ @@ -933,7 +935,9 @@ func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string, bootstrap ...Bootstr // Create main route table with local route (written directly to KV to avoid circular import) if s.rtbKV != nil { - s.createMainRouteTable(accountID, vpcID, DefaultVPCCidr) + if err := s.createMainRouteTable(accountID, vpcID, DefaultVPCCidr); err != nil { + slog.Error("Failed to create main route table for VPC", "vpcId", vpcID, "err", err) + } } slog.Info("Created default VPC and subnet", @@ -954,7 +958,7 @@ func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string, bootstrap ...Bootstr // createMainRouteTable writes a main route table record directly to the route table KV bucket. // This avoids a circular import (routetable package imports vpc package for validation). -func (s *VPCServiceImpl) createMainRouteTable(accountID, vpcID, vpcCidr string) { +func (s *VPCServiceImpl) createMainRouteTable(accountID, vpcID, vpcCidr string) error { type rtbRecord struct { RouteTableId string `json:"route_table_id"` VpcId string `json:"vpc_id"` @@ -1000,15 +1004,14 @@ func (s *VPCServiceImpl) createMainRouteTable(accountID, vpcID, vpcCidr string) data, err := json.Marshal(record) if err != nil { - slog.Error("Failed to marshal main route table", "vpcId", vpcID, "err", err) - return + return fmt.Errorf("marshal main route table: %w", err) } if _, err := s.rtbKV.Put(utils.AccountKey(accountID, rtbID), data); err != nil { - slog.Error("Failed to store main route table", "vpcId", vpcID, "err", err) - return + return fmt.Errorf("store main route table: %w", err) } slog.Info("Created main route table for VPC", "routeTableId", rtbID, "vpcId", vpcID, "accountID", accountID) + return nil } // GetDefaultSubnet returns the default subnet for RunInstances when no SubnetId is specified. diff --git a/tests/e2e/run-e2e.sh b/tests/e2e/run-e2e.sh index f86b14fbc..d4e7fa185 100755 --- a/tests/e2e/run-e2e.sh +++ b/tests/e2e/run-e2e.sh @@ -2604,37 +2604,38 @@ echo "========================================" echo "Step 1: Verify default VPC main route table" RTB_DEFAULT_VPC=$(aws ec2 describe-vpcs --query 'Vpcs[?IsDefault==`true`].VpcId | [0]' --output text) if [ -z "$RTB_DEFAULT_VPC" ] || [ "$RTB_DEFAULT_VPC" = "None" ]; then - echo "SKIP: No default VPC found" -else - MAIN_RTB=$(aws ec2 describe-route-tables \ - --filters "Name=vpc-id,Values=$RTB_DEFAULT_VPC" "Name=association.main,Values=true" \ - --query 'RouteTables[0].RouteTableId' --output text) - if [ -z "$MAIN_RTB" ] || [ "$MAIN_RTB" = "None" ]; then - echo "FAIL: No main route table found for default VPC $RTB_DEFAULT_VPC" - exit 1 - fi - echo " Main route table: $MAIN_RTB" - - # Verify local route exists - LOCAL_ROUTE=$(aws ec2 describe-route-tables --route-table-ids "$MAIN_RTB" \ - --query 'RouteTables[0].Routes[?GatewayId==`local`].DestinationCidrBlock | [0]' --output text) - if [ -z "$LOCAL_ROUTE" ] || [ "$LOCAL_ROUTE" = "None" ]; then - echo "FAIL: Main route table missing local route" - exit 1 - fi - echo " Local route: $LOCAL_ROUTE → local" - - # Verify IGW route exists - IGW_ROUTE=$(aws ec2 describe-route-tables --route-table-ids "$MAIN_RTB" \ - --query 'RouteTables[0].Routes[?DestinationCidrBlock==`0.0.0.0/0`].GatewayId | [0]' --output text) - if [ -z "$IGW_ROUTE" ] || [ "$IGW_ROUTE" = "None" ]; then - echo "FAIL: Main route table missing 0.0.0.0/0 → IGW route" - exit 1 - fi - echo " Default route: 0.0.0.0/0 → $IGW_ROUTE" - echo " PASS: Default VPC main route table verified" + echo "FAIL: No default VPC found — route table tests require a default VPC" + exit 1 fi +MAIN_RTB=$(aws ec2 describe-route-tables \ + --filters "Name=vpc-id,Values=$RTB_DEFAULT_VPC" "Name=association.main,Values=true" \ + --query 'RouteTables[0].RouteTableId' --output text) +if [ -z "$MAIN_RTB" ] || [ "$MAIN_RTB" = "None" ]; then + echo "FAIL: No main route table found for default VPC $RTB_DEFAULT_VPC" + exit 1 +fi +echo " Main route table: $MAIN_RTB" + +# Verify local route exists +LOCAL_ROUTE=$(aws ec2 describe-route-tables --route-table-ids "$MAIN_RTB" \ + --query 'RouteTables[0].Routes[?GatewayId==`local`].DestinationCidrBlock | [0]' --output text) +if [ -z "$LOCAL_ROUTE" ] || [ "$LOCAL_ROUTE" = "None" ]; then + echo "FAIL: Main route table missing local route" + exit 1 +fi +echo " Local route: $LOCAL_ROUTE → local" + +# Verify IGW route exists +IGW_ROUTE=$(aws ec2 describe-route-tables --route-table-ids "$MAIN_RTB" \ + --query 'RouteTables[0].Routes[?DestinationCidrBlock==`0.0.0.0/0`].GatewayId | [0]' --output text) +if [ -z "$IGW_ROUTE" ] || [ "$IGW_ROUTE" = "None" ]; then + echo "FAIL: Main route table missing 0.0.0.0/0 → IGW route" + exit 1 +fi +echo " Default route: 0.0.0.0/0 → $IGW_ROUTE" +echo " PASS: Default VPC main route table verified" + # Step 2: Custom route table lifecycle echo "" echo "Step 2: Custom route table CRUD lifecycle"