Conversation
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
4 times, most recently
from
July 15, 2026 12:02
84b9344 to
4234d9b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## staging #2817 +/- ##
===========================================
+ Coverage 49.52% 49.93% +0.40%
===========================================
Files 1286 1357 +71
Lines 27799 29381 +1582
Branches 8300 8695 +395
===========================================
+ Hits 13767 14670 +903
- Misses 11740 12259 +519
- Partials 2292 2452 +160
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
8 times, most recently
from
July 17, 2026 13:21
e140985 to
a2a0d1a
Compare
RemiBonnet
reviewed
Jul 20, 2026
RemiBonnet
left a comment
RemiBonnet
left a comment
Member
There was a problem hiding this comment.
Thanks @pggb25! I added few comments and I think you can probably share with @TheoGrandin74 to check if the UI is ok for him. I added after my comments:
- I'm not sure why I've an error when I click on the Qovery stack in the creation flow, is it something expected?
- Label aren't supported right now for this kind of cluster, could we hide it?
| } | ||
|
|
||
| // These endpoints are not generated by qovery-typescript-axios yet. Keeping the raw | ||
| // calls behind this data-access module avoids leaking their transport details into UI code. |
Member
There was a problem hiding this comment.
When do you want to add it in the qovery-typescript-axios package?
| type PlatformComponentConfigurationPreviewRequest, | ||
| PlatformConfigurationApi, | ||
| } from 'qovery-typescript-axios' | ||
| import { isHttpStatus } from '../http/is-http-status' |
Member
There was a problem hiding this comment.
I'm not sure to understand why do you need to return null if it's a 404? Same for cluster-operator.ts file
| enabled?: boolean | ||
| } | ||
|
|
||
| export function useClusterOperatorStatus({ organizationId, clusterId, enabled = true }: ClusterOperatorQueryProps) { |
Member
There was a problem hiding this comment.
You need to have one file per query
| <div className="rounded-lg border border-neutral bg-surface-neutral p-5"> | ||
| <div className="mb-5 flex items-start justify-between gap-3"> | ||
| <div> | ||
| <Heading level={2}>{formatCatalogKey(component.key)}</Heading> |
Member
There was a problem hiding this comment.
You need to have <Section> component before your Heading
| ) : null} | ||
|
|
||
| {fields.length > 0 ? ( | ||
| <section className="flex flex-col gap-3"> |
Member
There was a problem hiding this comment.
This one should be <Section>
| const { data: binding } = usePlatformBinding({ organizationId, clusterId, suspense: true }) | ||
| const { mutate: updateBinding, isLoading: isSaving } = useUpdatePlatformBinding() | ||
|
|
||
| const [state, setState] = useState<PlatformConfigurationState | null>(() => { |
Member
There was a problem hiding this comment.
Could you rename it with a more explicit name? platformConfigurationState and setPlatformConfigurationState, in react state is generic
| value: CatalogVariableValue | undefined | ||
| } | ||
|
|
||
| export function CatalogVariableInput({ |
Member
There was a problem hiding this comment.
This component is not enough generic to be in the UI shared library, could you add it in a domain?
| ) | ||
| } | ||
|
|
||
| const saveConfiguration = () => |
Member
There was a problem hiding this comment.
Inputs that are no longer required remain in customerProvidedInputs, could we filter them against preview.requirements, as we already do in the creation flow?
| })} | ||
| </Accordion.Root> | ||
|
|
||
| <div className="flex justify-between border-t border-neutral pt-4"> |
Member
There was a problem hiding this comment.
Can users continue without validating the components of an enabled layer?
| const { data: binding } = usePlatformBinding({ organizationId, clusterId, suspense: true }) | ||
| const { mutate: updateBinding, isLoading: isSaving } = useUpdatePlatformBinding() | ||
|
|
||
| const [state, setState] = useState<PlatformConfigurationState | null>(() => { |
Member
There was a problem hiding this comment.
The creation and settings flows duplicate template selection, preview, field updates and payload cleanup, could this shared controller logic live in a domain hook?
RemiBonnet
reviewed
Jul 20, 2026
| const isOperatorGuideLoading = | ||
| isFeatureFlagLoading || | ||
| (canUseOperator && (isOperatorStatusLoading || (isOperatorManaged && isOperatorBootstrapLoading))) | ||
| // Only a bootstrap failure of a confirmed operator-managed cluster blocks the guide; |
Member
There was a problem hiding this comment.
Should a status request failure fall back to the legacy guide? A transient API error could show incorrect instructions for an operator-managed cluster, I would expect an error state with Retry instead
| templateId: string | ||
| } | ||
|
|
||
| export function PlatformConfiguration({ |
Member
There was a problem hiding this comment.
Could you add small test for this one?
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
from
August 17, 2026 08:48
a2a0d1a to
2b24b90
Compare
|
View your CI Pipeline Execution ↗ for commit 89cdf60
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
2 times, most recently
from
August 18, 2026 12:23
5d4a623 to
dac7c40
Compare
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
2 times, most recently
from
September 11, 2026 09:34
d37438e to
f63e8f1
Compare
- guard operator queries against the undefined PostHog flag state (modal, create/settings platform routes) so flag-off orgs never hit operator endpoints and refreshes no longer bounce users off the platform step - make engine-v2 cluster creation idempotent on retry and redirect to the platform step when platform data is missing instead of failing silently - fix bool cluster-input requirements round-trip and keep requirement inputs mounted during preview refreshes (no more focus loss per keystroke) - keep cleared field values empty instead of resurrecting schema defaults; omit cleared markers from resolver and save payloads - make components without catalog fields openable so requirement-only components can be configured - exclude Stop / Upgrade K8s actions for self-managed clusters - restore bool-before-allowedValues precedence in CatalogVariableInput (blueprint regression) with a non-regression test - key PlatformConfiguration by cluster and re-seed when the template list changes; surface binding fetch errors instead of redirecting - add Back navigation to step-platform error/empty states - drop dead validationMode/componentsConfigurable props, use ts-pattern for enum mappers, import catalog types directly from util-js Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Use qovery-typescript-axios 1.1.972 for platform APIs and field schemas. Render nested objects and arrays, including per-row fields and validation. Preserve displayed defaults and existing component values when saving. Derive platform requests from generated types and share scalar rendering with Blueprint without changing its API contract.
pggb25
force-pushed
the
feat/engine-v2-slice4-platform-configuration
branch
from
September 14, 2026 08:46
f63e8f1 to
d0a6fbc
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
21 issues found and verified against the latest diff
Confidence score: 2/5
step-summary.tsxkeeps its retry guard only for the current summary-page lifetime, so a partial failure followed by navigation or re-entry can create the cluster again; use durable or server-side idempotency for the creation attempt.platform-configuration-catalog.tsxcan apply an older binding layer by key alone, whileplatform-component-configuration.tsxcan hide managed bindings and unmapped violations when no fields resolve; require matching template versions and surface all preview constraints before saving.step-platform.tsxcan omit resolved component defaults from the create binding and discard unsaved edits when navigating back, whilecatalog-configuration-input.tsxseeds scalar array rows with incorrect defaults; preserve the draft and initialize values through the schema/default resolution path.cluster-operator-status.tsxandplatform-configuration-catalog.tsxleave focused save/update controls keyboard-activatable during in-flight mutations, risking duplicate requests; disable them using the active mutation state. The platform route also redirects failed cluster queries to General instead of showing the error, so handle query errors separately.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/domains/clusters/feature/src/lib/platform-configuration/platform-component-configuration.spec.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-component-configuration.spec.tsx:93">
P3: The `queryByText('Ready')` assertion is vacuous: `PlatformComponentConfiguration` never renders the string 'Ready' (its status UI is only 'Checking…' and 'Action required'). The assertion passes regardless of behavior, so these tests don't actually pin anything about a ready status. Either assert on the meaningful signal (e.g. the 'Save configuration' button being enabled) or remove the 'Ready' check.</violation>
</file>
<file name="libs/shared/ui/src/lib/components/catalog-variable-input/catalog-variable-input.spec.tsx">
<violation number="1" location="libs/shared/ui/src/lib/components/catalog-variable-input/catalog-variable-input.spec.tsx:24">
P3: The first test's name claims it renders the field description, but its assertion checks the opposite (the description is suppressed because `error` is set). Rename the test or add a case without `error` that asserts the description is visible; otherwise the number-field description rendering is never validated.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.spec.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.spec.tsx:79">
P3: This assertion can never fail: the catalog never applies a `truncate` class to the layer name (the span has only `font-medium text-neutral`), so the test's first half — "shows complete layer names" — is not actually verified. Drop the `not.toHaveClass('truncate')` check or assert something the component really renders, otherwise the test title overclaims and the assertion only gives false confidence.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary.tsx:197">
P2: The `createdClusterRef` retry cache only protects back-to-back submits while the user stays on the summary page. StepSummary lives on the `summary` route, so after a partial failure (e.g. binding or operator attach throws) the natural recovery — going back to the platform/general step to fix the config — unmounts the component, resets the ref, and the next submit calls `createCluster` again, orphaning the already-created cluster. Persist the created cluster id outside the component (e.g. in the creation-flow context, keyed by cluster name) so it survives step navigation, or clear it intentionally after success, so retries across steps don't duplicate clusters.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-general/step-general.spec.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-general/step-general.spec.tsx:130">
P3: The new test changes two independent variables at once: it sets isEngineV2SelfManaged=true and generalData.installation_type='SELF_MANAGED'. The 'Provider details' heading assertion is correctly isolated to engine v2, but the absence of the 'fully managed' callout is caused by installation_type='SELF_MANAGED', not by engine v2, so that assertion does not exercise the engine-v2-specific behavior. The title also claims credentials are 'required', yet no assertion checks any requirement/validation (e.g. submit blocked without credentials). Consider isolating the engine-v2 flag from the installation_type change and asserting the behavior the title claims.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/platform-configuration/cluster-operator-status.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/platform-configuration/cluster-operator-status.tsx:121">
P2: While an operator update is in flight, this button is still keyboard-activatable because `loading` does not disable it, so users can submit duplicate update mutations. Include `isUpdating` in the disabled condition.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary-presentation.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary-presentation.tsx:319">
P3: The summary renders raw layer keys, so enabled layers appear as internal identifiers such as `log-infrastructure` instead of the user-facing labels shown in the platform configuration panel. Format each key with `formatCatalogKey` before joining the selected layers.</violation>
</file>
<file name="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/settings/platform.tsx">
<violation number="1" location="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/settings/platform.tsx:64">
P2: When the cluster query fails, `isClusterLoading` becomes false while `cluster` remains undefined, so this condition redirects to General instead of exposing the cluster error. Handle `useCluster` errors separately before redirecting.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.tsx:61">
P2: When the available template is newer than the binding, this still applies the old binding layer by key. Ignore binding layers unless the binding key and version match `template`, otherwise an old `SKIPPED` status can hide a current layer and disable its components.</violation>
<violation number="2" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.tsx:176">
P2: While `isSaving` is true, `loading` does not disable this button for keyboard activation. Pass `disabled={isSaving}` so a focused save control cannot submit the binding repeatedly during an in-flight save.</violation>
</file>
<file name="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/settings/route.tsx">
<violation number="1" location="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/settings/route.tsx:21">
P2: When the platform flag is enabled, this fetches a platform binding for every cluster type, including legacy managed and EKS Anywhere clusters that are not part of the Engine V2 self-managed flow. Gate `enabled` with `cluster?.kubernetes === 'SELF_MANAGED'` as the cluster actions do, avoiding an extra binding request on every legacy cluster settings page.</violation>
</file>
<file name="libs/shared/util-js/src/lib/catalog-configuration-field.ts">
<violation number="1" location="libs/shared/util-js/src/lib/catalog-configuration-field.ts:46">
P2: When an optional object field has no value or child defaults, this branch materializes it as `{}`. `omitEmptyCatalogValues` retains that object, so untouched settings are sent and persisted as present; preserve `undefined` when recursive defaults produce no values.</violation>
</file>
<file name="libs/shared/ui/src/lib/components/catalog-configuration-input/catalog-configuration-input.tsx">
<violation number="1" location="libs/shared/ui/src/lib/components/catalog-configuration-input/catalog-configuration-input.tsx:132">
P2: When adding a scalar array item, the form ignores the schema default and seeds `false` or `''`, so new rows do not use configured defaults. Initialize scalar rows through `getCatalogVariableValue` and `toCatalogConfigurationValue`, falling back to `false`/`''` only when no default exists.</violation>
</file>
<file name="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/overview.tsx">
<violation number="1" location="apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/overview.tsx:116">
P2: When a cluster reports node_pools in metrics but currently has zero nodes, `showNodePools` is true, so ClusterTableNodepool renders — and it returns null because `nodes?.length === 0` (cluster-table-nodepool.tsx:309). The table section then renders empty where the old code would have shown ClusterTableNode. Make the branch consistent with the component's own guard, e.g. only switch to the nodepool table when node pools exist AND nodes exist, otherwise keep the node table.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/platform-configuration/platform-component-configuration.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-component-configuration.tsx:70">
P2: `lastRequirementsRef.current` is written and read inside the render body. Mutating a ref during render is a React anti-pattern (render can run multiple times and ref writes are not guaranteed stable), and the state is fully derivable from the `preview` props already in scope. Compute the last-known fields/requirements from the previous `preview` for the same component without a render-time ref write, or move the write into an effect.</violation>
<violation number="2" location="libs/domains/clusters/feature/src/lib/platform-configuration/platform-component-configuration.tsx:107">
P2: When a resolver returns no fields or requirements but includes a managed binding or an unmapped violation, this branch hides that information and shows the no-configuration message. Include `preview.componentBindings` and `unmappedViolations` in the empty-state guard so users can see the binding or the reason Save is disabled.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-platform/step-platform.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-platform/step-platform.tsx:221">
P2: Clicking the “Platform layers” back button in the creation step calls `setComponentDraft(undefined)`, discarding every unsaved component edit because edits are held only in local `componentDraft` and are merged into `platformConfigurationData` exclusively on “Save configuration”. The user can then hit “Continue” and lose work silently. This differs from the settings page, where the back button keeps `state.draft` edits and only clears the selected component. Persist the draft (e.g. via `updateDraft`) when leaving the component editor, or add an explicit discard/confirm so edits cannot be lost without warning.</violation>
<violation number="2" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-platform/step-platform.tsx:251">
P2: When a component has defaulted fields and the user opens it without changing every field, this draft contains only `{}` or stored overrides, so the create binding omits the defaults even though the preview resolved them. Apply `applyPlatformConfigurationDefaults` when initializing `managedConfig`, as the existing platform configuration editor does, so the binding matches the configuration the user reviewed.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-creation-flow/cluster-creation-flow.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-creation-flow/cluster-creation-flow.tsx:115">
P2: When a user opens an unsupported provider slug such as `on-premise-self-managed` while the flag is enabled, this function activates the Engine V2 platform flow. Restrict the check to the four providers for which `ClusterNew` creates Engine V2 self-managed routes.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.spec.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.spec.tsx:48">
P3: This test covers only the operator-bootstrap error branch of ClusterInstallationGuideModal. The new operator-managed success guide (download values + helm command steps), the loading-spinner state, and the legacy ClusterSetup path are all untested, which is the exact regression surface this feature adds. Add cases that mock useClusterOperatorBootstrap with data to assert the success guide renders, and one asserting the loading state.</violation>
</file>
<file name="libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.tsx">
<violation number="1" location="libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.tsx:73">
P3: When `useClusterOperatorStatus` fails on first load (no cached data, e.g. a network error), `operatorStatus` is undefined so `isOperatorManaged` is false, `hasOperatorGuideError` is false, and the modal silently renders the legacy Qovery-CLI/`ClusterSetup` instructions with no error UI. For a self-managed cluster that is operator-managed, that shows the wrong instructions ("created via Qovery CLI / use the CLI to manage upgrades") with zero feedback that the operator status failed to load. The comment documents this as intended, but the consequence is a silent, misleading fallback. Consider surfacing a status-error state (retry) instead of falling through to legacy instructions when the cluster is SELF_MANAGED with the flag enabled.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ...awsLabelsGroups, | ||
| }, | ||
| })) | ||
| createdClusterRef.current = cluster |
Contributor
There was a problem hiding this comment.
P2: The createdClusterRef retry cache only protects back-to-back submits while the user stays on the summary page. StepSummary lives on the summary route, so after a partial failure (e.g. binding or operator attach throws) the natural recovery — going back to the platform/general step to fix the config — unmounts the component, resets the ref, and the next submit calls createCluster again, orphaning the already-created cluster. Persist the created cluster id outside the component (e.g. in the creation-flow context, keyed by cluster name) so it survives step navigation, or clear it intentionally after success, so retries across steps don't duplicate clusters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary.tsx, line 197:
<comment>The `createdClusterRef` retry cache only protects back-to-back submits while the user stays on the summary page. StepSummary lives on the `summary` route, so after a partial failure (e.g. binding or operator attach throws) the natural recovery — going back to the platform/general step to fix the config — unmounts the component, resets the ref, and the next submit calls `createCluster` again, orphaning the already-created cluster. Persist the created cluster id outside the component (e.g. in the creation-flow context, keyed by cluster name) so it survives step navigation, or clear it intentionally after success, so retries across steps don't duplicate clusters.</comment>
<file context>
@@ -144,28 +167,49 @@ export function StepSummary({ organizationId }: StepSummaryProps) {
+ ...awsLabelsGroups,
+ },
+ }))
+ createdClusterRef.current = cluster
+ if (cloudProviderCredentials) {
+ await editCloudProviderInfo({
</file context>
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| disabled={!data.desired_chart_version} |
Contributor
There was a problem hiding this comment.
P2: While an operator update is in flight, this button is still keyboard-activatable because loading does not disable it, so users can submit duplicate update mutations. Include isUpdating in the disabled condition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/platform-configuration/cluster-operator-status.tsx, line 121:
<comment>While an operator update is in flight, this button is still keyboard-activatable because `loading` does not disable it, so users can submit duplicate update mutations. Include `isUpdating` in the disabled condition.</comment>
<file context>
@@ -0,0 +1,161 @@
+ <Button
+ size="sm"
+ variant="outline"
+ disabled={!data.desired_chart_version}
+ loading={isUpdating}
+ onClick={() => {
</file context>
Suggested change
| disabled={!data.desired_chart_version} | |
| disabled={isUpdating || !data.desired_chart_version} |
| if ( | ||
| isPlatformConfigurationEnabled === false || | ||
| (!isPlatformBindingLoading && !platformBinding) || | ||
| (!isClusterLoading && (!clusterMode || !cloudProvider)) |
Contributor
There was a problem hiding this comment.
P2: When the cluster query fails, isClusterLoading becomes false while cluster remains undefined, so this condition redirects to General instead of exposing the cluster error. Handle useCluster errors separately before redirecting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/routes/_authenticated/organization/$organizationId/cluster/$clusterId/settings/platform.tsx, line 64:
<comment>When the cluster query fails, `isClusterLoading` becomes false while `cluster` remains undefined, so this condition redirects to General instead of exposing the cluster error. Handle `useCluster` errors separately before redirecting.</comment>
<file context>
@@ -0,0 +1,99 @@
+ if (
+ isPlatformConfigurationEnabled === false ||
+ (!isPlatformBindingLoading && !platformBinding) ||
+ (!isClusterLoading && (!clusterMode || !cloudProvider))
+ ) {
+ return (
</file context>
| ) : ( | ||
| <span /> | ||
| )} | ||
| <Button type="button" size="lg" loading={isSaving} onClick={onSave}> |
Contributor
There was a problem hiding this comment.
P2: While isSaving is true, loading does not disable this button for keyboard activation. Pass disabled={isSaving} so a focused save control cannot submit the binding repeatedly during an in-flight save.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.tsx, line 176:
<comment>While `isSaving` is true, `loading` does not disable this button for keyboard activation. Pass `disabled={isSaving}` so a focused save control cannot submit the binding repeatedly during an in-flight save.</comment>
<file context>
@@ -0,0 +1,182 @@
+ ) : (
+ <span />
+ )}
+ <Button type="button" size="lg" loading={isSaving} onClick={onSave}>
+ {saveLabel}
+ </Button>
</file context>
Suggested change
| <Button type="button" size="lg" loading={isSaving} onClick={onSave}> | |
| <Button type="button" size="lg" loading={isSaving} disabled={isSaving} onClick={onSave}> |
| defaultValue={template.layers.map((layer) => layer.key)} | ||
| > | ||
| {template.layers.map((layer) => { | ||
| const bindingLayer = binding?.layers.find((candidate) => candidate.key === layer.key) |
Contributor
There was a problem hiding this comment.
P2: When the available template is newer than the binding, this still applies the old binding layer by key. Ignore binding layers unless the binding key and version match template, otherwise an old SKIPPED status can hide a current layer and disable its components.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.tsx, line 61:
<comment>When the available template is newer than the binding, this still applies the old binding layer by key. Ignore binding layers unless the binding key and version match `template`, otherwise an old `SKIPPED` status can hide a current layer and disable its components.</comment>
<file context>
@@ -0,0 +1,182 @@
+ defaultValue={template.layers.map((layer) => layer.key)}
+ >
+ {template.layers.map((layer) => {
+ const bindingLayer = binding?.layers.find((candidate) => candidate.key === layer.key)
+ const applicableToCluster =
+ (!clusterMode || layer.modes.includes(clusterMode)) &&
</file context>
Suggested change
| const bindingLayer = binding?.layers.find((candidate) => candidate.key === layer.key) | |
| const bindingLayer = | |
| binding?.templateKey === template.key && binding.templateVersion === template.version | |
| ? binding.layers.find((candidate) => candidate.key === layer.key) | |
| : undefined |
| it('shows complete layer names and opens a component on a second depth', async () => { | ||
| const { userEvent } = renderWithProviders(<PlatformConfigurationCatalog {...defaultProps} />) | ||
|
|
||
| expect(screen.getByText('Log infrastructure')).not.toHaveClass('truncate') |
Contributor
There was a problem hiding this comment.
P3: This assertion can never fail: the catalog never applies a truncate class to the layer name (the span has only font-medium text-neutral), so the test's first half — "shows complete layer names" — is not actually verified. Drop the not.toHaveClass('truncate') check or assert something the component really renders, otherwise the test title overclaims and the assertion only gives false confidence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/platform-configuration/platform-configuration-catalog.spec.tsx, line 79:
<comment>This assertion can never fail: the catalog never applies a `truncate` class to the layer name (the span has only `font-medium text-neutral`), so the test's first half — "shows complete layer names" — is not actually verified. Drop the `not.toHaveClass('truncate')` check or assert something the component really renders, otherwise the test title overclaims and the assertion only gives false confidence.</comment>
<file context>
@@ -0,0 +1,201 @@
+ it('shows complete layer names and opens a component on a second depth', async () => {
+ const { userEvent } = renderWithProviders(<PlatformConfigurationCatalog {...defaultProps} />)
+
+ expect(screen.getByText('Log infrastructure')).not.toHaveClass('truncate')
+ await userEvent.click(screen.getByRole('button', { name: 'Loki HELM' }))
+
</file context>
Suggested change
| expect(screen.getByText('Log infrastructure')).not.toHaveClass('truncate') | |
| expect(screen.getByText('Log infrastructure')).toBeInTheDocument() |
| region: 'us-east-1', | ||
| production: false, | ||
| } | ||
| mockContextValue.isEngineV2SelfManaged = true |
Contributor
There was a problem hiding this comment.
P3: The new test changes two independent variables at once: it sets isEngineV2SelfManaged=true and generalData.installation_type='SELF_MANAGED'. The 'Provider details' heading assertion is correctly isolated to engine v2, but the absence of the 'fully managed' callout is caused by installation_type='SELF_MANAGED', not by engine v2, so that assertion does not exercise the engine-v2-specific behavior. The title also claims credentials are 'required', yet no assertion checks any requirement/validation (e.g. submit blocked without credentials). Consider isolating the engine-v2 flag from the installation_type change and asserting the behavior the title claims.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-general/step-general.spec.tsx, line 130:
<comment>The new test changes two independent variables at once: it sets isEngineV2SelfManaged=true and generalData.installation_type='SELF_MANAGED'. The 'Provider details' heading assertion is correctly isolated to engine v2, but the absence of the 'fully managed' callout is caused by installation_type='SELF_MANAGED', not by engine v2, so that assertion does not exercise the engine-v2-specific behavior. The title also claims credentials are 'required', yet no assertion checks any requirement/validation (e.g. submit blocked without credentials). Consider isolating the engine-v2 flag from the installation_type change and asserting the behavior the title claims.</comment>
<file context>
@@ -117,6 +120,22 @@ describe('StepGeneral', () => {
+ region: 'us-east-1',
+ production: false,
+ }
+ mockContextValue.isEngineV2SelfManaged = true
+
+ renderWithProviders(<StepGeneral {...defaultProps} />, { wrapper: Wrapper })
</file context>
| <strong className="font-medium">Optional layers enabled: </strong> | ||
| {Object.entries(props.platformConfigurationData?.layerSelections ?? {}) | ||
| .filter(([, enabled]) => enabled) | ||
| .map(([key]) => key) |
Contributor
There was a problem hiding this comment.
P3: The summary renders raw layer keys, so enabled layers appear as internal identifiers such as log-infrastructure instead of the user-facing labels shown in the platform configuration panel. Format each key with formatCatalogKey before joining the selected layers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/cluster-creation-flow/step-summary/step-summary-presentation.tsx, line 319:
<comment>The summary renders raw layer keys, so enabled layers appear as internal identifiers such as `log-infrastructure` instead of the user-facing labels shown in the platform configuration panel. Format each key with `formatCatalogKey` before joining the selected layers.</comment>
<file context>
@@ -292,25 +303,48 @@ export function StepSummaryPresentation(props: StepSummaryPresentationProps) {
+ <strong className="font-medium">Optional layers enabled: </strong>
+ {Object.entries(props.platformConfigurationData?.layerSelections ?? {})
+ .filter(([, enabled]) => enabled)
+ .map(([key]) => key)
+ .join(', ') || 'None'}
+ </li>
</file context>
| jest.restoreAllMocks() | ||
| }) | ||
|
|
||
| it('shows an actionable error when the operator bootstrap cannot be loaded', async () => { |
Contributor
There was a problem hiding this comment.
P3: This test covers only the operator-bootstrap error branch of ClusterInstallationGuideModal. The new operator-managed success guide (download values + helm command steps), the loading-spinner state, and the legacy ClusterSetup path are all untested, which is the exact regression surface this feature adds. Add cases that mock useClusterOperatorBootstrap with data to assert the success guide renders, and one asserting the loading state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.spec.tsx, line 48:
<comment>This test covers only the operator-bootstrap error branch of ClusterInstallationGuideModal. The new operator-managed success guide (download values + helm command steps), the loading-spinner state, and the legacy ClusterSetup path are all untested, which is the exact regression surface this feature adds. Add cases that mock useClusterOperatorBootstrap with data to assert the success guide renders, and one asserting the loading state.</comment>
<file context>
@@ -0,0 +1,66 @@
+ jest.restoreAllMocks()
+ })
+
+ it('shows an actionable error when the operator bootstrap cannot be loaded', async () => {
+ const cluster = {
+ id: 'cluster-123',
</file context>
| (canUseOperator && (isOperatorStatusLoading || (isOperatorManaged && isOperatorBootstrapLoading))) | ||
| // Only a bootstrap failure of a confirmed operator-managed cluster blocks the guide; | ||
| // a status failure falls back to the legacy instructions, which don't depend on the operator. | ||
| const hasOperatorGuideError = isOperatorManaged && isOperatorBootstrapError |
Contributor
There was a problem hiding this comment.
P3: When useClusterOperatorStatus fails on first load (no cached data, e.g. a network error), operatorStatus is undefined so isOperatorManaged is false, hasOperatorGuideError is false, and the modal silently renders the legacy Qovery-CLI/ClusterSetup instructions with no error UI. For a self-managed cluster that is operator-managed, that shows the wrong instructions ("created via Qovery CLI / use the CLI to manage upgrades") with zero feedback that the operator status failed to load. The comment documents this as intended, but the consequence is a silent, misleading fallback. Consider surfacing a status-error state (retry) instead of falling through to legacy instructions when the cluster is SELF_MANAGED with the flag enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/clusters/feature/src/lib/cluster-installation-guide-modal/cluster-installation-guide-modal.tsx, line 73:
<comment>When `useClusterOperatorStatus` fails on first load (no cached data, e.g. a network error), `operatorStatus` is undefined so `isOperatorManaged` is false, `hasOperatorGuideError` is false, and the modal silently renders the legacy Qovery-CLI/`ClusterSetup` instructions with no error UI. For a self-managed cluster that is operator-managed, that shows the wrong instructions ("created via Qovery CLI / use the CLI to manage upgrades") with zero feedback that the operator status failed to load. The comment documents this as intended, but the consequence is a silent, misleading fallback. Consider surfacing a status-error state (retry) instead of falling through to legacy instructions when the cluster is SELF_MANAGED with the flag enabled.</comment>
<file context>
@@ -32,6 +64,23 @@ export function ClusterInstallationGuideModal({ type, onClose, ...props }: Clust
+ (canUseOperator && (isOperatorStatusLoading || (isOperatorManaged && isOperatorBootstrapLoading)))
+ // Only a bootstrap failure of a confirmed operator-managed cluster blocks the guide;
+ // a status failure falls back to the legacy instructions, which don't depend on the operator.
+ const hasOperatorGuideError = isOperatorManaged && isOperatorBootstrapError
+
+ const retryOperatorGuide = () => {
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Here is a PR for self-managed clusters within the scope of the engine V2 POC.
All changes is under feature flag.
Preview can be found here
https://p80-z9693eb24-z7d62c7cc-gtw.zc531a994.rustrocks.cloud/
Existing test cluster is here
https://p80-z9693eb24-z7d62c7cc-gtw.zc531a994.rustrocks.cloud/organization/460616f0-94da-4d35-b631-6fa4ed08eb9a/cluster/8a494a2b-ce45-4499-a3c2-f5c75fbd38dd/settings/platform
What we reuse from the Service Catalog is the normalized field vocabulary and the generic form renderer: field types, defaults, constraints, sensitivity, local validation, and widget selection.
The change on the Service Catalog side was mainly an extraction: Blueprint-specific field utilities and rendering were moved into shared catalog primitives. Blueprints now adapt their DTOs to those primitives, and Platform Configuration does the same with its own q-core DTOs.
The goal is to share the configuration mechanism while keeping the two domains independent.
Screenshots / Recordings
Adding new "Platform configurations" panel in the edit and creation of a self-managed cluster
The idea of the review is to check the godd separation and the good utilisation of the existing service catalogue feature
Testing
yarn testoryarn test -u(if you need to regenerate snapshots)yarn formatyarn lintPR Checklist
.cursor/rules)feat(service): add new Terraform service) - required for semantic-release