Skip to content

Commit

Permalink
[v15] readGrpcCert: Remove Promise.any usage (#50683)
Browse files Browse the repository at this point in the history
* readGrpcCert: Remove Promise.any usage

* Handle errors from the initial fileExistsAndHasSize call
  • Loading branch information
ravicious authored Jan 2, 2025
1 parent 061b2cd commit dd8d59f
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 27 deletions.
84 changes: 84 additions & 0 deletions web/packages/teleterm/src/services/grpcCredentials/files.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* @jest-environment node
*/
/**
* Teleport
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import fs from 'node:fs/promises';
import path from 'node:path';
import os from 'node:os';
import timers from 'node:timers/promises';

import { readGrpcCert } from './files';

let tempDir: string;

beforeAll(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'grpc-files-test'));
});

afterAll(async () => {
await fs.rm(tempDir, { recursive: true, force: true });
});

beforeEach(() => {
jest.restoreAllMocks();
});

describe('readGrpcCert', () => {
it('reads the file if the file already exists', async () => {
await fs.writeFile(path.join(tempDir, 'already-exists'), 'foobar');

await expect(
readGrpcCert(tempDir, 'already-exists').then(buffer => buffer.toString())
).resolves.toEqual('foobar');
});

it('reads the file when the file is created after starting a watcher', async () => {
const readGrpcCertPromise = readGrpcCert(
tempDir,
'created-after-start'
).then(buffer => buffer.toString());
await timers.setTimeout(10);

await fs.writeFile(path.join(tempDir, 'created-after-start'), 'foobar');

await expect(readGrpcCertPromise).resolves.toEqual('foobar');
});

it('returns an error if the file is not created within the timeout', async () => {
await expect(
readGrpcCert(tempDir, 'non-existent', { timeoutMs: 1 })
).rejects.toMatchObject({
message: expect.stringContaining('within the timeout'),
});
});

it('returns an error if stat fails', async () => {
const expectedError = new Error('Something went wrong');
jest.spyOn(fs, 'stat').mockRejectedValue(expectedError);

await expect(
readGrpcCert(
tempDir,
'non-existent',
{ timeoutMs: 100 } // Make sure that the test doesn't hang for 10s on failure.
)
).rejects.toEqual(expectedError);
});
});
71 changes: 44 additions & 27 deletions web/packages/teleterm/src/services/grpcCredentials/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
*/

import path from 'path';
import { watch } from 'fs';
import { type Stats, watch } from 'fs';
import { readFile, writeFile, stat, rename } from 'fs/promises';

import { wait } from 'shared/utils/wait';

import { makeCert } from './makeCert';

/**
Expand Down Expand Up @@ -51,49 +53,64 @@ export async function generateAndSaveGrpcCert(

/**
* Reads a cert with given `certName` in the `certDir`.
* If the file doesn't exist, it will wait up to 10 seconds for it.
* If the file doesn't exist, by default it will wait up to 10 seconds for it.
*/
export async function readGrpcCert(
certsDir: string,
certName: string
certName: string,
{ timeoutMs = 10_000 } = {}
): Promise<Buffer> {
const fullPath = path.join(certsDir, certName);
const abortController = new AbortController();

async function fileExistsAndHasSize(): Promise<boolean> {
return !!(await stat(fullPath)).size;
let stats: Stats;
try {
stats = await stat(fullPath);
} catch (error) {
if (error?.code === 'ENOENT') {
return false;
}
throw error;
}

return !!stats.size;
}

function watchForFile(): Promise<Buffer> {
function waitForFile(): Promise<Buffer> {
return new Promise((resolve, reject) => {
abortController.signal.onabort = () => {
watcher.close();
clearTimeout(timeout);
};

const timeout = setTimeout(() => {
reject(
`Could not read ${certName} certificate. The operation timed out.`
);
}, 10_000);
wait(timeoutMs, abortController.signal).then(
() =>
reject(
new Error(
`Could not read ${certName} certificate within the timeout.`
)
),
() => {} // Ignore abort errors.
);

const watcher = watch(certsDir, async (event, filename) => {
if (certName === filename && (await fileExistsAndHasSize())) {
resolve(readFile(fullPath));
// Watching must be started before checking if the file already exists to avoid race
// conditions. If we checked if the file exists and then started the watcher, the file could
// in theory be created between those two actions.
watch(
certsDir,
{ signal: abortController.signal },
async (_, filename) => {
if (certName === filename && (await fileExistsAndHasSize())) {
resolve(readFile(fullPath));
}
}
});
});
}
);

async function checkIfFileAlreadyExists(): Promise<Buffer> {
if (await fileExistsAndHasSize()) {
return readFile(fullPath);
}
fileExistsAndHasSize().then(
exists => exists && resolve(readFile(fullPath)),
err => reject(err)
);
});
}

try {
// watching must be started before checking if the file already exists to avoid race conditions
return await Promise.any([watchForFile(), checkIfFileAlreadyExists()]);
return await waitForFile();
} finally {
abortController.abort();
}
Expand Down

0 comments on commit dd8d59f

Please sign in to comment.