Skip to content

Commit

Permalink
Web: remove s3 bucket option when creating/editing aws oidc integrati…
Browse files Browse the repository at this point in the history 
…on (#44104)

* Remove s3 related files

* Remove s3 reference

* Remove s3 fields when creating aws oidc

* Remove ability to edit s3 fields, and suggest reconfiguring

* Add a warning icon choice for ToolTip.tsx

* Improve messaging

* Address CRs

* Address CRs
  • Loading branch information
@kimlisa
kimlisa authored Jul 20, 2024
1 parent fc31399 commit ecfa26a
Show file tree
Hide file tree
Showing 19 changed files with 141 additions and 968 deletions.
18 changes: 17 additions & 1 deletion web/packages/shared/components/ToolTip/ToolTip.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ export const ToolTipInfo: React.FC<
muteIconColor?: boolean;
sticky?: boolean;
maxWidth?: number;
kind?: 'info' | 'warning';
}>
> = ({
children,
trigger = 'hover',
muteIconColor,
sticky = false,
maxWidth = 350,
kind = 'info',
}) => {
const [anchorEl, setAnchorEl] = useState();
const open = Boolean(anchorEl);
Expand Down Expand Up @@ -71,7 +73,12 @@ export const ToolTipInfo: React.FC<
height: 18px;
`}
>
<InfoIcon $muteIconColor={muteIconColor} size="medium" />
{kind === 'info' && (
<InfoIcon $muteIconColor={muteIconColor} size="medium" />
)}
{kind === 'warning' && (
<WarningIcon $muteIconColor={muteIconColor} size="medium" />
)}
</span>
<Popover
modalCss={() =>
Expand Down Expand Up @@ -108,3 +115,12 @@ const InfoIcon = styled(Icons.Info)<{ $muteIconColor?: boolean }>`
width: 18px;
color: ${p => (p.$muteIconColor ? p.theme.colors.text.disabled : 'inherit')};
`;

const WarningIcon = styled(Icons.Warning)<{ $muteIconColor?: boolean }>`
height: 18px;
width: 18px;
color: ${p =>
p.$muteIconColor
? p.theme.colors.text.disabled
: p.theme.colors.interactive.solid.alert.default.background};
`;
176 changes: 38 additions & 138 deletions web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {

import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog';

test('user acknowledging script was ran when s3 bucket fields are edited', async () => {
test('user acknowledging script was ran when reconfiguring', async () => {
render(
<EditAwsOidcIntegrationDialog
close={() => null}
Expand All @@ -37,8 +37,6 @@ test('user acknowledging script was ran when s3 bucket fields are edited', async
name: 'some-integration-name',
spec: {
roleArn: 'arn:aws:iam::123456789012:role/johndoe',
issuerS3Bucket: 'test-value',
issuerS3Prefix: '',
},
statusCode: IntegrationStatusCode.Running,
}}
Expand All @@ -49,31 +47,32 @@ test('user acknowledging script was ran when s3 bucket fields are edited', async
expect(screen.queryByTestId('scriptbox')).not.toBeInTheDocument();
expect(screen.queryByLabelText(/I ran the command/i)).not.toBeInTheDocument();
expect(
screen.queryByRole('button', { name: /generate command/i })
screen.queryByRole('button', { name: /reconfigure/i })
).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();

// Fill in the s3 prefix field.
fireEvent.change(screen.getByPlaceholderText(/prefix/i), {
target: { value: 'test-value' },
// Check s3 related fields are not rendered.
expect(screen.queryByText(/not recommended/)).not.toBeInTheDocument();
expect(screen.queryByText('Amazon S3')).not.toBeInTheDocument();

// change role arn
fireEvent.change(screen.getByPlaceholderText(/arn:aws:iam:/i), {
target: { value: 'arn:aws:iam::123456789011:role/other' },
});

await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
expect(screen.getByRole('button', { name: /reconfigure/i })).toBeEnabled()
);
// When clicking on generate command:
// When clicking on reconfigure:
// - script rendered
// - checkbox to confirm user has ran command
// - edit button replaces generate command button
// - edit button replaces reconfigure button
// - save button still disabled
await userEvent.click(
screen.getByRole('button', { name: /generate command/i })
);
await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));
await screen.findByRole('button', { name: /edit/i });
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
expect(
screen.queryByRole('button', { name: /generate command/i })
screen.queryByRole('button', { name: /reconfigure/i })
).not.toBeInTheDocument();
expect(screen.getByLabelText(/I ran the command/i)).toBeInTheDocument();
expect(screen.getByTestId('scriptbox')).toBeInTheDocument();
Expand All @@ -91,16 +90,14 @@ test('user acknowledging script was ran when s3 bucket fields are edited', async
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled()
);

// Click on edit, should replace it with generate command
// Click on edit, should replace it with reconfigure
await userEvent.click(screen.getByRole('button', { name: /edit/i }));
await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
expect(screen.getByRole('button', { name: /reconfigure/i })).toBeEnabled()
);
});

test('render warning on save when leaving s3 fields empty', async () => {
test('render warning when s3 buckets are present', async () => {
const edit = jest.fn(() => Promise.resolve());
render(
<EditAwsOidcIntegrationDialog
Expand All @@ -112,8 +109,8 @@ test('render warning on save when leaving s3 fields empty', async () => {
name: 'some-integration-name',
spec: {
roleArn: 'arn:aws:iam::123456789012:role/johndoe',
issuerS3Bucket: '',
issuerS3Prefix: '',
issuerS3Bucket: 'some-bucket',
issuerS3Prefix: 'some-prefix',
},
statusCode: IntegrationStatusCode.Running,
}}
Expand All @@ -124,112 +121,25 @@ test('render warning on save when leaving s3 fields empty', async () => {
expect(screen.queryByTestId('scriptbox')).not.toBeInTheDocument();
expect(screen.queryByLabelText(/I ran the command/i)).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
expect(
screen.queryByRole('button', { name: /generate command/i })
).not.toBeInTheDocument();

// Enable the generate command button by changing a field.
fireEvent.change(screen.getByPlaceholderText(/arn:aws:iam:/i), {
target: { value: 'arn:aws:iam::123456789012:role/someonelse' },
});
await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
);

expect(screen.queryByLabelText(/I ran the command/i)).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();

await userEvent.click(
screen.getByRole('button', { name: /generate command/i })
);
await screen.findByRole('button', { name: /edit/i });
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();

await userEvent.click(screen.getByLabelText(/I ran the command/i));
await waitFor(() =>
expect(screen.getByRole('button', { name: /save/i })).toBeEnabled()
);

// Clicking on save without defining s3 fields, should render
// a warning.
await userEvent.click(screen.getByRole('button', { name: /save/i }));
await screen.findByText(/recommended to use an S3 bucket/i);
expect(edit).not.toHaveBeenCalled();

// Canceling and saving should re-render the warning.
await userEvent.click(screen.getByRole('button', { name: /cancel/i }));
await screen.findByRole('button', { name: /save/i });

await userEvent.click(screen.getByRole('button', { name: /save/i }));
await screen.findByText(/recommended to use an S3 bucket/i);

await userEvent.click(screen.getByRole('button', { name: /continue/i }));
await waitFor(() => expect(edit).toHaveBeenCalledTimes(1));
});

test('render warning on save when deleting existing s3 fields', async () => {
const edit = jest.fn(() => Promise.resolve());
render(
<EditAwsOidcIntegrationDialog
close={() => null}
edit={edit}
integration={{
resourceType: 'integration',
kind: IntegrationKind.AwsOidc,
name: 'some-integration-name',
spec: {
roleArn: 'arn:aws:iam::123456789012:role/johndoe',
issuerS3Bucket: 'delete-me',
issuerS3Prefix: 'delete-me',
},
statusCode: IntegrationStatusCode.Running,
}}
/>
);

// Check s3 related fields/warnings are rendered.
expect(
screen.queryByRole('button', { name: /generate command/i })
).not.toBeInTheDocument();

// Delete the s3 fields.
fireEvent.change(screen.getByPlaceholderText(/bucket/i), {
target: { value: '' },
});
fireEvent.change(screen.getByPlaceholderText(/prefix/i), {
target: { value: '' },
});
await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
);

expect(screen.queryByLabelText(/I ran the command/i)).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
screen.getByRole('button', { name: /reconfigure/i })
).toBeInTheDocument();
expect(screen.getByText(/not recommended/)).toBeInTheDocument();
expect(screen.getByText(/Amazon S3 Location/)).toBeInTheDocument();

await userEvent.click(
screen.getByRole('button', { name: /generate command/i })
);
await screen.findByRole('button', { name: /edit/i });
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
// Clicking on reconfigure should hide s3 fields.
await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));
await screen.findByText(/AWS CloudShell/);
expect(screen.queryByText(/not recommended/)).not.toBeInTheDocument();
expect(screen.queryByText('/Amazon S3 Location/')).not.toBeInTheDocument();

await userEvent.click(screen.getByLabelText(/I ran the command/i));
await waitFor(() =>
expect(screen.getByRole('button', { name: /save/i })).toBeEnabled()
);

// Test for warning render.
await userEvent.click(screen.getByRole('button', { name: /save/i }));
await screen.findByText(/recommended to use an S3 bucket/i);
expect(edit).not.toHaveBeenCalled();
expect(
screen.getByText(/recommended to use an S3 bucket/i)
).toBeInTheDocument();
// Clicking on edit, should render it back.
await userEvent.click(screen.getByRole('button', { name: /edit/i }));

await userEvent.click(screen.getByRole('button', { name: /continue/i }));
await waitFor(() => expect(edit).toHaveBeenCalledTimes(1));
await screen.findByText(/not recommended/);
await screen.findByText(/Amazon S3 Location/);
});

test('edit invalid fields', async () => {
Expand All @@ -249,14 +159,10 @@ test('edit invalid fields', async () => {
});

await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
expect(screen.getByRole('button', { name: /reconfigure/i })).toBeEnabled()
);

await userEvent.click(
screen.getByRole('button', { name: /generate command/i })
);
await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));
await screen.findByText(/invalid role ARN format/i);
});

Expand Down Expand Up @@ -286,14 +192,10 @@ test('edit submit called with proper fields', async () => {
});

await waitFor(() =>
expect(
screen.getByRole('button', { name: /generate command/i })
).toBeEnabled()
expect(screen.getByRole('button', { name: /reconfigure/i })).toBeEnabled()
);

await userEvent.click(
screen.getByRole('button', { name: /generate command/i })
);
await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));
await screen.findByRole('button', { name: /edit/i });

await userEvent.click(screen.getByLabelText(/I ran the command/i));
Expand All @@ -305,8 +207,6 @@ test('edit submit called with proper fields', async () => {

expect(mockEditFn).toHaveBeenCalledWith({
roleArn: 'arn:aws:iam::123456789011:role/other',
s3Bucket: 'other-bucket',
s3Prefix: 'other-prefix',
});
});

Expand Down
Loading

0 comments on commit ecfa26a

Please sign in to comment.