Skip to content

Commit

Permalink
Web: Improve RDS enrollment flow
Browse files Browse the repository at this point in the history
- user selects a vpc to see RDS's
- restrict deploying service to user selected vpc
- allow and require selecting subnets and security groups
- refactor EnrollRdsDatabase.tsx into SingleEnrollment and AutoEnrollment
- for self hosted, move configuring discovery config into own step
- fixes a query bug where not all db servers were returning because
  possible duplicates were not accounted for (fixed by removing limit,
  default limit is 1k which should be plenty)
  • Loading branch information
kimlisa committed Aug 1, 2024
1 parent 2885f03 commit 584821c
Show file tree
Hide file tree
Showing 39 changed files with 2,400 additions and 754 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ const newDatabaseReq: CreateDatabaseRequest = {
};

jest.useFakeTimers();
const defaultIsCloud = cfg.isCloud;

describe('registering new databases, mainly error checking', () => {
const discoverCtx: DiscoverContextState = {
Expand Down Expand Up @@ -277,6 +278,7 @@ describe('registering new databases, mainly error checking', () => {
let wrapper;

beforeEach(() => {
cfg.isCloud = true;
jest.spyOn(api, 'get').mockResolvedValue([]); // required for fetchClusterAlerts

jest
Expand Down Expand Up @@ -313,6 +315,7 @@ describe('registering new databases, mainly error checking', () => {
});

afterEach(() => {
cfg.isCloud = defaultIsCloud;
jest.clearAllMocks();
});

Expand Down Expand Up @@ -344,6 +347,9 @@ describe('registering new databases, mainly error checking', () => {
// of steps to skip.
result.current.nextStep();
expect(discoverCtx.nextStep).toHaveBeenCalledWith(2);
cfg.isCloud = false;
result.current.nextStep();
expect(discoverCtx.nextStep).toHaveBeenCalledWith(3);
});

test('continue polling when poll result returns with iamPolicyStatus field set to "pending"', async () => {
Expand Down Expand Up @@ -399,6 +405,9 @@ describe('registering new databases, mainly error checking', () => {
result.current.nextStep();
// Skips both deploy service AND IAM policy step.
expect(discoverCtx.nextStep).toHaveBeenCalledWith(3);
cfg.isCloud = false;
result.current.nextStep();
expect(discoverCtx.nextStep).toHaveBeenCalledWith(4);
});

test('stops polling when poll result returns with iamPolicyStatus field set to "unspecified"', async () => {
Expand Down Expand Up @@ -467,6 +476,9 @@ describe('registering new databases, mainly error checking', () => {
// number of steps to skip defined.
result.current.nextStep();
expect(discoverCtx.nextStep).toHaveBeenCalledWith();
cfg.isCloud = false;
result.current.nextStep();
expect(discoverCtx.nextStep).toHaveBeenCalledWith(2);
});

test('when failed to create db, stops flow', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { compareByString } from 'teleport/lib/util';
import { ApiError } from 'teleport/services/api/parseError';
import { DatabaseLocation } from 'teleport/Discover/SelectResource';
import { IamPolicyStatus } from 'teleport/services/databases';
import cfg from 'teleport/config';

import { matchLabels } from '../common';

Expand Down Expand Up @@ -121,6 +122,7 @@ export function useCreateDatabase() {
...(agentMeta as DbMeta),
resourceName: createdDb.name,
awsRegion: createdDb.awsRegion,
awsVpcId: createdDb.awsVpcId,
agentMatcherLabels: dbPollingResult.labels,
db: dbPollingResult,
serviceDeployedMethod:
Expand Down Expand Up @@ -162,10 +164,9 @@ export function useCreateDatabase() {
});
}

function fetchDatabaseServers(query: string, limit: number) {
function fetchDatabaseServers(query: string) {
const request = {
query,
limit,
};
return ctx.databaseService.fetchDatabases(clusterId, request);
}
Expand Down Expand Up @@ -223,6 +224,7 @@ export function useCreateDatabase() {
awsRegion: db.awsRegion,
agentMatcherLabels: db.labels,
selectedAwsRdsDb: db.awsRds,
awsVpcId: db.awsVpcId,
});
setAttempt({ status: 'success' });
return;
Expand Down Expand Up @@ -315,6 +317,11 @@ export function useCreateDatabase() {
}

function handleNextStep() {
if (isAws && !cfg.isCloud) {
handleNextStepForSelfHostedAwsEnrollment();
return;
}

if (dbPollingResult) {
if (
isAws &&
Expand All @@ -326,13 +333,28 @@ export function useCreateDatabase() {
// Skips the deploy database service step.
return nextStep(2);
}
nextStep(); // Goes to deploy database service step.
}

const meta = agentMeta as DbMeta;
if (meta.autoDiscovery && meta.serviceDeployedMethod === 'skipped') {
// IAM policy setup is not required for auto discover.
/**
* self hosted AWS enrollment flow has one additional step
* called the Configure Discovery Service. This step is
* only required if user enabled auto discovery.
* If a user is here in "useCreateDatabase" then user did not
* opt for auto discovery (auto discovery will auto create dbs),
* so we need to skip this step here.
*/
function handleNextStepForSelfHostedAwsEnrollment() {
if (dbPollingResult) {
if (dbPollingResult.aws?.iamPolicyStatus === IamPolicyStatus.Success) {
// Skips configure discovery service, deploy db service AND
// setting up IAM policy step
return nextStep(4);
}
// Skips the configure discovery service and deploy database service step.
return nextStep(3);
}
nextStep(); // Goes to deploy database service step.
nextStep(2); // Skips the discovery service (goes to deploy database service step).
}

const access = ctx.storeUser.getDatabaseAccess();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,27 @@ export const Init = () => {
Init.parameters = {
msw: {
handlers: [
http.post(cfg.getListSecurityGroupsUrl('test-integration'), () =>
http.post(cfg.api.awsSecurityGroupsListPath, () =>
HttpResponse.json({ securityGroups: securityGroupsResponse })
),
http.post(cfg.api.awsDeployTeleportServicePath, () =>
HttpResponse.json({ serviceDashboardUrl: 'some-dashboard-url' })
),
http.post(cfg.api.awsSubnetListPath, () =>
HttpResponse.json({ subnets: subnetsResponse })
),
],
},
};

export const InitWithAutoEnroll = () => {
export const InitWithAutoDiscover = () => {
return (
<TeleportProvider
resourceKind={ResourceKind.Database}
agentMeta={{
...getDbMeta(),
autoDiscovery: {
config: { name: '', discoveryGroup: '', aws: [] },
requiredVpcsAndSubnets: {},
},
}}
resourceSpec={getDbResourceSpec(
Expand All @@ -81,17 +83,20 @@ export const InitWithAutoEnroll = () => {
</TeleportProvider>
);
};
InitWithAutoEnroll.parameters = {
InitWithAutoDiscover.parameters = {
msw: {
handlers: [
http.post(cfg.getListSecurityGroupsUrl('test-integration'), () =>
http.post(cfg.api.awsSecurityGroupsListPath, () =>
HttpResponse.json({ securityGroups: securityGroupsResponse })
),
http.post(cfg.getAwsRdsDbsDeployServicesUrl('test-integration'), () =>
HttpResponse.json({
clusterDashboardUrl: 'some-cluster-dashboard-url',
})
),
http.post(cfg.api.awsSubnetListPath, () =>
HttpResponse.json({ subnets: subnetsResponse })
),
],
},
};
Expand Down Expand Up @@ -119,7 +124,7 @@ export const InitWithLabelsWithDeployFailure = () => {
InitWithLabelsWithDeployFailure.parameters = {
msw: {
handlers: [
http.post(cfg.getListSecurityGroupsUrl('test-integration'), () =>
http.post(cfg.api.awsSecurityGroupsListPath, () =>
HttpResponse.json({ securityGroups: securityGroupsResponse })
),
http.post(cfg.api.awsDeployTeleportServicePath, () =>
Expand All @@ -130,6 +135,9 @@ InitWithLabelsWithDeployFailure.parameters = {
{ status: 500 }
)
),
http.post(cfg.api.awsSubnetListPath, () =>
HttpResponse.json({ subnets: subnetsResponse })
),
],
},
};
Expand All @@ -145,14 +153,22 @@ export const InitSecurityGroupsLoadingFailed = () => {
InitSecurityGroupsLoadingFailed.parameters = {
msw: {
handlers: [
http.post(cfg.getListSecurityGroupsUrl('test-integration'), () =>
http.post(cfg.api.awsSecurityGroupsListPath, () =>
HttpResponse.json(
{
message: 'some error when trying to list security groups',
},
{ status: 403 }
)
),
http.post(cfg.api.awsSubnetListPath, () =>
HttpResponse.json(
{
error: { message: 'Whoops, error getting subnets' },
},
{ status: 403 }
)
),
],
},
};
Expand All @@ -168,13 +184,45 @@ export const InitSecurityGroupsLoading = () => {
InitSecurityGroupsLoading.parameters = {
msw: {
handlers: [
http.post(cfg.getListSecurityGroupsUrl('test-integration'), () =>
delay('infinite')
),
http.post(cfg.api.awsSecurityGroupsListPath, () => delay('infinite')),
http.post(cfg.api.awsSubnetListPath, () => delay('infinite')),
],
},
};

const subnetsResponse = [
{
name: 'aws-something-PrivateSubnet1A',
id: 'subnet-e40cd872-74de-54e3-a081',
availability_zone: 'us-east-1c',
},
{
name: 'aws-something-PrivateSubnet2A',
id: 'subnet-e6f9e40e-a7c7-52ab-b8e8',
availability_zone: 'us-east-1a',
},
{
name: '',
id: 'subnet-9106bc09-ea32-5216-ae3b',
availability_zone: 'us-east-1b',
},
{
name: '',
id: 'subnet-0ee385cf-b090-5cf7-b692',
availability_zone: 'us-east-1c',
},
{
name: 'something-long-test-1-cluster/SubnetPublicU',
id: 'subnet-0f0b563e-629f-5921-841d',
availability_zone: 'us-east-1c',
},
{
name: 'something-long-test-1-cluster/SubnetPrivateUS',
id: 'subnet-30c9e2f6-65ce-5422-bbc0',
availability_zone: 'us-east-1c',
},
];

const securityGroupsResponse = [
{
name: 'security-group-1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@

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

import { ContextProvider } from 'teleport';
import {
Expand Down Expand Up @@ -85,23 +91,55 @@ describe('test AutoDeploy.tsx', () => {
jest.useFakeTimers();

beforeEach(() => {
jest.spyOn(integrationService, 'fetchAwsSubnets').mockResolvedValue({
nextToken: '',
subnets: [
{
name: 'subnet-name',
id: 'subnet-id',
availabilityZone: 'subnet-az',
},
],
});
jest.spyOn(integrationService, 'fetchSecurityGroups').mockResolvedValue({
nextToken: '',
securityGroups: [
{
name: 'sg-name',
id: 'sg-id',
description: 'sg-desc',
inboundRules: [],
outboundRules: [],
},
],
});
});

afterEach(() => {
jest.restoreAllMocks();
});

test('init: labels are rendered, command is not rendered yet', () => {
async function waitForSubnetsAndSecurityGroups() {
await screen.findByText('sg-id');
await screen.findByText('subnet-id');
}

test('init: labels are rendered, command is not rendered yet', async () => {
const { teleCtx, discoverCtx } = getMockedContexts();

renderAutoDeploy(teleCtx, discoverCtx);
await waitForSubnetsAndSecurityGroups();

expect(screen.getByText(/env: prod/i)).toBeInTheDocument();
expect(screen.queryByText(/copy\/paste/i)).not.toBeInTheDocument();
expect(screen.queryByText(/curl/i)).not.toBeInTheDocument();
});

test('clicking button renders command', () => {
test('clicking button renders command', async () => {
const { teleCtx, discoverCtx } = getMockedContexts();

renderAutoDeploy(teleCtx, discoverCtx);
await waitForSubnetsAndSecurityGroups();

fireEvent.click(screen.getByText(/generate command/i));

Expand All @@ -113,10 +151,11 @@ describe('test AutoDeploy.tsx', () => {
).toBeInTheDocument();
});

test('invalid role name', () => {
test('invalid role name', async () => {
const { teleCtx, discoverCtx } = getMockedContexts();

renderAutoDeploy(teleCtx, discoverCtx);
await waitForSubnetsAndSecurityGroups();

expect(
screen.queryByText(/name can only contain/i)
Expand All @@ -140,6 +179,23 @@ describe('test AutoDeploy.tsx', () => {
const { teleCtx, discoverCtx } = getMockedContexts();

renderAutoDeploy(teleCtx, discoverCtx);
await waitForSubnetsAndSecurityGroups();

fireEvent.click(screen.getByText(/Deploy Teleport Service/i));

// select required subnet
expect(
screen.getByText(/one subnet selection is required/i)
).toBeInTheDocument();
fireEvent.click(screen.getByTestId(/subnet-id/i));

fireEvent.click(screen.getByText(/Deploy Teleport Service/i));

// select required sg
expect(
screen.getByText(/one security group selection is required/i)
).toBeInTheDocument();
fireEvent.click(screen.getByTestId(/sg-id/i));

fireEvent.click(screen.getByText(/Deploy Teleport Service/i));

Expand Down
Loading

0 comments on commit 584821c

Please sign in to comment.