diff --git a/.github/workflows/hive.yml b/.github/workflows/hive.yml index 26f59397f..e5aa1998c 100644 --- a/.github/workflows/hive.yml +++ b/.github/workflows/hive.yml @@ -29,14 +29,12 @@ jobs: uses: actions/checkout@v6 with: repository: mulgadc/viperblock - ref: ${{ github.ref_name }} path: viperblock - name: Checkout Predastore uses: actions/checkout@v6 with: repository: mulgadc/predastore - ref: ${{ github.ref_name }} path: predastore - name: Setup Go Workspace @@ -71,14 +69,12 @@ jobs: uses: actions/checkout@v6 with: repository: mulgadc/viperblock - ref: ${{ github.ref_name }} path: viperblock - name: Checkout Predastore uses: actions/checkout@v6 with: repository: mulgadc/predastore - ref: ${{ github.ref_name }} path: predastore - name: Setup Go Workspace @@ -107,14 +103,12 @@ jobs: uses: actions/checkout@v6 with: repository: mulgadc/viperblock - ref: ${{ github.ref_name }} path: viperblock - name: Checkout Predastore uses: actions/checkout@v6 with: repository: mulgadc/predastore - ref: ${{ github.ref_name }} path: predastore - name: Setup Go Workspace diff --git a/hive/handlers/ec2/key/service_impl_test.go b/hive/handlers/ec2/key/service_impl_test.go index 998f00a3b..17f29c1cf 100644 --- a/hive/handlers/ec2/key/service_impl_test.go +++ b/hive/handlers/ec2/key/service_impl_test.go @@ -2,401 +2,381 @@ package handlers_ec2_key import ( "encoding/json" + "os/exec" "strings" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/mulgadc/hive/hive/awserrors" "github.com/mulgadc/hive/hive/objectstore" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// TestKeyPairMetadataMarshaling tests that CreateKeyPairOutput (used as metadata) can be marshaled/unmarshaled correctly +const ( + testBucket = "test-bucket" + testAccountID = "123456789" +) + +// Valid ed25519 public key for import tests +const testED25519PubKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl" + +// Valid RSA public key for import tests (generated locally, 2048-bit) +const testRSAPubKey = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDP9LrByKWpgbX+prxBwnlf7lrmI5AfDwuiCofuvOAzt9/PwIDMMIAhlvlpm09jjOuuH/MRQApJB5A714Auv+hBKVK0lCq9KcTrnKZOpRj2aGgIZgaoO6P/POoZc+kBf9Y/GP18DCKc4y/XyBsp69dPP6XRdYBlEdeIeVZdgCPYrM7s5FjT7aML2ba2Y2EvH116hPxv+nJZGwM6yqWxWRyTOoNMMTAfNYmoKkNy2zC1FARUyqDwumJ2z5Fvo4ZdN1qoFPOsfPc3iB0NUtSZbLU1awScvHb0rwR5PRnelTZ3Nbkw8I8A0IAhBTE5veW9D38hDIJhRd4nW73BUhgmzDL7" + +func newTestKeyService() (*KeyServiceImpl, *objectstore.MemoryObjectStore) { + store := objectstore.NewMemoryObjectStore() + svc := NewKeyServiceImplWithStore(store, testBucket, testAccountID) + return svc, store +} + +// requireSSHKeygen skips the test if ssh-keygen is not available. +func requireSSHKeygen(t *testing.T) { + t.Helper() + if _, err := exec.LookPath("ssh-keygen"); err != nil { + t.Skip("ssh-keygen not available, skipping") + } +} + +// importTestKey is a helper that imports an ed25519 key and returns the output. +func importTestKey(t *testing.T, svc *KeyServiceImpl, keyName string) *ec2.ImportKeyPairOutput { + t.Helper() + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String(keyName), + PublicKeyMaterial: []byte(testED25519PubKey), + }) + require.NoError(t, err) + require.NotNil(t, out) + return out +} + +// --- TestKeyPairMetadataMarshaling (kept — validates JSON roundtrip independently) --- + func TestKeyPairMetadataMarshaling(t *testing.T) { metadata := ec2.CreateKeyPairOutput{ KeyPairId: aws.String("key-12345"), KeyFingerprint: aws.String("SHA256:abcdef1234567890"), KeyName: aws.String("test-key"), - // Note: KeyMaterial is not stored in metadata (only returned once during creation) } - // Marshal to JSON data, err := json.Marshal(metadata) - if err != nil { - t.Fatalf("Failed to marshal CreateKeyPairOutput: %v", err) - } + require.NoError(t, err) - // Unmarshal from JSON var unmarshaled ec2.CreateKeyPairOutput err = json.Unmarshal(data, &unmarshaled) - if err != nil { - t.Fatalf("Failed to unmarshal CreateKeyPairOutput: %v", err) - } + require.NoError(t, err) - // Verify fields match - if *unmarshaled.KeyPairId != *metadata.KeyPairId { - t.Errorf("KeyPairId mismatch: got %s, want %s", *unmarshaled.KeyPairId, *metadata.KeyPairId) - } - if *unmarshaled.KeyFingerprint != *metadata.KeyFingerprint { - t.Errorf("KeyFingerprint mismatch: got %s, want %s", *unmarshaled.KeyFingerprint, *metadata.KeyFingerprint) - } - if *unmarshaled.KeyName != *metadata.KeyName { - t.Errorf("KeyName mismatch: got %s, want %s", *unmarshaled.KeyName, *metadata.KeyName) - } + assert.Equal(t, *metadata.KeyPairId, *unmarshaled.KeyPairId) + assert.Equal(t, *metadata.KeyFingerprint, *unmarshaled.KeyFingerprint) + assert.Equal(t, *metadata.KeyName, *unmarshaled.KeyName) } -// TestDetermineKeyTypeFromFingerprint tests the key type detection logic -func TestDetermineKeyTypeFromFingerprint(t *testing.T) { - tests := []struct { - name string - fingerprint string - expected string - }{ - { - name: "ED25519 key (SHA256 prefix)", - fingerprint: "SHA256:abcdef1234567890", - expected: "ed25519", - }, - { - name: "RSA key (hex fingerprint)", - fingerprint: "ab:cd:ef:12:34:56:78:90", - expected: "rsa", - }, - { - name: "RSA key (uppercase hex)", - fingerprint: "AB:CD:EF:12:34:56:78:90", - expected: "rsa", - }, - { - name: "Empty fingerprint", - fingerprint: "", - expected: "rsa", // default fallback - }, - { - name: "RSA key (MD5 format)", - fingerprint: "1a2b3c4d5e6f7890abcdef1234567890", - expected: "rsa", - }, - } +// ============================================================ +// CreateKeyPair Tests +// ============================================================ - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Simulate the exact logic from DescribeKeyPairs (line 537-541) - var keyType string - if strings.HasPrefix(tt.fingerprint, "SHA256:") { - keyType = "ed25519" - } else { - keyType = "rsa" - } +func TestCreateKeyPair_ED25519(t *testing.T) { + requireSSHKeygen(t) + svc, store := newTestKeyService() - if keyType != tt.expected { - t.Errorf("Expected key type %s, got %s for fingerprint %s", tt.expected, keyType, tt.fingerprint) - } - }) - } + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("my-ed25519-key"), + }) + require.NoError(t, err) + require.NotNil(t, out) + + assert.Equal(t, "my-ed25519-key", *out.KeyName) + assert.NotEmpty(t, *out.KeyPairId) + assert.True(t, strings.HasPrefix(*out.KeyPairId, "key-")) + assert.NotEmpty(t, *out.KeyFingerprint) + assert.True(t, strings.HasPrefix(*out.KeyFingerprint, "SHA256:"), "ed25519 fingerprint should be SHA256 format") + assert.NotEmpty(t, *out.KeyMaterial) + assert.Contains(t, *out.KeyMaterial, "PRIVATE KEY") + + // Verify public key stored in S3 + keyPath := "keys/" + testAccountID + "/my-ed25519-key" + getOut, err := store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(keyPath), + }) + require.NoError(t, err) + assert.NotNil(t, getOut) + + // Verify metadata stored in S3 + metaPath := "keys/" + testAccountID + "/" + *out.KeyPairId + ".json" + metaOut, err := store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(metaPath), + }) + require.NoError(t, err) + assert.NotNil(t, metaOut) } -// TestDescribeKeyPairsInputValidation tests input validation for DescribeKeyPairs -func TestDescribeKeyPairsInputValidation(t *testing.T) { - tests := []struct { - name string - input *ec2.DescribeKeyPairsInput - expectErr bool - }{ - { - name: "Nil input", - input: nil, - expectErr: false, // DescribeKeyPairs accepts nil input (all optional) - }, - { - name: "Empty input", - input: &ec2.DescribeKeyPairsInput{}, - expectErr: false, - }, - { - name: "With KeyNames filter", - input: &ec2.DescribeKeyPairsInput{ - KeyNames: []*string{aws.String("test-key")}, - }, - expectErr: false, - }, - { - name: "With KeyPairIds filter", - input: &ec2.DescribeKeyPairsInput{ - KeyPairIds: []*string{aws.String("key-12345")}, - }, - expectErr: false, - }, - { - name: "With both filters", - input: &ec2.DescribeKeyPairsInput{ - KeyNames: []*string{aws.String("test-key")}, - KeyPairIds: []*string{aws.String("key-12345")}, - }, - expectErr: false, - }, - } +func TestCreateKeyPair_RSA(t *testing.T) { + requireSSHKeygen(t) + svc, _ := newTestKeyService() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Since DescribeKeyPairs has no required parameters, all inputs should be valid - // This test documents the expected behavior - if tt.input == nil && tt.expectErr { - t.Error("Unexpected error expectation for nil input") - } - }) - } + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("my-rsa-key"), + KeyType: aws.String("rsa"), + }) + require.NoError(t, err) + require.NotNil(t, out) + + assert.Equal(t, "my-rsa-key", *out.KeyName) + assert.NotEmpty(t, *out.KeyFingerprint) + // RSA fingerprint is MD5 hex (no "SHA256:" prefix, no colons in our format) + assert.False(t, strings.HasPrefix(*out.KeyFingerprint, "SHA256:"), "RSA fingerprint should not be SHA256 format") } -// TestKeyPairFiltering tests the filtering logic for DescribeKeyPairs -func TestKeyPairFiltering(t *testing.T) { - // Create sample key pairs - keyPairs := []*ec2.KeyPairInfo{ - { - KeyPairId: aws.String("key-11111"), - KeyName: aws.String("test-key-1"), - KeyFingerprint: aws.String("SHA256:abc123"), - }, - { - KeyPairId: aws.String("key-22222"), - KeyName: aws.String("test-key-2"), - KeyFingerprint: aws.String("ab:cd:ef:12"), - }, - { - KeyPairId: aws.String("key-33333"), - KeyName: aws.String("prod-key-1"), - KeyFingerprint: aws.String("SHA256:def456"), - }, - } +func TestCreateKeyPair_NilInput(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.CreateKeyPair(nil) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) +} + +func TestCreateKeyPair_MissingKeyName(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{}) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) +} + +func TestCreateKeyPair_InvalidKeyName(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("invalid key name!@#"), + }) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidKeyPairFormat, err.Error()) +} + +func TestCreateKeyPair_Duplicate(t *testing.T) { + requireSSHKeygen(t) + svc, _ := newTestKeyService() + + _, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("dup-key"), + }) + require.NoError(t, err) + + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("dup-key"), + }) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidKeyPairDuplicate, err.Error()) +} + +func TestCreateKeyPair_InvalidKeyType(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("bad-type-key"), + KeyType: aws.String("dsa"), + }) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) +} + +// ============================================================ +// ImportKeyPair Tests +// ============================================================ + +func TestImportKeyPair_Success_ED25519(t *testing.T) { + svc, store := newTestKeyService() + + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("imported-ed25519"), + PublicKeyMaterial: []byte(testED25519PubKey), + }) + require.NoError(t, err) + require.NotNil(t, out) + + assert.Equal(t, "imported-ed25519", *out.KeyName) + assert.NotEmpty(t, *out.KeyPairId) + assert.True(t, strings.HasPrefix(*out.KeyPairId, "key-")) + assert.NotEmpty(t, *out.KeyFingerprint) + assert.True(t, strings.HasPrefix(*out.KeyFingerprint, "SHA256:")) + + // Verify public key stored in S3 + keyPath := "keys/" + testAccountID + "/imported-ed25519" + getOut, err := store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(keyPath), + }) + require.NoError(t, err) + assert.NotNil(t, getOut) + + // Verify metadata stored in S3 + metaPath := "keys/" + testAccountID + "/" + *out.KeyPairId + ".json" + metaOut, err := store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(metaPath), + }) + require.NoError(t, err) + assert.NotNil(t, metaOut) +} + +func TestImportKeyPair_Success_RSA(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("imported-rsa"), + PublicKeyMaterial: []byte(testRSAPubKey), + }) + require.NoError(t, err) + require.NotNil(t, out) + + assert.Equal(t, "imported-rsa", *out.KeyName) + assert.NotEmpty(t, *out.KeyFingerprint) + assert.False(t, strings.HasPrefix(*out.KeyFingerprint, "SHA256:"), "RSA fingerprint should be MD5 format") +} + +func TestImportKeyPair_Duplicate(t *testing.T) { + svc, _ := newTestKeyService() + + _, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("dup-import"), + PublicKeyMaterial: []byte(testED25519PubKey), + }) + require.NoError(t, err) + + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("dup-import"), + PublicKeyMaterial: []byte(testED25519PubKey), + }) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidKeyPairDuplicate, err.Error()) +} + +func TestImportKeyPair_InvalidKeyName(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("bad name with spaces!"), + PublicKeyMaterial: []byte(testED25519PubKey), + }) + require.Error(t, err) + assert.Nil(t, out) + assert.Equal(t, awserrors.ErrorInvalidKeyPairFormat, err.Error()) +} + +func TestImportKeyPairInvalidKeyFormat(t *testing.T) { + svc, _ := newTestKeyService() tests := []struct { name string - input *ec2.DescribeKeyPairsInput - expectedCount int - expectedKeyIds []string + publicKey string + expectedErrMsg string }{ { - name: "No filters - return all", - input: &ec2.DescribeKeyPairsInput{}, - expectedCount: 3, - expectedKeyIds: []string{"key-11111", "key-22222", "key-33333"}, + name: "SingleFieldNoKeyData", + publicKey: "ssh-rsa", + expectedErrMsg: awserrors.ErrorInvalidKeyFormat, }, { - name: "Filter by single KeyName", - input: &ec2.DescribeKeyPairsInput{ - KeyNames: []*string{aws.String("test-key-1")}, - }, - expectedCount: 1, - expectedKeyIds: []string{"key-11111"}, + name: "UnsupportedAlgorithm", + publicKey: "ssh-dss AAAAB3NzaC1kc3MAAACB", + expectedErrMsg: awserrors.ErrorInvalidKeyFormat, }, { - name: "Filter by multiple KeyNames", - input: &ec2.DescribeKeyPairsInput{ - KeyNames: []*string{aws.String("test-key-1"), aws.String("test-key-2")}, - }, - expectedCount: 2, - expectedKeyIds: []string{"key-11111", "key-22222"}, + name: "InvalidBase64", + publicKey: "ssh-rsa not-valid-base64!!!", + expectedErrMsg: awserrors.ErrorInvalidKeyFormat, }, { - name: "Filter by single KeyPairId", - input: &ec2.DescribeKeyPairsInput{ - KeyPairIds: []*string{aws.String("key-22222")}, - }, - expectedCount: 1, - expectedKeyIds: []string{"key-22222"}, - }, - { - name: "Filter by multiple KeyPairIds", - input: &ec2.DescribeKeyPairsInput{ - KeyPairIds: []*string{aws.String("key-11111"), aws.String("key-33333")}, - }, - expectedCount: 2, - expectedKeyIds: []string{"key-11111", "key-33333"}, - }, - { - name: "Filter by non-existent KeyName", - input: &ec2.DescribeKeyPairsInput{ - KeyNames: []*string{aws.String("nonexistent-key")}, - }, - expectedCount: 0, - expectedKeyIds: []string{}, + name: "EmptyKeyData", + publicKey: "ssh-ed25519 ", + expectedErrMsg: awserrors.ErrorInvalidKeyFormat, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Simulate filtering logic from DescribeKeyPairs - var filtered []*ec2.KeyPairInfo - - if len(tt.input.KeyNames) > 0 { - // Filter by KeyName - nameSet := make(map[string]bool) - for _, name := range tt.input.KeyNames { - nameSet[*name] = true - } - for _, kp := range keyPairs { - if nameSet[*kp.KeyName] { - filtered = append(filtered, kp) - } - } - } else if len(tt.input.KeyPairIds) > 0 { - // Filter by KeyPairId - idSet := make(map[string]bool) - for _, id := range tt.input.KeyPairIds { - idSet[*id] = true - } - for _, kp := range keyPairs { - if idSet[*kp.KeyPairId] { - filtered = append(filtered, kp) - } - } - } else { - // No filters - return all - filtered = keyPairs - } - - if len(filtered) != tt.expectedCount { - t.Errorf("Expected %d key pairs, got %d", tt.expectedCount, len(filtered)) - } - - // Verify expected key IDs are present - for _, expectedId := range tt.expectedKeyIds { - found := false - for _, kp := range filtered { - if *kp.KeyPairId == expectedId { - found = true - break - } - } - if !found { - t.Errorf("Expected key pair ID %s not found in filtered results", expectedId) - } - } + _, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("test-key"), + PublicKeyMaterial: []byte(tt.publicKey), + }) + require.Error(t, err) + assert.Equal(t, tt.expectedErrMsg, err.Error()) }) } } -// TestKeyPairMetadataPath tests the S3 path construction logic -func TestKeyPairMetadataPath(t *testing.T) { - accountID := "123456789" +// ============================================================ +// DeleteKeyPair Tests +// ============================================================ - tests := []struct { - name string - keyPairId string - expected string - }{ - { - name: "Standard key pair ID", - keyPairId: "key-abcdef123456", - expected: "keys/123456789/key-abcdef123456.json", - }, - { - name: "Short key pair ID", - keyPairId: "key-12345", - expected: "keys/123456789/key-12345.json", - }, - } +func TestDeleteKeyPair_ByKeyName(t *testing.T) { + svc, store := newTestKeyService() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Simulate path construction from the code - metadataPath := "keys/" + accountID + "/" + tt.keyPairId + ".json" + imported := importTestKey(t, svc, "to-delete-by-name") - if metadataPath != tt.expected { - t.Errorf("Expected path %s, got %s", tt.expected, metadataPath) - } - }) - } + result, err := svc.DeleteKeyPair(&ec2.DeleteKeyPairInput{ + KeyName: aws.String("to-delete-by-name"), + }) + require.NoError(t, err) + assert.NotNil(t, result) + + // Verify public key removed from S3 + keyPath := "keys/" + testAccountID + "/to-delete-by-name" + _, err = store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(keyPath), + }) + assert.Error(t, err, "public key should be deleted") + + // Verify metadata removed from S3 + metaPath := "keys/" + testAccountID + "/" + *imported.KeyPairId + ".json" + _, err = store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(metaPath), + }) + assert.Error(t, err, "metadata should be deleted") } -// TestImportKeyPairKeyTypeDetection tests key type detection from public key material -func TestImportKeyPairKeyTypeDetection(t *testing.T) { - tests := []struct { - name string - publicKeyData string - expectedType string - expectError bool - }{ - { - name: "ED25519 key", - publicKeyData: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl", - expectedType: "ed25519", - expectError: false, - }, - { - name: "RSA key", - publicKeyData: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCz...", - expectedType: "rsa", - expectError: false, - }, - { - name: "ECDSA key", - publicKeyData: "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTY...", - expectedType: "ecdsa", - expectError: false, - }, - { - name: "Invalid format - no key data", - publicKeyData: "ssh-rsa", - expectedType: "", - expectError: true, - }, - { - name: "Empty key material", - publicKeyData: "", - expectedType: "", - expectError: true, - }, - { - name: "Unsupported key type", - publicKeyData: "ssh-dss AAAAB3NzaC1kc3MAAACB...", - expectedType: "", - expectError: true, - }, - } +func TestDeleteKeyPair_ByKeyPairId(t *testing.T) { + svc, store := newTestKeyService() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Simulate the key type detection logic from ImportKeyPair - parts := strings.Fields(tt.publicKeyData) + imported := importTestKey(t, svc, "to-delete-by-id") - if len(parts) < 2 { - if !tt.expectError { - t.Errorf("Expected no error but got invalid format") - } - return - } + result, err := svc.DeleteKeyPair(&ec2.DeleteKeyPairInput{ + KeyPairId: imported.KeyPairId, + }) + require.NoError(t, err) + assert.NotNil(t, result) - var keyType string - algorithmPrefix := parts[0] + // Verify public key removed from S3 + keyPath := "keys/" + testAccountID + "/to-delete-by-id" + _, err = store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(keyPath), + }) + assert.Error(t, err, "public key should be deleted") - switch { - case strings.HasPrefix(algorithmPrefix, "ssh-ed25519"): - keyType = "ed25519" - case strings.HasPrefix(algorithmPrefix, "ssh-rsa"): - keyType = "rsa" - case strings.HasPrefix(algorithmPrefix, "ecdsa-sha2-"): - keyType = "ecdsa" - default: - if !tt.expectError { - t.Errorf("Expected no error but got unsupported key type: %s", algorithmPrefix) - } - return - } - - if keyType != tt.expectedType { - t.Errorf("Expected key type %s, got %s", tt.expectedType, keyType) - } - }) - } + // Verify metadata removed from S3 + metaPath := "keys/" + testAccountID + "/" + *imported.KeyPairId + ".json" + _, err = store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(metaPath), + }) + assert.Error(t, err, "metadata should be deleted") } -// TestDeleteKeyPairIdempotent verifies AWS-compatible idempotent behavior: -// deleting a non-existent key pair returns success instead of NotFound. func TestDeleteKeyPairIdempotent(t *testing.T) { - store := objectstore.NewMemoryObjectStore() - svc := NewKeyServiceImplWithStore(store, "test-bucket", "123456789") + svc, _ := newTestKeyService() t.Run("NonExistentKeyName", func(t *testing.T) { result, err := svc.DeleteKeyPair(&ec2.DeleteKeyPairInput{ @@ -415,109 +395,139 @@ func TestDeleteKeyPairIdempotent(t *testing.T) { }) } -// TestImportKeyPairInvalidKeyFormat verifies that ImportKeyPair returns -// InvalidKey.Format for malformed public key material (not InvalidKeyPair.Format). -func TestImportKeyPairInvalidKeyFormat(t *testing.T) { - store := objectstore.NewMemoryObjectStore() - svc := NewKeyServiceImplWithStore(store, "test-bucket", "123456789") +func TestDeleteKeyPair_NilInput(t *testing.T) { + svc, _ := newTestKeyService() - tests := []struct { - name string - publicKey string - expectedErrMsg string - }{ - { - name: "SingleFieldNoKeyData", - publicKey: "ssh-rsa", - expectedErrMsg: "InvalidKey.Format", - }, - { - name: "UnsupportedAlgorithm", - publicKey: "ssh-dss AAAAB3NzaC1kc3MAAACB", - expectedErrMsg: "InvalidKey.Format", - }, - { - name: "InvalidBase64", - publicKey: "ssh-rsa not-valid-base64!!!", - expectedErrMsg: "InvalidKey.Format", - }, - { - name: "EmptyKeyData", - publicKey: "ssh-ed25519 ", - expectedErrMsg: "InvalidKey.Format", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ - KeyName: aws.String("test-key"), - PublicKeyMaterial: []byte(tt.publicKey), - }) - require.Error(t, err) - assert.Equal(t, tt.expectedErrMsg, err.Error()) - }) - } + result, err := svc.DeleteKeyPair(nil) + require.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) } -// TestImportKeyPairInputValidation tests input validation for ImportKeyPair -func TestImportKeyPairInputValidation(t *testing.T) { - tests := []struct { - name string - input *ec2.ImportKeyPairInput - expectErr bool - }{ - { - name: "Nil input", - input: nil, - expectErr: true, - }, - { - name: "Missing KeyName", - input: &ec2.ImportKeyPairInput{ - PublicKeyMaterial: []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ..."), - }, - expectErr: true, - }, - { - name: "Missing PublicKeyMaterial", - input: &ec2.ImportKeyPairInput{ - KeyName: aws.String("test-key"), - }, - expectErr: true, - }, - { - name: "Empty PublicKeyMaterial", - input: &ec2.ImportKeyPairInput{ - KeyName: aws.String("test-key"), - PublicKeyMaterial: []byte(""), - }, - expectErr: true, - }, - { - name: "Valid input", - input: &ec2.ImportKeyPairInput{ - KeyName: aws.String("test-key"), - PublicKeyMaterial: []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ..."), - }, - expectErr: false, - }, - } +func TestDeleteKeyPair_EmptyNameAndId(t *testing.T) { + svc, _ := newTestKeyService() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Simulate validation logic from ImportKeyPair - hasError := false - - if tt.input == nil || tt.input.KeyName == nil { - hasError = true - } else if len(tt.input.PublicKeyMaterial) == 0 { - hasError = true - } - - if hasError != tt.expectErr { - t.Errorf("Expected error=%v, got error=%v", tt.expectErr, hasError) - } - }) - } + result, err := svc.DeleteKeyPair(&ec2.DeleteKeyPairInput{}) + require.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) +} + +func TestDeleteKeyPair_InvalidKeyPairIdFormat(t *testing.T) { + svc, _ := newTestKeyService() + + result, err := svc.DeleteKeyPair(&ec2.DeleteKeyPairInput{ + KeyPairId: aws.String("bad id format!!!"), + }) + require.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, awserrors.ErrorInvalidKeyPairFormat, err.Error()) +} + +// ============================================================ +// DescribeKeyPairs Tests +// ============================================================ + +func TestDescribeKeyPairs_Empty(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{}) + require.NoError(t, err) + require.NotNil(t, out) + assert.Empty(t, out.KeyPairs) +} + +func TestDescribeKeyPairs_AllKeys(t *testing.T) { + svc, _ := newTestKeyService() + + importTestKey(t, svc, "key-alpha") + importTestKey(t, svc, "key-beta") + + out, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{}) + require.NoError(t, err) + require.NotNil(t, out) + assert.Len(t, out.KeyPairs, 2) + + names := make(map[string]bool) + for _, kp := range out.KeyPairs { + names[*kp.KeyName] = true + assert.NotEmpty(t, *kp.KeyPairId) + assert.NotEmpty(t, *kp.KeyFingerprint) + assert.NotEmpty(t, *kp.KeyType) + } + assert.True(t, names["key-alpha"]) + assert.True(t, names["key-beta"]) +} + +func TestDescribeKeyPairs_FilterByKeyName(t *testing.T) { + svc, _ := newTestKeyService() + + importTestKey(t, svc, "find-me") + importTestKey(t, svc, "ignore-me") + + out, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + KeyNames: []*string{aws.String("find-me")}, + }) + require.NoError(t, err) + require.NotNil(t, out) + require.Len(t, out.KeyPairs, 1) + assert.Equal(t, "find-me", *out.KeyPairs[0].KeyName) +} + +func TestDescribeKeyPairs_FilterByKeyPairId(t *testing.T) { + svc, _ := newTestKeyService() + + imported := importTestKey(t, svc, "find-by-id") + importTestKey(t, svc, "other-key") + + out, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + KeyPairIds: []*string{imported.KeyPairId}, + }) + require.NoError(t, err) + require.NotNil(t, out) + require.Len(t, out.KeyPairs, 1) + assert.Equal(t, "find-by-id", *out.KeyPairs[0].KeyName) +} + +func TestDescribeKeyPairs_FilterNoMatch(t *testing.T) { + svc, _ := newTestKeyService() + + importTestKey(t, svc, "exists") + + out, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + KeyNames: []*string{aws.String("does-not-exist")}, + }) + require.NoError(t, err) + require.NotNil(t, out) + assert.Empty(t, out.KeyPairs) +} + +func TestDescribeKeyPairs_NilInput(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.DescribeKeyPairs(nil) + require.NoError(t, err) + require.NotNil(t, out) + assert.Empty(t, out.KeyPairs) +} + +// ============================================================ +// ValidateKeyPairExists Tests +// ============================================================ + +func TestValidateKeyPairExists_Found(t *testing.T) { + svc, _ := newTestKeyService() + + importTestKey(t, svc, "existing-key") + + err := svc.ValidateKeyPairExists("existing-key") + assert.NoError(t, err) +} + +func TestValidateKeyPairExists_NotFound(t *testing.T) { + svc, _ := newTestKeyService() + + err := svc.ValidateKeyPairExists("ghost-key") + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidKeyPairNotFound, err.Error()) } diff --git a/tests/e2e/TEST_COVERAGE.md b/tests/e2e/TEST_COVERAGE.md index 3be343ab7..7434eabf3 100644 --- a/tests/e2e/TEST_COVERAGE.md +++ b/tests/e2e/TEST_COVERAGE.md @@ -104,10 +104,15 @@ - `attach-volume` to stopped instance (expect `IncorrectInstanceState` error) - `delete-volume` (cleanup) -### Phase 7 (cont): Start and Restart -- `start-instances` (poll -> running) +### Phase 7b: ModifyInstanceAttribute +- `modify-instance-attribute` (change instance type from nano → xlarge while stopped) +- `describe-instances` (verify type updated in KV) +- `describe-instance-types` (get expected vCPU count and memory for new type) +- `start-instances` (poll -> running with new type) +- SSH: `nproc` — verify vCPU count matches xlarge (4 vCPUs) +- SSH: `/proc/meminfo` MemTotal — verify memory matches xlarge (within 85% of expected) -### Phase 7b: RunInstances with count > 1 +### Phase 7c: RunInstances with count > 1 - `run-instances --count 2` (launch 2 instances in a single call) - Verify 2 instances returned in response - Poll both to running state @@ -130,6 +135,7 @@ - `describe-images` with non-existent AMI ID (expect `InvalidAMIID.NotFound`) - `create-image` with duplicate name (expect `InvalidAMIName.Duplicate`) - `delete-key-pair` on non-existent key (expect success — idempotent, matches AWS) +- `modify-instance-attribute` on running instance (expect `InvalidInstanceID.NotFound` — running instances not in stopped KV) ### Phase 9: Terminate and Verify Cleanup - `delete-snapshot` (CreateImage backing snapshot, so DeleteOnTermination is not blocked) diff --git a/tests/e2e/run-e2e.sh b/tests/e2e/run-e2e.sh index 9ae34f4cf..804d2035e 100755 --- a/tests/e2e/run-e2e.sh +++ b/tests/e2e/run-e2e.sh @@ -990,12 +990,41 @@ echo "Attach-to-stopped correctly rejected" $AWS_EC2 delete-volume --volume-id "$STOPPED_VOL_ID" echo "Cleaned up test volume $STOPPED_VOL_ID" -# Start instance (start-instances) and verify transition back to running (describe-instances) -echo "Starting instance..." +# Phase 7b: ModifyInstanceAttribute (change instance type while stopped, verify via SSH) +echo "Phase 7b: ModifyInstanceAttribute" +echo "Instance is stopped — modifying instance type to verify changes take effect on restart" + +# Derive an upsized type in the same family: nano → xlarge (4 vCPUs instead of 2) +MODIFY_TYPE="${INSTANCE_TYPE%.nano}.xlarge" +echo "Changing instance type from $INSTANCE_TYPE to $MODIFY_TYPE..." + +# Get expected vCPU and memory for the new type +# Note: --instance-types filter may not be supported; use jq to select the correct type +TYPES_JSON=$($AWS_EC2 describe-instance-types) +EXPECTED_VCPUS=$(echo "$TYPES_JSON" | jq -r ".InstanceTypes[] | select(.InstanceType==\"$MODIFY_TYPE\") | .VCpuInfo.DefaultVCpus") +EXPECTED_MEM_MIB=$(echo "$TYPES_JSON" | jq -r ".InstanceTypes[] | select(.InstanceType==\"$MODIFY_TYPE\") | .MemoryInfo.SizeInMiB") +echo "Expected resources after modify: ${EXPECTED_VCPUS} vCPUs, ${EXPECTED_MEM_MIB} MiB RAM" + +# Modify the instance type +$AWS_EC2 modify-instance-attribute --instance-id "$INSTANCE_ID" \ + --instance-type "{\"Value\": \"$MODIFY_TYPE\"}" + +# Verify describe-instances reflects the new type +MODIFIED_TYPE=$($AWS_EC2 describe-instances --instance-ids "$INSTANCE_ID" \ + --query 'Reservations[0].Instances[0].InstanceType' --output text) +if [ "$MODIFIED_TYPE" != "$MODIFY_TYPE" ]; then + echo "ModifyInstanceAttribute failed: expected type $MODIFY_TYPE, got $MODIFIED_TYPE" + exit 1 +fi +echo "Instance type updated to $MODIFIED_TYPE" + +# Start instance with the new type +echo "Starting instance with modified type..." $AWS_EC2 start-instances --instance-ids "$INSTANCE_ID" COUNT=0 while [ $COUNT -lt 30 ]; do - STATE=$($AWS_EC2 describe-instances --instance-ids "$INSTANCE_ID" --query 'Reservations[0].Instances[0].State.Name' --output text) + STATE=$($AWS_EC2 describe-instances --instance-ids "$INSTANCE_ID" \ + --query 'Reservations[0].Instances[0].State.Name' --output text) echo "Instance state: $STATE" if [ "$STATE" == "running" ]; then break @@ -1005,12 +1034,60 @@ while [ $COUNT -lt 30 ]; do done if [ "$STATE" != "running" ]; then - echo "Instance failed to reach running state after restart" + echo "Instance failed to reach running state after type change" exit 1 fi -# Phase 7b: RunInstances with count > 1 -echo "Phase 7b: RunInstances with MinCount/MaxCount > 1" +# Get SSH port (may have changed after restart with new QEMU config) +echo "Getting SSH port for restarted instance..." +SSH_PORT=$(get_ssh_port "$INSTANCE_ID") +SSH_HOST=$(get_ssh_host "$INSTANCE_ID") +echo "SSH endpoint: $SSH_HOST:$SSH_PORT" + +# Wait for SSH to become ready +wait_for_ssh "$SSH_HOST" "$SSH_PORT" "test-key-1.pem" 30 + +# Verify vCPU count matches the new instance type (nproc reports online CPUs) +echo "Verifying vCPU count inside the VM..." +VM_VCPUS=$(ssh -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null \ + -o LogLevel=ERROR \ + -o ConnectTimeout=5 \ + -o BatchMode=yes \ + -p "$SSH_PORT" \ + -i "test-key-1.pem" \ + ec2-user@"$SSH_HOST" 'nproc' | tr -d '[:space:]') +echo "VM reports $VM_VCPUS vCPUs (expected $EXPECTED_VCPUS)" +if [ "$VM_VCPUS" != "$EXPECTED_VCPUS" ]; then + echo "vCPU count mismatch after ModifyInstanceAttribute: VM reports $VM_VCPUS, expected $EXPECTED_VCPUS" + exit 1 +fi +echo "vCPU count verified" + +# Verify memory matches the new instance type (MemTotal from /proc/meminfo) +echo "Verifying memory inside the VM..." +VM_MEM_KB=$(ssh -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null \ + -o LogLevel=ERROR \ + -o ConnectTimeout=5 \ + -o BatchMode=yes \ + -p "$SSH_PORT" \ + -i "test-key-1.pem" \ + ec2-user@"$SSH_HOST" "awk '/MemTotal/ {print \$2}' /proc/meminfo" | tr -d '[:space:]') +VM_MEM_MIB=$((VM_MEM_KB / 1024)) +# Allow 15% margin for kernel reserved memory +EXPECTED_MEM_LOW=$((EXPECTED_MEM_MIB * 85 / 100)) +echo "VM reports ${VM_MEM_MIB} MiB total RAM (expected ~${EXPECTED_MEM_MIB} MiB, threshold ${EXPECTED_MEM_LOW} MiB)" +if [ "$VM_MEM_MIB" -lt "$EXPECTED_MEM_LOW" ]; then + echo "Memory too low after ModifyInstanceAttribute: VM reports ${VM_MEM_MIB} MiB, expected at least ${EXPECTED_MEM_LOW} MiB" + exit 1 +fi +echo "Memory verified" + +echo "ModifyInstanceAttribute test passed (type change + vCPU + memory verified via SSH)" + +# Phase 7c: RunInstances with count > 1 +echo "Phase 7c: RunInstances with MinCount/MaxCount > 1" echo "Launching 2 instances in a single run-instances call..." MULTI_RUN_OUTPUT=$($AWS_EC2 run-instances \ --image-id "$AMI_ID" \ @@ -1161,6 +1238,11 @@ echo "8p: DeleteKeyPair non-existent key (idempotent)..." $AWS_EC2 delete-key-pair --key-name nonexistent-key-99999 echo " DeleteKeyPair for non-existent key succeeded (idempotent)" +# 8q: ModifyInstanceAttribute on running instance (instance not in stopped KV → NotFound) +echo "8q: ModifyInstanceAttribute on running instance..." +expect_error "InvalidInstanceID.NotFound" $AWS_EC2 modify-instance-attribute \ + --instance-id "$INSTANCE_ID" --instance-type "{\"Value\": \"$INSTANCE_TYPE\"}" + echo "Negative test suite passed" # Phase 9: Terminate and Verify Cleanup