Skip to content

Commit

Permalink
fix(cli): user agent is reported as undefined/undefined (#24663)
Browse files Browse the repository at this point in the history
Our CLI's user agent is reported as `undefined/undefined`. This is because we are reading the package name and version from the CLI's `package.json` by using a relative path to the source file (using `__dirname`). However, since a good long while, our production CLI is being bundled using `esbuild` into a single JavaScript file. This means that at runtime, `__dirname` points to a completely different directory than the one it's been coded against, and so reading the `package.json` fails.

Account for this by using a function that searches for `package.json`; still do it defensively so that if some other condition we didn't predict causes the search to fail, our CLI doesn't fail.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Mar 17, 2023
1 parent 160ac48 commit 3e8d8d8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 8 deletions.
19 changes: 16 additions & 3 deletions packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { cached } from './cached';
import { CredentialPlugins } from './credential-plugins';
import { Mode } from './credentials';
import { ISDK, SDK, isUnrecoverableAwsError } from './sdk';
import { rootDir } from '../../util/directories';
import { traceMethods } from '../../util/tracing';


Expand Down Expand Up @@ -417,9 +418,7 @@ function parseHttpOptions(options: SdkHttpOptions) {

let userAgent = options.userAgent;
if (userAgent == null) {
// Find the package.json from the main toolkit
const pkg = JSON.parse(readIfPossible(path.join(__dirname, '..', '..', '..', 'package.json')) ?? '{}');
userAgent = `${pkg.name}/${pkg.version}`;
userAgent = defaultCliUserAgent();
}
config.customUserAgent = userAgent;

Expand All @@ -444,6 +443,20 @@ function parseHttpOptions(options: SdkHttpOptions) {
return config;
}

/**
* Find the package.json from the main toolkit.
*
* If we can't read it for some reason, try to do something reasonable anyway.
* Fall back to argv[1], or a standard string if that is undefined for some reason.
*/
export function defaultCliUserAgent() {
const root = rootDir(false);
const pkg = JSON.parse((root ? readIfPossible(path.join(root, 'package.json')) : undefined) ?? '{}');
const name = pkg.name ?? path.basename(process.argv[1] ?? 'cdk-cli');
const version = pkg.version ?? '<unknown>';
return `${name}/${version}`;
}

/**
* Find and return a CA certificate bundle path to be passed into the SDK.
*/
Expand Down
20 changes: 16 additions & 4 deletions packages/aws-cdk/lib/util/directories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,27 @@ export function cdkCacheDir() {
return path.join(cdkHomeDir(), 'cache');
}

export function rootDir() {

function _rootDir(dirname: string): string {
/**
* From the current file, find the directory that contains the CLI's package.json
*
* Can't use `__dirname` in production code, as the CLI will get bundled as it's
* released and `__dirname` will refer to a different location in the `.ts` form
* as it will in the final executing form.
*/
export function rootDir(): string;
export function rootDir(fail: true): string;
export function rootDir(fail: false): string | undefined;
export function rootDir(fail?: boolean) {
function _rootDir(dirname: string): string | undefined {
const manifestPath = path.join(dirname, 'package.json');
if (fs.existsSync(manifestPath)) {
return dirname;
}
if (path.dirname(dirname) === dirname) {
throw new Error('Unable to find package manifest');
if (fail ?? true) {
throw new Error('Unable to find package manifest');
}
return undefined;
}
return _rootDir(path.dirname(dirname));
}
Expand Down
6 changes: 5 additions & 1 deletion packages/aws-cdk/test/api/sdk-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
import * as promptly from 'promptly';
import * as uuid from 'uuid';
import { FakeSts, RegisterRoleOptions, RegisterUserOptions } from './fake-sts';
import { ISDK, Mode, SDK, SdkProvider } from '../../lib/api/aws-auth';
import { ISDK, Mode, SDK, SdkProvider, defaultCliUserAgent } from '../../lib/api/aws-auth';
import { PluginHost } from '../../lib/api/plugin';
import * as logging from '../../lib/logging';
import * as bockfs from '../bockfs';
Expand Down Expand Up @@ -623,6 +623,10 @@ test('even when using a profile to assume another profile, STS calls goes throug
expect(called).toEqual(true);
});

test('default useragent is reasonable', () => {
expect(defaultCliUserAgent()).toContain('aws-cdk/');
});

/**
* Use object hackery to get the credentials out of the SDK object
*/
Expand Down

0 comments on commit 3e8d8d8

Please sign in to comment.