Skip to content

Commit

Permalink
Address CRs
Browse files Browse the repository at this point in the history
  • Loading branch information
kimlisa committed Jan 13, 2025
1 parent 5485a1f commit 0e1787d
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ import {
DiscoverEventResource,
userEventService,
} from 'teleport/services/userEvent';
import { ProxyRequiresUpgrade } from 'teleport/services/version/unsupported';
import TeleportContext from 'teleport/teleportContext';

import { CreateAppAccess } from './CreateAppAccess';

beforeEach(() => {
jest.spyOn(integrationService, 'createAwsAppAccess').mockResolvedValue(app);
jest.spyOn(integrationService, 'createAwsAppAccessV2').mockResolvedValue(app);
jest
.spyOn(userEventService, 'captureDiscoverEvent')
.mockResolvedValue(undefined as never);
Expand All @@ -60,6 +61,25 @@ test('create app access', async () => {
renderCreateAppAccess(ctx, discoverCtx);
await screen.findByText(/bash/i);

await userEvent.click(screen.getByRole('button', { name: /next/i }));
await screen.findByText(/aws-console/i);
expect(integrationService.createAwsAppAccessV2).toHaveBeenCalledTimes(1);
});

test('create app access with v1 endpoint', async () => {
jest
.spyOn(integrationService, 'createAwsAppAccessV2')
.mockRejectedValueOnce(new Error(ProxyRequiresUpgrade));
jest.spyOn(integrationService, 'createAwsAppAccess').mockResolvedValue(app);

const { ctx, discoverCtx } = getMockedContexts();

renderCreateAppAccess(ctx, discoverCtx);
await screen.findByText(/bash/i);

await userEvent.click(screen.getByRole('button', { name: /next/i }));
await screen.findByText(ProxyRequiresUpgrade);

await userEvent.click(screen.getByRole('button', { name: /next/i }));
await screen.findByText(/aws-console/i);
expect(integrationService.createAwsAppAccess).toHaveBeenCalledTimes(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ import { Container } from 'teleport/Discover/Shared/CommandBox';
import { ResourceLabelTooltip } from 'teleport/Discover/Shared/ResourceLabelTooltip';
import { useDiscover } from 'teleport/Discover/useDiscover';
import { ResourceLabel } from 'teleport/services/agents';
import { App } from 'teleport/services/apps/types';
import { integrationService } from 'teleport/services/integrations';
import { splitAwsIamArn } from 'teleport/services/integrations/aws';
import { ProxyRequiresUpgrade } from 'teleport/services/version/unsupported';

import { ActionButtons, Header, LabelsCreater } from '../../Shared';
import { AppCreatedDialog } from './AppCreatedDialog';
Expand All @@ -47,20 +49,31 @@ export function CreateAppAccess() {
const { awsIntegration } = agentMeta;
const [labels, setLabels] = useState<ResourceLabel[]>([]);

// TODO(kimlisa): DELETE IN 19.0
const [requiresProxyUpgrade, setRequiresProxyUpgrade] = useState(false);

const [attempt, createApp] = useAsync(async () => {
const mapOfLabels: Record<string, string> = {};
labels.forEach(l => (mapOfLabels[l.name] = l.value));
const labelsMap: Record<string, string> = {};
labels.forEach(l => (labelsMap[l.name] = l.value));
try {
const app = await integrationService.createAwsAppAccess(
awsIntegration.name,
{ labels: mapOfLabels }
);
let app: App;
if (requiresProxyUpgrade && !labels.length) {
app = await integrationService.createAwsAppAccess(awsIntegration.name);
} else {
app = await integrationService.createAwsAppAccessV2(
awsIntegration.name,
{ labels: labelsMap }
);
}
updateAgentMeta({
...agentMeta,
app,
resourceName: app.name,
});
} catch (err) {
if (err.message.includes(ProxyRequiresUpgrade)) {
setRequiresProxyUpgrade(true);
}
emitErrorEvent(err.message);
throw err;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,32 +244,6 @@ test('enrollEksClusters with labels calls v2', async () => {
);
});

test('createAwsAppAccess without labels calls v1', async () => {
jest.spyOn(api, 'post').mockResolvedValue({});

await integrationService.createAwsAppAccess('integration', {});

expect(api.post).toHaveBeenCalledWith(
cfg.getAwsAppAccessUrl('integration'),
{}
);
});

test('createAwsAppAccess with labels calls v2', async () => {
jest.spyOn(api, 'post').mockResolvedValue({});

await integrationService.createAwsAppAccess('integration', {
labels: { env: 'staging' },
});

expect(api.post).toHaveBeenCalledWith(
cfg.getAwsAppAccessUrlV2('integration'),
{
labels: { env: 'staging' },
}
);
});

describe('fetchAwsDatabases() request body formatting', () => {
test.each`
protocol | expectedEngines | expectedRdsType
Expand Down
17 changes: 9 additions & 8 deletions web/packages/teleport/src/services/integrations/integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,10 @@ export const integrationService = {
.then(resp => resp.serviceDashboardUrl);
},

async createAwsAppAccess(
async createAwsAppAccessV2(
integrationName,
req: CreateAwsAppAccessRequest
): Promise<App> {
// TODO(kimlisa): DELETE IN 19.0 - replaced by v2 endpoint.
if (!req.labels || !Object.keys(req.labels).length) {
return api
.post(cfg.getAwsAppAccessUrl(integrationName), req)
.then(makeApp);
}

return (
api
.post(cfg.getAwsAppAccessUrlV2(integrationName), req)
Expand All @@ -313,6 +306,14 @@ export const integrationService = {
);
},

// TODO(kimlisa): DELETE IN 19.0
// replaced by createAwsAppAccessV2 that accepts request body
async createAwsAppAccess(integrationName): Promise<App> {
return api
.post(cfg.getAwsAppAccessUrl(integrationName), null)
.then(makeApp);
},

async deployDatabaseServices(
integrationName,
req: AwsOidcDeployDatabaseServicesRequest
Expand Down
9 changes: 9 additions & 0 deletions web/packages/teleport/src/services/integrations/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,15 @@ export type AwsDatabaseVpcsResponse = {
nextToken: string;
};

/**
* Object that contains request fields for
* when requesting to create an AWS console app.
*
* This request object is only supported with v2 endpoint.
*/
export type CreateAwsAppAccessRequest = {
/**
* resource labels that will be set as app_server's labels
*/
labels?: Record<string, string>;
};
5 changes: 4 additions & 1 deletion web/packages/teleport/src/services/version/unsupported.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

import { ApiError } from '../api/parseError';

export const ProxyRequiresUpgrade =
'Ensure all proxies are upgraded or remove labels and try again.';

export function withUnsupportedLabelFeatureErrorConversion(
err: unknown
): never {
Expand All @@ -26,7 +29,7 @@ export function withUnsupportedLabelFeatureErrorConversion(
'We could not complete your request. ' +
'Your proxy may be behind the minimum required version ' +
`(v17.2.0) to support adding resource labels. ` +
'Ensure all proxies are upgraded or remove labels and try again.'
ProxyRequiresUpgrade
);
}
throw err;
Expand Down

0 comments on commit 0e1787d

Please sign in to comment.