Skip to content

Commit

Permalink
src: start sending get request with query params
Browse files Browse the repository at this point in the history
We are incorrectly using formData in a get request. To move
away from this we send both query params and formData until
the server is fully upgraded. After which we can stop sending
formData.
  • Loading branch information
adityamaru committed Dec 9, 2024
1 parent 50ccb6c commit 1308752
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 48 deletions.
6 changes: 3 additions & 3 deletions dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

48 changes: 24 additions & 24 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"@docker/actions-toolkit": "0.37.1",
"@iarna/toml": "^2.2.5",
"axios-retry": "^4.5.0",
"form-data": "^4.0.1",
"handlebars": "^4.7.7",
"portfinder": "^1.0.32"
},
Expand Down
52 changes: 52 additions & 0 deletions src/__tests__/setup-builder.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import * as reporter from '../reporter';
import { getStickyDisk } from '../setup_builder';
import FormData from 'form-data';

jest.mock('../reporter');

describe('getStickyDisk', () => {
const mockGet = jest.fn();

beforeEach(() => {
jest.resetAllMocks();
process.env.GITHUB_REPO_NAME = 'test-repo';
process.env.BLACKSMITH_REGION = 'test-region';
process.env.BLACKSMITH_INSTALLATION_MODEL_ID = 'test-model';
process.env.VM_ID = 'test-vm';

(reporter.createBlacksmithAgentClient as jest.Mock).mockResolvedValue({});
(reporter.get as jest.Mock).mockImplementation(mockGet);
mockGet.mockResolvedValue({
data: {
expose_id: 'test-expose-id',
disk_identifier: 'test-device'
}
});
});

it('sets both FormData and query parameters correctly', async () => {
await getStickyDisk();

// Verify the get call was made
expect(mockGet).toHaveBeenCalledTimes(1);

const [, url, formData] = mockGet.mock.calls[0];

// Verify query parameters
expect(url).toContain('stickyDiskKey=test-repo');
expect(url).toContain('region=test-region');
expect(url).toContain('installationModelID=test-model');
expect(url).toContain('vmID=test-vm');

// Verify FormData by checking what was actually sent
// This tests the behavior, not the implementation
expect(formData instanceof FormData).toBeTruthy();

// If you need to verify the content, you can convert to string or buffer
const formDataString = formData.getBuffer().toString();
expect(formDataString).toContain('test-repo');
expect(formDataString).toContain('test-region');
expect(formDataString).toContain('test-model');
expect(formDataString).toContain('test-vm');
});
});
7 changes: 3 additions & 4 deletions src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as core from '@actions/core';
import axios, {AxiosError, AxiosInstance, AxiosResponse, AxiosStatic } from 'axios';
import axiosRetry from 'axios-retry';
import {ExportRecordResponse} from '@docker/actions-toolkit/lib/types/buildx/history';
import FormData from 'form-data';

// Configure base axios instance for Blacksmith API.
const createBlacksmithAPIClient = () => {
Expand Down Expand Up @@ -79,9 +80,7 @@ export async function reportBuildCompleted(exportRes?: ExportRecordResponse, bla
formData.append('stickyDiskKey', process.env.GITHUB_REPO_NAME || '');

await agentClient.post('/stickydisks', formData, {
headers: {
'Content-Type': 'multipart/form-data'
}
headers: formData.getHeaders()
});

// Report success to Blacksmith API
Expand Down Expand Up @@ -182,7 +181,7 @@ export async function get(client: AxiosInstance, url: string, formData: FormData
headers: {
Authorization: `Bearer ${process.env.BLACKSMITH_STICKYDISK_TOKEN}`,
'X-Github-Repo-Name': process.env.GITHUB_REPO_NAME || '',
'Content-Type': 'multipart/form-data'
...(formData && {'Content-Type': 'multipart/form-data'})
},
signal: options?.signal
});
Expand Down
46 changes: 30 additions & 16 deletions src/setup_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {exec} from 'child_process';
import {promisify} from 'util';
import * as TOML from '@iarna/toml';
import * as reporter from './reporter';
import FormData from 'form-data';

const mountPoint = '/var/lib/buildkit';
const execAsync = promisify(exec);
Expand Down Expand Up @@ -143,25 +144,38 @@ async function getDiskSize(device: string): Promise<number> {
}
}

async function getStickyDisk(options?: {signal?: AbortSignal}): Promise<{expose_id: string; device: string}> {
export async function getStickyDisk(options?: {signal?: AbortSignal}): Promise<{expose_id: string; device: string}> {
const client = await reporter.createBlacksmithAgentClient();
const formData = new FormData();
// TODO(adityamaru): Support a stickydisk-per-build flag that will namespace the stickydisks by Dockerfile.
// For now, we'll use the repo name as the stickydisk key.
const repoName = process.env.GITHUB_REPO_NAME || '';
if (repoName === '') {

// Prepare data for both FormData and query params
const stickyDiskKey = process.env.GITHUB_REPO_NAME || '';
if (stickyDiskKey === '') {
throw new Error('GITHUB_REPO_NAME is not set');
}
formData.append('stickyDiskKey', repoName);
formData.append('region', process.env.BLACKSMITH_REGION || 'eu-central');
formData.append('installationModelID', process.env.BLACKSMITH_INSTALLATION_MODEL_ID || '');
formData.append('vmID', process.env.VM_ID || '');
core.debug(`Getting sticky disk for ${repoName}`);
core.debug('FormData contents:');
for (const pair of formData.entries()) {
core.debug(`${pair[0]}: ${pair[1]}`);
}
const response = await reporter.get(client, '/stickydisks', formData, options);
const region = process.env.BLACKSMITH_REGION || 'eu-central';
const installationModelID = process.env.BLACKSMITH_INSTALLATION_MODEL_ID || '';
const vmID = process.env.VM_ID || '';

// Create FormData (for backwards compatibility).
// TODO(adityamaru): Remove this once all of our VM agents are reading query params.
const formData = new FormData();
formData.append('stickyDiskKey', stickyDiskKey);
formData.append('region', region);
formData.append('installationModelID', installationModelID);
formData.append('vmID', vmID);

// Create query params string.
const queryParams = new URLSearchParams({
stickyDiskKey,
region,
installationModelID,
vmID
}).toString();

core.debug(`Getting sticky disk for ${stickyDiskKey}`);

// Send request with both FormData and query params
const response = await reporter.get(client, `/stickydisks?${queryParams}`, formData, options);
const exposeId = response.data?.expose_id || '';
const device = response.data?.disk_identifier || '';
return {expose_id: exposeId, device: device};
Expand Down

0 comments on commit 1308752

Please sign in to comment.