fix(core): should use nx cloud if access token specified by env (#19975)

This commit is contained in:
Craigory Coppola 2023-11-08 12:26:11 -05:00 committed by GitHub
parent 933051334f
commit 4f23523ba0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 136 additions and 60 deletions

View File

@ -428,7 +428,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -479,7 +478,6 @@ describe('migrate', () => {
// runs migrations
runCLI('migrate --run-migrations=migrations.json', {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -493,7 +491,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -503,7 +500,6 @@ describe('migrate', () => {
// runs migrations with createCommits enabled
runCLI('migrate --run-migrations=migrations.json --create-commits', {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -522,7 +518,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -534,7 +529,6 @@ describe('migrate', () => {
`migrate --run-migrations=migrations.json --create-commits --commit-prefix="'chore(core): AUTOMATED - '"`,
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -553,7 +547,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -565,7 +558,6 @@ describe('migrate', () => {
`migrate --run-migrations=migrations.json --commit-prefix CUSTOM_PREFIX`,
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -584,7 +576,6 @@ describe('migrate', () => {
// Invalid: runs migrations with a custom commit-prefix but without enabling --create-commits
const output = runCLI(`migrate --run-migrations`, {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
@ -602,7 +593,6 @@ describe('migrate', () => {
// Invalid: runs migrations with a custom commit-prefix but without enabling --create-commits
const output = runCLI(`migrate --run-migrations --if-exists`, {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},

View File

@ -11,7 +11,6 @@ describe('Nx Cloud', () => {
beforeAll(() => {
runCLI('connect --no-interactive', {
env: {
...process.env,
NX_CLOUD_API: 'https://staging.nx.app',
},
});

View File

@ -331,13 +331,13 @@ describe('Nx Running Tests', () => {
runCLI(`generate @nx/web:app ${myapp}`);
const buildWithDaemon = runCLI(`build ${myapp}`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});
expect(buildWithDaemon).toContain('Successfully ran target build');
const buildAgain = runCLI(`build ${myapp}`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});
expect(buildAgain).toContain('[local cache]');
@ -614,7 +614,7 @@ describe('Nx Running Tests', () => {
// testing run many with daemon disabled
const buildWithDaemon = runCLI(`run-many --target=build`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});
expect(buildWithDaemon).toContain(`Successfully ran target build`);
}, 1000000);

View File

@ -425,7 +425,6 @@ describe('CLI - Environment Variables', () => {
`run-many --target build --outputHashing=none --optimization=false`,
{
env: {
...process.env,
NODE_ENV: 'test',
NX_BUILD: '52',
NX_API: 'QA',

View File

@ -1,69 +1,118 @@
import { withEnvironmentVariables } from '../../internal-testing-utils/with-environment';
import { onlyDefaultRunnerIsUsed } from './connect-to-nx-cloud';
describe('connect-to-nx-cloud', () => {
describe('onlyDefaultRunnerIsUsed', () => {
it('should say no if tasks runner options is undefined and nxCloudAccessToken is set', () => {
expect(
onlyDefaultRunnerIsUsed({
nxCloudAccessToken: 'xxx-xx-xxx',
})
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
() =>
onlyDefaultRunnerIsUsed({
nxCloudAccessToken: 'xxx-xx-xxx',
})
)
).toBe(false);
});
it('should say no if cloud access token is in env', () => {
const defaultRunnerUsed = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: 'xxx-xx-xxx',
},
() => onlyDefaultRunnerIsUsed({})
);
expect(defaultRunnerUsed).toBe(false);
});
it('should say yes if tasks runner options is undefined and nxCloudAccessToken is not set', () => {
expect(onlyDefaultRunnerIsUsed({})).toBe(true);
expect(
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
() => onlyDefaultRunnerIsUsed({})
)
).toBe(true);
});
it('should say yes if tasks runner options is set to default runner', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'nx/tasks-runners/default',
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'nx/tasks-runners/default',
},
},
})
)
).toBeTruthy();
});
it('should say no if tasks runner is set to a custom runner', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'custom-runner',
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'custom-runner',
},
},
})
)
).toBeFalsy();
});
it('should say yes if tasks runner has options, but no runner and not using cloud', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
},
})
)
).toBeTruthy();
});
it('should say no if tasks runner has options, but no runner and using cloud', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
nxCloudAccessToken: 'xxx-xx-xxx',
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
},
nxCloudAccessToken: 'xxx-xx-xxx',
})
)
).toBeFalsy();
});
});

View File

@ -16,7 +16,7 @@ export function onlyDefaultRunnerIsUsed(nxJson: NxJsonConfiguration) {
// No tasks runner options OR no default runner defined:
// - If access token defined, uses cloud runner
// - If no access token defined, uses default
return !nxJson.nxCloudAccessToken;
return !(nxJson.nxCloudAccessToken ?? process.env.NX_CLOUD_ACCESS_TOKEN);
}
return defaultRunner === 'nx/tasks-runners/default';

View File

