fix(helm): don't block install with dry-run errors [r8s-454] (#976)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
develop
Ali 2025-08-05 18:53:41 +12:00 committed by GitHub
parent 497b16e942
commit dc273b2d63
11 changed files with 408 additions and 26 deletions

View File

@ -98,7 +98,7 @@ export const ngModule = angular
r2a(Tooltip, ['message', 'position', 'className', 'setHtmlMessage', 'size'])
)
.component('terminalTooltip', r2a(TerminalTooltip, []))
.component('badge', r2a(Badge, ['type', 'className']))
.component('badge', r2a(Badge, ['type', 'className', 'data-cy']))
.component('fileUploadField', fileUploadField)
.component('porSwitchField', switchField)
.component(

View File

@ -1,6 +1,8 @@
import clsx from 'clsx';
import { PropsWithChildren } from 'react';
import { AutomationTestingProps } from '@/types';
export type BadgeType =
| 'success'
| 'danger'
@ -73,7 +75,8 @@ export function Badge({
type = 'info',
className,
children,
}: PropsWithChildren<Props>) {
'data-cy': dataCy,
}: PropsWithChildren<Props> & Partial<AutomationTestingProps>) {
const baseClasses =
'inline-flex w-fit items-center !text-xs font-medium rounded-full px-2 py-0.5';
@ -81,6 +84,7 @@ export function Badge({
<span
className={clsx(baseClasses, typeClasses[type], className)}
role="status"
data-cy={dataCy}
>
{children}
</span>

View File

@ -0,0 +1,115 @@
import { Meta, StoryObj } from '@storybook/react';
import { ExpandableMessageByLines } from './ExpandableMessageByLines';
export default {
component: ExpandableMessageByLines,
title: 'Components/ExpandableMessageByLines',
argTypes: {
maxLines: {
control: {
type: 'select',
options: [2, 5, 10, 20, 50],
},
description: 'Maximum number of lines to show before truncating',
},
children: {
control: 'text',
description: 'The text content to display',
},
},
} as Meta;
interface Args {
children: string;
maxLines?: 2 | 5 | 10 | 20 | 50;
}
// Short text that won't be truncated
export const ShortText: StoryObj<Args> = {
args: {
children: 'This is a short message that should not be truncated.',
maxLines: 10,
},
};
// Long text that will be truncated
export const LongText: StoryObj<Args> = {
args: {
children: `This is a very long message that should be truncated after the specified number of lines.
It contains multiple lines of text to demonstrate the expandable functionality.
The component will show a "Show more" button when the content exceeds the maxLines limit.
When clicked, it will expand to show the full content and change to "Show less".
This is useful for displaying long error messages, logs, or any text content that might be too long for the UI.`,
maxLines: 5,
},
};
// Text with line breaks
export const TextWithLineBreaks: StoryObj<Args> = {
args: {
children: `Line 1: This is the first line
Line 2: This is the second line
Line 3: This is the third line
Line 4: This is the fourth line
Line 5: This is the fifth line
Line 6: This is the sixth line
Line 7: This is the seventh line
Line 8: This is the eighth line
Line 9: This is the ninth line
Line 10: This is the tenth line`,
maxLines: 5,
},
};
// Very short maxLines
export const VeryShortMaxLines: StoryObj<Args> = {
args: {
children: `This text will be truncated after just 2 lines.
This is the second line.
This is the third line that should be hidden initially.
This is the fourth line that should also be hidden.`,
maxLines: 2,
},
};
// Error message example
export const ErrorMessage: StoryObj<Args> = {
args: {
children: `Error: Failed to connect to the Docker daemon at unix:///var/run/docker.sock.
Is the docker daemon running?
This error typically occurs when:
1. Docker daemon is not running
2. User doesn't have permission to access the Docker socket
3. Docker socket path is incorrect
4. Docker service has crashed
To resolve this issue:
1. Start the Docker daemon: sudo systemctl start docker
2. Add user to docker group: sudo usermod -aG docker $USER
3. Verify Docker is running: docker ps
4. Check Docker socket permissions: ls -la /var/run/docker.sock`,
maxLines: 5,
},
};
// Log output example
export const LogOutput: StoryObj<Args> = {
args: {
children: `2024-01-15T10:30:45.123Z INFO [ContainerService] Starting container nginx:latest
2024-01-15T10:30:45.234Z DEBUG [ContainerService] Container ID: abc123def456
2024-01-15T10:30:45.345Z INFO [ContainerService] Container started successfully
2024-01-15T10:30:45.456Z DEBUG [NetworkService] Creating network bridge
2024-01-15T10:30:45.567Z INFO [NetworkService] Network created: portainer_network
2024-01-15T10:30:45.678Z DEBUG [VolumeService] Mounting volume /data
2024-01-15T10:30:45.789Z INFO [VolumeService] Volume mounted successfully
2024-01-15T10:30:45.890Z DEBUG [ContainerService] Setting up port mapping 80:80
2024-01-15T10:30:45.901Z INFO [ContainerService] Port mapping configured
2024-01-15T10:30:45.912Z DEBUG [ContainerService] Setting environment variables
2024-01-15T10:30:45.923Z INFO [ContainerService] Environment variables set
2024-01-15T10:30:45.934Z DEBUG [ContainerService] Starting container process
2024-01-15T10:30:45.945Z INFO [ContainerService] Container process started`,
maxLines: 10,
},
};

View File

@ -0,0 +1,137 @@
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { vi } from 'vitest';
import { ExpandableMessageByLines } from '@@/ExpandableMessageByLines';
describe('ExpandableMessageByLines', () => {
// Mock scrollHeight and clientHeight for testing truncation
const mockScrollHeight = vi.fn();
const mockClientHeight = vi.fn();
beforeEach(() => {
// Mock the properties on HTMLDivElement prototype
Object.defineProperty(HTMLDivElement.prototype, 'scrollHeight', {
get: mockScrollHeight,
configurable: true,
});
Object.defineProperty(HTMLDivElement.prototype, 'clientHeight', {
get: mockClientHeight,
configurable: true,
});
});
afterEach(() => {
vi.clearAllMocks();
});
describe('Basic Rendering', () => {
it('should render text content', () => {
const text = 'This is test content';
// Mock non-truncated content (scrollHeight === clientHeight)
mockScrollHeight.mockReturnValue(100);
mockClientHeight.mockReturnValue(100);
render(<ExpandableMessageByLines>{text}</ExpandableMessageByLines>);
expect(screen.getByText(text)).toBeInTheDocument();
});
it('should show expand button only when text is truncated', () => {
const text = 'This is test content that should be truncated';
// Mock truncated content (scrollHeight > clientHeight)
mockScrollHeight.mockReturnValue(300);
mockClientHeight.mockReturnValue(200);
render(<ExpandableMessageByLines>{text}</ExpandableMessageByLines>);
expect(
screen.getByRole('button', { name: 'Show more' })
).toBeInTheDocument();
expect(
screen.getByTestId('expandable-message-lines-button')
).toBeInTheDocument();
});
it('should hide expand button when text is not truncated', () => {
const text = 'Short text';
// Mock non-truncated content (scrollHeight === clientHeight)
mockScrollHeight.mockReturnValue(50);
mockClientHeight.mockReturnValue(50);
render(<ExpandableMessageByLines>{text}</ExpandableMessageByLines>);
expect(screen.queryByRole('button')).not.toBeInTheDocument();
expect(
screen.queryByTestId('expandable-message-lines-button')
).not.toBeInTheDocument();
});
});
describe('Expand/Collapse Functionality', () => {
it('should toggle between Show more and Show less when button is clicked', async () => {
const user = userEvent.setup();
const text =
'This is a long text that should be truncated and show the expand button';
// Mock truncated content (scrollHeight > clientHeight)
mockScrollHeight.mockReturnValue(400);
mockClientHeight.mockReturnValue(200);
render(<ExpandableMessageByLines>{text}</ExpandableMessageByLines>);
const button = screen.getByRole('button');
// Initially should show "Show more"
expect(screen.getByText('Show more')).toBeInTheDocument();
// Click to expand
await user.click(button);
expect(screen.getByText('Show less')).toBeInTheDocument();
// Click to collapse
await user.click(button);
expect(screen.getByText('Show more')).toBeInTheDocument();
});
});
describe('Text Content Handling', () => {
it('should not show button for single space strings when not truncated', () => {
// Mock non-truncated content (scrollHeight === clientHeight)
mockScrollHeight.mockReturnValue(20);
mockClientHeight.mockReturnValue(20);
render(<ExpandableMessageByLines> </ExpandableMessageByLines>);
expect(screen.queryByRole('button')).not.toBeInTheDocument();
});
it('should show button for single space strings when truncated', () => {
// Mock truncated content (scrollHeight > clientHeight)
mockScrollHeight.mockReturnValue(100);
mockClientHeight.mockReturnValue(50);
render(<ExpandableMessageByLines> </ExpandableMessageByLines>);
expect(screen.getByRole('button')).toBeInTheDocument();
});
it('should handle different maxLines values', () => {
const longText =
'Line 1\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6\nLine 7\nLine 8\nLine 9\nLine 10\nLine 11\nLine 12';
// Mock truncated content for 5 lines (scrollHeight > clientHeight)
mockScrollHeight.mockReturnValue(240);
mockClientHeight.mockReturnValue(100);
render(
<ExpandableMessageByLines maxLines={5}>
{longText}
</ExpandableMessageByLines>
);
expect(screen.getByRole('button')).toBeInTheDocument();
expect(screen.getByText('Show more')).toBeInTheDocument();
});
});
});

View File

@ -0,0 +1,76 @@
import { useRef, useState, useEffect, useCallback } from 'react';
import { Button } from '@@/buttons';
// use enum so that the tailwind classes aren't interpolated
type MaxLines = 2 | 5 | 10 | 20 | 50;
const lineClampClasses: Record<MaxLines, string> = {
2: 'line-clamp-[2]',
5: 'line-clamp-[5]',
10: 'line-clamp-[10]',
20: 'line-clamp-[20]',
50: 'line-clamp-[50]',
};
interface LineBasedProps {
children: string;
maxLines?: MaxLines;
}
export function ExpandableMessageByLines({
children,
maxLines = 10,
}: LineBasedProps) {
const [isExpanded, setIsExpanded] = useState(false);
const [isTruncated, setIsTruncated] = useState(false);
const contentRef = useRef<HTMLDivElement>(null);
const checkTruncation = useCallback(() => {
const el = contentRef.current;
if (el) {
setIsTruncated(el.scrollHeight > el.clientHeight);
}
}, []);
useEffect(() => {
checkTruncation();
// Use requestAnimationFrame for better performance
let rafId: number;
function handleResize() {
if (rafId) cancelAnimationFrame(rafId);
rafId = requestAnimationFrame(checkTruncation);
}
window.addEventListener('resize', handleResize);
return () => {
window.removeEventListener('resize', handleResize);
if (rafId) cancelAnimationFrame(rafId);
};
}, [children, maxLines, checkTruncation, isExpanded]);
return (
<div className="flex flex-col items-start">
<div
ref={contentRef}
className={`whitespace-pre-wrap break-words overflow-hidden ${
isExpanded ? '' : lineClampClasses[maxLines]
}`}
>
{children}
</div>
{(isTruncated || isExpanded) && (
<Button
color="link"
size="xsmall"
onClick={() => setIsExpanded(!isExpanded)}
className="!ml-0 !p-0 mt-1"
data-cy="expandable-message-lines-button"
>
{isExpanded ? 'Show less' : 'Show more'}
</Button>
)}
</div>
);
}

View File

@ -7,6 +7,7 @@ import { ChartVersion } from '@/react/kubernetes/helm/helmChartSourceQueries/use
import { EnvironmentId } from '@/react/portainer/environments/types';
import { Modal, OnSubmit, openModal } from '@@/modals';
import { confirm } from '@@/modals/confirm';
import { Button } from '@@/buttons';
import { Input } from '@@/form-components/Input';
import { FormControl } from '@@/form-components/FormControl';
@ -181,8 +182,19 @@ export function UpgradeHelmModal({
Cancel
</Button>
<Button
onClick={() => onSubmit(submitPayload)}
disabled={!previewIsValid}
onClick={async () => {
if (!previewIsValid) {
const confirmed = await confirm({
title: 'Chart validation failed',
message:
'The Helm manifest preview validation failed, which may indicate configuration issues. This can be normal when creating new resources. Do you want to proceed with the upgrade?',
});
if (!confirmed) {
return;
}
}
onSubmit(submitPayload);
}}
color="primary"
key="update-button"
size="medium"

View File

@ -1,4 +1,4 @@
import { useRef } from 'react';
import { useRef, useState } from 'react';
import { Formik, FormikProps } from 'formik';
import { useRouter } from '@uirouter/react';
@ -7,7 +7,7 @@ import { useAnalytics } from '@/react/hooks/useAnalytics';
import { useCanExit } from '@/react/hooks/useCanExit';
import { useEnvironmentId } from '@/react/hooks/useEnvironmentId';
import { confirmGenericDiscard } from '@@/modals/confirm';
import { confirm, confirmGenericDiscard } from '@@/modals/confirm';
import { Option } from '@@/form-components/PortainerSelect';
import { Chart } from '../types';
@ -34,6 +34,7 @@ export function HelmInstallForm({
isRepoAvailable,
}: Props) {
const environmentId = useEnvironmentId();
const [previewIsValid, setPreviewIsValid] = useState(false);
const router = useRouter();
const analytics = useAnalytics();
const helmRepoVersionsQuery = useHelmRepoVersions(
@ -78,6 +79,7 @@ export function HelmInstallForm({
versionOptions={versionOptions}
isVersionsLoading={helmRepoVersionsQuery.isInitialLoading}
isRepoAvailable={isRepoAvailable}
setPreviewIsValid={setPreviewIsValid}
/>
</Formik>
);
@ -88,6 +90,17 @@ export function HelmInstallForm({
return;
}
if (!previewIsValid) {
const confirmed = await confirm({
title: 'Chart validation failed',
message:
'The Helm manifest preview validation failed, which may indicate configuration issues. This can be normal when creating new resources. Do you want to proceed with the installation?',
});
if (!confirmed) {
return;
}
}
await installHelmChartMutation.mutateAsync(
{
name,

View File

@ -1,5 +1,5 @@
import { Form, useFormikContext } from 'formik';
import { useMemo, useState } from 'react';
import { useMemo } from 'react';
import { useEnvironmentId } from '@/react/hooks/useEnvironmentId';
@ -23,6 +23,7 @@ type Props = {
versionOptions: Option<ChartVersion>[];
isVersionsLoading: boolean;
isRepoAvailable: boolean;
setPreviewIsValid: (isValid: boolean) => void;
};
export function HelmInstallInnerForm({
@ -32,9 +33,9 @@ export function HelmInstallInnerForm({
versionOptions,
isVersionsLoading,
isRepoAvailable,
setPreviewIsValid,
}: Props) {
const environmentId = useEnvironmentId();
const [previewIsValid, setPreviewIsValid] = useState(false);
const { values, setFieldValue, isSubmitting } =
useFormikContext<HelmInstallFormValues>();
@ -129,7 +130,7 @@ export function HelmInstallInnerForm({
className="!ml-0 mt-5"
loadingText="Installing Helm chart"
isLoading={isSubmitting}
disabled={!namespace || !name || !isRepoAvailable || !previewIsValid}
disabled={!namespace || !name || !isRepoAvailable}
data-cy="helm-install"
>
Install

View File

@ -73,7 +73,7 @@ export function HelmTemplatesList({
/>
))}
{filteredCharts.length === 0 && textFilter && (
{filteredCharts.length === 0 && textFilter && !isLoadingCharts && (
<div className="text-muted small mt-4">No Helm charts found</div>
)}

View File

@ -106,7 +106,7 @@ describe('ManifestPreviewFormSection', () => {
expect(screen.queryByText('Manifest Preview')).not.toBeInTheDocument();
});
it('should show error and no form section when error', () => {
it("should show error when there's an error", async () => {
mockUseHelmDryRun.mockReturnValue({
isInitialLoading: false,
isError: true,
@ -116,11 +116,18 @@ describe('ManifestPreviewFormSection', () => {
renderComponent();
expect(screen.queryByText('Manifest Preview')).toBeInTheDocument();
// there should be an error badge
expect(
screen.queryByTestId('helm-manifest-preview-error-badge')
).toBeInTheDocument();
const expandButton = screen.getByLabelText('Expand');
await userEvent.click(expandButton);
expect(
screen.getByText('Error with Helm chart configuration')
).toBeInTheDocument();
expect(screen.getByText('Invalid chart configuration')).toBeInTheDocument();
expect(screen.queryByText('Manifest Preview')).not.toBeInTheDocument();
});
it('should show single code editor when only the generated manifest is available', async () => {

View File

@ -1,14 +1,18 @@
import { useEffect, useState } from 'react';
import { AlertTriangle } from 'lucide-react';
import { useDebouncedValue } from '@/react/hooks/useDebouncedValue';
import { EnvironmentId } from '@/react/portainer/environments/types';
import { ExpandableMessageByLines } from '@@/ExpandableMessageByLines';
import { FormSection } from '@@/form-components/FormSection';
import { CodeEditor } from '@@/CodeEditor';
import { DiffViewer } from '@@/CodeEditor/DiffViewer';
import { InlineLoader } from '@@/InlineLoader';
import { Alert } from '@@/Alert';
import { TextTip } from '@@/Tip/TextTip';
import { Badge } from '@@/Badge';
import { Icon } from '@@/Icon';
import { useHelmDryRun } from '../helmReleaseQueries/useHelmDryRun';
import { UpdateHelmReleasePayload } from '../types';
@ -49,26 +53,39 @@ export function ManifestPreviewFormSection({
return <InlineLoader>Generating manifest preview...</InlineLoader>;
}
if (manifestPreviewQuery.isError) {
return (
<Alert color="error" title="Error with Helm chart configuration">
{manifestPreviewQuery.error?.message ||
'Error generating manifest preview'}
</Alert>
);
}
return (
<FormSection
title={title}
title={
<>
{title}
{manifestPreviewQuery.isError && (
<Badge
type="dangerSecondary"
className="ml-2"
data-cy="helm-manifest-preview-error-badge"
>
<Icon icon={AlertTriangle} size="md" />
</Badge>
)}
</>
}
isFoldable
defaultFolded={isFolded}
setIsDefaultFolded={setIsFolded}
>
<ManifestPreview
currentManifest={currentManifest}
newManifest={manifestPreviewQuery.data?.manifest ?? ''}
/>
{manifestPreviewQuery.isError ? (
<Alert color="error" title="Error with Helm chart configuration">
<ExpandableMessageByLines>
{manifestPreviewQuery.error?.message ||
'Error generating manifest preview'}
</ExpandableMessageByLines>
</Alert>
) : (
<ManifestPreview
currentManifest={currentManifest}
newManifest={manifestPreviewQuery.data?.manifest ?? ''}
/>
)}
</FormSection>
);
}