feat(ui): Display and manage deployment registry credentials

[nfebe]

Add UI for viewing and changing registry credentials associated with deployments. Changes include:

• Add credential_id to ServiceMetadata in types and API service
• Display credential name in deployment detail General Information card
• Add modal to select/change registry credential for a deployment
• Preserve credential_id when saving domain settings

[sourceant]

Code Review Summary

✨ This pull request introduces the ability to configure registry credentials for deployments, allowing them to pull images from private repositories. Key changes include updating ServiceMetadata interfaces, refactoring the updateMetadata API call to accept partial updates, and adding a new UI component for managing registry credentials within the deployment detail view.

🚀 Key Improvements

• Updated ServiceMetadata to include credential_id in src/types/index.ts and src/services/api.ts for consistency.
• Refactored deploymentsApi.updateMetadata to accept Partial<ServiceMetadata>, enabling more precise and robust updates to deployment metadata.
• Implemented a new UI section in DeploymentDetailView.vue to display and manage registry credentials, improving the user experience for private registry integrations.

💡 Minor Suggestions

• Consider adding a notification or explicit error handling within openCredentialModal for when credentialsApi.list() might fail, although the global error interceptor might cover this. The current behavior of allCredentials.value = [] is acceptable as a fallback.

[sourceant]

Review complete. See the overview comment for a summary.

[cloudflare-workers-and-pages]

Deploying flatrun-ui with    Cloudflare Pages

Latest commit:	b2a9c1f
Status:	✅  Deploy successful!
Preview URL:	https://60ddedff.flatrun-ui.pages.dev
Branch Preview URL:	https://feat-track-registry-credenti.flatrun-ui.pages.dev

View logs

[sourceant]

🔍 Code Review

💡 1. **src/services/api.ts** (Lines 44-63) - REFACTOR

The ServiceMetadata interface is defined here locally, but also imported from src/types/index.ts. This duplication can lead to confusion and maintenance overhead. It's best practice to define interfaces once as the source of truth.

To address this, consider removing the local definition and importing the ServiceMetadata from src/types/index.ts. If the updateMetadata endpoint truly expects a subset of the full ServiceMetadata (e.g., without quick_actions or security), you can define a new type like DeploymentMetadataUpdatePayload using utility types such as Pick or Omit from the central ServiceMetadata interface.

Suggested Code:

import type {
  Deployment,
  Network,
  Certificate,
  ProxyStatus,
  ProxySetupResult,
  VirtualHost,
  RegistryType,
  RegistryCredential,
  SecurityEvent,
  SecurityStats,
  BlockedIP,
  ProtectedRoute,
  DeploymentSecurityConfig,
  ServiceMetadata as DeploymentServiceMetadata
} from "@/types";

export interface DeploymentMetadataUpdatePayload extends Pick<DeploymentServiceMetadata, "name" | "type" | "networking" | "ssl" | "healthcheck" | "credential_id" | "quick_actions" | "security"> {}

// Then, update the `updateMetadata` API call to use `DeploymentMetadataUpdatePayload`
// For example: updateMetadata: (name: string, metadata: DeploymentMetadataUpdatePayload) => apiClient.put(`/deployments/${name}/metadata`, metadata),

Current Code:

export interface ServiceMetadata {
  name: string;
  type: string;
  networking: {
    expose: boolean;
    domain: string;
    container_port: number;
    protocol: string;
    proxy_type: string;
  };
  ssl: {
    enabled: boolean;
    auto_cert: boolean;
  };
  healthcheck: {
    path: string;
    interval: string;
  };
  credential_id?: string;
}

💡 2. **src/views/DeploymentDetailView.vue** (Lines 2104-2122) - BUG

When updating metadata, it's crucial to preserve existing fields that are not being explicitly changed by the current form. The current implementation of saveDomainSettings could unintentionally remove fields like quick_actions or security from the deployment.value?.metadata if they are not explicitly included in the new object.

To ensure all existing metadata properties are preserved, use the spread operator (...currentMetadata) at the beginning of the object when constructing the metadata payload, and then override specific fields as needed.

Suggested Code:

    const currentMetadata = deployment.value?.metadata || {};
    await deploymentsApi.updateMetadata(route.params.name as string, {
      ...currentMetadata, // Spread current metadata to preserve existing fields
      networking: {
        ...(currentMetadata.networking || {}), // Preserve existing networking fields
        expose: domainSettings.value.expose,
        domain: domainSettings.value.domain,
        container_port: domainSettings.value.containerPort,
        protocol: "http", // Assuming protocol is always http for now
        proxy_type: "http", // Assuming proxy_type is always http for now
      },
      ssl: {
        ...(currentMetadata.ssl || {}), // Preserve existing SSL fields
        enabled: domainSettings.value.sslEnabled,
        auto_cert: domainSettings.value.autoCert,
      },
      // Explicitly include other fields if they are always part of the metadata payload
      // otherwise, rely on `...currentMetadata` for them.
      name: deployment.value?.name || currentMetadata.name || "",
      type: currentMetadata.type || "custom",
      healthcheck: currentMetadata.healthcheck || { path: "/", interval: "30s" },
      credential_id: registryCredential.value?.id || currentMetadata.credential_id,
    });

Current Code:

    const currentMetadata = deployment.value?.metadata || {};
    await deploymentsApi.updateMetadata(route.params.name as string, {
      ...currentMetadata,
      name: deployment.value?.name || "",
      type: currentMetadata.type || "custom",
      networking: {
        expose: domainSettings.value.expose,
        domain: domainSettings.value.domain,
        container_port: domainSettings.value.containerPort,
        protocol: "http",
        proxy_type: "http",
      },
      ssl: {
        enabled: domainSettings.value.sslEnabled,
        auto_cert: domainSettings.value.autoCert,
      },
      healthcheck: currentMetadata.healthcheck || {
        path: "/",
        interval: "30s",
      },
      credential_id: registryCredential.value?.id,
    });

💡 3. **src/views/DeploymentDetailView.vue** (Lines 2149-2154) - BUG

Similar to saveDomainSettings, the saveCredential function also needs to ensure it preserves all existing metadata properties when updating. Currently, it only explicitly sets name, type, and credential_id, potentially discarding other crucial metadata fields like networking, ssl, healthcheck, quick_actions, or security.

Modify this to use the spread operator for currentMetadata and only override the credential_id explicitly.

Suggested Code:

    const currentMetadata = deployment.value?.metadata || {};
    await deploymentsApi.updateMetadata(route.params.name as string, {
      ...currentMetadata, // Spread current metadata to preserve existing fields
      credential_id: selectedCredentialId.value || undefined,
    });

Current Code:

    const currentMetadata = deployment.value?.metadata || {};
    await deploymentsApi.updateMetadata(route.params.name as string, {
      ...currentMetadata,
      name: currentMetadata.name || deployment.value?.name || "",
      type: currentMetadata.type || "custom",
      credential_id: selectedCredentialId.value || undefined,
    });

Verdict: APPROVE

Posted as a comment because posting a review failed.

[sourceant]

Review complete. See the overview comment for a summary.

[sourceant]

Review complete. See the overview comment for a summary.