@ -1,11 +1,16 @@
export function withEnvironmentVariables(
env: Record<string, string>,
callback: () => void | Promise<void>
): void | Promise<void> {
export function withEnvironmentVariables<T>(
env: Record<string, string | false | null | undefined>,
callback: () => T
): T {
const originalValues: Record<string, string> = {};
for (const key in env) {
originalValues[key] = process.env[key];
process.env[key] = env[key];
const value = env[key];
if (value) {
process.env[key] = value;
} else {
delete process.env[key];
}
}
const cleanup = () => {
for (const key in env) {
@ -14,8 +19,9 @@ export function withEnvironmentVariables(
};
const p = callback();
if (p instanceof Promise) {
return p.finally(cleanup);
return p.finally(cleanup) as T;
} else {
cleanup();
return p;
}
}

View File

@ -3,6 +3,7 @@ import { getRunner } from './run-command';
import { NxJsonConfiguration } from '../config/nx-json';
import { join } from 'path';
import { nxCloudTasksRunnerShell } from '../nx-cloud/nx-cloud-tasks-runner-shell';
import { withEnvironmentVariables } from '../internal-testing-utils/with-environment';
describe('getRunner', () => {
let nxJson: NxJsonConfiguration;
@ -77,7 +78,12 @@ describe('getRunner', () => {
it('uses default runner when no tasksRunnerOptions are present', () => {
jest.mock(join(__dirname, './default-tasks-runner.ts'), () => mockRunner);
const { tasksRunner } = getRunner({}, {});
const { tasksRunner } = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: undefined,
},
() => getRunner({}, {})
);
expect(tasksRunner).toEqual(mockRunner);
});
@ -100,6 +106,16 @@ describe('getRunner', () => {
`);
});
it('uses cloud runner when tasksRunnerOptions are not present and accessToken is set in env', () => {
const { tasksRunner } = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: 'xxx-xx-xxx',
},
() => getRunner({}, {})
);
expect(tasksRunner).toEqual(nxCloudTasksRunnerShell);
});
it('reads options from base properties if no runner options provided', () => {
jest.mock(join(__dirname, './default-tasks-runner.ts'), () => mockRunner);

View File

@ -493,7 +493,9 @@ function getTasksRunnerPath(
// No tasksRunnerOptions for given --runner
nxJson.nxCloudAccessToken ||
// No runner prop in tasks runner options, check if access token is set.
nxJson.tasksRunnerOptions?.[runner]?.options?.accessToken;
nxJson.tasksRunnerOptions?.[runner]?.options?.accessToken ||
// Cloud access token specified in env var.
process.env.NX_CLOUD_ACCESS_TOKEN;
return isCloudRunner ? 'nx-cloud' : require.resolve('./default-tasks-runner');
}
@ -517,6 +519,10 @@ export function getRunnerOptions(
...nxArgs,
};
// NOTE: we don't pull from env here because the cloud package
// supports it within nx-cloud's implementation. We could
// normalize it here, and that may make more sense, but
// leaving it as is for now.
if (nxJson.nxCloudAccessToken && isCloudDefault) {
result.accessToken ??= nxJson.nxCloudAccessToken;
}

View File

@ -2,6 +2,7 @@ import { NxJsonConfiguration, readNxJson } from '../config/nx-json';
export function isNxCloudUsed(nxJson: NxJsonConfiguration) {
return (
process.env.NX_CLOUD_ACCESS_TOKEN ||
!!nxJson.nxCloudAccessToken ||
!!Object.values(nxJson.tasksRunnerOptions ?? {}).find(
(r) => r.runner == '@nrwl/nx-cloud' || r.runner == 'nx-cloud'
@ -13,7 +14,10 @@ export function getNxCloudUrl(nxJson: NxJsonConfiguration): string {
const cloudRunner = Object.values(nxJson.tasksRunnerOptions ?? {}).find(
(r) => r.runner == '@nrwl/nx-cloud' || r.runner == 'nx-cloud'
);
if (!cloudRunner && !nxJson.nxCloudAccessToken)
if (
!cloudRunner &&
!(nxJson.nxCloudAccessToken || process.env.NX_CLOUD_ACCESS_TOKEN)
)
throw new Error('nx-cloud runner not found in nx.json');
return cloudRunner?.options?.url ?? nxJson.nxCloudUrl ?? 'https://nx.app';
}
@ -23,8 +27,15 @@ export function getNxCloudToken(nxJson: NxJsonConfiguration): string {
(r) => r.runner == '@nrwl/nx-cloud' || r.runner == 'nx-cloud'
);
if (!cloudRunner && !nxJson.nxCloudAccessToken)
if (
!cloudRunner &&
!(nxJson.nxCloudAccessToken || process.env.NX_CLOUD_ACCESS_TOKEN)
)
throw new Error('nx-cloud runner not found in nx.json');
return cloudRunner?.options.accessToken ?? nxJson.nxCloudAccessToken;
return (
process.env.NX_CLOUD_ACCESS_TOKEN ??
cloudRunner?.options.accessToken ??
nxJson.nxCloudAccessToken
);
}