Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 68 additions & 2 deletions packages/plugins/apps/src/upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2019-Present Datadog, Inc.

import { getData, getIntakeUrl, uploadArchive } from '@dd/apps-plugin/upload';
import { getData, getIntakeUrl, getReleaseUrl, uploadArchive } from '@dd/apps-plugin/upload';
import { getDDEnvValue } from '@dd/core/helpers/env';
import { getFile } from '@dd/core/helpers/fs';
import {
Expand Down Expand Up @@ -73,11 +73,43 @@ describe('Apps Plugin - upload', () => {
expect(getIntakeUrl('datadoghq.com', 'my-app')).toBe('https://custom.apps');
});

test('Should fallback to default intake url', () => {
test('Should prefix for all Datadog sites', () => {
getDDEnvValueMock.mockReturnValue(undefined);
expect(getIntakeUrl('datadoghq.com', 'my-app')).toBe(
'https://api.datadoghq.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('datadoghq.eu', 'my-app')).toBe(
'https://api.datadoghq.eu/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('ddog-gov.com', 'my-app')).toBe(
'https://api.ddog-gov.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('us5.datadoghq.com', 'my-app')).toBe(
'https://api.us5.datadoghq.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('dd.datad0g.com', 'my-app')).toBe(
'https://api.dd.datad0g.com/api/unstable/app-builder-code/apps/my-app/upload',
);
Comment on lines +78 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need to test for all DD sites.
One should be well enough to verify the string update.
Same for the following test.

Suggested change
expect(getIntakeUrl('datadoghq.com', 'my-app')).toBe(
'https://api.datadoghq.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('datadoghq.eu', 'my-app')).toBe(
'https://api.datadoghq.eu/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('ddog-gov.com', 'my-app')).toBe(
'https://api.ddog-gov.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('us5.datadoghq.com', 'my-app')).toBe(
'https://api.us5.datadoghq.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('dd.datad0g.com', 'my-app')).toBe(
'https://api.dd.datad0g.com/api/unstable/app-builder-code/apps/my-app/upload',
);
expect(getIntakeUrl('datadoghq.eu', 'my-app')).toBe(
'https://api.datadoghq.eu/api/unstable/app-builder-code/apps/my-app/upload',
);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're all valid DD sites, and imo more tests/more explicit tests are better than fewer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just string substitution, apart from adding overhead and unnecessary execution time, there is no added value to test these.

We could test for getIntakeUrl('toto', 'hello') giving us 'https://api.toto/api/unstable/app-builder-code/apps/hello/upload' with the same result in coverage and testing efficiency.

Here, more tests only means more executions for no added value at all.

});
});

describe('getReleaseUrl', () => {
test('Should prefix for all Datadog sites', () => {
expect(getReleaseUrl('datadoghq.com', 'my-app')).toBe(
'https://api.datadoghq.com/api/unstable/app-builder-code/apps/my-app/release/live',
);
expect(getReleaseUrl('datadoghq.eu', 'my-app')).toBe(
'https://api.datadoghq.eu/api/unstable/app-builder-code/apps/my-app/release/live',
);
expect(getReleaseUrl('ddog-gov.com', 'my-app')).toBe(
'https://api.ddog-gov.com/api/unstable/app-builder-code/apps/my-app/release/live',
);
expect(getReleaseUrl('us5.datadoghq.com', 'my-app')).toBe(
'https://api.us5.datadoghq.com/api/unstable/app-builder-code/apps/my-app/release/live',
);
expect(getReleaseUrl('dd.datad0g.com', 'my-app')).toBe(
'https://api.dd.datad0g.com/api/unstable/app-builder-code/apps/my-app/release/live',
);
});
});

Expand Down Expand Up @@ -226,6 +258,40 @@ describe('Apps Plugin - upload', () => {
);
});

test('Should make PUT request to release version when APPS_VERSION_NAME is set', async () => {
getDDEnvValueMock.mockImplementation((key) => {
if (key === 'APPS_VERSION_NAME') {
return 'my-version';
}
return undefined;
});
doRequestMock
.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
})
.mockResolvedValueOnce({});

const { errors, warnings } = await uploadArchive(archive, context, logger);

expect(errors).toHaveLength(0);
expect(warnings).toHaveLength(0);
expect(doRequestMock).toHaveBeenCalledTimes(2);
expect(doRequestMock).toHaveBeenNthCalledWith(2, {
auth: { apiKey: 'api-key', appKey: 'app-key' },
url: 'https://api.datadoghq.com/api/unstable/app-builder-code/apps/repo:app/release/live',
method: 'PUT',
type: 'json',
getData: expect.any(Function),
onRetry: expect.any(Function),
});
expect(mockLogFn).toHaveBeenCalledWith(
expect.stringContaining('Released version'),
'info',
);
});

test('Should collect warnings on retries', async () => {
doRequestMock.mockImplementation(async (opts) => {
opts.onRetry?.(new Error('network'), 2);
Expand Down
31 changes: 31 additions & 0 deletions packages/plugins/apps/src/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { prettyObject } from '@dd/core/helpers/strings';
import type { Logger } from '@dd/core/types';
import chalk from 'chalk';
import prettyBytes from 'pretty-bytes';
import { Readable } from 'stream';

import type { Archive } from './archive';
import { APPS_API_PATH, ARCHIVE_FILENAME } from './constants';
Expand Down Expand Up @@ -41,6 +42,10 @@ export const getIntakeUrl = (site: string, appId: string) => {
return envIntake || `https://api.${site}/${APPS_API_PATH}/${appId}/upload`;
};

export const getReleaseUrl = (site: string, appId: string) => {
return `https://api.${site}/${APPS_API_PATH}/${appId}/release/live`;
};

export const getData =
(archivePath: string, defaultHeaders: Record<string, string> = {}, name: string) =>
async (): Promise<DataResponse> => {
Expand Down Expand Up @@ -135,6 +140,32 @@ Would have uploaded ${summary}`,
`Your application is available at:\n${bold('Standalone :')}\n ${cyan(appUrl)}\n\n${bold('AppBuilder :')}\n ${cyan(appBuilderUrl)}`,
);
}

const versionName = getDDEnvValue('APPS_VERSION_NAME')?.trim();
if (versionName) {
const releaseUrl = getReleaseUrl(context.site, context.identifier);
await doRequest({
auth: { apiKey: context.apiKey, appKey: context.appKey },
url: releaseUrl,
method: 'PUT',
type: 'json',
getData: async () => ({
data: Readable.from(JSON.stringify({ version_id: versionName })),
headers: {
'Content-Type': 'application/json',
...defaultHeaders,
},
}),
onRetry: (error: Error, attempt: number) => {
const message = `Failed to release version (attempt ${yellow(
`${attempt}/${NB_RETRIES}`,
)}): ${error.message}`;
warnings.push(message);
log.warn(message);
},
});
log.info(`Released version ${bold(versionName)} to live.`);
}
} catch (error: unknown) {
const err = error instanceof Error ? error : new Error(String(error));
errors.push(err);
Expand Down