Skip to content

Quadlet spec#371

Merged
celdrake merged 2 commits into
flightctl:mainfrom
celdrake:quadlet-spec
Oct 29, 2025
Merged

Quadlet spec#371
celdrake merged 2 commits into
flightctl:mainfrom
celdrake:quadlet-spec

Conversation

@celdrake

@celdrake celdrake commented Oct 29, 2025

Copy link
Copy Markdown
Collaborator

Adapts the UI to the new types introduced in flightctl/flightctl#1855 to support Quadlet and Artifact applications.

Support not yet added either on the Backend or UI, that will be done later.

Summary by CodeRabbit

  • New Features
    • Added support for artifact-based application provider specifications alongside existing image and inline types.
    • UI now recognizes artifact-based apps in device workflows and adjusts form/patch handling (scaffolding for future full support).
    • Introduced Quadlet as a new application type option.
    • Added tracking of rendered device versions for enrollment requests.

@coderabbitai

coderabbitai Bot commented Oct 29, 2025

Copy link
Copy Markdown

Walkthrough

Adds an ArtifactApplicationProviderSpec type and integrates artifact-app handling: extends ApplicationProviderSpec union, adds AppTypeQuadlet enum member, exports the new type, adds an isArtifactAppProvider type guard, and updates device spec utilities to skip/omit artifact apps in patching and value generation.

Changes

Cohort / File(s) Summary
Core type & model files
libs/types/models/ArtifactApplicationProviderSpec.ts, libs/types/models/AppType.ts, libs/types/models/ApplicationProviderSpec.ts, libs/types/models/EnrollmentRequestSpec.ts
New ArtifactApplicationProviderSpec type (artifact: string); added AppTypeQuadlet = 'quadlet' to AppType; extended ApplicationProviderSpec union to include artifact provider; added optional knownRenderedVersion?: string to EnrollmentRequestSpec.
Type exports
libs/types/index.ts
Exported ArtifactApplicationProviderSpec from the public types index.
UI types & guards
libs/ui-components/src/types/extraTypes.ts, libs/ui-components/src/types/deviceSpec.ts
Imported ArtifactApplicationProviderSpec and added it to ApplicationProviderSpecFixed union; added isArtifactAppProvider type guard (checks 'artifact' in app).
Device spec utilities
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
Added artifact-aware handling: getApplicationPatches short-circuits/returns no patch for artifact apps; getApplicationValues filters out artifact apps and maps remaining apps (image apps preserved, inline apps mapped), with TODOs for future artifact support.

Sequence Diagram(s)

sequenceDiagram
    participant UI as EditDeviceWizard
    participant Utils as deviceSpecUtils
    participant Types as typeGuards

    UI->>Utils: getApplicationPatches(currentApps, formApps)
    Utils->>Types: isArtifactAppProvider(app)?
    alt artifact app
        Types-->>Utils: true
        Utils-->>UI: no patch (skip artifact app)
    else not artifact
        Types-->>Utils: false
        Utils-->>UI: compute patch (image/inline logic)
    end
Loading
sequenceDiagram
    participant UI as EditDeviceWizard
    participant Utils as deviceSpecUtils
    participant Types as typeGuards

    UI->>Utils: getApplicationValues(deviceSpec)
    Utils->>Types: isArtifactAppProvider(app)?
    alt artifact app
        Types-->>Utils: true
        Utils-->>UI: filter out artifact app (exclude from values)
    else not artifact
        Types-->>Utils: false
        Utils-->>UI: map to AppForm (OCI_IMAGE or INLINE)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Attention areas:
    • Logic in deviceSpecUtils.ts for skipping artifact apps and ensuring no unintended side effects in patch calculation.
    • Correctness and exhaustiveness of the updated discriminated union (ApplicationProviderSpec) across UI types.
    • Type guard (isArtifactAppProvider) correctness and places where artifact apps are now excluded.

Suggested reviewers

  • rawagner

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Quadlet spec" refers to a real aspect of the changeset: the addition of AppTypeQuadlet = 'quadlet' to the AppType enum, which enables Quadlet application support. However, the title is somewhat incomplete, as the PR's main structural changes center on adding comprehensive support for ArtifactApplicationProviderSpec alongside Quadlet, with the Artifact type being equally significant to the overall changes. The title is also mildly vague—"spec" doesn't clearly communicate that this is about adding new application provider type support. Despite these limitations, the title does clearly identify a real part of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@celdrake celdrake requested a review from kkyrazis October 29, 2025 09:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80cb7b5 and cb08652.

📒 Files selected for processing (8)
  • libs/types/index.ts (1 hunks)
  • libs/types/models/AppType.ts (1 hunks)
  • libs/types/models/ApplicationProviderSpec.ts (2 hunks)
  • libs/types/models/ArtifactApplicationProviderSpec.ts (1 hunks)
  • libs/types/models/EnrollmentRequestSpec.ts (1 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (4 hunks)
  • libs/ui-components/src/types/deviceSpec.ts (2 hunks)
  • libs/ui-components/src/types/extraTypes.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • libs/types/models/ArtifactApplicationProviderSpec.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
  • libs/ui-components/src/types/deviceSpec.ts
  • libs/types/models/ApplicationProviderSpec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
libs/ui-components/src/types/extraTypes.ts (2)
libs/types/index.ts (2)
  • ImageApplicationProviderSpec (109-109)
  • ArtifactApplicationProviderSpec (16-16)
libs/types/models/ImageApplicationProviderSpec.ts (1)
  • ImageApplicationProviderSpec (6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: integration-tests
  • GitHub Check: Lint
  • GitHub Check: Build
🔇 Additional comments (3)
libs/types/models/EnrollmentRequestSpec.ts (1)

19-22: LGTM!

The optional field addition maintains backward compatibility and is appropriately documented.

libs/ui-components/src/types/extraTypes.ts (1)

5-5: LGTM!

The import is correctly added to support the new artifact-based application provider type.

libs/types/index.ts (1)

16-16: All verification checks pass. The source file exists and correctly exports the type.

The verification confirms that ArtifactApplicationProviderSpec.ts exists in the expected location, exports the correct type, and the re-export statement in libs/types/index.ts is valid.

Comment thread libs/types/models/AppType.ts
Comment thread libs/ui-components/src/types/extraTypes.ts
@celdrake celdrake merged commit 3fcbe30 into flightctl:main Oct 29, 2025
6 checks passed
@celdrake celdrake deleted the quadlet-spec branch October 29, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants