From c46197f389ae3bea2d38b078f8061c0d8d7a6cb1 Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 3 Aug 2026 17:35:31 +0530 Subject: [PATCH 1/8] fix: add Type field to ApiKey struct for internal/external classification --- src/data/models/api_key.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/data/models/api_key.go b/src/data/models/api_key.go index 6ff52373..e0f2c799 100644 --- a/src/data/models/api_key.go +++ b/src/data/models/api_key.go @@ -11,6 +11,7 @@ type ApiKey struct { RevokedAt string `json:"revoked_at"` ExpiresAt string `json:"expires_at"` UserID string `json:"user_id,omitempty"` + Type string `json:"type,omitempty"` // "internal" or "external". Default: "external" *DbRecord `json:"db_record"` } From 8e19a4a7bae64f2e61301bb0fc1d9c4000e7862b Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 3 Aug 2026 17:52:18 +0530 Subject: [PATCH 2/8] fix: implement normalization and filtering for API key types --- src/data/api_key.go | 30 ++++ src/data/api_key_test.go | 339 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 369 insertions(+) diff --git a/src/data/api_key.go b/src/data/api_key.go index de2f9b3c..3d85aae1 100644 --- a/src/data/api_key.go +++ b/src/data/api_key.go @@ -32,6 +32,27 @@ func GetOwnRecords[T interface{ GetUserID() string }](ctx basecontext.ApiContext return result } +// normalizeApiKeyType converts empty Type to "external" for backward compat +func normalizeApiKeyType(key *models.ApiKey) { + if key.Type == "" { + key.Type = "external" + } +} + +// filterInternalKeys removes internal keys from a list +// Note: This creates a new slice, so normalization should happen after this +func filterInternalKeys(keys []models.ApiKey) []models.ApiKey { + result := make([]models.ApiKey, 0, len(keys)) + for _, k := range keys { + // Normalize before checking type + normalizeApiKeyType(&k) + if k.Type != "internal" { + result = append(result, k) + } + } + return result +} + func (j *JsonDatabase) GetApiKeys(ctx basecontext.ApiContext, filter string) ([]models.ApiKey, error) { if !j.IsConnected() { return nil, ErrDatabaseNotConnected @@ -46,6 +67,9 @@ func (j *JsonDatabase) GetApiKeys(ctx basecontext.ApiContext, filter string) ([] return nil, err } + // Filter internal keys (function also normalizes types) - do this BEFORE auth check + filteredData = filterInternalKeys(filteredData) + authContext := ctx.GetAuthorizationContext() if authContext == nil || authContext.User == nil { return filteredData, nil @@ -78,6 +102,7 @@ func (j *JsonDatabase) GetApiKey(ctx basecontext.ApiContext, idOrName string) (* } } + normalizeApiKeyType(&apiKey) return &apiKey, nil } } @@ -120,6 +145,11 @@ func (j *JsonDatabase) CreateApiKey(ctx basecontext.ApiContext, apiKey models.Ap return nil, ErrApiKeyAlreadyExists } + // Set default type if not provided + if apiKey.Type == "" { + apiKey.Type = "external" + } + passwdSvc := password.Get() hashSecret, err := passwdSvc.Hash(apiKey.Secret, apiKey.ID) if err != nil { diff --git a/src/data/api_key_test.go b/src/data/api_key_test.go index ba4d87ae..a919ee92 100644 --- a/src/data/api_key_test.go +++ b/src/data/api_key_test.go @@ -264,3 +264,342 @@ func TestCreateApiKeyWithoutUserIdAutoAssignsToUser(t *testing.T) { // After creation, the UserID should be empty (data layer doesn't auto-assign) assert.Empty(t, createdKey.UserID) } + +// ============================================================================ +// Tests for API Key Type Field (Task 2) +// ============================================================================ + +func TestNormalizeApiKeyType(t *testing.T) { + t.Run("empty type becomes external", func(t *testing.T) { + key := &models.ApiKey{Type: ""} + normalizeApiKeyType(key) + assert.Equal(t, "external", key.Type) + }) + + t.Run("internal type stays internal", func(t *testing.T) { + key := &models.ApiKey{Type: "internal"} + normalizeApiKeyType(key) + assert.Equal(t, "internal", key.Type) + }) + + t.Run("external type stays external", func(t *testing.T) { + key := &models.ApiKey{Type: "external"} + normalizeApiKeyType(key) + assert.Equal(t, "external", key.Type) + }) +} + +func TestFilterInternalKeys(t *testing.T) { + keys := []models.ApiKey{ + {ID: "key1", Type: "external"}, + {ID: "key2", Type: "internal"}, + {ID: "key3", Type: "external"}, + {ID: "key4", Type: "internal"}, + {ID: "key5", Type: ""}, + } + + filtered := filterInternalKeys(keys) + + assert.Equal(t, 3, len(filtered)) + assert.Equal(t, "key1", filtered[0].ID) + assert.Equal(t, "key3", filtered[1].ID) + assert.Equal(t, "key5", filtered[2].ID) +} + +func TestCreateApiKeyDefaultTypeIsExternal(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + apiKey := models.ApiKey{ + ID: "test-key-default-type", + Name: "Test Key Default Type", + Key: "TEST_KEY_DEFAULT", + Secret: "secret", + // Type is not set + } + + createdKey, err := db.CreateApiKey(ctx, apiKey) + require.NoError(t, err) + assert.Equal(t, "external", createdKey.Type) + + // Verify persistence + loadedKey, err := db.GetApiKey(ctx, "test-key-default-type") + require.NoError(t, err) + assert.Equal(t, "external", loadedKey.Type) +} + +func TestCreateApiKeyExplicitTypePreserved(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + t.Run("explicit internal type", func(t *testing.T) { + apiKey := models.ApiKey{ + ID: "test-key-internal", + Name: "Test Key Internal", + Key: "TEST_KEY_INTERNAL", + Secret: "secret", + Type: "internal", + } + + createdKey, err := db.CreateApiKey(ctx, apiKey) + require.NoError(t, err) + assert.Equal(t, "internal", createdKey.Type) + + // Verify persistence + loadedKey, err := db.GetApiKey(ctx, "test-key-internal") + require.NoError(t, err) + assert.Equal(t, "internal", loadedKey.Type) + }) + + t.Run("explicit external type", func(t *testing.T) { + apiKey := models.ApiKey{ + ID: "test-key-external", + Name: "Test Key External", + Key: "TEST_KEY_EXTERNAL", + Secret: "secret", + Type: "external", + } + + createdKey, err := db.CreateApiKey(ctx, apiKey) + require.NoError(t, err) + assert.Equal(t, "external", createdKey.Type) + }) +} + +func TestGetApiKeyNormalizesType(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Directly insert a key with empty Type (simulating old data) + oldKey := models.ApiKey{ + ID: "old-key", + Name: "Old Key", + Key: "OLD_KEY", + Secret: "hashed_secret", + Type: "", // Empty, like old data + CreatedAt: helpers.GetUtcCurrentDateTime(), + UpdatedAt: helpers.GetUtcCurrentDateTime(), + } + db.data.ApiKeys = append(db.data.ApiKeys, oldKey) + + // Retrieve the key - should normalize to "external" + loadedKey, err := db.GetApiKey(ctx, "old-key") + require.NoError(t, err) + assert.Equal(t, "external", loadedKey.Type) +} + +func TestGetApiKeysFiltersInternalKeys(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Create external keys + externalKey1 := models.ApiKey{ + ID: "external-1", + Name: "External Key 1", + Key: "EXTERNAL_1", + Secret: "secret1", + Type: "external", + } + _, err := db.CreateApiKey(ctx, externalKey1) + require.NoError(t, err) + + externalKey2 := models.ApiKey{ + ID: "external-2", + Name: "External Key 2", + Key: "EXTERNAL_2", + Secret: "secret2", + Type: "external", + } + _, err = db.CreateApiKey(ctx, externalKey2) + require.NoError(t, err) + + // Create internal keys + internalKey1 := models.ApiKey{ + ID: "internal-1", + Name: "Internal Key 1", + Key: "INTERNAL_1", + Secret: "secret3", + Type: "internal", + } + _, err = db.CreateApiKey(ctx, internalKey1) + require.NoError(t, err) + + internalKey2 := models.ApiKey{ + ID: "internal-2", + Name: "Internal Key 2", + Key: "INTERNAL_2", + Secret: "secret4", + Type: "internal", + } + _, err = db.CreateApiKey(ctx, internalKey2) + require.NoError(t, err) + + // Get all keys - should only return external ones + allKeys, err := db.GetApiKeys(ctx, "") + require.NoError(t, err) + assert.Equal(t, 2, len(allKeys), "Only external keys should be returned") + + // Verify all returned keys are external + for _, key := range allKeys { + assert.Equal(t, "external", key.Type) + assert.NotContains(t, []string{"internal-1", "internal-2"}, key.ID) + } + + // Verify internal keys are still in DB and retrievable by ID + loadedInternal, err := db.GetApiKey(ctx, "internal-1") + require.NoError(t, err) + assert.Equal(t, "internal", loadedInternal.Type) + assert.Equal(t, "internal-1", loadedInternal.ID) +} + +func TestGetApiKeysNormalizesTypes(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Directly insert keys with empty Type (simulating old data) + oldKey1 := models.ApiKey{ + ID: "old-key-1", + Name: "Old Key 1", + Key: "OLD_KEY_1", + Secret: "hashed_secret_1", + Type: "", // Empty, like old data + CreatedAt: helpers.GetUtcCurrentDateTime(), + UpdatedAt: helpers.GetUtcCurrentDateTime(), + } + db.data.ApiKeys = append(db.data.ApiKeys, oldKey1) + + oldKey2 := models.ApiKey{ + ID: "old-key-2", + Name: "Old Key 2", + Key: "OLD_KEY_2", + Secret: "hashed_secret_2", + Type: "", // Empty, like old data + CreatedAt: helpers.GetUtcCurrentDateTime(), + UpdatedAt: helpers.GetUtcCurrentDateTime(), + } + db.data.ApiKeys = append(db.data.ApiKeys, oldKey2) + + // Get all keys - should normalize all to "external" + allKeys, err := db.GetApiKeys(ctx, "") + require.NoError(t, err) + assert.Equal(t, 2, len(allKeys)) + + for _, key := range allKeys { + assert.Equal(t, "external", key.Type, "All keys should be normalized to external") + } +} + +func TestInternalKeysNotInListButRetrievableByID(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Create a temp internal key (like temp-vm-* keys) + tempKey := models.ApiKey{ + ID: "temp-vm-job123", + Name: "temp-vm-job123", + Key: "temp-vm-job123", + Secret: "temporary-secret", + Type: "internal", + ExpiresAt: time.Now().Add(2 * time.Hour).Format(time.RFC3339), + } + _, err := db.CreateApiKey(ctx, tempKey) + require.NoError(t, err) + + // Create a regular external key + regularKey := models.ApiKey{ + ID: "regular-key", + Name: "Regular Key", + Key: "REGULAR_KEY", + Secret: "regular-secret", + Type: "external", + } + _, err = db.CreateApiKey(ctx, regularKey) + require.NoError(t, err) + + // List keys - should NOT include temp internal key + allKeys, err := db.GetApiKeys(ctx, "") + require.NoError(t, err) + assert.Equal(t, 1, len(allKeys), "Only external key should be in list") + assert.Equal(t, "regular-key", allKeys[0].ID) + + // Verify internal key is still retrievable by ID (needed for auth) + loadedTemp, err := db.GetApiKey(ctx, "temp-vm-job123") + require.NoError(t, err) + assert.Equal(t, "temp-vm-job123", loadedTemp.ID) + assert.Equal(t, "internal", loadedTemp.Type) + + // Verify can also retrieve by Name + loadedByName, err := db.GetApiKey(ctx, "temp-vm-job123") + require.NoError(t, err) + assert.Equal(t, "temp-vm-job123", loadedByName.ID) + assert.Equal(t, "internal", loadedByName.Type) +} + +func TestBackwardCompatibilityWithOldApiKeys(t *testing.T) { + db, tmpDir := setupTestDB(t) + defer cleanupTestDB(t, tmpDir, db) + + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Simulate old API keys without Type field + oldKeys := []models.ApiKey{ + { + ID: "legacy-1", + Name: "Legacy Key 1", + Key: "LEGACY_1", + Secret: "hashed_secret_1", + Type: "", // No Type field in old data + CreatedAt: helpers.GetUtcCurrentDateTime(), + UpdatedAt: helpers.GetUtcCurrentDateTime(), + }, + { + ID: "legacy-2", + Name: "Legacy Key 2", + Key: "LEGACY_2", + Secret: "hashed_secret_2", + Type: "", // No Type field in old data + CreatedAt: helpers.GetUtcCurrentDateTime(), + UpdatedAt: helpers.GetUtcCurrentDateTime(), + }, + } + + // Directly insert into DB (simulating existing data) + db.data.ApiKeys = append(db.data.ApiKeys, oldKeys...) + + // Get all keys - should normalize and return them as external + allKeys, err := db.GetApiKeys(ctx, "") + require.NoError(t, err) + assert.Equal(t, 2, len(allKeys)) + + for _, key := range allKeys { + assert.Equal(t, "external", key.Type, "Legacy keys should be normalized to external") + } + + // Retrieve individual keys - should also normalize + legacy1, err := db.GetApiKey(ctx, "legacy-1") + require.NoError(t, err) + assert.Equal(t, "external", legacy1.Type) + + legacy2, err := db.GetApiKey(ctx, "legacy-2") + require.NoError(t, err) + assert.Equal(t, "external", legacy2.Type) +} From 2e7011440054b85caafe9e0003c30088789de290 Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 3 Aug 2026 19:01:39 +0530 Subject: [PATCH 3/8] feat: add OrchestratorPublicUrl method and related constants for catalog connection --- src/config/main.go | 8 + src/constants/main.go | 1 + src/controllers/machines.go | 68 ++++ .../machines_local_catalog_test.go | 297 ++++++++++++++++++ 4 files changed, 374 insertions(+) diff --git a/src/config/main.go b/src/config/main.go index 5c7ba43c..e0f0aa1f 100644 --- a/src/config/main.go +++ b/src/config/main.go @@ -518,6 +518,14 @@ func (c *Config) OrchestratorPullFrequency() int { return intVal } +func (c *Config) OrchestratorPublicUrl() string { + url := c.GetKey(constants.ORCHESTRATOR_PUBLIC_URL) + if url == "" { + return "localhost" + } + return url +} + func (c *Config) DatabaseFolder() string { return c.GetKey(constants.DATABASE_FOLDER_ENV_VAR) } diff --git a/src/constants/main.go b/src/constants/main.go index a5db803c..a184a5eb 100644 --- a/src/constants/main.go +++ b/src/constants/main.go @@ -96,6 +96,7 @@ const ( MODE_ENV_VAR = "MODE" USE_ORCHESTRATOR_RESOURCES_ENV_VAR = "USE_ORCHESTRATOR_RESOURCES" ORCHESTRATOR_PULL_FREQUENCY_SECONDS_ENV_VAR = "ORCHESTRATOR_PULL_FREQUENCY_SECONDS" + ORCHESTRATOR_PUBLIC_URL = "ORCHESTRATOR_PUBLIC_URL" DATABASE_FOLDER_ENV_VAR = "DATABASE_FOLDER" DATABASE_NUMBER_BACKUP_FILES_ENV_VAR = "DATABASE_NUMBER_BACKUP_FILES" DATABASE_BACKUP_INTERVAL_ENV_VAR = "DATABASE_BACKUP_INTERVAL_MINUTES" diff --git a/src/controllers/machines.go b/src/controllers/machines.go index 9ee1f894..123271a9 100644 --- a/src/controllers/machines.go +++ b/src/controllers/machines.go @@ -13,7 +13,9 @@ import ( catalog_models "github.com/Parallels/prl-devops-service/catalog/models" "github.com/Parallels/prl-devops-service/config" "github.com/Parallels/prl-devops-service/constants" + data_models "github.com/Parallels/prl-devops-service/data/models" "github.com/Parallels/prl-devops-service/errors" + "github.com/Parallels/prl-devops-service/helpers" "github.com/Parallels/prl-devops-service/jobs" "github.com/Parallels/prl-devops-service/mappers" "github.com/Parallels/prl-devops-service/models" @@ -2072,3 +2074,69 @@ func populateMachineRequestArchitecture(ctx basecontext.ApiContext, request *mod request.Architecture = arch return nil } + +// buildLocalCatalogConnection creates a temporary internal API key for catalog access +// Used when orchestrator IS the catalog and needs to provide credentials to remote hosts +func buildLocalCatalogConnection(ctx basecontext.ApiContext, callerID string, jobID string) (string, string, error) { + cfg := config.Get() + apiPort := cfg.ApiPort() + if apiPort == "" { + return "", "", fmt.Errorf("API port not configured") + } + + // Determine schema and host + schema := "http" + if cfg.TlsEnabled() { + schema = "https" + } + + host := cfg.OrchestratorPublicUrl() + + // Sanitize host to handle various input formats: + // - Remove protocol prefixes (http://, https://) + // - Remove trailing slashes + // - Remove port numbers (we add apiPort separately) + host = strings.TrimSpace(host) + host = strings.TrimPrefix(host, "https://") + host = strings.TrimPrefix(host, "http://") + host = strings.TrimSuffix(host, "/") + if idx := strings.Index(host, ":"); idx != -1 { + host = host[:idx] // Remove port if present + } + if host == "" { + host = "localhost" // Fallback to localhost if empty after sanitization + } + + // Generate unique key name and secret + keyName := "temp-vm-" + jobID + fullSecret := helpers.GenerateId() + // Limit secret to 40 chars (password hashing limitation) + plaintextSecret := fullSecret + if len(plaintextSecret) > 40 { + plaintextSecret = plaintextSecret[:40] + } + + db := serviceprovider.Get().JsonDatabase + if err := db.Connect(ctx); err != nil { + return "", "", fmt.Errorf("failed to connect to database: %w", err) + } + + tempKey := data_models.ApiKey{ + ID: helpers.GenerateId(), + Name: keyName, + Key: keyName, + Secret: plaintextSecret, // Will be hashed by CreateApiKey + Type: "internal", + UserID: callerID, + ExpiresAt: time.Now().Add(2 * time.Hour).Format(time.RFC3339), + } + + _, err := db.CreateApiKey(ctx, tempKey) + if err != nil { + return "", "", fmt.Errorf("failed to create temp API key: %w", err) + } + + // Use plaintext secret in connection string (before it was hashed) + connStr := fmt.Sprintf("host=%s@%s://%s:%s", plaintextSecret, schema, host, apiPort) + return connStr, keyName, nil +} diff --git a/src/controllers/machines_local_catalog_test.go b/src/controllers/machines_local_catalog_test.go index 04b8f70f..d8a4e4f0 100644 --- a/src/controllers/machines_local_catalog_test.go +++ b/src/controllers/machines_local_catalog_test.go @@ -2,11 +2,18 @@ package controllers import ( "net/http" + "os" + "path/filepath" + "strings" "testing" + "time" "github.com/Parallels/prl-devops-service/basecontext" + "github.com/Parallels/prl-devops-service/config" "github.com/Parallels/prl-devops-service/constants" + "github.com/Parallels/prl-devops-service/data" "github.com/Parallels/prl-devops-service/models" + "github.com/Parallels/prl-devops-service/serviceprovider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -86,3 +93,293 @@ func TestResolveCatalogMachineConnection_BothEmpty_CatalogDisabled(t *testing.T) } assert.Contains(t, err.Error(), "local catalog is not enabled") } + +// setupTestDBForController creates a temporary test database and configures +// the service provider for controller tests +func setupTestDBForController(t *testing.T) (*data.JsonDatabase, string, basecontext.ApiContext) { + t.Helper() + + // Create temporary directory for test database + tmpDir, err := os.MkdirTemp("", "prl-devops-controller-test-*") + require.NoError(t, err) + + dbFile := filepath.Join(tmpDir, "test_db.json") + + // Create context + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + + // Reset config + cfg := config.New(ctx) + require.NotNil(t, cfg) + + // Create and connect database + db := data.NewJsonDatabase(ctx, dbFile) + require.NotNil(t, db) + require.NoError(t, db.Connect(ctx)) + + // Set up service provider with test database + // Use NewMockProvider to ensure globalProvider is initialized + sp := serviceprovider.NewMockProvider() + require.NotNil(t, sp) + sp.JsonDatabase = db + + return db, tmpDir, ctx +} + +// cleanupTestDBForController cleans up test database and temp directory +func cleanupTestDBForController(t *testing.T, tmpDir string, db *data.JsonDatabase) { + t.Helper() + + if db != nil { + ctx := basecontext.NewBaseContext() + ctx.DisableLog() + _ = db.Disconnect(ctx) + } + + time.Sleep(50 * time.Millisecond) + + err := os.RemoveAll(tmpDir) + require.NoError(t, err) +} + +// TestBuildLocalCatalogConnection_CleanDNS tests normal DNS hostname +func TestBuildLocalCatalogConnection_CleanDNS(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "my-service-orchestrator.com") + t.Setenv(constants.API_PORT_ENV_VAR, "9999") + // Note: TLS enabled but without valid certs will default to HTTP + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + // Reload config to pick up env vars + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, keyName, err := buildLocalCatalogConnection(ctx, "caller-123", "job-456") + + require.NoError(t, err) + assert.Equal(t, "temp-vm-job-456", keyName) + + // Verify connection string format: host=@http://my-service-orchestrator.com:9999 + assert.Contains(t, connStr, "@http://my-service-orchestrator.com:9999") + assert.True(t, strings.HasPrefix(connStr, "host=")) + + // Verify API key was created + apiKey, err := db.GetApiKey(ctx, keyName) + require.NoError(t, err) + assert.NotNil(t, apiKey) + assert.Equal(t, "internal", apiKey.Type) + assert.Equal(t, "caller-123", apiKey.UserID) +} + +// TestBuildLocalCatalogConnection_HTTPSPrefix tests DNS with https:// prefix +func TestBuildLocalCatalogConnection_HTTPSPrefix(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "https://my-orchestrator.com") + t.Setenv(constants.API_PORT_ENV_VAR, "8080") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, keyName, err := buildLocalCatalogConnection(ctx, "user-1", "job-1") + + require.NoError(t, err) + assert.Equal(t, "temp-vm-job-1", keyName) + + // Should strip https:// prefix and use http (TLS disabled) + assert.Contains(t, connStr, "@http://my-orchestrator.com:8080") + assert.NotContains(t, connStr, "https://https://") +} + +// TestBuildLocalCatalogConnection_HTTPPrefix tests DNS with http:// prefix +func TestBuildLocalCatalogConnection_HTTPPrefix(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "http://orchestrator.local") + t.Setenv(constants.API_PORT_ENV_VAR, "7777") + // TLS disabled for this test + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "admin", "job-999") + + require.NoError(t, err) + // Should strip http:// prefix and use http (TLS disabled) + assert.Contains(t, connStr, "@http://orchestrator.local:7777") + assert.NotContains(t, connStr, "http://http://") +} + +// TestBuildLocalCatalogConnection_TrailingSlash tests DNS with trailing slash +func TestBuildLocalCatalogConnection_TrailingSlash(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "my-service.com/") + t.Setenv(constants.API_PORT_ENV_VAR, "5000") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "test-user", "job-abc") + + require.NoError(t, err) + // Should strip trailing slash + assert.Contains(t, connStr, "@http://my-service.com:5000") + assert.NotContains(t, connStr, "my-service.com/:") +} + +// TestBuildLocalCatalogConnection_WithPort tests DNS with port number +func TestBuildLocalCatalogConnection_WithPort(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "orchestrator.example.com:8888") + t.Setenv(constants.API_PORT_ENV_VAR, "9999") + // TLS disabled + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "user", "job-xyz") + + require.NoError(t, err) + // Should strip port 8888 and use apiPort 9999 with HTTP + assert.Contains(t, connStr, "@http://orchestrator.example.com:9999") + assert.NotContains(t, connStr, ":8888") +} + +// TestBuildLocalCatalogConnection_LocalhostDefault tests empty/whitespace falls back to localhost +func TestBuildLocalCatalogConnection_LocalhostDefault(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + // Don't set ORCHESTRATOR_PUBLIC_URL, should default to localhost + t.Setenv(constants.API_PORT_ENV_VAR, "3000") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "default-user", "job-default") + + require.NoError(t, err) + // Should use localhost default + assert.Contains(t, connStr, "@http://localhost:3000") +} + +// TestBuildLocalCatalogConnection_ComplexScenario tests multiple edge cases combined +func TestBuildLocalCatalogConnection_ComplexScenario(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + // Combination: https prefix + port + trailing slash + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "https://my-complex-host.io:7777/") + t.Setenv(constants.API_PORT_ENV_VAR, "9999") + // TLS disabled + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "complex-user", "job-complex") + + require.NoError(t, err) + // Should clean all: remove https://, remove :7777, remove /, use :9999 with HTTP + assert.Contains(t, connStr, "@http://my-complex-host.io:9999") + assert.NotContains(t, connStr, "https://https://") + assert.NotContains(t, connStr, ":7777") + assert.NotContains(t, connStr, "/:") +} + +// TestBuildLocalCatalogConnection_WhitespaceHandling tests whitespace in config +func TestBuildLocalCatalogConnection_WhitespaceHandling(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, " orchestrator.space.com ") + t.Setenv(constants.API_PORT_ENV_VAR, "4000") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, _, err := buildLocalCatalogConnection(ctx, "space-user", "job-space") + + require.NoError(t, err) + // Should trim whitespace + assert.Contains(t, connStr, "@http://orchestrator.space.com:4000") + assert.NotContains(t, connStr, " ") +} + +// TestBuildLocalCatalogConnection_APIKeyExpiration tests that temp keys have proper expiration +func TestBuildLocalCatalogConnection_APIKeyExpiration(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "test-host.local") + t.Setenv(constants.API_PORT_ENV_VAR, "8080") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + beforeCreate := time.Now() + _, keyName, err := buildLocalCatalogConnection(ctx, "test", "job-exp") + afterCreate := time.Now() + + require.NoError(t, err) + + // Verify key has expiration ~2 hours from now + apiKey, err := db.GetApiKey(ctx, keyName) + require.NoError(t, err) + require.NotEmpty(t, apiKey.ExpiresAt) + + expiresAt, err := time.Parse(time.RFC3339, apiKey.ExpiresAt) + require.NoError(t, err) + + // Should expire approximately 2 hours from creation (allow 1 minute tolerance) + expectedExpiry := beforeCreate.Add(2 * time.Hour) + assert.WithinDuration(t, expectedExpiry, expiresAt, 1*time.Minute) + assert.True(t, expiresAt.After(afterCreate)) +} + +// TestBuildLocalCatalogConnection_SecretInConnectionString tests that plaintext secret is used +func TestBuildLocalCatalogConnection_SecretInConnectionString(t *testing.T) { + db, tmpDir, ctx := setupTestDBForController(t) + defer cleanupTestDBForController(t, tmpDir, db) + + t.Setenv(constants.ORCHESTRATOR_PUBLIC_URL, "secret-test.com") + t.Setenv(constants.API_PORT_ENV_VAR, "6000") + t.Setenv(constants.TLS_ENABLED_ENV_VAR, "false") + + cfg := config.New(ctx) + require.NotNil(t, cfg) + + connStr, keyName, err := buildLocalCatalogConnection(ctx, "secret-user", "job-secret") + require.NoError(t, err) + + // Extract secret from connection string: host=@http://... + parts := strings.Split(connStr, "@") + require.Len(t, parts, 2, "connection string should have format: host=@") + + secretPart := strings.TrimPrefix(parts[0], "host=") + assert.NotEmpty(t, secretPart, "secret should be present in connection string") + + // Verify the secret in connection string is NOT the hashed version stored in DB + apiKey, err := db.GetApiKey(ctx, keyName) + require.NoError(t, err) + + // The secret in DB should be hashed, not matching the plaintext in connection string + assert.NotEqual(t, secretPart, apiKey.Secret, "DB should store hashed secret, not plaintext") + assert.NotEmpty(t, apiKey.Secret, "DB secret should be present") +} From bf941d30be3db2be7813bd499a934ed647225952 Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 3 Aug 2026 19:43:06 +0530 Subject: [PATCH 4/8] fix: enhance async VM creation handler to build connection string for catalog --- src/controllers/orchestrator.go | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/controllers/orchestrator.go b/src/controllers/orchestrator.go index 5b76b53b..7285b29c 100644 --- a/src/controllers/orchestrator.go +++ b/src/controllers/orchestrator.go @@ -4229,14 +4229,42 @@ func AsyncCreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { return } - go func(jobID string, req models.CreateVirtualMachineRequest) { + go func(jobID string, callerID string, req models.CreateVirtualMachineRequest) { asyncCtx := basecontext.NewRootBaseContext() + + // Track temp key name for cleanup + var tempKeyName string + + // CLEANUP DEFER: Must be FIRST so it runs LAST (after panic recovery) + defer func() { + if tempKeyName != "" { + db := serviceprovider.Get().JsonDatabase + if db != nil { + _ = db.Connect(asyncCtx) + _ = db.DeleteApiKey(asyncCtx, tempKeyName) + } + } + }() + + // PANIC RECOVERY: Second defer, runs before cleanup defer func() { if rec := recover(); rec != nil { asyncCtx.LogErrorf("[Orchestrator] Panic in async create goroutine for job %s: %v", jobID, rec) _ = jobManager.MarkJobError(jobID, fmt.Errorf("internal error: %v", rec)) } }() + + // NEW: Build connection string if orchestrator is the catalog + if req.CatalogManifest != nil && req.CatalogManifest.Connection == "" { + connStr, keyName, err := buildLocalCatalogConnection(asyncCtx, callerID, jobID) + if err != nil { + _ = jobManager.MarkJobError(jobID, fmt.Errorf("failed to resolve catalog connection: %w", err)) + return + } + req.CatalogManifest.Connection = connStr + tempKeyName = keyName + } + _, _ = jobManager.UpdateJobProgress(jobID, 1, constants.JobStateRunning) orchSvc := orchestrator.NewOrchestratorService(asyncCtx) result, apiErr := orchSvc.DispatchCreateVirtualMachine(asyncCtx, jobID, req) @@ -4249,7 +4277,7 @@ func AsyncCreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { return } _ = jobManager.MarkJobCompleteWithRecord(jobID, fmt.Sprintf("Virtual machine %s created", result.ID), result.ID, result.Name, "virtual_machine", result.Host) - }(job.ID, request) + }(job.ID, callerID, request) response := mappers.MapJobToApiJob(*job) w.WriteHeader(http.StatusAccepted) From 0d60baa19e54980d0321ddbc18baa40b2314bacf Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 3 Aug 2026 19:52:03 +0530 Subject: [PATCH 5/8] feat: implement cleanup for orphaned temporary API keys and add method to retrieve all API keys including internal ones --- src/data/api_key.go | 22 +++++++++++++ src/orchestrator/main.go | 67 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/src/data/api_key.go b/src/data/api_key.go index 3d85aae1..e8836fe3 100644 --- a/src/data/api_key.go +++ b/src/data/api_key.go @@ -85,6 +85,28 @@ func (j *JsonDatabase) GetApiKeys(ctx basecontext.ApiContext, filter string) ([] return filteredData, nil } +// GetAllApiKeysIncludingInternal returns all API keys including internal ones +// This is used for administrative tasks like cleanup and should NOT be exposed via API +func (j *JsonDatabase) GetAllApiKeysIncludingInternal(ctx basecontext.ApiContext) ([]models.ApiKey, error) { + if !j.IsConnected() { + return nil, ErrDatabaseNotConnected + } + + j.dataMutex.RLock() + defer j.dataMutex.RUnlock() + + // Return a copy to avoid mutation issues + result := make([]models.ApiKey, len(j.data.ApiKeys)) + copy(result, j.data.ApiKeys) + + // Normalize types for consistency + for i := range result { + normalizeApiKeyType(&result[i]) + } + + return result, nil +} + func (j *JsonDatabase) GetApiKey(ctx basecontext.ApiContext, idOrName string) (*models.ApiKey, error) { if !j.IsConnected() { return nil, ErrDatabaseNotConnected diff --git a/src/orchestrator/main.go b/src/orchestrator/main.go index 9f79d912..3451f982 100644 --- a/src/orchestrator/main.go +++ b/src/orchestrator/main.go @@ -2,6 +2,7 @@ package orchestrator import ( "context" + "strings" "sync" "time" @@ -126,6 +127,21 @@ func (s *OrchestratorService) Start(waitForInit bool) { // Background: periodic full refresh (self-healing) on a longer interval. go s.runFullRefreshLoop() + // Background: periodic cleanup of orphaned temp keys (every hour) + go func() { + cleanupTicker := time.NewTicker(1 * time.Hour) + defer cleanupTicker.Stop() + + for { + select { + case <-s.syncContext.Done(): + return + case <-cleanupTicker.C: + cleanupOrphanedTempKeys(s.ctx) + } + } + }() + // Background: lightweight health check on every refreshInterval tick. for { select { @@ -659,3 +675,54 @@ func getHostName(host models.OrchestratorHost) string { } return host.Host } + +// cleanupOrphanedTempKeys removes expired temporary API keys created for VM operations +// This is a safety net for keys that weren't cleaned up by the normal defer mechanism +func cleanupOrphanedTempKeys(ctx basecontext.ApiContext) { + db := serviceprovider.Get().JsonDatabase + if db == nil { + return + } + + if err := db.Connect(ctx); err != nil { + ctx.LogErrorf("[Orchestrator] Failed to connect to database for temp key cleanup: %v", err) + return + } + + // CRITICAL: Use GetAllApiKeysIncludingInternal to see internal keys + // GetApiKeys() filters out internal keys after Task 2, so we can't use it + allKeys, err := db.GetAllApiKeysIncludingInternal(ctx) + if err != nil { + ctx.LogErrorf("[Orchestrator] Failed to get API keys for cleanup: %v", err) + return + } + + cutoff := time.Now().Add(-3 * time.Hour) + deletedCount := 0 + + for _, k := range allKeys { + // Only process internal temp keys + if k.Type != "internal" { + continue + } + if !strings.HasPrefix(k.Name, "temp-vm-") { + continue + } + + // Check if expired more than 3 hours ago (temp keys expire in 2 hours) + if k.ExpiresAt != "" { + expiresAt, err := time.Parse(time.RFC3339, k.ExpiresAt) + if err == nil && expiresAt.Before(cutoff) { + if err := db.DeleteApiKey(ctx, k.ID); err == nil { + deletedCount++ + } else { + ctx.LogErrorf("[Orchestrator] Failed to delete orphaned temp key %s: %v", k.Name, err) + } + } + } + } + + if deletedCount > 0 { + ctx.LogInfof("[Orchestrator] Cleaned up %d orphaned temporary API keys", deletedCount) + } +} From b4c05398f4fb6efd90f5d585760467cd91ea87c7 Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Sun, 9 Aug 2026 11:35:21 +0530 Subject: [PATCH 6/8] feat: enhance temp API key handling and logging for orchestrator VM creation --- src/controllers/machines.go | 16 +++++- src/controllers/orchestrator.go | 95 ++++++++++++++++++++++++--------- src/startup/main.go | 3 ++ 3 files changed, 86 insertions(+), 28 deletions(-) diff --git a/src/controllers/machines.go b/src/controllers/machines.go index 123271a9..7049d824 100644 --- a/src/controllers/machines.go +++ b/src/controllers/machines.go @@ -1,6 +1,7 @@ package controllers import ( + "encoding/base64" "encoding/json" "fmt" "net/http" @@ -2091,6 +2092,8 @@ func buildLocalCatalogConnection(ctx basecontext.ApiContext, callerID string, jo } host := cfg.OrchestratorPublicUrl() + ctx.LogDebugf("[Temp API Key] ORCHESTRATOR_PUBLIC_URL from config: '%s'", host) + ctx.LogDebugf("[Temp API Key] API Port: %s, TLS Enabled: %v, Schema: %s", apiPort, cfg.TlsEnabled(), schema) // Sanitize host to handle various input formats: // - Remove protocol prefixes (http://, https://) @@ -2136,7 +2139,16 @@ func buildLocalCatalogConnection(ctx basecontext.ApiContext, callerID string, jo return "", "", fmt.Errorf("failed to create temp API key: %w", err) } - // Use plaintext secret in connection string (before it was hashed) - connStr := fmt.Sprintf("host=%s@%s://%s:%s", plaintextSecret, schema, host, apiPort) + // Connection string format for API key authentication: + // host=@protocol://host:port + // API key format: base64(keyName:secret) + apiKeyValue := keyName + ":" + plaintextSecret + encodedApiKey := base64.StdEncoding.EncodeToString([]byte(apiKeyValue)) + connStr := fmt.Sprintf("host=%s@%s://%s:%s", encodedApiKey, schema, host, apiPort) + + ctx.LogInfof("[Temp API Key] Created temp key '%s' for job %s", keyName, jobID) + ctx.LogInfof("[Temp API Key] Connection string: host=@%s://%s:%s", schema, host, apiPort) + ctx.LogInfof("[Temp API Key] Base64 API key starts with: %s...", encodedApiKey[:20]) + return connStr, keyName, nil } diff --git a/src/controllers/orchestrator.go b/src/controllers/orchestrator.go index 7285b29c..109d3559 100644 --- a/src/controllers/orchestrator.go +++ b/src/controllers/orchestrator.go @@ -4213,6 +4213,75 @@ func AsyncCreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { ReturnApiError(ctx, w, models.NewFromError(connErr)) return } + + ctx.LogDebugf("[Async VM] catalogConnection='%s', IsCatalog=%v", catalogConnection, config.Get().IsCatalog()) + + // TEMP API KEY LOGIC: If orchestrator IS catalog and connection is empty, + // build temp API key connection string before entering goroutine + if catalogConnection == "" && config.Get().IsCatalog() { + ctx.LogInfof("[Temp API Key] Building temp credentials for async VM creation") + + jobManager := jobs.Get(ctx) + if jobManager == nil { + ReturnApiError(ctx, w, models.NewFromErrorWithCode(fmt.Errorf("job manager not available"), http.StatusInternalServerError)) + return + } + + job, err := jobManager.CreateNewJob(callerID, "orchestrator", "create", "Initializing orchestrator virtual machine creation") + if err != nil { + ReturnApiError(ctx, w, models.NewFromErrorWithCode(err, http.StatusInternalServerError)) + return + } + + connStr, _, err := buildLocalCatalogConnection(ctx, callerID, job.ID) + if err != nil { + ReturnApiError(ctx, w, models.ApiErrorResponse{ + Message: "Failed to create temporary catalog credentials: " + err.Error(), + Code: http.StatusInternalServerError, + }) + return + } + request.CatalogManifest.Connection = connStr + request.CatalogManifest.CatalogManagerId = "" + + // Spawn dedicated goroutine for async dispatch + // NOTE: Temp key cleanup is handled by: + // 1. Background cleanup process (hourly) for orphaned keys + // 2. Key expiration (2 hours) + // We DON'T clean up here because the remote host needs the key + // to pull the catalog AFTER this goroutine completes. + go func(jobID string, req models.CreateVirtualMachineRequest) { + asyncCtx := basecontext.NewRootBaseContext() + + defer func() { + if rec := recover(); rec != nil { + asyncCtx.LogErrorf("[Orchestrator] Panic in async create with temp key for job %s: %v", jobID, rec) + _ = jobManager.MarkJobError(jobID, fmt.Errorf("internal error: %v", rec)) + } + }() + + _, _ = jobManager.UpdateJobProgress(jobID, 1, constants.JobStateRunning) + orchSvc := orchestrator.NewOrchestratorService(asyncCtx) + result, apiErr := orchSvc.DispatchCreateVirtualMachine(asyncCtx, jobID, req) + if apiErr != nil { + _ = jobManager.MarkJobError(jobID, fmt.Errorf("%s", apiErr.Message)) + return + } + if result == nil { + // Async dispatch to remote host - remote will complete the job + asyncCtx.LogInfof("[Temp API Key] Remote host will use temp key for job %s", jobID) + return + } + _ = jobManager.MarkJobCompleteWithRecord(jobID, fmt.Sprintf("Virtual machine %s created", result.ID), result.ID, result.Name, "virtual_machine", result.Host) + }(job.ID, request) + + response := mappers.MapJobToApiJob(*job) + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(response) + ctx.LogInfof("Async orchestrator machine create started with temp API key, job ID: %v", response.ID) + return + } + request.CatalogManifest.Connection = catalogConnection request.CatalogManifest.CatalogManagerId = "" } @@ -4232,21 +4301,6 @@ func AsyncCreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { go func(jobID string, callerID string, req models.CreateVirtualMachineRequest) { asyncCtx := basecontext.NewRootBaseContext() - // Track temp key name for cleanup - var tempKeyName string - - // CLEANUP DEFER: Must be FIRST so it runs LAST (after panic recovery) - defer func() { - if tempKeyName != "" { - db := serviceprovider.Get().JsonDatabase - if db != nil { - _ = db.Connect(asyncCtx) - _ = db.DeleteApiKey(asyncCtx, tempKeyName) - } - } - }() - - // PANIC RECOVERY: Second defer, runs before cleanup defer func() { if rec := recover(); rec != nil { asyncCtx.LogErrorf("[Orchestrator] Panic in async create goroutine for job %s: %v", jobID, rec) @@ -4254,17 +4308,6 @@ func AsyncCreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { } }() - // NEW: Build connection string if orchestrator is the catalog - if req.CatalogManifest != nil && req.CatalogManifest.Connection == "" { - connStr, keyName, err := buildLocalCatalogConnection(asyncCtx, callerID, jobID) - if err != nil { - _ = jobManager.MarkJobError(jobID, fmt.Errorf("failed to resolve catalog connection: %w", err)) - return - } - req.CatalogManifest.Connection = connStr - tempKeyName = keyName - } - _, _ = jobManager.UpdateJobProgress(jobID, 1, constants.JobStateRunning) orchSvc := orchestrator.NewOrchestratorService(asyncCtx) result, apiErr := orchSvc.DispatchCreateVirtualMachine(asyncCtx, jobID, req) diff --git a/src/startup/main.go b/src/startup/main.go index ddad11a4..09fc3bd0 100644 --- a/src/startup/main.go +++ b/src/startup/main.go @@ -164,6 +164,9 @@ func Start(ctx basecontext.ApiContext) { if cfg.IsOrchestrator() { ctx := basecontext.NewRootBaseContext() ctx.LogInfof("Starting Orchestrator Background Service") + ctx.LogInfof("[Orchestrator] Public URL: %s", cfg.OrchestratorPublicUrl()) + ctx.LogInfof("[Orchestrator] API Port: %s, TLS: %v", cfg.ApiPort(), cfg.TlsEnabled()) + ctx.LogInfof("[Orchestrator] Catalog Enabled: %v", cfg.IsCatalog()) canUseOwnResources := false if system.GetOperatingSystem() == "linux" { canUseOwnResources = false From 0d6aa1401291d3dfb44588b6f528807cbe7ec13d Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Sun, 9 Aug 2026 11:54:13 +0530 Subject: [PATCH 7/8] feat: enhance catalog connection handling in VM creation with temp API key generation --- src/controllers/orchestrator.go | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/controllers/orchestrator.go b/src/controllers/orchestrator.go index 109d3559..51f3aa20 100644 --- a/src/controllers/orchestrator.go +++ b/src/controllers/orchestrator.go @@ -3084,16 +3084,6 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { } } - if request.CatalogManifest != nil { - catalogConnection, connErr := resolveCatalogMachineConnection(ctx, request.CatalogManifest) - if connErr != nil { - ReturnApiError(ctx, w, models.NewFromError(connErr)) - return - } - request.CatalogManifest.Connection = catalogConnection - request.CatalogManifest.CatalogManagerId = "" - } - callerID, ok := getEffectiveCallerID(ctx) if !ok { ReturnApiError(ctx, w, models.ApiErrorResponse{Code: http.StatusUnauthorized, Message: "User not found"}) @@ -3112,6 +3102,29 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler { return } + if request.CatalogManifest != nil { + catalogConnection, connErr := resolveCatalogMachineConnection(ctx, request.CatalogManifest) + if connErr != nil { + ReturnApiError(ctx, w, models.NewFromError(connErr)) + return + } + + // Generate temp API key if orchestrator IS the catalog and no connection provided + if catalogConnection == "" && config.Get().IsCatalog() { + ctx.LogInfof("[Temp API Key] Building temp credentials for direct VM creation") + connStr, _, buildErr := buildLocalCatalogConnection(ctx, callerID, job.ID) + if buildErr != nil { + _ = jobManager.MarkJobError(job.ID, buildErr) + ReturnApiError(ctx, w, models.NewFromErrorWithCode(buildErr, http.StatusInternalServerError)) + return + } + catalogConnection = connStr + } + + request.CatalogManifest.Connection = catalogConnection + request.CatalogManifest.CatalogManagerId = "" + } + _, _ = jobManager.UpdateJobProgress(job.ID, 1, constants.JobStateRunning) orchestratorSvc := orchestrator.NewOrchestratorService(ctx) response, err := orchestratorSvc.CreateVirtualMachine(ctx, job.ID, request) From d3ed585917f67b94d3c4eccacfd127e8a98bd089 Mon Sep 17 00:00:00 2001 From: Sai Kumar P Date: Mon, 10 Aug 2026 14:36:53 +0530 Subject: [PATCH 8/8] refactor: update key naming convention from 'temp-vm-' to 'local-catalog-' across relevant functions and tests --- src/controllers/machines.go | 2 +- src/controllers/machines_local_catalog_test.go | 4 ++-- src/data/api_key_test.go | 16 ++++++++-------- src/orchestrator/main.go | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/controllers/machines.go b/src/controllers/machines.go index 7049d824..95c6370d 100644 --- a/src/controllers/machines.go +++ b/src/controllers/machines.go @@ -2111,7 +2111,7 @@ func buildLocalCatalogConnection(ctx basecontext.ApiContext, callerID string, jo } // Generate unique key name and secret - keyName := "temp-vm-" + jobID + keyName := "local-catalog-" + jobID fullSecret := helpers.GenerateId() // Limit secret to 40 chars (password hashing limitation) plaintextSecret := fullSecret diff --git a/src/controllers/machines_local_catalog_test.go b/src/controllers/machines_local_catalog_test.go index d8a4e4f0..6c149a43 100644 --- a/src/controllers/machines_local_catalog_test.go +++ b/src/controllers/machines_local_catalog_test.go @@ -160,7 +160,7 @@ func TestBuildLocalCatalogConnection_CleanDNS(t *testing.T) { connStr, keyName, err := buildLocalCatalogConnection(ctx, "caller-123", "job-456") require.NoError(t, err) - assert.Equal(t, "temp-vm-job-456", keyName) + assert.Equal(t, "local-catalog-job-456", keyName) // Verify connection string format: host=@http://my-service-orchestrator.com:9999 assert.Contains(t, connStr, "@http://my-service-orchestrator.com:9999") @@ -189,7 +189,7 @@ func TestBuildLocalCatalogConnection_HTTPSPrefix(t *testing.T) { connStr, keyName, err := buildLocalCatalogConnection(ctx, "user-1", "job-1") require.NoError(t, err) - assert.Equal(t, "temp-vm-job-1", keyName) + assert.Equal(t, "local-catalog-job-1", keyName) // Should strip https:// prefix and use http (TLS disabled) assert.Contains(t, connStr, "@http://my-orchestrator.com:8080") diff --git a/src/data/api_key_test.go b/src/data/api_key_test.go index a919ee92..82c9cb95 100644 --- a/src/data/api_key_test.go +++ b/src/data/api_key_test.go @@ -511,11 +511,11 @@ func TestInternalKeysNotInListButRetrievableByID(t *testing.T) { ctx := basecontext.NewBaseContext() ctx.DisableLog() - // Create a temp internal key (like temp-vm-* keys) + // Create a temp internal key (like local-catalog-* keys) tempKey := models.ApiKey{ - ID: "temp-vm-job123", - Name: "temp-vm-job123", - Key: "temp-vm-job123", + ID: "local-catalog-job123", + Name: "local-catalog-job123", + Key: "local-catalog-job123", Secret: "temporary-secret", Type: "internal", ExpiresAt: time.Now().Add(2 * time.Hour).Format(time.RFC3339), @@ -541,15 +541,15 @@ func TestInternalKeysNotInListButRetrievableByID(t *testing.T) { assert.Equal(t, "regular-key", allKeys[0].ID) // Verify internal key is still retrievable by ID (needed for auth) - loadedTemp, err := db.GetApiKey(ctx, "temp-vm-job123") + loadedTemp, err := db.GetApiKey(ctx, "local-catalog-job123") require.NoError(t, err) - assert.Equal(t, "temp-vm-job123", loadedTemp.ID) + assert.Equal(t, "local-catalog-job123", loadedTemp.ID) assert.Equal(t, "internal", loadedTemp.Type) // Verify can also retrieve by Name - loadedByName, err := db.GetApiKey(ctx, "temp-vm-job123") + loadedByName, err := db.GetApiKey(ctx, "local-catalog-job123") require.NoError(t, err) - assert.Equal(t, "temp-vm-job123", loadedByName.ID) + assert.Equal(t, "local-catalog-job123", loadedByName.ID) assert.Equal(t, "internal", loadedByName.Type) } diff --git a/src/orchestrator/main.go b/src/orchestrator/main.go index 3451f982..53a724a8 100644 --- a/src/orchestrator/main.go +++ b/src/orchestrator/main.go @@ -705,7 +705,7 @@ func cleanupOrphanedTempKeys(ctx basecontext.ApiContext) { if k.Type != "internal" { continue } - if !strings.HasPrefix(k.Name, "temp-vm-") { + if !strings.HasPrefix(k.Name, "local-catalog-") { continue }