Skip to content

Commit

Permalink
[v17] Web: add aws oidc integration health check before editing and w…
Browse files Browse the repository at this point in the history
…hen selecting in discover (#49009)

* Add ping aws integration req/resp boilerplate

* Add aws health check when selecting existing aws integration for discover flow

* Add health check when editing aws oidc integration

* Fix lint

* Add health check when creating integration
  • Loading branch information
kimlisa authored Nov 15, 2024
1 parent 28bec86 commit 4db7ebb
Show file tree
Hide file tree
Showing 13 changed files with 316 additions and 29 deletions.
4 changes: 4 additions & 0 deletions lib/web/integrations_awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,10 @@ func (h *Handler) awsOIDCPing(w http.ResponseWriter, r *http.Request, p httprout
return nil, trace.Wrap(err)
}

if req.RoleARN != "" {
integrationName = ""
}

pingResp, err := clt.IntegrationAWSOIDCClient().Ping(ctx, &integrationv1.PingRequest{
Integration: integrationName,
RoleArn: req.RoleARN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import React from 'react';
import { MemoryRouter } from 'react-router';
import { render, screen } from 'design/utils/testing';
import { render, screen, fireEvent } from 'design/utils/testing';

import { ContextProvider } from 'teleport';
import {
Expand Down Expand Up @@ -144,6 +144,27 @@ test('missing permissions for integrations', async () => {
expect(screen.getByRole('button', { name: /back/i })).toBeInTheDocument();
});

test('health check is called after selecting an aws integration', async () => {
const { ctx, discoverCtx, spyPing } = getMockedContexts({
kind: ResourceKind.Application,
appMeta: { awsConsole: true },
name: '',
icon: undefined,
keywords: [],
event: DiscoverEventResource.ApplicationHttp,
});

renderAwsAccount(ctx, discoverCtx);

await screen.findByText(/AWS Integrations/i);

const selectContainer = screen.getByRole('combobox');
fireEvent.mouseDown(selectContainer);
fireEvent.keyPress(selectContainer, { key: 'Enter' });

expect(spyPing).toHaveBeenCalledTimes(1);
});

function getMockedContexts(resourceSpec: ResourceSpec) {
const ctx = createTeleportContext();
const discoverCtx: DiscoverContextState = {
Expand All @@ -167,7 +188,21 @@ function getMockedContexts(resourceSpec: ResourceSpec) {
.spyOn(userEventService, 'captureDiscoverEvent')
.mockResolvedValue(undefined as never);

return { ctx, discoverCtx };
const spyPing = jest
.spyOn(integrationService, 'fetchIntegrations')
.mockResolvedValue({
items: [
{
resourceType: 'integration',
name: 'aws-oidc-1',
kind: IntegrationKind.AwsOidc,
spec: { roleArn: '111' },
statusCode: IntegrationStatusCode.Running,
},
],
});

return { ctx, discoverCtx, spyPing };
}

function renderAwsAccount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ export function AwsAccount() {
eventState,
resourceSpec,
currentStep,
emitErrorEvent,
} = useDiscover();

const [selectedAwsIntegration, setSelectedAwsIntegration] =
useState<Option>();

// if true, requires an additional step where we fetch for
// apps matching fetched aws integrations to determine
// if an app already exists for the integration the user
Expand Down Expand Up @@ -99,6 +103,18 @@ export function AwsAccount() {
}, [clusterId, isAddingAwsApp])
);

const [healthCheckAttempt, healthCheckSelectedIntegration] = useAsync(
async () => {
await integrationService.pingAwsOidcIntegration(
{
clusterId,
integrationName: selectedAwsIntegration.value.name,
},
{ roleArn: '' }
);
}
);

const integrationAccess = storeUser.getIntegrationsAccess();

let roleTemplate = integrationRWE;
Expand Down Expand Up @@ -137,9 +153,6 @@ export function AwsAccount() {
appAccess.read;
}

const [selectedAwsIntegration, setSelectedAwsIntegration] =
useState<Option>();

useEffect(() => {
if (hasAccess && attempt.status === '') {
fetch();
Expand Down Expand Up @@ -193,11 +206,17 @@ export function AwsAccount() {
);
}

function proceedWithExistingIntegration(validator: Validator) {
async function proceedWithExistingIntegration(validator: Validator) {
if (!validator.validate()) {
return;
}

const [, err] = await healthCheckSelectedIntegration();
if (err) {
emitErrorEvent(`failed to health check selected aws integration: ${err}`);
return;
}

if (
isAddingAwsApp &&
attempt.status === 'success' &&
Expand Down Expand Up @@ -250,6 +269,12 @@ export function AwsAccount() {
return (
<Box maxWidth="700px">
<Heading />
{healthCheckAttempt.status === 'error' && (
<Alert
kind="danger"
children={`Health check failed for the selected AWS integration: ${healthCheckAttempt.statusText}`}
/>
)}
<Box mb={3}>
<Validation>
{({ validator }) => (
Expand Down Expand Up @@ -289,7 +314,11 @@ export function AwsAccount() {
<ActionButtons
onPrev={prevStep}
onProceed={() => proceedWithExistingIntegration(validator)}
disableProceed={!hasAwsIntegrations || !selectedAwsIntegration}
disableProceed={
!hasAwsIntegrations ||
!selectedAwsIntegration ||
healthCheckAttempt.status === 'processing'
}
/>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import { useEffect } from 'react';
import { render, screen, fireEvent, waitFor } from 'design/utils/testing';
import userEvent from '@testing-library/user-event';
import { MemoryRouter } from 'react-router';

import {
Integration,
IntegrationKind,
integrationService,
IntegrationStatusCode,
} from 'teleport/services/integrations';
import cfg from 'teleport/config';

import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog';
import { useIntegrationOperation } from './Operations';

test('user acknowledging script was ran when reconfiguring', async () => {
render(
Expand Down Expand Up @@ -97,6 +102,46 @@ test('user acknowledging script was ran when reconfiguring', async () => {
);
});

test('health check is called before calling update', async () => {
const spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockResolvedValue({} as any); // response doesn't matter

const spyUpdate = jest
.spyOn(integrationService, 'updateIntegration')
.mockResolvedValue({} as any); // response doesn't matter

render(
<MemoryRouter initialEntries={[cfg.getClusterRoute('some-cluster')]}>
<ComponentWithEditOperation />
</MemoryRouter>
);

// 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: /reconfigure/i })).toBeEnabled()
);
await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));

// Click on checkbox to enable save button.
await userEvent.click(screen.getByRole('checkbox'));
await waitFor(() =>
expect(screen.getByRole('button', { name: /save/i })).toBeEnabled()
);
await userEvent.click(screen.getByRole('button', { name: /save/i }));

await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
await waitFor(() => expect(spyUpdate).toHaveBeenCalledTimes(1));

const pingOrder = spyPing.mock.invocationCallOrder[0];
const createOrder = spyUpdate.mock.invocationCallOrder[0];
expect(pingOrder).toBeLessThan(createOrder);
});

test('render warning when s3 buckets are present', async () => {
const edit = jest.fn(() => Promise.resolve());
render(
Expand Down Expand Up @@ -205,7 +250,7 @@ test('edit submit called with proper fields', async () => {
await userEvent.click(screen.getByRole('button', { name: /save/i }));
await waitFor(() => expect(mockEditFn).toHaveBeenCalledTimes(1));

expect(mockEditFn).toHaveBeenCalledWith({
expect(mockEditFn).toHaveBeenCalledWith(integration, {
roleArn: 'arn:aws:iam::123456789011:role/other',
});
});
Expand All @@ -221,3 +266,18 @@ const integration: Integration = {
},
statusCode: IntegrationStatusCode.Running,
};

function ComponentWithEditOperation() {
const integrationOps = useIntegrationOperation();
useEffect(() => {
integrationOps.onEdit(integration);
}, []);

return (
<EditAwsOidcIntegrationDialog
close={() => null}
edit={(integration, req) => integrationOps.edit(integration, req).then()}
integration={integration}
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ import Dialog, {
DialogFooter,
} from 'design/DialogConfirmation';
import { OutlineInfo, OutlineWarn } from 'design/Alert/Alert';
import useAttempt from 'shared/hooks/useAttemptNext';
import FieldInput from 'shared/components/FieldInput';
import Validation, { Validator } from 'shared/components/Validation';
import { requiredRoleArn } from 'shared/components/Validation/rules';
import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy';

import { FieldCheckbox } from 'shared/components/FieldCheckbox';
import { useAsync } from 'shared/hooks/useAsync';

import {
Integration,
Expand All @@ -54,24 +54,26 @@ import { S3BucketConfiguration } from './Enroll/AwsOidc/S3BucketConfiguration';

type Props = {
close(): void;
edit(req: EditableIntegrationFields): Promise<void>;
edit(integration: Integration, req: EditableIntegrationFields): Promise<void>;
integration: Integration;
};

export function EditAwsOidcIntegrationDialog(props: Props) {
const { close, edit, integration } = props;
const { attempt, run } = useAttempt();
const [updateAttempt, runUpdate] = useAsync(async () => {
await edit(integration, { roleArn });
});

const [roleArn, setRoleArn] = useState(integration.spec.roleArn);
const [scriptUrl, setScriptUrl] = useState('');
const [confirmed, setConfirmed] = useState(false);

function handleEdit(validator: Validator) {
async function handleEdit(validator: Validator) {
if (!validator.validate()) {
return;
}

run(() => edit({ roleArn }));
await runUpdate();
}

function generateAwsOidcConfigIdpScript(
Expand Down Expand Up @@ -100,7 +102,7 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
const s3Prefix = integration.spec.issuerS3Prefix;
const showReadonlyS3Fields = s3Bucket || s3Prefix;

const isProcessing = attempt.status === 'processing';
const isProcessing = updateAttempt.status === 'processing';
const showGenerateCommand =
integration.spec.roleArn !== roleArn || showReadonlyS3Fields;

Expand All @@ -122,8 +124,8 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
<DialogTitle>Edit Integration</DialogTitle>
</DialogHeader>
<DialogContent width="650px">
{attempt.status === 'failed' && (
<Alert children={attempt.statusText} />
{updateAttempt.status === 'error' && (
<Alert children={updateAttempt.statusText} />
)}
<FieldInput
label="Integration Name"
Expand Down Expand Up @@ -238,6 +240,7 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
onChange={e => {
setConfirmed(e.target.checked);
}}
disabled={isProcessing}
/>
)}
<ButtonPrimary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import { MemoryRouter } from 'react-router';

import { userEventService } from 'teleport/services/userEvent';

import { integrationService } from 'teleport/services/integrations';
import { ApiError } from 'teleport/services/api/parseError';

import { AwsOidc } from './AwsOidc';

test('render', async () => {
Expand Down Expand Up @@ -56,6 +59,14 @@ test('generate command', async () => {
.spyOn(userEventService, 'captureIntegrationEnrollEvent')
.mockImplementation();

let spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockResolvedValue({} as any); // response doesn't matter

const spyCreate = jest
.spyOn(integrationService, 'createIntegration')
.mockResolvedValue({} as any); // response doesn't matter

window.prompt = jest.fn();

render(
Expand Down Expand Up @@ -95,4 +106,42 @@ test('generate command', async () => {
const clipboardText = await navigator.clipboard.readText();
expect(clipboardText).toContain(`integrationName=${pluginConfig.name}`);
expect(clipboardText).toContain(`role=${pluginConfig.roleName}`);

// Fill out arn.
fireEvent.change(screen.getByLabelText(/role arn/i), {
target: {
value: `arn:aws:iam::123456789012:role/${pluginConfig.roleName}`,
},
});

// Test ping is called before create.
fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(1));

let pingOrder = spyPing.mock.invocationCallOrder[0];
let createOrder = spyCreate.mock.invocationCallOrder[0];
expect(pingOrder).toBeLessThan(createOrder);

// Test create is still called with 404 ping error.
jest.clearAllMocks();
let error = new ApiError('', { status: 404 } as Response);
spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockRejectedValue(error);

fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(1));

// Test create isn't called with non 404 error
jest.clearAllMocks();
error = new ApiError('', { status: 400 } as Response);
spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockRejectedValue(error);

fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(0));
});
Loading

0 comments on commit 4db7ebb

Please sign in to comment.