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 4833ac8
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 14 deletions.
2 changes: 1 addition & 1 deletion 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.

47 changes: 47 additions & 0 deletions src/__tests__/setup-builder.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import * as reporter from '../reporter';
import { getStickyDisk } from '../setup_builder';

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);

// Get the arguments from the mock call
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
expect(formData.get('stickyDiskKey')).toBe('test-repo');
expect(formData.get('region')).toBe('test-region');
expect(formData.get('installationModelID')).toBe('test-model');
expect(formData.get('vmID')).toBe('test-vm');
});
});
41 changes: 29 additions & 12 deletions src/setup_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,25 +143,42 @@ 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}`);
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}`);
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);

// 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 4833ac8

Please sign in to comment.