Skip to content

Commit 42721df

Browse files
committed
Address CRs
1 parent 5485a1f commit 42721df

File tree

6 files changed

+61
-42
lines changed

6 files changed

+61
-42
lines changed

web/packages/teleport/src/Discover/AwsMangementConsole/CreateAppAccess/CreateAppAccess.test.tsx

+21-1
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@ import {
3939
DiscoverEventResource,
4040
userEventService,
4141
} from 'teleport/services/userEvent';
42+
import { ProxyRequiresUpgrade } from 'teleport/services/version/unsupported';
4243
import TeleportContext from 'teleport/teleportContext';
4344

4445
import { CreateAppAccess } from './CreateAppAccess';
4546

4647
beforeEach(() => {
47-
jest.spyOn(integrationService, 'createAwsAppAccess').mockResolvedValue(app);
48+
jest.spyOn(integrationService, 'createAwsAppAccessV2').mockResolvedValue(app);
4849
jest
4950
.spyOn(userEventService, 'captureDiscoverEvent')
5051
.mockResolvedValue(undefined as never);
@@ -60,6 +61,25 @@ test('create app access', async () => {
6061
renderCreateAppAccess(ctx, discoverCtx);
6162
await screen.findByText(/bash/i);
6263

64+
await userEvent.click(screen.getByRole('button', { name: /next/i }));
65+
await screen.findByText(/aws-console/i);
66+
expect(integrationService.createAwsAppAccessV2).toHaveBeenCalledTimes(1);
67+
});
68+
69+
test('create app access with v1 endpoint', async () => {
70+
jest
71+
.spyOn(integrationService, 'createAwsAppAccessV2')
72+
.mockRejectedValueOnce(new Error(ProxyRequiresUpgrade));
73+
jest.spyOn(integrationService, 'createAwsAppAccess').mockResolvedValue(app);
74+
75+
const { ctx, discoverCtx } = getMockedContexts();
76+
77+
renderCreateAppAccess(ctx, discoverCtx);
78+
await screen.findByText(/bash/i);
79+
80+
await userEvent.click(screen.getByRole('button', { name: /next/i }));
81+
await screen.findByText(ProxyRequiresUpgrade);
82+
6383
await userEvent.click(screen.getByRole('button', { name: /next/i }));
6484
await screen.findByText(/aws-console/i);
6585
expect(integrationService.createAwsAppAccess).toHaveBeenCalledTimes(1);

web/packages/teleport/src/Discover/AwsMangementConsole/CreateAppAccess/CreateAppAccess.tsx

+19-6
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ import { Container } from 'teleport/Discover/Shared/CommandBox';
3333
import { ResourceLabelTooltip } from 'teleport/Discover/Shared/ResourceLabelTooltip';
3434
import { useDiscover } from 'teleport/Discover/useDiscover';
3535
import { ResourceLabel } from 'teleport/services/agents';
36+
import { App } from 'teleport/services/apps/types';
3637
import { integrationService } from 'teleport/services/integrations';
3738
import { splitAwsIamArn } from 'teleport/services/integrations/aws';
39+
import { ProxyRequiresUpgrade } from 'teleport/services/version/unsupported';
3840

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

52+
// TODO(kimlisa): DELETE IN 19.0
53+
const [requiresProxyUpgrade, setRequiresProxyUpgrade] = useState(false);
54+
5055
const [attempt, createApp] = useAsync(async () => {
51-
const mapOfLabels: Record<string, string> = {};
52-
labels.forEach(l => (mapOfLabels[l.name] = l.value));
56+
const labelsMap: Record<string, string> = {};
57+
labels.forEach(l => (labelsMap[l.name] = l.value));
5358
try {
54-
const app = await integrationService.createAwsAppAccess(
55-
awsIntegration.name,
56-
{ labels: mapOfLabels }
57-
);
59+
let app: App;
60+
if (requiresProxyUpgrade && !labels.length) {
61+
app = await integrationService.createAwsAppAccess(awsIntegration.name);
62+
} else {
63+
app = await integrationService.createAwsAppAccessV2(
64+
awsIntegration.name,
65+
{ labels: labelsMap }
66+
);
67+
}
5868
updateAgentMeta({
5969
...agentMeta,
6070
app,
6171
resourceName: app.name,
6272
});
6373
} catch (err) {
74+
if (err.message.includes(ProxyRequiresUpgrade)) {
75+
setRequiresProxyUpgrade(true);
76+
}
6477
emitErrorEvent(err.message);
6578
throw err;
6679
}

web/packages/teleport/src/services/integrations/integrations.test.ts

-26
Original file line numberDiff line numberDiff line change
@@ -244,32 +244,6 @@ test('enrollEksClusters with labels calls v2', async () => {
244244
);
245245
});
246246

247-
test('createAwsAppAccess without labels calls v1', async () => {
248-
jest.spyOn(api, 'post').mockResolvedValue({});
249-
250-
await integrationService.createAwsAppAccess('integration', {});
251-
252-
expect(api.post).toHaveBeenCalledWith(
253-
cfg.getAwsAppAccessUrl('integration'),
254-
{}
255-
);
256-
});
257-
258-
test('createAwsAppAccess with labels calls v2', async () => {
259-
jest.spyOn(api, 'post').mockResolvedValue({});
260-
261-
await integrationService.createAwsAppAccess('integration', {
262-
labels: { env: 'staging' },
263-
});
264-
265-
expect(api.post).toHaveBeenCalledWith(
266-
cfg.getAwsAppAccessUrlV2('integration'),
267-
{
268-
labels: { env: 'staging' },
269-
}
270-
);
271-
});
272-
273247
describe('fetchAwsDatabases() request body formatting', () => {
274248
test.each`
275249
protocol | expectedEngines | expectedRdsType

web/packages/teleport/src/services/integrations/integrations.ts

+9-8
Original file line numberDiff line numberDiff line change
@@ -293,17 +293,10 @@ export const integrationService = {
293293
.then(resp => resp.serviceDashboardUrl);
294294
},
295295

296-
async createAwsAppAccess(
296+
async createAwsAppAccessV2(
297297
integrationName,
298298
req: CreateAwsAppAccessRequest
299299
): Promise<App> {
300-
// TODO(kimlisa): DELETE IN 19.0 - replaced by v2 endpoint.
301-
if (!req.labels || !Object.keys(req.labels).length) {
302-
return api
303-
.post(cfg.getAwsAppAccessUrl(integrationName), req)
304-
.then(makeApp);
305-
}
306-
307300
return (
308301
api
309302
.post(cfg.getAwsAppAccessUrlV2(integrationName), req)
@@ -313,6 +306,14 @@ export const integrationService = {
313306
);
314307
},
315308

309+
// TODO(kimlisa): DELETE IN 19.0
310+
// replaced by createAwsAppAccessV2 that accepts request body
311+
async createAwsAppAccess(integrationName): Promise<App> {
312+
return api
313+
.post(cfg.getAwsAppAccessUrl(integrationName), null)
314+
.then(makeApp);
315+
},
316+
316317
async deployDatabaseServices(
317318
integrationName,
318319
req: AwsOidcDeployDatabaseServicesRequest

web/packages/teleport/src/services/integrations/types.ts

+9
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,15 @@ export type AwsDatabaseVpcsResponse = {
760760
nextToken: string;
761761
};
762762

763+
/**
764+
* Object that contains request fields for
765+
* when requesting to create an AWS console app.
766+
*
767+
* This request object is only supported with v2 endpoint.
768+
*/
763769
export type CreateAwsAppAccessRequest = {
770+
/**
771+
* resource labels that will be set as app_server's labels
772+
*/
764773
labels?: Record<string, string>;
765774
};

web/packages/teleport/src/services/version/unsupported.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

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

21+
export const ProxyRequiresUpgrade = 'Ensure all proxies are upgraded';
22+
2123
export function withUnsupportedLabelFeatureErrorConversion(
2224
err: unknown
2325
): never {
@@ -26,7 +28,7 @@ export function withUnsupportedLabelFeatureErrorConversion(
2628
'We could not complete your request. ' +
2729
'Your proxy may be behind the minimum required version ' +
2830
`(v17.2.0) to support adding resource labels. ` +
29-
'Ensure all proxies are upgraded or remove labels and try again.'
31+
`${ProxyRequiresUpgrade} or remove labels and try again.`
3032
);
3133
}
3234
throw err;

0 commit comments

Comments
 (0)