From 35f2104325916e8825177f83655ccde76bea7717 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 5 Mar 2026 15:09:24 +1000 Subject: [PATCH] feat: more tests --- Makefile | 3 +- .../ec2/volume/DescribeVolumeStatus_test.go | 51 ++++ hive/gateway/gateway_test.go | 74 +++++ hive/handlers/ec2/instance/service_impl.go | 109 ++++--- .../ec2/instance/service_impl_test.go | 92 ++++++ hive/handlers/ec2/key/service_impl_test.go | 79 +++++ hive/handlers/iam/service_impl_test.go | 144 +++++++++ hive/nbd/nbd.go | 25 +- hive/nbd/nbd_test.go | 284 ++++++++++++++++++ hive/services/vpcd/nbdb/model_test.go | 38 +++ scripts/check-coverage.sh | 8 +- scripts/diff-coverage.sh | 2 +- 12 files changed, 849 insertions(+), 60 deletions(-) create mode 100644 hive/nbd/nbd_test.go create mode 100644 hive/services/vpcd/nbdb/model_test.go diff --git a/Makefile b/Makefile index 6200a1048..9880f2e19 100644 --- a/Makefile +++ b/Makefile @@ -78,11 +78,10 @@ test: # Note: go test may exit non-zero due to Go version mismatch in coverage instrumentation # for packages without test files. We check actual test results + coverage threshold instead. COVERPROFILE ?= coverage.out -MIN_COVERAGE ?= 60.0 test-cover: @echo -e "\n....Running tests with coverage for $(GO_PROJECT_NAME)...." $(_Q)LOG_IGNORE=1 go test -timeout 120s -coverprofile=$(COVERPROFILE) -covermode=atomic ./hive/... $(_COVQ) - @scripts/check-coverage.sh $(COVERPROFILE) $(MIN_COVERAGE) $(QUIET) + @scripts/check-coverage.sh $(COVERPROFILE) $(QUIET) # Run unit tests with race detector test-race: diff --git a/hive/gateway/ec2/volume/DescribeVolumeStatus_test.go b/hive/gateway/ec2/volume/DescribeVolumeStatus_test.go index 9981fc6fb..7f7931d4b 100644 --- a/hive/gateway/ec2/volume/DescribeVolumeStatus_test.go +++ b/hive/gateway/ec2/volume/DescribeVolumeStatus_test.go @@ -8,6 +8,57 @@ import ( "github.com/stretchr/testify/assert" ) +func TestValidateDescribeVolumeStatusInput(t *testing.T) { + tests := []struct { + name string + input *ec2.DescribeVolumeStatusInput + wantErr bool + }{ + { + name: "NilInput", + input: nil, + wantErr: false, + }, + { + name: "EmptyInput", + input: &ec2.DescribeVolumeStatusInput{}, + wantErr: false, + }, + { + name: "ValidVolumeId", + input: &ec2.DescribeVolumeStatusInput{ + VolumeIds: []*string{aws.String("vol-abc123")}, + }, + wantErr: false, + }, + { + name: "MultipleValidVolumeIds", + input: &ec2.DescribeVolumeStatusInput{ + VolumeIds: []*string{aws.String("vol-abc123"), aws.String("vol-def456")}, + }, + wantErr: false, + }, + { + name: "NilVolumeIdEntry", + input: &ec2.DescribeVolumeStatusInput{ + VolumeIds: []*string{nil, aws.String("vol-abc123")}, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateDescribeVolumeStatusInput(tt.input) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + func TestDescribeVolumeStatus_InputValidation(t *testing.T) { tests := []struct { name string diff --git a/hive/gateway/gateway_test.go b/hive/gateway/gateway_test.go index fc8def3e2..31929657b 100644 --- a/hive/gateway/gateway_test.go +++ b/hive/gateway/gateway_test.go @@ -186,6 +186,80 @@ func TestErrorHandler_IAMService(t *testing.T) { assert.NotContains(t, xmlStr, "") } +func TestErrorHandler_UnknownError(t *testing.T) { + gw := &GatewayConfig{DisableLogging: true} + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), ctxService, "ec2") + r = r.WithContext(ctx) + gw.ErrorHandler(w, r, errors.New("SomeCompletelyBogusError")) + }) + + req := httptest.NewRequest("POST", "/", nil) + resp := doRequest(handler, req) + assert.Equal(t, 500, resp.StatusCode) + + body, _ := io.ReadAll(resp.Body) + xmlStr := string(body) + // Unknown errors should be remapped to InternalError + assert.Contains(t, xmlStr, "InternalError") + // EC2 format + assert.Contains(t, xmlStr, "") +} + +func TestErrorHandler_EC2Service(t *testing.T) { + gw := &GatewayConfig{DisableLogging: true} + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), ctxService, "ec2") + r = r.WithContext(ctx) + gw.ErrorHandler(w, r, errors.New(awserrors.ErrorInvalidParameterValue)) + }) + + req := httptest.NewRequest("POST", "/", nil) + resp := doRequest(handler, req) + assert.Equal(t, 400, resp.StatusCode) + + body, _ := io.ReadAll(resp.Body) + xmlStr := string(body) + // EC2 uses root, not + assert.Contains(t, xmlStr, "") + assert.Contains(t, xmlStr, "") + assert.Contains(t, xmlStr, "InvalidParameterValue") + assert.NotContains(t, xmlStr, "") +} + +func TestErrorHandler_UsesRequestIDHeader(t *testing.T) { + gw := &GatewayConfig{DisableLogging: true} + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), ctxService, "ec2") + r = r.WithContext(ctx) + gw.ErrorHandler(w, r, errors.New(awserrors.ErrorInternalError)) + }) + + req := httptest.NewRequest("POST", "/", nil) + req.Header.Set("x-amz-request-id", "custom-req-id-123") + resp := doRequest(handler, req) + + body, _ := io.ReadAll(resp.Body) + assert.Contains(t, string(body), "custom-req-id-123") +} + +func TestErrorHandler_ContentTypeXML(t *testing.T) { + gw := &GatewayConfig{DisableLogging: true} + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), ctxService, "ec2") + r = r.WithContext(ctx) + gw.ErrorHandler(w, r, errors.New(awserrors.ErrorInternalError)) + }) + + req := httptest.NewRequest("POST", "/", nil) + resp := doRequest(handler, req) + assert.Equal(t, "application/xml", resp.Header.Get("Content-Type")) +} + func startTestNATS(t *testing.T) *nats.Conn { t.Helper() ns, err := server.NewServer(&server.Options{ diff --git a/hive/handlers/ec2/instance/service_impl.go b/hive/handlers/ec2/instance/service_impl.go index 2638a6461..c97c24f28 100644 --- a/hive/handlers/ec2/instance/service_impl.go +++ b/hive/handlers/ec2/instance/service_impl.go @@ -99,6 +99,57 @@ type VolumeInfo struct { DeleteOnTermination bool } +// volumeParams holds parsed block device mapping parameters for volume creation. +type volumeParams struct { + size int + deviceName string + volumeType string + iops int + imageId string + snapshotId string + deleteOnTermination bool +} + +// parseVolumeParams extracts volume parameters from RunInstancesInput, +// applying defaults and resolving AMI-based image IDs. +func parseVolumeParams(input *ec2.RunInstancesInput) volumeParams { + p := volumeParams{ + size: 4 * 1024 * 1024 * 1024, // 4GB default + deviceName: "/dev/vda", + deleteOnTermination: true, // matches AWS RunInstances behavior + } + + if len(input.BlockDeviceMappings) > 0 { + bdm := input.BlockDeviceMappings[0] + if bdm.DeviceName != nil { + p.deviceName = *bdm.DeviceName + } + if bdm.Ebs != nil { + if bdm.Ebs.VolumeSize != nil { + p.size = int(*bdm.Ebs.VolumeSize) * 1024 * 1024 * 1024 + } + if bdm.Ebs.VolumeType != nil { + p.volumeType = *bdm.Ebs.VolumeType + } + if bdm.Ebs.Iops != nil { + p.iops = int(*bdm.Ebs.Iops) + } + if bdm.Ebs.DeleteOnTermination != nil { + p.deleteOnTermination = *bdm.Ebs.DeleteOnTermination + } + } + } + + if strings.HasPrefix(*input.ImageId, "ami-") { + p.imageId = utils.GenerateResourceID("vol") + p.snapshotId = *input.ImageId + } else { + p.imageId = *input.ImageId + } + + return p +} + // InstanceServiceImpl handles daemon-side EC2 instance operations type InstanceServiceImpl struct { config *config.Config @@ -161,61 +212,29 @@ func (s *InstanceServiceImpl) RunInstance(input *ec2.RunInstancesInput) (*vm.VM, } func (s *InstanceServiceImpl) GenerateVolumes(input *ec2.RunInstancesInput, instance *vm.VM) ([]VolumeInfo, error) { - - var size int = 4 * 1024 * 1024 * 1024 // 4GB default size - var deviceName = "/dev/vda" // Default device name (virtio-blk-pci) - var volumeType string - var iops int - var imageId string - var snapshotId string - var deleteOnTermination = true // Default to true (matches AWS RunInstances behavior) - - // Handle block device mappings - if len(input.BlockDeviceMappings) > 0 { - bdm := input.BlockDeviceMappings[0] - if bdm.DeviceName != nil { - deviceName = *bdm.DeviceName - } - if bdm.Ebs != nil { - if bdm.Ebs.VolumeSize != nil { - size = int(*bdm.Ebs.VolumeSize) * 1024 * 1024 * 1024 // AWS API sends GiB, convert to bytes - } - if bdm.Ebs.VolumeType != nil { - volumeType = *bdm.Ebs.VolumeType - } - if bdm.Ebs.Iops != nil { - iops = int(*bdm.Ebs.Iops) - } - if bdm.Ebs.DeleteOnTermination != nil { - deleteOnTermination = *bdm.Ebs.DeleteOnTermination - } - } - } - - // Determine image ID and snapshot ID - if strings.HasPrefix(*input.ImageId, "ami-") { - imageId = utils.GenerateResourceID("vol") - snapshotId = *input.ImageId - } else { - imageId = *input.ImageId - } + p := parseVolumeParams(input) // Capture attach time for the root volume attachTime := time.Now() volumeConfig := viperblock.VolumeConfig{ VolumeMetadata: viperblock.VolumeMetadata{ - VolumeID: imageId, - SizeGiB: utils.SafeIntToUint64(size / 1024 / 1024 / 1024), + VolumeID: p.imageId, + SizeGiB: utils.SafeIntToUint64(p.size / 1024 / 1024 / 1024), CreatedAt: attachTime, - DeviceName: deviceName, - VolumeType: volumeType, - IOPS: iops, - SnapshotID: snapshotId, - DeleteOnTermination: deleteOnTermination, + DeviceName: p.deviceName, + VolumeType: p.volumeType, + IOPS: p.iops, + SnapshotID: p.snapshotId, + DeleteOnTermination: p.deleteOnTermination, }, } + size := p.size + imageId := p.imageId + deviceName := p.deviceName + deleteOnTermination := p.deleteOnTermination + // Step 1: Create or validate root volume err := s.prepareRootVolume(input, imageId, size, volumeConfig, instance, deleteOnTermination) if err != nil { diff --git a/hive/handlers/ec2/instance/service_impl_test.go b/hive/handlers/ec2/instance/service_impl_test.go index 05b790c87..74e64c9f7 100644 --- a/hive/handlers/ec2/instance/service_impl_test.go +++ b/hive/handlers/ec2/instance/service_impl_test.go @@ -276,6 +276,98 @@ func TestRunInstance_NoImageId(t *testing.T) { assert.Nil(t, ec2Instance.KeyName) } +func TestParseVolumeParams_Defaults(t *testing.T) { + input := &ec2.RunInstancesInput{ + ImageId: aws.String("ami-0abcdef1234567890"), + } + + p := parseVolumeParams(input) + + assert.Equal(t, 4*1024*1024*1024, p.size, "default size should be 4GB") + assert.Equal(t, "/dev/vda", p.deviceName, "default device should be /dev/vda") + assert.True(t, p.deleteOnTermination, "deleteOnTermination should default to true") + assert.Empty(t, p.volumeType) + assert.Zero(t, p.iops) + // AMI-based: imageId should be a generated vol-*, snapshotId should be the AMI + assert.True(t, strings.HasPrefix(p.imageId, "vol-"), "AMI launch should generate vol- ID") + assert.Equal(t, "ami-0abcdef1234567890", p.snapshotId) +} + +func TestParseVolumeParams_CustomBlockDeviceMapping(t *testing.T) { + input := &ec2.RunInstancesInput{ + ImageId: aws.String("ami-test123"), + BlockDeviceMappings: []*ec2.BlockDeviceMapping{ + { + DeviceName: aws.String("/dev/sda1"), + Ebs: &ec2.EbsBlockDevice{ + VolumeSize: aws.Int64(20), + VolumeType: aws.String("gp3"), + Iops: aws.Int64(3000), + DeleteOnTermination: aws.Bool(false), + }, + }, + }, + } + + p := parseVolumeParams(input) + + assert.Equal(t, 20*1024*1024*1024, p.size, "size should be 20 GiB in bytes") + assert.Equal(t, "/dev/sda1", p.deviceName) + assert.Equal(t, "gp3", p.volumeType) + assert.Equal(t, 3000, p.iops) + assert.False(t, p.deleteOnTermination) +} + +func TestParseVolumeParams_BlockDeviceMappingNoEbs(t *testing.T) { + input := &ec2.RunInstancesInput{ + ImageId: aws.String("ami-test"), + BlockDeviceMappings: []*ec2.BlockDeviceMapping{ + { + DeviceName: aws.String("/dev/xvda"), + }, + }, + } + + p := parseVolumeParams(input) + + assert.Equal(t, "/dev/xvda", p.deviceName) + assert.Equal(t, 4*1024*1024*1024, p.size, "size should stay at default without Ebs") + assert.True(t, p.deleteOnTermination) +} + +func TestParseVolumeParams_NonAMIImageId(t *testing.T) { + rawImageId := "vol-existing-volume-id" + input := &ec2.RunInstancesInput{ + ImageId: aws.String(rawImageId), + } + + p := parseVolumeParams(input) + + assert.Equal(t, rawImageId, p.imageId, "non-AMI imageId should be used directly") + assert.Empty(t, p.snapshotId, "non-AMI launch should have no snapshotId") +} + +func TestParseVolumeParams_PartialEbs(t *testing.T) { + input := &ec2.RunInstancesInput{ + ImageId: aws.String("ami-partial"), + BlockDeviceMappings: []*ec2.BlockDeviceMapping{ + { + Ebs: &ec2.EbsBlockDevice{ + VolumeSize: aws.Int64(8), + }, + }, + }, + } + + p := parseVolumeParams(input) + + assert.Equal(t, 8*1024*1024*1024, p.size) + assert.Equal(t, "/dev/vda", p.deviceName, "device should stay at default") + assert.Empty(t, p.volumeType, "volumeType should stay empty") + assert.Zero(t, p.iops, "iops should stay zero") + assert.True(t, p.deleteOnTermination, "deleteOnTermination should stay at default") +} + func TestMockInstanceService(t *testing.T) { svc := NewMockInstanceService() require.NotNil(t, svc) diff --git a/hive/handlers/ec2/key/service_impl_test.go b/hive/handlers/ec2/key/service_impl_test.go index a483a8ba7..70fd244a7 100644 --- a/hive/handlers/ec2/key/service_impl_test.go +++ b/hive/handlers/ec2/key/service_impl_test.go @@ -531,3 +531,82 @@ func TestValidateKeyPairExists_NotFound(t *testing.T) { require.Error(t, err) assert.Equal(t, awserrors.ErrorInvalidKeyPairNotFound, err.Error()) } + +// ============================================================ +// formatFingerprint Tests +// ============================================================ + +func TestFormatFingerprint_MD5(t *testing.T) { + hash := []byte{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99} + result := formatFingerprint(hash, "MD5") + assert.Equal(t, "aabbccddeeff00112233445566778899", result) + // Should be lowercase hex, no colons + assert.False(t, strings.Contains(result, ":")) + assert.Equal(t, strings.ToLower(result), result) +} + +func TestFormatFingerprint_SHA256(t *testing.T) { + hash := []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08} + result := formatFingerprint(hash, "SHA256") + assert.True(t, strings.HasPrefix(result, "SHA256:")) + // Should be raw base64 (no padding) + assert.False(t, strings.HasSuffix(result, "=")) +} + +func TestFormatFingerprint_EmptyHash(t *testing.T) { + result := formatFingerprint([]byte{}, "MD5") + assert.Equal(t, "", result) + + result = formatFingerprint([]byte{}, "SHA256") + assert.Equal(t, "SHA256:", result) +} + +// ============================================================ +// calculateFingerprint Tests +// ============================================================ + +func TestCalculateFingerprint_ED25519(t *testing.T) { + svc, _ := newTestKeyService() + + fp, err := svc.calculateFingerprint([]byte(testED25519PubKey), "ed25519") + require.NoError(t, err) + assert.True(t, strings.HasPrefix(fp, "SHA256:"), "ed25519 should use SHA256 fingerprint") + assert.NotEqual(t, "SHA256:", fp, "fingerprint should have content after prefix") +} + +func TestCalculateFingerprint_RSA(t *testing.T) { + svc, _ := newTestKeyService() + + fp, err := svc.calculateFingerprint([]byte(testRSAPubKey), "rsa") + require.NoError(t, err) + assert.False(t, strings.HasPrefix(fp, "SHA256:"), "RSA should use MD5 fingerprint") + assert.NotEmpty(t, fp) + // MD5 of RSA key data = 32 hex chars + assert.Len(t, fp, 32) +} + +func TestCalculateFingerprint_Deterministic(t *testing.T) { + svc, _ := newTestKeyService() + + fp1, err := svc.calculateFingerprint([]byte(testED25519PubKey), "ed25519") + require.NoError(t, err) + fp2, err := svc.calculateFingerprint([]byte(testED25519PubKey), "ed25519") + require.NoError(t, err) + assert.Equal(t, fp1, fp2, "same key should produce same fingerprint") +} + +func TestCalculateFingerprint_InvalidFormat(t *testing.T) { + svc, _ := newTestKeyService() + + _, err := svc.calculateFingerprint([]byte("no-space-here"), "ed25519") + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid public key format") +} + +func TestCalculateFingerprint_InvalidBase64(t *testing.T) { + svc, _ := newTestKeyService() + + _, err := svc.calculateFingerprint([]byte("ssh-ed25519 !!!not-base64!!!"), "ed25519") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to decode public key") +} diff --git a/hive/handlers/iam/service_impl_test.go b/hive/handlers/iam/service_impl_test.go index 57494f6e8..9392183ef 100644 --- a/hive/handlers/iam/service_impl_test.go +++ b/hive/handlers/iam/service_impl_test.go @@ -1654,3 +1654,147 @@ func TestGeneratePolicyID(t *testing.T) { id2 := generateIAMID("ANPA") assert.NotEqual(t, id, id2) } + +// ============================================================================ +// Validator Tests +// ============================================================================ + +func TestIsIAMNameChar(t *testing.T) { + // Valid characters + for _, c := range "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+=,.@-_" { + assert.True(t, isIAMNameChar(byte(c)), "expected valid: %c", c) + } + // Invalid characters + for _, c := range " !#$%^&*(){}[]|\\:;\"'<>?/`~\t\n" { + assert.False(t, isIAMNameChar(byte(c)), "expected invalid: %c", c) + } +} + +func TestValidateUserName(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + {"valid simple", "alice", false}, + {"valid with special chars", "alice.bob+test@example_com", false}, + {"valid single char", "a", false}, + {"valid 64 chars", strings.Repeat("a", 64), false}, + {"empty", "", true}, + {"too long 65 chars", strings.Repeat("a", 65), true}, + {"invalid space", "alice bob", true}, + {"invalid slash", "alice/bob", true}, + {"invalid colon", "alice:bob", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateUserName(tt.input) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidatePolicyName(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + {"valid simple", "MyPolicy", false}, + {"valid with special chars", "My.Policy-v2_test+1", false}, + {"valid single char", "P", false}, + {"valid 128 chars", strings.Repeat("x", 128), false}, + {"empty", "", true}, + {"too long 129 chars", strings.Repeat("x", 129), true}, + {"invalid space", "My Policy", true}, + {"invalid slash", "My/Policy", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePolicyName(tt.input) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidatePath(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + {"root path", "/", false}, + {"nested path", "/division/engineering/", false}, + {"no leading slash", "division/", true}, + {"no trailing slash", "/division", true}, + {"empty string", "", true}, + {"just text", "division", true}, + {"max length 512", "/" + strings.Repeat("a", 510) + "/", false}, + {"over max length 513", "/" + strings.Repeat("a", 511) + "/", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePath(tt.input) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestParseCreatedAt(t *testing.T) { + // Valid RFC3339 + ts := "2024-01-15T10:30:00Z" + result := parseCreatedAt(ts) + assert.Equal(t, 2024, result.Year()) + assert.Equal(t, time.January, result.Month()) + assert.Equal(t, 15, result.Day()) + assert.Equal(t, 10, result.Hour()) + assert.Equal(t, 30, result.Minute()) + + // With timezone offset + ts2 := "2024-06-01T12:00:00+05:00" + result2 := parseCreatedAt(ts2) + assert.Equal(t, 2024, result2.Year()) + assert.Equal(t, time.June, result2.Month()) + + // Invalid — returns zero time (fallback) + result3 := parseCreatedAt("not-a-date") + assert.True(t, result3.IsZero()) + + // Empty string — returns zero time + result4 := parseCreatedAt("") + assert.True(t, result4.IsZero()) +} + +func TestGenerateIAMID_AllUpperHex(t *testing.T) { + for range 20 { + id := generateIAMID("AIDA") + suffix := id[4:] + for _, c := range suffix { + assert.True(t, (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F'), + "expected uppercase hex char, got %c in ID %s", c, id) + } + } +} + +func TestGenerateAccessKeyID_AllUpperHex(t *testing.T) { + for range 20 { + id := generateAccessKeyID() + suffix := id[4:] + for _, c := range suffix { + assert.True(t, (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F'), + "expected uppercase hex char, got %c in ID %s", c, id) + } + } +} diff --git a/hive/nbd/nbd.go b/hive/nbd/nbd.go index 61b6797b4..56382021f 100644 --- a/hive/nbd/nbd.go +++ b/hive/nbd/nbd.go @@ -27,14 +27,8 @@ type NBDKitConfig struct { UseTCP bool `json:"use_tcp"` // If true, use TCP transport; otherwise use Unix socket } -func (cfg *NBDKitConfig) Execute() (*exec.Cmd, error) { - // TODO: Consider setting threads to X% of the host availability (preference for VMs) - /* - --threads=THREADS - Set the number of threads to be used per connection, which in turn controls the number of outstanding requests that can - be processed at once. Only matters for plugins with thread_model=parallel (where it defaults to 16). To force - serialized behavior (useful if the client is not prepared for out-of-order responses), set this to 1. - */ +// buildArgs constructs the nbdkit command-line arguments from the config. +func (cfg *NBDKitConfig) buildArgs() ([]string, error) { args := []string{ "-f", // foreground required for Golang plugin via nbdkit "--pidfile", cfg.PidFile, @@ -73,6 +67,21 @@ func (cfg *NBDKitConfig) Execute() (*exec.Cmd, error) { } args = append(args, pluginArgs...) + return args, nil +} + +func (cfg *NBDKitConfig) Execute() (*exec.Cmd, error) { + // TODO: Consider setting threads to X% of the host availability (preference for VMs) + /* + --threads=THREADS + Set the number of threads to be used per connection, which in turn controls the number of outstanding requests that can + be processed at once. Only matters for plugins with thread_model=parallel (where it defaults to 16). To force + serialized behavior (useful if the client is not prepared for out-of-order responses), set this to 1. + */ + args, err := cfg.buildArgs() + if err != nil { + return nil, err + } cmd := exec.Command("nbdkit", args...) cmd.Stdout = os.Stdout diff --git a/hive/nbd/nbd_test.go b/hive/nbd/nbd_test.go new file mode 100644 index 000000000..e34c2fb87 --- /dev/null +++ b/hive/nbd/nbd_test.go @@ -0,0 +1,284 @@ +package nbd + +import ( + "strconv" + "testing" +) + +func TestBuildArgs_TCPTransport(t *testing.T) { + cfg := &NBDKitConfig{ + Port: 10809, + PidFile: "/tmp/nbd.pid", + PluginPath: "/usr/lib/nbdkit/plugins/vb.so", + UseTCP: true, + Size: 1073741824, + Volume: "vol-abc123", + Bucket: "my-bucket", + Region: "us-east-1", + AccessKey: "AKIA123", + SecretKey: "secret", + BaseDir: "/data", + Host: "localhost:9000", + CacheSize: 256, + ShardWAL: true, + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := []string{ + "-f", + "--pidfile", "/tmp/nbd.pid", + "-p", "10809", + "/usr/lib/nbdkit/plugins/vb.so", + "size=1073741824", + "volume=vol-abc123", + "bucket=my-bucket", + "region=us-east-1", + "access_key=AKIA123", + "secret_key=secret", + "base_dir=/data", + "host=localhost:9000", + "cache_size=256", + "shardwal=true", + } + + assertArgs(t, expected, args) +} + +func TestBuildArgs_UnixSocketTransport(t *testing.T) { + cfg := &NBDKitConfig{ + Socket: "/tmp/nbd.sock", + PidFile: "/tmp/nbd.pid", + PluginPath: "/usr/lib/nbdkit/plugins/vb.so", + UseTCP: false, + Size: 536870912, + Volume: "vol-def456", + Bucket: "bucket-2", + Region: "eu-west-1", + AccessKey: "AKIA456", + SecretKey: "topsecret", + BaseDir: "/mnt/data", + Host: "10.0.0.1:9000", + CacheSize: 128, + ShardWAL: false, + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := []string{ + "-f", + "--pidfile", "/tmp/nbd.pid", + "--unix", "/tmp/nbd.sock", + "/usr/lib/nbdkit/plugins/vb.so", + "size=536870912", + "volume=vol-def456", + "bucket=bucket-2", + "region=eu-west-1", + "access_key=AKIA456", + "secret_key=topsecret", + "base_dir=/mnt/data", + "host=10.0.0.1:9000", + "cache_size=128", + "shardwal=false", + } + + assertArgs(t, expected, args) +} + +func TestBuildArgs_SocketTransport_MissingSocket(t *testing.T) { + cfg := &NBDKitConfig{ + PidFile: "/tmp/nbd.pid", + PluginPath: "/usr/lib/nbdkit/plugins/vb.so", + UseTCP: false, + Socket: "", // empty socket path + } + + _, err := cfg.buildArgs() + if err == nil { + t.Fatal("expected error for missing socket path, got nil") + } + + want := "socket path is required when not using TCP transport" + if err.Error() != want { + t.Errorf("error = %q, want %q", err.Error(), want) + } +} + +func TestBuildArgs_Verbose(t *testing.T) { + cfg := &NBDKitConfig{ + Socket: "/tmp/nbd.sock", + PidFile: "/tmp/nbd.pid", + PluginPath: "/usr/lib/nbdkit/plugins/vb.so", + Verbose: true, + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // -v should appear right after plugin path + pluginIdx := indexOf(args, cfg.PluginPath) + if pluginIdx < 0 { + t.Fatal("plugin path not found in args") + } + if pluginIdx+1 >= len(args) || args[pluginIdx+1] != "-v" { + t.Errorf("expected -v after plugin path, got args: %v", args) + } +} + +func TestBuildArgs_NotVerbose(t *testing.T) { + cfg := &NBDKitConfig{ + Socket: "/tmp/nbd.sock", + PidFile: "/tmp/nbd.pid", + PluginPath: "/usr/lib/nbdkit/plugins/vb.so", + Verbose: false, + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, arg := range args { + if arg == "-v" { + t.Error("expected -v to be absent when Verbose=false") + } + } +} + +func TestBuildArgs_PluginArgValues(t *testing.T) { + cfg := &NBDKitConfig{ + Socket: "/tmp/nbd.sock", + PidFile: "/tmp/nbd.pid", + PluginPath: "/plugin.so", + Size: 0, + Volume: "", + CacheSize: 0, + ShardWAL: false, + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertContains(t, args, "size=0") + assertContains(t, args, "volume=") + assertContains(t, args, "cache_size=0") + assertContains(t, args, "shardwal=false") +} + +func TestBuildArgs_TCPPortValue(t *testing.T) { + tests := []struct { + port int + want string + }{ + {10809, "10809"}, + {0, "0"}, + {65535, "65535"}, + } + + for _, tt := range tests { + t.Run("port_"+tt.want, func(t *testing.T) { + cfg := &NBDKitConfig{ + UseTCP: true, + Port: tt.port, + PidFile: "/tmp/nbd.pid", + PluginPath: "/plugin.so", + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + pIdx := indexOf(args, "-p") + if pIdx < 0 || pIdx+1 >= len(args) { + t.Fatal("-p flag not found in args") + } + if args[pIdx+1] != strconv.Itoa(tt.port) { + t.Errorf("port = %q, want %q", args[pIdx+1], tt.want) + } + }) + } +} + +func TestBuildArgs_ArgOrdering(t *testing.T) { + cfg := &NBDKitConfig{ + Socket: "/tmp/nbd.sock", + PidFile: "/tmp/nbd.pid", + PluginPath: "/plugin.so", + Verbose: true, + Volume: "vol-test", + } + + args, err := cfg.buildArgs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // -f must be first + if args[0] != "-f" { + t.Errorf("first arg = %q, want -f", args[0]) + } + + // --pidfile before transport args + pidIdx := indexOf(args, "--pidfile") + unixIdx := indexOf(args, "--unix") + pluginIdx := indexOf(args, cfg.PluginPath) + verboseIdx := indexOf(args, "-v") + volumeIdx := indexOf(args, "volume=vol-test") + + if pidIdx < 0 || unixIdx < 0 || pluginIdx < 0 || verboseIdx < 0 || volumeIdx < 0 { + t.Fatalf("missing expected args in: %v", args) + } + + // Order: -f, --pidfile, transport, plugin, -v, plugin-args + if pidIdx >= unixIdx { + t.Error("--pidfile should come before --unix") + } + if unixIdx >= pluginIdx { + t.Error("--unix should come before plugin path") + } + if pluginIdx >= verboseIdx { + t.Error("plugin path should come before -v") + } + if verboseIdx >= volumeIdx { + t.Error("-v should come before plugin args") + } +} + +func assertArgs(t *testing.T, expected, got []string) { + t.Helper() + if len(expected) != len(got) { + t.Fatalf("args length = %d, want %d\ngot: %v\nwant: %v", len(got), len(expected), got, expected) + } + for i := range expected { + if expected[i] != got[i] { + t.Errorf("args[%d] = %q, want %q", i, got[i], expected[i]) + } + } +} + +func assertContains(t *testing.T, args []string, val string) { + t.Helper() + if indexOf(args, val) < 0 { + t.Errorf("expected args to contain %q, got: %v", val, args) + } +} + +func indexOf(args []string, val string) int { + for i, a := range args { + if a == val { + return i + } + } + return -1 +} diff --git a/hive/services/vpcd/nbdb/model_test.go b/hive/services/vpcd/nbdb/model_test.go new file mode 100644 index 000000000..62410e99a --- /dev/null +++ b/hive/services/vpcd/nbdb/model_test.go @@ -0,0 +1,38 @@ +package nbdb + +import ( + "testing" +) + +func TestFullDatabaseModel(t *testing.T) { + dbModel, err := FullDatabaseModel() + if err != nil { + t.Fatalf("FullDatabaseModel() returned error: %v", err) + } + + if dbModel.Name() != "OVN_Northbound" { + t.Errorf("database name = %q, want %q", dbModel.Name(), "OVN_Northbound") + } + + expectedTables := []string{ + "Logical_Switch", + "Logical_Switch_Port", + "Logical_Router", + "Logical_Router_Port", + "DHCP_Options", + "NAT", + "Logical_Router_Static_Route", + "Gateway_Chassis", + } + + types := dbModel.Types() + for _, table := range expectedTables { + if _, ok := types[table]; !ok { + t.Errorf("missing table %q in database model", table) + } + } + + if len(types) != len(expectedTables) { + t.Errorf("table count = %d, want %d", len(types), len(expectedTables)) + } +} diff --git a/scripts/check-coverage.sh b/scripts/check-coverage.sh index cee01ca66..566df0a84 100755 --- a/scripts/check-coverage.sh +++ b/scripts/check-coverage.sh @@ -2,11 +2,11 @@ set -euo pipefail # Check total coverage meets minimum threshold. -# Usage: scripts/check-coverage.sh [quiet] +# Usage: scripts/check-coverage.sh [quiet] -PROFILE="${1:?Usage: check-coverage.sh [quiet]}" -MIN="${2:-60.0}" -QUIET="${3:-}" +PROFILE="${1:?Usage: check-coverage.sh [quiet]}" +MIN=65.0 +QUIET="${2:-}" if [[ ! -s "$PROFILE" ]]; then echo "ERROR: No coverage data generated — tests may have failed to compile" diff --git a/scripts/diff-coverage.sh b/scripts/diff-coverage.sh index dd6ea2fc2..aa280b5d1 100755 --- a/scripts/diff-coverage.sh +++ b/scripts/diff-coverage.sh @@ -13,7 +13,7 @@ set -euo pipefail PROFILE="" BASE_REF="" -THRESHOLD=50 +THRESHOLD=65 while [[ $# -gt 0 ]]; do case "$1" in