diff --git a/docs/configuration.md b/docs/configuration.md index 3e07dccf30..1aa2abb707 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -440,6 +440,8 @@ Tools that write a new temporary kubeconfig for each shell produce a new key each time; launch Radar with `--kubeconfig` pointing at the stable file instead. That section groups saved integrations by kubeconfig entry; removal is offered for entries no longer loaded, with a confirmation for the selected integration. +Each integration reads as saved, or as auto-discovery (Cost: Automatic) when +it was switched back and no endpoint, credential or cluster mapping is left. Overview's collapsed **Configuration files** section explains the three local files and credential storage; the info button beside the cluster name in each integration tab identifies its kubeconfig entry. In-cluster installations show operator diff --git a/internal/connections/resolver.go b/internal/connections/resolver.go index 00ffdd2377..66d82ed4be 100644 --- a/internal/connections/resolver.go +++ b/internal/connections/resolver.go @@ -43,11 +43,13 @@ type StoredSettingsView struct { InFileName string `json:"inFileName,omitempty"` Availability string `json:"availability"` Revision string `json:"revision"` + Mode string `json:"mode"` URL string `json:"url"` HeaderKeys []string `json:"headerKeys"` EnvHeaderKeys []string `json:"envHeaderKeys"` SecretSet bool `json:"secretSet"` InsecureTLS bool `json:"insecureTls"` + ClusterID string `json:"clusterId"` Error string `json:"error,omitempty"` } @@ -179,6 +181,7 @@ func (p *Resolver) Catalog() ([]StoredSettingsView, error) { for kind, settings := range profile.Integrations { v := settingsView(settings) v.Binding, v.Integration, v.Context = binding, kind, profile.Context + v.Mode, v.ClusterID = settings.EffectiveMode(kind), settings.ClusterID v.Source, v.InFileName = profile.Source, profile.InFileName v.Revision = p.integrationRevision(file, kind, binding) v.Availability = "unavailable" diff --git a/internal/connections/update_test.go b/internal/connections/update_test.go index c41f53e5be..d11a3c7ef2 100644 --- a/internal/connections/update_test.go +++ b/internal/connections/update_test.go @@ -252,6 +252,15 @@ func TestDecliningPausedSettingsSwitchesToDiscovery(t *testing.T) { r, a, _ := setupResolver(t) apply(t, r, a, Update{Kind: config.IntegrationArgoCD, Action: "save", Secret: &SecretEdit{Action: "set", Value: "discovery-token"}}) apply(t, r, a, Update{Kind: config.IntegrationCost, Action: "save", URL: stringPtr("https://cost.example"), Secret: &SecretEdit{Action: "set", Value: "key"}, ClusterID: stringPtr("cluster-a")}) + saved, err := r.Catalog() + if err != nil { + t.Fatal(err) + } + if !slices.ContainsFunc(saved, func(entry StoredSettingsView) bool { + return entry.Integration == config.IntegrationCost && entry.ClusterID == "cluster-a" + }) { + t.Fatalf("catalog omits the saved Kubecost cluster ID: %+v", saved) + } a.Fingerprint = "changed" for _, kind := range []config.Integration{config.IntegrationArgoCD, config.IntegrationCost} { paused := r.Resolve(a, kind, false).View @@ -286,7 +295,7 @@ func TestDecliningPausedSettingsSwitchesToDiscovery(t *testing.T) { } for _, kind := range []config.Integration{config.IntegrationArgoCD, config.IntegrationCost} { if !slices.ContainsFunc(catalog, func(entry StoredSettingsView) bool { - return entry.Binding == a.Binding && entry.Integration == kind && entry.URL == "" && !entry.SecretSet + return entry.Binding == a.Binding && entry.Integration == kind && entry.Mode == "auto" && entry.URL == "" && !entry.SecretSet && entry.ClusterID == "" }) { t.Fatalf("declined %s missing from the catalog as an empty record: %+v", kind, catalog) } diff --git a/packages/k8s-ui/src/components/ui/SelectMenu.test.tsx b/packages/k8s-ui/src/components/ui/SelectMenu.test.tsx new file mode 100644 index 0000000000..881cc4ad45 --- /dev/null +++ b/packages/k8s-ui/src/components/ui/SelectMenu.test.tsx @@ -0,0 +1,25 @@ +// @vitest-environment jsdom +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { expect, it, vi } from 'vitest' +import { SelectMenu } from './SelectMenu' + +vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) + +it('keeps one option tabbable when the options shrink while the menu is open', async () => { + const element = document.createElement('div') + document.body.append(element) + const root = createRoot(element) + const render = (options: { value: string; label: string }[]) => + root.render( {}} options={options} ariaLabel="Source" />) + await act(async () => render([{ value: 'a', label: 'A' }, { value: 'b', label: 'B' }, { value: 'c', label: 'C' }])) + await act(async () => { element.querySelector('button[aria-haspopup="listbox"]')!.click() }) + expect(element.querySelectorAll('[role="option"]')).toHaveLength(3) + + await act(async () => render([{ value: 'a', label: 'A' }])) + const tabbable = element.querySelectorAll('[role="option"][tabindex="0"]') + expect(tabbable).toHaveLength(1) + expect(tabbable[0].textContent).toBe('A') + await act(async () => root.unmount()) + element.remove() +}) diff --git a/packages/k8s-ui/src/components/ui/SelectMenu.tsx b/packages/k8s-ui/src/components/ui/SelectMenu.tsx index a0c11cc8aa..99c84cdb54 100644 --- a/packages/k8s-ui/src/components/ui/SelectMenu.tsx +++ b/packages/k8s-ui/src/components/ui/SelectMenu.tsx @@ -55,7 +55,9 @@ export function SelectMenu({ if (!normalized) return options return options.filter((option) => `${option.label} ${option.description ?? ''}`.toLowerCase().includes(normalized)) }, [options, query]) - const selectedIsVisible = filteredOptions.some((option) => option.value === value) + // Options can be replaced while the menu is open, leaving the highlight past + // the end; without the clamp no option is tabbable and Tab skips the list. + const activeIndex = Math.min(highlightedIndex, filteredOptions.length - 1) const focusTabbableOption = () => { listRef.current?.querySelector('[role="option"][tabindex="0"]')?.focus() @@ -153,7 +155,9 @@ export function SelectMenu({ ref={triggerRef} id={id} type="button" - aria-label={ariaLabel} + // The fixed label would otherwise hide the visible value from screen + // readers, which a native select announces. + aria-label={selected ? `${ariaLabel}: ${selected.label}` : ariaLabel} aria-describedby={ariaDescribedBy} aria-haspopup="listbox" aria-expanded={open} @@ -216,11 +220,11 @@ export function SelectMenu({ onKeyDown={(event) => { if (event.key === 'Enter' && filteredOptions.length > 0) { event.preventDefault() - selectOption(filteredOptions[Math.min(highlightedIndex, filteredOptions.length - 1)].value) + selectOption(filteredOptions[activeIndex].value) } else if (event.key === 'ArrowDown') { event.preventDefault() const optionElements = listRef.current?.querySelectorAll('[role="option"]') - optionElements?.[Math.min(highlightedIndex, optionElements.length - 1)]?.focus() + optionElements?.[activeIndex]?.focus() } }} aria-label={searchPlaceholder} @@ -230,7 +234,7 @@ export function SelectMenu({ aria-expanded="true" aria-activedescendant={ filteredOptions.length > 0 - ? `${listboxId}-option-${Math.min(highlightedIndex, filteredOptions.length - 1)}` + ? `${listboxId}-option-${activeIndex}` : undefined } placeholder={searchPlaceholder} @@ -283,15 +287,13 @@ export function SelectMenu({ role="option" aria-selected={active} aria-disabled={option.disabled || undefined} - tabIndex={ - searchPlaceholder ? (index === highlightedIndex ? 0 : -1) : active || (!selectedIsVisible && index === 0) ? 0 : -1 - } + tabIndex={index === activeIndex ? 0 : -1} onClick={() => selectOption(option.value)} onFocus={() => setHighlightedIndex(index)} className={clsx( 'flex w-full items-center gap-2 px-2.5 py-1.5 text-left text-xs text-theme-text-secondary transition-colors', option.disabled ? 'cursor-not-allowed opacity-60' : 'hover:bg-theme-hover hover:text-theme-text-primary', - searchPlaceholder && index === highlightedIndex && 'bg-theme-hover text-theme-text-primary', + searchPlaceholder && index === activeIndex && 'bg-theme-hover text-theme-text-primary', !searchPlaceholder && 'whitespace-nowrap' )} > diff --git a/web/e2e/settings-connections.spec.ts b/web/e2e/settings-connections.spec.ts index 95f3e3d93a..f2650d45d3 100644 --- a/web/e2e/settings-connections.spec.ts +++ b/web/e2e/settings-connections.spec.ts @@ -90,7 +90,7 @@ function profile(kind: IntegrationKind): IntegrationProfile { } } -const discoverySettings = { url: '', headerKeys: [], envHeaderKeys: [], secretSet: false, insecureTls: false } +const discoverySettings = { mode: 'auto', url: '', headerKeys: [], envHeaderKeys: [], secretSet: false, insecureTls: false, clusterId: '' } async function fixture(page: Page) { const profiles: IntegrationProfiles = { metrics: profile('metrics'), argocd: profile('argocd'), cost: profile('cost') } @@ -655,7 +655,7 @@ test('copy after a target change warns before replacement and reload clears stal }) async function chooseCostSource(page: Page, label: 'Automatic' | 'OpenCost metrics' | 'Kubecost') { - await page.getByRole('button', { name: 'Cost source', exact: true }).click() + await page.getByRole('button', { name: /^Cost source:/ }).click() await page.getByRole('option', { name: new RegExp(`^${label}`) }).click() } @@ -702,10 +702,10 @@ test('old context cleanup lives in Connection and clearly scopes credential remo await page.getByRole('tab', { name: 'Connection', exact: true }).click() await expect(page.getByRole('heading', { name: 'Integration settings by cluster', exact: true })).toBeVisible() await expect(page.getByText('Not in kubeconfig', { exact: true })).toBeVisible() - const remove = page.getByRole('button', { name: 'Remove saved Metrics connection for old-cluster', exact: true }) + const remove = page.getByRole('button', { name: 'Remove saved Metrics settings for old-cluster', exact: true }) await remove.click() - await expect(page.getByText('Remove the saved Metrics connection and credentials for old-cluster?', { exact: false })).toBeVisible() - await expect(page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved connection?' }) })).toBeVisible() + await expect(page.getByText('Remove the saved Metrics settings and credentials for old-cluster?', { exact: false })).toBeVisible() + await expect(page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved settings?' }) })).toBeVisible() await page.keyboard.press('Escape') await expect(remove).toBeFocused() await remove.click() @@ -722,12 +722,12 @@ test('Connection groups stale integrations by context and removes only the confi await page.getByRole('textbox', { name: 'Metrics backend URL' }).fill('https://metrics.example/draft') await page.getByRole('tab', { name: 'Connection', exact: true }).click() await expect(page.getByText('old-cluster', { exact: true })).toHaveCount(1) - await page.getByRole('button', { name: 'Remove saved Cost connection for old-cluster', exact: true }).click() - await page.getByRole('button', { name: 'Remove connection', exact: true }).click() - await expect(page.getByText('Removed the saved Cost connection for old-cluster.', { exact: true })).toBeVisible() + await page.getByRole('button', { name: 'Remove saved Cost settings for old-cluster', exact: true }).click() + await page.getByRole('button', { name: 'Remove settings', exact: true }).click() + await expect(page.getByText('Removed the saved Cost settings for old-cluster.', { exact: true })).toBeVisible() expect(state.writes).toEqual([{ action: 'forget', kind: 'cost', binding: 'old', sourceRevision: 'old-cost', confirmRemoval: true }]) - await expect(page.getByRole('button', { name: 'Remove saved Cost connection for old-cluster', exact: true })).toHaveCount(0) - await expect(page.getByRole('button', { name: 'Remove saved Argo CD connection for old-cluster', exact: true })).toBeVisible() + await expect(page.getByRole('button', { name: 'Remove saved Cost settings for old-cluster', exact: true })).toHaveCount(0) + await expect(page.getByRole('button', { name: 'Remove saved Argo CD settings for old-cluster', exact: true })).toBeVisible() await page.getByRole('tab', { name: 'Metrics', exact: true }).click() await expect(page.getByRole('textbox', { name: 'Metrics backend URL' })).toHaveValue('https://metrics.example/draft') }) @@ -745,34 +745,50 @@ test('Connection preserves the removed-versus-unavailable distinction and cannot await expect(page.getByText('config.json', { exact: true })).toBeVisible() await page.getByRole('tab', { name: 'Connection', exact: true }).click() await expect(page.getByText('Kubeconfig not loaded', { exact: true })).toBeVisible() - await expect(page.getByRole('button', { name: 'Remove saved Metrics connection for development', exact: true })).toHaveCount(0) - await expect(page.getByRole('button', { name: 'Remove saved Cost connection for staging', exact: true })).toBeVisible() - await page.getByRole('button', { name: 'Remove saved Cost connection for staging', exact: true }).click() + await expect(page.getByRole('button', { name: 'Remove saved Metrics settings for development', exact: true })).toHaveCount(0) + await expect(page.getByRole('button', { name: 'Remove saved Cost settings for staging', exact: true })).toBeVisible() + await page.getByRole('button', { name: 'Remove saved Cost settings for staging', exact: true }).click() await expect(page.getByText(/This entry may still exist in another kubeconfig/)).toBeVisible() }) +test('the cluster list separates auto-discovery from saved settings', async ({ page }) => { + const state = await fixture(page) + const current = { binding: 'development', context: 'development', source: '/test/kubeconfig', inFileName: 'development', availability: 'available' as const } + const staging = { binding: 'staging', context: 'staging', source: '/test/kubeconfig', inFileName: 'staging', availability: 'available' as const } + state.connections.push( + { ...discoverySettings, ...current, integration: 'argocd', secretSet: true, revision: 'argocd' }, + { ...discoverySettings, ...current, integration: 'cost', revision: 'cost' }, + { ...discoverySettings, ...current, integration: 'metrics', revision: 'metrics' }, + { ...discoverySettings, ...staging, integration: 'cost', clusterId: 'cluster-a', revision: 'staging-cost' }, + { ...discoverySettings, ...staging, integration: 'metrics', error: 'metrics settings use only a URL and optional headers', revision: 'staging-metrics' }, + ) + await openSettings(page, 'Connection') + await expect(page.getByText('Argo CD: saved · Cost: Automatic · Metrics: auto-discovery', { exact: true })).toBeVisible() + await expect(page.getByText('Cost: saved · Metrics: saved', { exact: true })).toBeVisible() +}) + test('saved settings load failure offers recovery rather than an empty-state claim', async ({ page }) => { await fixture(page) let fail = true await page.route('**/api/integrations/connections', route => fail ? route.fulfill({ status: 500, json: { error: 'Could not read saved settings.' } }) : route.fallback()) await openSettings(page, 'Connection') await expect(page.getByRole('alert')).toContainText('Could not read saved settings.') - await expect(page.getByText('No saved connections yet.', { exact: true })).toHaveCount(0) + await expect(page.getByText('No integration settings saved yet.', { exact: true })).toHaveCount(0) fail = false - await page.getByRole('button', { name: 'Reload saved connections', exact: true }).click() - await expect(page.getByText('No saved connections yet.', { exact: true })).toBeVisible() + await page.getByRole('button', { name: 'Reload integration settings', exact: true }).click() + await expect(page.getByText('No integration settings saved yet.', { exact: true })).toBeVisible() }) test('cleanup blocks close while committing and keeps remaining cards mounted without a refetch', async ({ page }) => { const state = await fixture(page) state.connections.push(...(['metrics', 'cost'] as const).map(integration => ({ ...discoverySettings, binding: 'old', integration, context: 'old-cluster', source: '/test/old', inFileName: 'old-cluster', availability: 'removed' as const, revision: `old-${integration}` }))) await openSettings(page, 'Connection') - const remaining = page.getByRole('button', { name: 'Remove saved Cost connection for old-cluster', exact: true }) + const remaining = page.getByRole('button', { name: 'Remove saved Cost settings for old-cluster', exact: true }) const node = await remaining.elementHandle() - await page.getByRole('button', { name: 'Remove saved Metrics connection for old-cluster', exact: true }).click() - const confirmation = page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved connection?', exact: true }) }) + await page.getByRole('button', { name: 'Remove saved Metrics settings for old-cluster', exact: true }).click() + const confirmation = page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved settings?', exact: true }) }) state.delayApply() - await confirmation.getByRole('button', { name: 'Remove connection', exact: true }).click() + await confirmation.getByRole('button', { name: 'Remove settings', exact: true }).click() await expect.poll(() => state.writes.length).toBe(1) await expect(page.getByRole('button', { name: 'Close settings', exact: true })).toBeDisabled() await expect(confirmation.getByRole('button', { name: 'Cancel', exact: true })).toBeDisabled() @@ -781,7 +797,7 @@ test('cleanup blocks close while committing and keeps remaining cards mounted wi // Hold catalog refreshes so a redundant reload cannot hide the row unnoticed. await page.route('**/api/integrations/connections', async route => { if (route.request().method() === 'GET') return; await route.fallback() }) state.releaseApply() - await expect(page.getByText('Removed the saved Metrics connection for old-cluster.', { exact: true })).toBeVisible() + await expect(page.getByText('Removed the saved Metrics settings for old-cluster.', { exact: true })).toBeVisible() await expect(remaining).toBeVisible() expect(await node!.evaluate(el => el.isConnected)).toBe(true) await expect(page.getByRole('heading', { name: 'Integration settings by cluster', exact: true })).toBeFocused() @@ -795,17 +811,17 @@ test('stale cleanup revisions require reload and keep keyboard focus in the conf ? route.fulfill({ status: 409, json: { error: 'Settings changed; reload latest settings.' } }) : route.fallback()) await openSettings(page, 'Connection') - await page.getByRole('button', { name: 'Remove saved Metrics connection for old-cluster', exact: true }).click() - const confirmation = page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved connection?', exact: true }) }) - await confirmation.getByRole('button', { name: 'Remove connection', exact: true }).click() + await page.getByRole('button', { name: 'Remove saved Metrics settings for old-cluster', exact: true }).click() + const confirmation = page.getByRole('dialog').filter({ has: page.getByRole('heading', { name: 'Remove saved settings?', exact: true }) }) + await confirmation.getByRole('button', { name: 'Remove settings', exact: true }).click() await expect(confirmation.getByRole('alert')).toBeFocused() await page.keyboard.press('Shift+Tab') await expect.poll(() => confirmation.evaluate(el => el.contains(document.activeElement))).toBe(true) - await expect(confirmation.getByRole('button', { name: 'Remove connection', exact: true })).toBeDisabled() + await expect(confirmation.getByRole('button', { name: 'Remove settings', exact: true })).toBeDisabled() await confirmation.getByRole('button', { name: 'Cancel', exact: true }).click() - await page.getByRole('button', { name: 'Reload saved connections', exact: true }).click() + await page.getByRole('button', { name: 'Reload integration settings', exact: true }).click() await expect(page.getByRole('alert')).toHaveCount(0) - await expect(page.getByRole('button', { name: 'Remove saved Metrics connection for old-cluster', exact: true })).toBeVisible() + await expect(page.getByRole('button', { name: 'Remove saved Metrics settings for old-cluster', exact: true })).toBeVisible() }) for (const management of ['operator', 'cloud']) { @@ -1229,12 +1245,14 @@ test('confirming one changed integration removes it from the sibling review pick state.profiles.cost.state = 'target_changed' await openSettings(page, 'Cost') await page.getByRole('button', { name: 'Review changes', exact: true }).click() + await expect(page.getByText('Metrics needs a separate review in its own tab.', { exact: true })).toBeVisible() await page.getByRole('checkbox', { name: /Cost/ }).check() await page.getByRole('button', { name: 'Keep selected settings for this cluster', exact: true }).click() await expect(page.getByRole('tabpanel').getByText('Saved · Connected', { exact: true })).toBeVisible() await page.getByRole('tab', { name: 'Metrics', exact: true }).click() await page.getByRole('button', { name: 'Review changes', exact: true }).click() await expect(page.getByRole('checkbox', { name: /Cost/ })).toHaveCount(0) + await expect(page.getByText(/different previous cluster/)).toHaveCount(0) await page.getByRole('checkbox', { name: /Metrics/ }).check() await page.getByRole('button', { name: 'Keep selected settings for this cluster', exact: true }).click() await expect.poll(() => state.writes.length).toBe(2) @@ -1512,7 +1530,7 @@ test('Cost reset focuses the source picker, which works from the keyboard', asyn const state = await fixture(page) await openSettings(page, 'Cost') const reset = page.getByRole('button', { name: 'Reset to Automatic', exact: true }) - const source = page.getByRole('button', { name: 'Cost source', exact: true }) + const source = page.getByRole('button', { name: /^Cost source:/ }) await reset.focus() await page.keyboard.press('Enter') await expect(source).toBeFocused() @@ -1523,7 +1541,16 @@ test('Cost reset focuses the source picker, which works from the keyboard', asyn await expect(page.getByRole('option', { name: /^Kubecost/ })).toBeFocused() await page.keyboard.press('Enter') await expect(source).toBeFocused() - await expect(source).toContainText('Kubecost') + await expect(source).toHaveAccessibleName('Cost source: Kubecost') + await page.keyboard.press('Enter') + await expect(page.getByRole('option', { name: /^Kubecost/ })).toBeFocused() + await page.keyboard.press('ArrowUp') + await expect(page.getByRole('option', { name: /^OpenCost metrics/ })).toBeFocused() + await page.keyboard.press('Tab') + await expect(page.getByRole('option', { name: /^Kubecost/ })).not.toBeFocused() + await expect(source).toHaveAttribute('aria-expanded', 'false') + await expect(source).toHaveAccessibleName('Cost source: Kubecost') + await source.focus() await page.keyboard.press('Enter') await expect(page.getByRole('option', { name: /^Kubecost/ })).toBeFocused() await page.keyboard.press('Escape') @@ -1541,6 +1568,7 @@ test('the paused-settings review names only what changed', async ({ page }) => { const identity = state.profiles.metrics.target.identity Object.assign(state.profiles.metrics, { state: 'target_changed', previousIdentity: { ...identity, server: 'https://old.example' } }) Object.assign(state.profiles.argocd, { state: 'target_changed', previousIdentity: { ...identity, trust: 'old-ca', proxy: 'http://proxy.example' } }) + Object.assign(state.profiles.cost, { state: 'target_changed', previousIdentity: { ...identity, server: 'https://old.example' } }) const dialog = await openSettings(page, 'Metrics') await dialog.getByRole('button', { name: 'Review changes', exact: true }).click() const rows = dialog.getByRole('tabpanel').locator('dl') @@ -1548,12 +1576,27 @@ test('the paused-settings review names only what changed', async ({ page }) => { await expect(rows).toContainText('https://old.example') await expect(rows).toContainText('developer (unchanged)') await expect(rows).toContainText('Unchanged') + await expect(dialog.getByRole('checkbox', { name: /Argo CD/ })).toHaveCount(0) + await expect(dialog.getByRole('checkbox', { name: /Cost/ })).toBeVisible() + await expect(dialog.getByText('Argo CD was saved for a different previous cluster, so review it in its own tab.', { exact: true })).toBeVisible() await dialog.getByRole('button', { name: 'Back to Metrics', exact: true }).click() await dialog.getByRole('tab', { name: 'Argo CD', exact: true }).click() await dialog.getByRole('button', { name: 'Review changes', exact: true }).click() await expect(rows).toContainText('https://cluster.example (unchanged)') await expect(rows).not.toContainText('Previous server') await expect(rows).toContainText('CA trust changed · Proxy changed') + await expect(dialog.getByText('Metrics and Cost were saved for a different previous cluster, so review them in their own tabs.', { exact: true })).toBeVisible() +}) + +test('an unrecorded previous identity is reviewed separately without claiming a different cluster', async ({ page }) => { + const state = await fixture(page) + const identity = state.profiles.metrics.target.identity + Object.assign(state.profiles.metrics, { state: 'target_changed', previousIdentity: { ...identity, server: 'https://old.example' } }) + Object.assign(state.profiles.cost, { state: 'target_changed' }) + const dialog = await openSettings(page, 'Metrics') + await dialog.getByRole('button', { name: 'Review changes', exact: true }).click() + await expect(dialog.getByRole('checkbox', { name: /Cost/ })).toHaveCount(0) + await expect(dialog.getByText('Cost needs a separate review in its own tab.', { exact: true })).toBeVisible() }) test('AI investigations saves through the shared row and is guarded on close', async ({ page }) => { diff --git a/web/src/components/settings/LocalConfigurationDetails.tsx b/web/src/components/settings/LocalConfigurationDetails.tsx index f1c48298ff..ebd7c56c36 100644 --- a/web/src/components/settings/LocalConfigurationDetails.tsx +++ b/web/src/components/settings/LocalConfigurationDetails.tsx @@ -73,6 +73,13 @@ export function PreviousIntegrationSettingsNotice({ profiles, onNavigate, onDism export const integrationSettingsByClusterId = 'integration-settings-by-cluster' +// Switching to auto-discovery keeps an empty record, so a record alone doesn't +// mean anything is saved. An invalid record disables the integration instead. +function integrationStatus(use: StoredConnection) { + if (use.error || use.mode !== 'auto' || use.url || use.secretSet || use.clusterId) return 'saved' + return use.integration === 'cost' ? 'Automatic' : 'auto-discovery' +} + export function LocalConfigurationDetails({ onNavigate }: { onNavigate: (section: SettingsSectionId) => void }) { return (
@@ -168,7 +175,7 @@ export function SavedClusterConnections({ .then(async (response) => { const data = (await response.json().catch(() => ({}))) as ConnectionResponse if (!response.ok) - throw new Error(data.error || 'Could not load saved connections.') + throw new Error(data.error || 'Could not load integration settings.') if (controller.signal.aborted || getApiBase() !== base) return setUses(data.connections) }) @@ -215,10 +222,10 @@ export function SavedClusterConnections({ const data = (await response.json().catch(() => ({}))) as ConnectionResponse if (controller.signal.aborted || getApiBase() !== base) return if (!response.ok) - throw new Error(data.error || 'Could not remove the saved connection.') + throw new Error(data.error || 'Could not remove the saved settings.') setUses(data.connections) setMessage( - `Removed the saved ${integrationNames[selected.integration]} connection for ${selected.context}.`, + `Removed the saved ${integrationNames[selected.integration]} settings for ${selected.context}.`, ) focusSummary.current = true setSelected(null) @@ -261,12 +268,12 @@ export function SavedClusterConnections({

{loading && (

- Loading saved connections… + Loading integration settings…

)} {!loading && !error && clusters.size === 0 && (

- No saved connections yet. + No integration settings saved yet.

)} {!loading && @@ -300,17 +307,16 @@ export function SavedClusterConnections({

{!current && entry.availability === 'removed' && (

- This kubeconfig entry is gone, but its connections are still - saved. To reuse one, select the new context and choose “Copy + This kubeconfig entry is gone, but its integration settings + are still saved. To reuse one, select the new context and choose “Copy settings from…” in that integration’s settings.

)} {(current || entries.every((use) => use.availability === 'available')) && (

- Saved:{' '} {entries - .map((use) => integrationNames[use.integration]) + .map((use) => `${integrationNames[use.integration]}: ${integrationStatus(use)}`) .join(' · ')}

)} @@ -325,7 +331,7 @@ export function SavedClusterConnections({ type="button" disabled={busy} className="text-accent-text hover:underline disabled:opacity-50" - aria-label={`Remove saved ${integrationNames[use.integration]} connection for ${use.context}`} + aria-label={`Remove saved ${integrationNames[use.integration]} settings for ${use.context}`} onClick={() => { setSelected(use) setError('') @@ -353,7 +359,7 @@ export function SavedClusterConnections({ className="underline" onClick={() => setVersion((value) => value + 1)} > - Reload saved connections + Reload integration settings

)} @@ -362,13 +368,13 @@ export function SavedClusterConnections({ open={!!selected} onClose={() => setSelected(null)} onConfirm={() => void remove()} - title="Remove saved connection?" + title="Remove saved settings?" message={ selected - ? `Remove the saved ${integrationNames[selected.integration]} connection and credentials for ${selected.context}? Other saved connections are unchanged. Nothing is deleted from Kubernetes or the backend.${selected.availability === 'unavailable' ? ' This entry may still exist in another kubeconfig; removal also affects any CLI or Desktop using that entry.' : ''}` + ? `Remove the saved ${integrationNames[selected.integration]} settings and credentials for ${selected.context}? Other saved settings are unchanged. Nothing is deleted from Kubernetes or the backend.${selected.availability === 'unavailable' ? ' This entry may still exist in another kubeconfig; removal also affects any CLI or Desktop using that entry.' : ''}` : '' } - confirmLabel="Remove connection" + confirmLabel="Remove settings" isLoading={busy} confirmDisabled={!!error} showWarning={false} @@ -380,7 +386,7 @@ export function SavedClusterConnections({ role="alert" className="text-sm text-warning-text" > - {error} Cancel and reload saved connections before trying again. + {error} Cancel and reload integration settings before trying again.

)} diff --git a/web/src/components/settings/LocalConnectionSettings.tsx b/web/src/components/settings/LocalConnectionSettings.tsx index 3658cfbed8..d9a53a87cf 100644 --- a/web/src/components/settings/LocalConnectionSettings.tsx +++ b/web/src/components/settings/LocalConnectionSettings.tsx @@ -47,11 +47,13 @@ export interface StoredConnection { source: string inFileName: string availability: 'available' | 'removed' | 'unavailable' + mode: string url: string headerKeys: string[] envHeaderKeys: string[] secretSet: boolean insecureTls: boolean + clusterId: string error?: string } export interface IntegrationProfile { @@ -126,6 +128,11 @@ const automaticAction: Record = { argocd: 'Use auto-discovery', cost: 'Reset to Automatic' } +function sameIdentity(a?: TargetIdentity, b?: TargetIdentity) { + return !!a && !!b && a.server === b.server && a.user === b.user && + a.tlsName === b.tlsName && a.trust === b.trust && a.proxy === b.proxy && + !!a.insecureTls === !!b.insecureTls +} const names: Record = { metrics: 'Metrics', argocd: 'Argo CD', @@ -180,9 +187,18 @@ export function LocalConnectionSettings({ const confirmationCopy = useRef({ title: '', message: '', label: '' }) const [confirmBack, setConfirmBack] = useState(false) const [accepted, setAccepted] = useState([]) - const acceptedChanges = accepted.filter( + // The review page compares against one previous identity, so it can only + // approve integrations that were paused from that same identity. + const pausedKinds = (Object.keys(snapshot) as IntegrationKind[]).filter( (k) => snapshot[k].state === 'target_changed' ) + const reviewKinds = pausedKinds.filter( + (k) => k === kind || sameIdentity(snapshot[k].previousIdentity, profile.previousIdentity) + ) + const separateReviewKinds = pausedKinds.filter((k) => !reviewKinds.includes(k)) + const separateIdentityKnown = !!profile.previousIdentity && + separateReviewKinds.every((k) => !!snapshot[k].previousIdentity) + const acceptedChanges = accepted.filter((k) => reviewKinds.includes(k)) const request = useRef(null) const catalogRequest = useRef(null) const heading = useRef(null) @@ -220,7 +236,7 @@ export function LocalConnectionSettings({ .then(async (response) => { const data = (await response.json().catch(() => ({}))) as ConnectionResponse if (!response.ok) - throw new Error(data.error || 'Could not load saved connections.') + throw new Error(data.error || 'Could not load other clusters’ settings.') if (!controller.signal.aborted && getApiBase() === base) { setCatalog(data.connections) setCatalogError(false) @@ -836,7 +852,7 @@ export function LocalConnectionSettings({ ) : null} )} -
+
{editor && (kind === 'metrics' ? ( ))}
- {!editor && + {!editor && task === 'main' &&

{feedback?.message}

} - {(Object.keys(snapshot) as IntegrationKind[]) - .filter((k) => snapshot[k].state === 'target_changed') + {reviewKinds .map((k) => (
)} {error && !pending && (