Conversation
- 1.1 Add AzureConfigSchema to schema.ts - 1.2 Add Azure parsing to ConfigService.parseIntegrationsConfig() - 1.3 Add getAzureConfig() accessor to ConfigService - 1.4 Update .env.example with Azure section - 2.1 Create Azure types and AzureAuthenticationError - 3.1 Install Azure SDK dependencies - 3.2-3.6 Implement AzureService with credential validation, inventory, groups, facts, provisioning, lifecycle, and resource discovery methods
- 5.1-5.6 Implement AzurePlugin extending BasePlugin with InformationSourcePlugin and ExecutionToolPlugin interfaces - 6.1 Add 'azure' to JournalSourceSchema - 6.2 Add collectAzureVMStateEntry to JournalCollectors - 7.1 Add 'azure' entry to IntegrationColorService
…, and color service - 6.2 Add collectAzureVMStateEntry to JournalCollectors - 7.1 Add 'azure' entry to IntegrationColorService - 8.1 Create Azure API routes with all endpoints - 8.2 Register Azure routes and plugin in server.ts
- 10.1 Create AzureSetupGuide.svelte with form fields, env snippet generation, masking, and clipboard copy - 10.2 Export AzureSetupGuide in components/index.ts - 10.3 Register AzureSetupGuide in IntegrationSetupPage
- 11.1 Create docs/integrations/azure.md with prerequisites, configuration, authentication, RBAC, features, troubleshooting - 11.2 Update scripts/setup.sh with Azure integration prompts
There was a problem hiding this comment.
Pull request overview
This PR adds first-party Microsoft Azure VM integration to Pabawi, including backend plugin/service/router support, frontend setup guidance, and documentation/setup-script wiring.
Changes:
- Add a new Azure integration plugin/service (inventory, facts, lifecycle, provisioning, resource discovery) and expose API routes under
/api/integrations/azure. - Add a frontend
AzureSetupGuideand register it in the integration setup page. - Add Azure configuration parsing/docs/.env example/setup.sh prompts, plus Azure SDK dependencies and journal/source/color support.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup.sh | Adds Azure prompts and writes AZURE_* vars to generated backend/.env. |
| package-lock.json | Locks newly added Azure SDK dependency tree. |
| frontend/src/pages/IntegrationSetupPage.svelte | Adds an integration === 'azure' branch rendering AzureSetupGuide. |
| frontend/src/components/index.ts | Exports AzureSetupGuide. |
| frontend/src/components/AzureSetupGuide.svelte | New UI to generate/copy a .env snippet for Azure integration. |
| docs/integrations/azure.md | New Azure integration documentation (auth/config/features/troubleshooting). |
| backend/src/services/journal/types.ts | Adds "azure" to JournalSourceSchema. |
| backend/src/services/journal/JournalCollectors.ts | Adds Azure VM power-state journal collector utilities. |
| backend/src/services/IntegrationColorService.ts | Adds Azure color palette entry. |
| backend/src/server.ts | Registers AzurePlugin and mounts /api/integrations/azure routes conditionally. |
| backend/src/routes/integrations/azure.ts | New Azure integration router: inventory, provision, lifecycle, test, discovery endpoints. |
| backend/src/integrations/azure/types.ts | Defines Azure config/resource types and AzureAuthenticationError. |
| backend/src/integrations/azure/AzureService.ts | Azure SDK wrapper: inventory, facts, provisioning, lifecycle, discovery. |
| backend/src/integrations/azure/AzurePlugin.ts | BasePlugin implementation for Azure (info source + execution tool + journaling). |
| backend/src/integrations/azure/AzureHelpers.ts | Pure helpers: node/facts transforms + grouping + nodeId parsing. |
| backend/src/config/schema.ts | Adds AzureConfigSchema and includes it in IntegrationsConfigSchema. |
| backend/src/config/ConfigService.ts | Parses AZURE_* env vars and adds getAzureConfig(). |
| backend/package.json | Adds Azure SDK dependencies. |
| backend/.env.example | Documents Azure env vars in the example env file. |
| README.md | Updates feature list and version history entry. |
| CHANGELOG.md | Adds a 1.1.0 changelog section and adjusts older formatting. |
| .kiro/specs/azure-support/requirements.md | Removes older Azure spec requirements doc. |
| .kiro/specs/azure-support/design.md | Removes older Azure spec design doc. |
| .kiro/specs/azure-support/.config.kiro | Removes older Azure spec config. |
| .kiro/specs/azure-integration/tasks.md | Adds new Azure integration implementation plan. |
| .kiro/specs/azure-integration/requirements.md | Adds new Azure integration requirements document. |
| .kiro/specs/azure-integration/design.md | Adds new Azure integration design document. |
| .kiro/specs/azure-integration/.config.kiro | Adds new Azure integration spec config. |
| async getImages(publisher?: string, offer?: string, sku?: string): Promise<AzureImageInfo[]> { | ||
| if (!publisher || !offer || !sku) return []; | ||
| const result = await this.computeClient.virtualMachineImages.list("eastus", publisher, offer, sku); | ||
| return result.map((img) => ({ publisher, offer, sku, version: img.name ?? "latest" })); |
There was a problem hiding this comment.
AzureService.getImages hardcodes the location to "eastus". This makes image discovery incorrect for other regions and conflicts with the UI’s "Default Location" concept. Consider accepting a location parameter (query param) and passing it through from the router, or deriving a default from configuration instead of hardcoding.
| async getImages(publisher?: string, offer?: string, sku?: string): Promise<AzureImageInfo[]> { | |
| if (!publisher || !offer || !sku) return []; | |
| const result = await this.computeClient.virtualMachineImages.list("eastus", publisher, offer, sku); | |
| return result.map((img) => ({ publisher, offer, sku, version: img.name ?? "latest" })); | |
| async getImages(location?: string, publisher?: string, offer?: string, sku?: string): Promise<AzureImageInfo[]> { | |
| if (!publisher || !offer || !sku) return []; | |
| const resolvedLocation = location ?? (await this.getLocations())[0]?.name; | |
| if (!resolvedLocation) return []; | |
| const result = await this.computeClient.virtualMachineImages.list(resolvedLocation, publisher, offer, sku); | |
| const images = result.map((img) => ({ publisher, offer, sku, version: img.name ?? "latest" })); | |
| this.logger.info("Azure VM images fetched", { | |
| component: "AzureService", | |
| operation: "getImages", | |
| metadata: { location: resolvedLocation, publisher, offer, sku, count: images.length }, | |
| }); | |
| return images; |
| private buildFailedResult( | ||
| action: Action, | ||
| startedAt: string, | ||
| target: string, | ||
| errorMessage: string, | ||
| ): ExecutionResult { | ||
| return { | ||
| id: `azure-error-${String(Date.now())}`, | ||
| type: "command", | ||
| targetNodes: [target], | ||
| action: action.action, | ||
| status: "failed", | ||
| startedAt, | ||
| completedAt: new Date().toISOString(), | ||
| results: [ | ||
| { | ||
| nodeId: target, | ||
| status: "failed", | ||
| error: errorMessage, | ||
| duration: 0, | ||
| }, | ||
| ], | ||
| error: errorMessage, | ||
| }; |
There was a problem hiding this comment.
buildFailedResult always sets ExecutionResult.type to "command". For provisioning actions (type "task"), returning a failed result with type "command" is inconsistent and may break UI logic that relies on the type. Consider setting type based on the incoming action (e.g., action.type) or based on action.action (provision/create_vm → "task").
| /** | ||
| * Collect Azure VM state change entry for a virtual machine. | ||
| * Calls getNodeFacts to get current state, compares against last recorded | ||
| * state in journal_entries. Returns 0 or 1 JournalEntry with deterministic ID. | ||
| */ | ||
| export async function collectAzureVMStateEntry( | ||
| azureService: AzureServiceLike, | ||
| vmName: string, | ||
| resourceGroup: string, | ||
| db: DatabaseAdapter, | ||
| nodeId: string, | ||
| ): Promise<JournalEntry[]> { | ||
| const logger = new LoggerService(); | ||
|
|
||
| // 1. Get current state from Azure | ||
| let currentState: string; | ||
| try { | ||
| const factsResult = await azureService.getNodeFacts(nodeId); | ||
| const system = factsResult.facts.categories?.system; | ||
| if (!system?.powerState) { | ||
| logger.warn("Azure facts missing system powerState", { | ||
| component: "JournalCollectors", | ||
| integration: "azure", | ||
| operation: "collectAzureVMStateEntry", | ||
| }); | ||
| return []; | ||
| } | ||
| currentState = system.powerState; | ||
| } catch (error: unknown) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| logger.warn(`Failed to fetch Azure facts for ${nodeId}: ${message}`, { | ||
| component: "JournalCollectors", | ||
| integration: "azure", | ||
| operation: "collectAzureVMStateEntry", | ||
| }); | ||
| return []; | ||
| } | ||
|
|
||
| // 2. Query last recorded state from journal_entries | ||
| let previousState: string | undefined; | ||
| try { | ||
| const rows = await db.query<{ details: string }>( | ||
| `SELECT details FROM journal_entries | ||
| WHERE nodeId = ? AND source = 'azure' AND action LIKE 'Azure VM state change:%' | ||
| ORDER BY timestamp DESC | ||
| LIMIT 1`, | ||
| [nodeId], | ||
| ); | ||
| if (rows.length > 0) { | ||
| const details = typeof rows[0].details === "string" | ||
| ? (JSON.parse(rows[0].details) as Record<string, unknown>) | ||
| : (rows[0].details as Record<string, unknown>); | ||
| previousState = details.currentState as string | undefined; | ||
| } | ||
| } catch (error: unknown) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| logger.warn(`Failed to query last Azure state for ${nodeId}: ${message}`, { | ||
| component: "JournalCollectors", | ||
| integration: "azure", | ||
| operation: "collectAzureVMStateEntry", | ||
| }); | ||
| // Treat as "no previous state known" — record the current state | ||
| } | ||
|
|
||
| // 3. If state hasn't changed, return empty | ||
| if (previousState === currentState) { | ||
| return []; | ||
| } | ||
|
|
||
| // 4. Create new journal entry for the state change | ||
| const eventType = mapAzurePowerStateToEventType(currentState); | ||
| const timestamp = new Date().toISOString(); | ||
|
|
||
| return [ | ||
| { | ||
| id: `azure:state:${vmName}:${currentState}:${timestamp}`, | ||
| nodeId, | ||
| nodeUri: `azure:${resourceGroup}:${vmName}`, | ||
| eventType, | ||
| source: "azure", | ||
| action: `Azure VM state change: ${previousState ?? "unknown"} → ${currentState}`, | ||
| summary: `Azure VM ${vmName} ${currentState}`, |
There was a problem hiding this comment.
Comment says the collector returns a journal entry with a deterministic ID, but the ID includes timestamp (ISO string), making it non-deterministic and causing a new ID on every run even if state hasn’t changed. Either update the comment or change the ID scheme (e.g., hash of nodeId+currentState+minute bucket, or let the DB generate IDs) to match the intended behavior.
| function generateEnvSnippet(): string { | ||
| const lines: string[] = [ | ||
| '# Azure Integration Configuration', | ||
| 'AZURE_ENABLED=true', | ||
| `AZURE_SUBSCRIPTION_ID=${config.subscriptionId}`, | ||
| ]; | ||
|
|
||
| if (config.tenantId) { | ||
| lines.push(`AZURE_TENANT_ID=${config.tenantId}`); | ||
| } | ||
|
|
||
| if (config.clientId) { | ||
| lines.push(`AZURE_CLIENT_ID=${config.clientId}`); | ||
| } | ||
|
|
||
| if (config.clientSecret) { | ||
| lines.push(`AZURE_CLIENT_SECRET=${config.clientSecret}`); // pragma: allowlist secret | ||
| } | ||
|
|
||
| if (config.resourceGroups) { | ||
| lines.push(`AZURE_RESOURCE_GROUPS=${config.resourceGroups}`); | ||
| } | ||
|
|
||
| if (config.defaultLocation) { | ||
| lines.push(`AZURE_DEFAULT_LOCATION=${config.defaultLocation}`); | ||
| } | ||
|
|
||
| return lines.join('\n'); | ||
| } | ||
|
|
||
| function maskSensitiveValues(snippet: string): string { | ||
| return snippet | ||
| .split('\n') | ||
| .map((line) => { | ||
| if (line.startsWith('#')) return line; | ||
| const eqIndex = line.indexOf('='); | ||
| if (eqIndex === -1) return line; | ||
| const key = line.substring(0, eqIndex); | ||
| if (sensitiveKeys.has(key)) { | ||
| const value = line.substring(eqIndex + 1); | ||
| if (value) { | ||
| return `${key}=${'*'.repeat(Math.min(value.length, 20))}`; | ||
| } | ||
| } | ||
| return line; | ||
| }) | ||
| .join('\n'); | ||
| } | ||
|
|
||
| const envSnippet = $derived(generateEnvSnippet()); | ||
| const maskedSnippet = $derived(maskSensitiveValues(envSnippet)); | ||
|
|
||
| async function copyToClipboard(): Promise<void> { | ||
| try { | ||
| await navigator.clipboard.writeText(envSnippet); | ||
| copied = true; | ||
| showSuccess('Copied to clipboard'); | ||
| setTimeout(() => { copied = false; }, 2000); | ||
| } catch { | ||
| showError('Failed to copy — please select and copy manually'); | ||
| } | ||
| } |
There was a problem hiding this comment.
AzureSetupGuide is a new setup wizard component, but there are no frontend tests added for snippet generation/masking and copy-to-clipboard behavior. Other setup guides in this repo (e.g., AWSSetupGuide.test.ts) are covered; adding similar tests here would help ensure secrets stay masked in the preview and required fields gate the UI correctly.
| ask AZURE_TENANT_ID "Azure Tenant ID" "" | ||
| ask AZURE_CLIENT_ID "Azure Client ID (Service Principal)" "" | ||
| ask AZURE_CLIENT_SECRET "Azure Client Secret" "" | ||
| ask AZURE_SUBSCRIPTION_ID "Azure Subscription ID" "" |
There was a problem hiding this comment.
The setup script allows enabling Azure integration without ensuring AZURE_SUBSCRIPTION_ID is provided, but the Azure plugin requires it and will fail initialization if it's empty. Consider validating that AZURE_SUBSCRIPTION_ID is non-empty when AZURE_ENABLED=true (re-prompt or abort) before writing backend/.env.
| ask AZURE_SUBSCRIPTION_ID "Azure Subscription ID" "" | |
| ask AZURE_SUBSCRIPTION_ID "Azure Subscription ID" "" | |
| while [[ -z "$AZURE_SUBSCRIPTION_ID" ]]; do | |
| error "Azure Subscription ID is required when Azure integration is enabled." | |
| ask AZURE_SUBSCRIPTION_ID "Azure Subscription ID" "" | |
| done |
| const entry: CreateJournalEntry = { | ||
| nodeId: target, | ||
| nodeUri: `azure:${target}`, | ||
| eventType, | ||
| source: "azure" as JournalSource, | ||
| action: action.action, | ||
| summary: | ||
| result.status === "success" | ||
| ? `Azure ${action.action} succeeded on ${target}` | ||
| : `Azure ${action.action} failed on ${target}: ${result.error ?? "unknown error"}`, | ||
| details: { | ||
| status: result.status, | ||
| parameters: action.parameters, | ||
| ...(result.error ? { error: result.error } : {}), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Journal entries are recorded with nodeId=target and nodeUri=azure:${target}. When target already starts with "azure:", this produces double-prefixed URIs (e.g., "azure:azure:...") and, for lifecycle routes, nodeId is currently in the non-canonical "azure:{rg}:{vm}" format. This will prevent journal entries from resolving/linking to inventory nodes whose IDs are "azure:{subscriptionId}:{resourceGroup}:{vmName}". Recommend normalizing to the canonical nodeId and using a canonical nodeUri (e.g., azure:${resourceGroup}:${vmName}) before recording.
| export function createAzureRouter( | ||
| azurePlugin: AzurePlugin, | ||
| integrationManager?: IntegrationManager, | ||
| options?: { allowDestructiveActions?: boolean }, | ||
| ): Router { |
There was a problem hiding this comment.
New Azure routes are added but there are no corresponding backend route tests. The repo has comprehensive integration tests for similar routers (e.g., backend/test/routes/aws.test.ts); adding a matching azure router test suite would help prevent regressions in auth/validation/403 guard behavior.
| async executeAction(action: Action): Promise<ExecutionResult> { | ||
| this.ensureInitialized(); | ||
|
|
||
| const startedAt = new Date().toISOString(); | ||
| const target = Array.isArray(action.target) ? action.target[0] : action.target; | ||
|
|
||
| try { | ||
| let result: ExecutionResult; | ||
|
|
||
| switch (action.action) { | ||
| case "provision": | ||
| case "create_vm": | ||
| result = await this.handleProvision(action, startedAt, target); | ||
| break; | ||
| case "start": | ||
| case "stop": | ||
| case "restart": | ||
| case "deallocate": | ||
| result = await this.handleLifecycle(action, startedAt, target); | ||
| break; | ||
| default: | ||
| throw new Error(`Unsupported Azure action: ${action.action}`); | ||
| } | ||
|
|
||
| await this.recordJournal(action, target, result); | ||
| return result; | ||
| } catch (error) { | ||
| if (error instanceof AzureAuthenticationError) { | ||
| await this.recordJournalFailure(action, target, startedAt, error.message); | ||
| throw error; | ||
| } | ||
|
|
||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
| const failedResult = this.buildFailedResult(action, startedAt, target, errorMessage); | ||
| await this.recordJournal(action, target, failedResult); | ||
| return failedResult; | ||
| } |
There was a problem hiding this comment.
AzurePlugin is new production-critical logic (initialization, healthCheck, executeAction, journal recording) but there are no unit tests added. There is an established pattern for AWSPlugin tests (backend/test/integrations/aws/AWSPlugin.executeAction.test.ts); adding analogous tests for AzurePlugin (including auth error propagation and journal recording) would improve confidence.
| "@azure/arm-network": "^36.0.0", | ||
| "@azure/arm-resources": "^7.0.0", | ||
| "@azure/arm-resources-subscriptions": "^2.1.0", | ||
| "@azure/arm-subscriptions": "^6.0.0", |
There was a problem hiding this comment.
@azure/arm-network and @azure/arm-subscriptions are added as runtime dependencies, but they are not imported anywhere in backend/src (AzureService uses @azure/arm-compute, @azure/arm-resources, and @azure/arm-resources-subscriptions). Removing unused deps will reduce install size and vulnerability surface.
| "@azure/arm-network": "^36.0.0", | |
| "@azure/arm-resources": "^7.0.0", | |
| "@azure/arm-resources-subscriptions": "^2.1.0", | |
| "@azure/arm-subscriptions": "^6.0.0", | |
| "@azure/arm-resources": "^7.0.0", | |
| "@azure/arm-resources-subscriptions": "^2.1.0", |
| if (config.defaultLocation) { | ||
| lines.push(`AZURE_DEFAULT_LOCATION=${config.defaultLocation}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
AzureSetupGuide always emits AZURE_DEFAULT_LOCATION (defaulting to "eastus"), but the backend ConfigService/schema don’t parse this env var and AzureService.getImages also hardcodes "eastus". Either add backend support for AZURE_DEFAULT_LOCATION (and wire it into image/resource discovery and provisioning defaults) or remove it from the generated snippet to avoid misleading configuration.
| if (config.defaultLocation) { | |
| lines.push(`AZURE_DEFAULT_LOCATION=${config.defaultLocation}`); | |
| } |
|
@copilot apply changes based on the comments in this thread |
- Add explicit type annotations to variables in transformVMToNode and transformToFacts - Replace unsafe non-null assertions (!) with proper null checks in grouping functions - Use typeof checks instead of type assertions for safer type narrowing - Remove redundant error checks that are already handled by throwIfAuthError - Replace nullish coalescing with explicit null checks where appropriate - Improve null safety in Map.get() operations to prevent potential runtime errors
…iledResult type, partial credentials, journal nodeId, remove unused deps, subnetId/tags schema, setup.sh validation, and add tests Agent-Logs-Url: https://github.com/example42/pabawi/sessions/6f2bcf42-120d-408a-8ef2-0bced181842a Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>
…warn log, clearer test secrets, reduce numRuns Agent-Logs-Url: https://github.com/example42/pabawi/sessions/6f2bcf42-120d-408a-8ef2-0bced181842a Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>
Applied all changes from the review thread in commits c08d983 and 9fa1fd4:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
- Add null coalescing operator to AzureService VM size name field - Update integration color test to include 'azure' in expected integrations list - Update IntegrationColorService test to include 'azure' in valid integrations - Increase RBAC performance test timeout from default to 30000ms - Add conditional check in cache performance test to handle near-zero timing values
| async getResourceGroups(): Promise<AzureResourceGroupInfo[]> { | ||
| const groups: AzureResourceGroupInfo[] = []; | ||
| for await (const rg of this.resourceClient.resourceGroups.list()) { | ||
| groups.push({ name: rg.name ?? "", location: rg.location, tags: (rg.tags as Record<string, string> | undefined) ?? {} }); |
There was a problem hiding this comment.
getResourceGroups() pushes rg.location into AzureResourceGroupInfo.location, but rg.location can be undefined. This is a type mismatch against location: string and can leak undefined to API consumers. Default location (e.g., "unknown") or skip entries missing required fields.
| groups.push({ name: rg.name ?? "", location: rg.location, tags: (rg.tags as Record<string, string> | undefined) ?? {} }); | |
| groups.push({ | |
| name: rg.name ?? "", | |
| location: rg.location ?? "unknown", | |
| tags: (rg.tags as Record<string, string> | undefined) ?? {}, | |
| }); |
| const eventType = this.mapActionToEventType(action.action); | ||
| // Normalize to canonical nodeId (azure:{subscriptionId}:{resourceGroup}:{vmName}) | ||
| // and avoid double-prefixing nodeUri | ||
| const canonicalNodeRef = target.startsWith("azure:") ? target : `azure:${target}`; | ||
| const entry: CreateJournalEntry = { | ||
| nodeId: canonicalNodeRef, | ||
| nodeUri: canonicalNodeRef, | ||
| eventType, | ||
| source: "azure" as JournalSource, | ||
| action: action.action, |
There was a problem hiding this comment.
For provision actions the router passes target: "new", and recordJournal() currently derives nodeId/nodeUri from target (producing azure:new). This will make journal entries impossible to link back to real Azure nodes. Build nodeId/nodeUri for provision entries from the provision parameters (subscriptionId/resourceGroup/vmName) or from the created resource id in the execution result, and keep the format consistent with inventory node IDs (azure:{subscriptionId}:{resourceGroup}:{vmName}).
| const AzureProvisionSchema = z.object({ | ||
| resourceGroup: z.string().min(1), | ||
| vmName: z.string().min(1), | ||
| location: z.string().min(1), | ||
| vmSize: z.string().optional().default("Standard_B1s"), | ||
| imageReference: z.object({ | ||
| publisher: z.string(), | ||
| offer: z.string(), | ||
| sku: z.string(), | ||
| version: z.string().optional().default("latest"), | ||
| }), | ||
| adminUsername: z.string().min(1), | ||
| adminPassword: z.string().optional(), | ||
| sshPublicKey: z.string().optional(), | ||
| networkInterfaceId: z.string().min(1, "networkInterfaceId is required for VM creation"), | ||
| }); |
There was a problem hiding this comment.
AzureProvisionSchema allows both adminPassword and sshPublicKey to be omitted. In AzureService.provisionVM() that produces an OS profile without any authentication material, which Azure will reject. Add a schema refinement to require at least one of adminPassword or sshPublicKey (and ideally require exactly one, or validate by OS type if you later add Windows support).
| const AzureProvisionSchema = z.object({ | |
| resourceGroup: z.string().min(1), | |
| vmName: z.string().min(1), | |
| location: z.string().min(1), | |
| vmSize: z.string().optional().default("Standard_B1s"), | |
| imageReference: z.object({ | |
| publisher: z.string(), | |
| offer: z.string(), | |
| sku: z.string(), | |
| version: z.string().optional().default("latest"), | |
| }), | |
| adminUsername: z.string().min(1), | |
| adminPassword: z.string().optional(), | |
| sshPublicKey: z.string().optional(), | |
| networkInterfaceId: z.string().min(1, "networkInterfaceId is required for VM creation"), | |
| }); | |
| const AzureProvisionSchema = z | |
| .object({ | |
| resourceGroup: z.string().min(1), | |
| vmName: z.string().min(1), | |
| location: z.string().min(1), | |
| vmSize: z.string().optional().default("Standard_B1s"), | |
| imageReference: z.object({ | |
| publisher: z.string(), | |
| offer: z.string(), | |
| sku: z.string(), | |
| version: z.string().optional().default("latest"), | |
| }), | |
| adminUsername: z.string().min(1), | |
| adminPassword: z.string().optional(), | |
| sshPublicKey: z.string().optional(), | |
| networkInterfaceId: z.string().min(1, "networkInterfaceId is required for VM creation"), | |
| }) | |
| .refine( | |
| (data) => | |
| (data.adminPassword !== undefined && data.sshPublicKey === undefined) || | |
| (data.adminPassword === undefined && data.sshPublicKey !== undefined), | |
| { | |
| message: "Provide exactly one of adminPassword or sshPublicKey", | |
| path: ["adminPassword"], | |
| }, | |
| ) | |
| .refine( | |
| (data) => | |
| (data.adminPassword !== undefined && data.sshPublicKey === undefined) || | |
| (data.adminPassword === undefined && data.sshPublicKey !== undefined), | |
| { | |
| message: "Provide exactly one of adminPassword or sshPublicKey", | |
| path: ["sshPublicKey"], | |
| }, | |
| ); |
| { | ||
| id: `azure:state:${vmName}:${currentState}:${timestamp}`, | ||
| nodeId, | ||
| nodeUri: `azure:${resourceGroup}:${vmName}`, |
There was a problem hiding this comment.
collectAzureVMStateEntry() sets nodeUri to azure:${resourceGroup}:${vmName}, but nodeId passed into the collector is in the inventory format (azure:{subscriptionId}:{resourceGroup}:{vmName}). This mismatch will break journal→node linking and can create duplicate identities. Use the provided nodeId for nodeUri as well (or consistently include subscriptionId in both).
| nodeUri: `azure:${resourceGroup}:${vmName}`, | |
| nodeUri: nodeId, |
| class="px-4 py-1.5 text-white text-sm rounded transition-colors flex items-center gap-2 {copied | ||
| ? 'bg-green-600' | ||
| : 'bg-blue-600 hover:bg-blue-700'}" | ||
| onclick={copyToClipboard} |
There was a problem hiding this comment.
The "Copy to Clipboard" button isn’t gated by isFormValid, and copyToClipboard() will copy a snippet even when subscriptionId is empty (generating AZURE_SUBSCRIPTION_ID=). Since validateForm()/isFormValid already exist, disable the button when invalid and/or show an error toast instead of copying.
| class="px-4 py-1.5 text-white text-sm rounded transition-colors flex items-center gap-2 {copied | |
| ? 'bg-green-600' | |
| : 'bg-blue-600 hover:bg-blue-700'}" | |
| onclick={copyToClipboard} | |
| class="px-4 py-1.5 text-white text-sm rounded transition-colors flex items-center gap-2 disabled:cursor-not-allowed disabled:opacity-50 {copied | |
| ? 'bg-green-600' | |
| : 'bg-blue-600 hover:bg-blue-700 disabled:bg-blue-400 disabled:hover:bg-blue-400'}" | |
| onclick={copyToClipboard} | |
| disabled={!isFormValid} | |
| aria-disabled={!isFormValid} |
| const target = `azure:${validatedBody.resourceGroup}:${validatedBody.vmName}`; | ||
|
|
||
| const result = await azurePlugin.executeAction({ | ||
| type: "command", | ||
| target, |
There was a problem hiding this comment.
The lifecycle endpoint builds target as azure:${resourceGroup}:${vmName}, but inventory node IDs are azure:{subscriptionId}:{resourceGroup}:{vmName} (see AzureHelpers). Returning/recording a different identifier format makes it hard to correlate lifecycle results and journal entries with inventory nodes. Consider using the configured subscriptionId to build the full target, or accept a full nodeId in the request and use it consistently throughout.
| const message = error instanceof Error ? error.message : String(error); | ||
| throw new AzureAuthenticationError(message); |
There was a problem hiding this comment.
validateCredentials() converts all errors into AzureAuthenticationError (it always throws new AzureAuthenticationError(message) after throwIfAuthError). This will misclassify connectivity/SDK failures as auth failures and can cause routes to return 401 incorrectly. Only wrap and throw AzureAuthenticationError when the error is actually auth/authz-related; otherwise rethrow the original error (or throw a generic Error).
| const message = error instanceof Error ? error.message : String(error); | |
| throw new AzureAuthenticationError(message); | |
| if (error instanceof Error) { | |
| throw error; | |
| } | |
| throw new Error(String(error)); |
| const images = result.map((img) => ({ publisher, offer, sku, version: img.name })); | ||
|
|
There was a problem hiding this comment.
getImages() maps img.name directly into version, but Azure SDK types allow name to be undefined. This doesn’t satisfy the AzureImageInfo.version: string type and can break at runtime. Filter out images without a version or default to a safe string (and consider logging/skipping).
| const images = result.map((img) => ({ publisher, offer, sku, version: img.name })); | |
| const images = result | |
| .filter((img): img is typeof img & { name: string } => typeof img.name === "string" && img.name.length > 0) | |
| .map((img) => ({ publisher, offer, sku, version: img.name })); | |
| const skippedCount = result.length - images.length; | |
| if (skippedCount > 0) { | |
| this.logger.warn("Skipped Azure VM images with missing version", { | |
| component: "AzureService", | |
| operation: "getImages", | |
| metadata: { location: resolvedLocation, publisher, offer, sku, skippedCount }, | |
| }); | |
| } |
No description provided.