Skip to content

Commit 4c3aaa8

Browse files
authored
feat(toolkit-lib): make the plugin host configurable (#375)
This makes the plugin host configurable for toolkit users: ## Design decisions - Every `Toolkit` instance gets a fresh `PluginHost` by default (not a global singleton `PluginHost`) - ...in fact, the concept of a global singleton `PluginHost` gets fully moved to the CLI. That concept disappears from the toolkit and the helpers, and all functions that used to implicitly assume a global plugin host now get an explicit parameter, that defaults to a fresh plugin host if not supplied. These choices lead to a lot of downstream test changes. - Plugin module resolution, module deduplication and logging was moved from the CLI into the plugin host. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent a096830 commit 4c3aaa8

File tree

25 files changed

+247
-81
lines changed

25 files changed

+247
-81
lines changed

.projenrc.ts

+6
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,12 @@ const tmpToolkitHelpers = configureProject(
756756
target: 'es2022',
757757
lib: ['es2022', 'esnext.disposable', 'dom'],
758758
module: 'NodeNext',
759+
760+
// Temporarily, because it makes it impossible for us to use @internal functions
761+
// from other packages. Annoyingly, VSCode won't show an error if you use @internal
762+
// because it looks at the .ts, but the compilation will fail because it will use
763+
// the .d.ts.
764+
stripInternal: false,
759765
},
760766
},
761767

packages/@aws-cdk/tmp-toolkit-helpers/src/api/aws-auth/credential-plugins.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import type { AwsCredentialIdentity, AwsCredentialIdentityProvider } from '@smit
44
import { credentialsAboutToExpire, makeCachingProvider } from './provider-caching';
55
import { formatErrorMessage } from '../../util';
66
import { IO, type IoHelper } from '../io/private';
7+
import type { PluginHost } from '../plugin';
78
import type { Mode } from '../plugin/mode';
8-
import type { PluginHost } from '../plugin/plugin';
99
import { AuthenticationError } from '../toolkit-error';
1010

1111
/**
@@ -22,12 +22,8 @@ import { AuthenticationError } from '../toolkit-error';
2222
*/
2323
export class CredentialPlugins {
2424
private readonly cache: { [key: string]: PluginCredentialsFetchResult | undefined } = {};
25-
private readonly host: PluginHost;
26-
private readonly ioHelper: IoHelper;
2725

28-
constructor(host: PluginHost, ioHelper: IoHelper) {
29-
this.host = host;
30-
this.ioHelper = ioHelper;
26+
constructor(private readonly host: PluginHost, private readonly ioHelper: IoHelper) {
3127
}
3228

3329
public async fetchCredentialsFor(awsAccountId: string, mode: Mode): Promise<PluginCredentialsFetchResult | undefined> {

packages/@aws-cdk/tmp-toolkit-helpers/src/api/aws-auth/sdk-provider.ts

+39-8
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { SDK } from './sdk';
1414
import { callTrace, traceMemberMethods } from './tracing';
1515
import { formatErrorMessage } from '../../util';
1616
import { IO, type IoHelper } from '../io/private';
17-
import { Mode, PluginHost } from '../plugin';
17+
import { PluginHost, Mode } from '../plugin';
1818
import { AuthenticationError } from '../toolkit-error';
1919

2020
export type AssumeRoleAdditionalOptions = Partial<Omit<AssumeRoleCommandInput, 'ExternalId' | 'RoleArn'>>;
@@ -44,6 +44,13 @@ export interface SdkProviderOptions {
4444
* The logger for sdk calls.
4545
*/
4646
readonly logger?: Logger;
47+
48+
/**
49+
* The plugin host to use
50+
*
51+
* @default - an empty plugin host
52+
*/
53+
readonly pluginHost?: PluginHost;
4754
}
4855

4956
/**
@@ -136,7 +143,7 @@ export class SdkProvider {
136143

137144
const region = await builder.region(options.profile);
138145
const requestHandler = await builder.requestHandlerBuilder(options.httpOptions);
139-
return new SdkProvider(credentialProvider, region, requestHandler, options.ioHelper, options.logger);
146+
return new SdkProvider(credentialProvider, region, requestHandler, options.pluginHost ?? new PluginHost(), options.ioHelper, options.logger);
140147
}
141148

142149
private readonly plugins;
@@ -148,10 +155,11 @@ export class SdkProvider {
148155
*/
149156
public readonly defaultRegion: string,
150157
private readonly requestHandler: NodeHttpHandlerOptions = {},
158+
pluginHost: PluginHost,
151159
private readonly ioHelper: IoHelper,
152160
private readonly logger?: Logger,
153161
) {
154-
this.plugins = new CredentialPlugins(PluginHost.instance, ioHelper);
162+
this.plugins = new CredentialPlugins(pluginHost, ioHelper);
155163
}
156164

157165
/**
@@ -183,7 +191,7 @@ export class SdkProvider {
183191

184192
// Our current credentials must be valid and not expired. Confirm that before we get into doing
185193
// actual CloudFormation calls, which might take a long time to hang.
186-
const sdk = new SDK(baseCreds.credentials, env.region, this.requestHandler, this.ioHelper, this.logger);
194+
const sdk = this._makeSdk(baseCreds.credentials, env.region);
187195
await sdk.validateCredentials();
188196
return { sdk, didAssumeRole: false };
189197
}
@@ -216,7 +224,7 @@ export class SdkProvider {
216224
`${fmtObtainedCredentials(baseCreds)} could not be used to assume '${options.assumeRoleArn}', but are for the right account. Proceeding anyway.`,
217225
));
218226
return {
219-
sdk: new SDK(baseCreds.credentials, env.region, this.requestHandler, this.ioHelper, this.logger),
227+
sdk: this._makeSdk(baseCreds.credentials, env.region),
220228
didAssumeRole: false,
221229
};
222230
}
@@ -236,7 +244,7 @@ export class SdkProvider {
236244
if (baseCreds.source === 'none') {
237245
return undefined;
238246
}
239-
return (await new SDK(baseCreds.credentials, env.region, this.requestHandler, this.ioHelper, this.logger).currentAccount()).partition;
247+
return (await this._makeSdk(baseCreds.credentials, env.region).currentAccount()).partition;
240248
}
241249

242250
/**
@@ -282,7 +290,7 @@ export class SdkProvider {
282290
public async defaultAccount(): Promise<Account | undefined> {
283291
return cached(this, CACHED_ACCOUNT, async () => {
284292
try {
285-
return await new SDK(this.defaultCredentialProvider, this.defaultRegion, this.requestHandler, this.ioHelper, this.logger).currentAccount();
293+
return await this._makeSdk(this.defaultCredentialProvider, this.defaultRegion).currentAccount();
286294
} catch (e: any) {
287295
// Treat 'ExpiredToken' specially. This is a common situation that people may find themselves in, and
288296
// they are complaining about if we fail 'cdk synth' on them. We loudly complain in order to show that
@@ -384,7 +392,7 @@ export class SdkProvider {
384392
// Call the provider at least once here, to catch an error if it occurs
385393
await credentials();
386394

387-
return new SDK(credentials, region, this.requestHandler, this.ioHelper, this.logger);
395+
return this._makeSdk(credentials, region);
388396
} catch (err: any) {
389397
if (err.name === 'ExpiredToken') {
390398
throw err;
@@ -402,6 +410,29 @@ export class SdkProvider {
402410
);
403411
}
404412
}
413+
414+
/**
415+
* Factory function that creates a new SDK instance
416+
*
417+
* This is a function here, instead of all the places where this is used creating a `new SDK`
418+
* instance, so that it is trivial to mock from tests.
419+
*
420+
* Use like this:
421+
*
422+
* ```ts
423+
* const mockSdk = jest.spyOn(SdkProvider.prototype, '_makeSdk').mockReturnValue(new MockSdk());
424+
* // ...
425+
* mockSdk.mockRestore();
426+
* ```
427+
*
428+
* @internal
429+
*/
430+
public _makeSdk(
431+
credProvider: AwsCredentialIdentityProvider,
432+
region: string,
433+
) {
434+
return new SDK(credProvider, region, this.requestHandler, this.ioHelper, this.logger);
435+
}
405436
}
406437

407438
/**

packages/@aws-cdk/tmp-toolkit-helpers/src/api/plugin/plugin.ts

+45-12
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { inspect } from 'util';
22
import type { CredentialProviderSource, IPluginHost, Plugin } from '@aws-cdk/cli-plugin-contract';
33
import { type ContextProviderPlugin, isContextProviderPlugin } from './context-provider-plugin';
4+
import type { IIoHost } from '../io';
5+
import { IoDefaultMessages, IoHelper } from '../private';
46
import { ToolkitError } from '../toolkit-error';
57

68
export let TESTING = false;
@@ -10,12 +12,13 @@ export function markTesting() {
1012
}
1113

1214
/**
13-
* A utility to manage plug-ins.
15+
* Class to manage a plugin collection
1416
*
17+
* It provides a `load()` function that loads a JavaScript
18+
* module from disk, and gives it access to the `IPluginHost` interface
19+
* to register itself.
1520
*/
1621
export class PluginHost implements IPluginHost {
17-
public static instance = new PluginHost();
18-
1922
/**
2023
* Access the currently registered CredentialProviderSources. New sources can
2124
* be registered using the +registerCredentialProviderSource+ method.
@@ -24,30 +27,60 @@ export class PluginHost implements IPluginHost {
2427

2528
public readonly contextProviderPlugins: Record<string, ContextProviderPlugin> = {};
2629

27-
constructor() {
28-
if (!TESTING && PluginHost.instance && PluginHost.instance !== this) {
29-
throw new ToolkitError('New instances of PluginHost must not be built. Use PluginHost.instance instead!');
30-
}
31-
}
30+
public ioHost?: IIoHost;
31+
32+
private readonly alreadyLoaded = new Set<string>();
3233

3334
/**
3435
* Loads a plug-in into this PluginHost.
3536
*
37+
* Will use `require.resolve()` to get the most accurate representation of what
38+
* code will get loaded in error messages. As such, it will not work in
39+
* unit tests with Jest virtual modules becauase of <https://github.com/jestjs/jest/issues/9543>.
40+
*
3641
* @param moduleSpec the specification (path or name) of the plug-in module to be loaded.
42+
* @param ioHost the I/O host to use for printing progress information
3743
*/
38-
public load(moduleSpec: string) {
44+
public load(moduleSpec: string, ioHost?: IIoHost) {
3945
try {
46+
const resolved = require.resolve(moduleSpec);
47+
if (ioHost) {
48+
new IoDefaultMessages(IoHelper.fromIoHost(ioHost, 'init')).debug(`Loading plug-in: ${resolved} from ${moduleSpec}`);
49+
}
50+
return this._doLoad(resolved);
51+
} catch (e: any) {
52+
// according to Node.js docs `MODULE_NOT_FOUND` is the only possible error here
53+
// @see https://nodejs.org/api/modules.html#requireresolverequest-options
54+
// Not using `withCause()` here, since the node error contains a "Require Stack"
55+
// as part of the error message that is inherently useless to our users.
56+
throw new ToolkitError(`Unable to resolve plug-in: Cannot find module '${moduleSpec}': ${e}`);
57+
}
58+
}
59+
60+
/**
61+
* Do the loading given an already-resolved module name
62+
*
63+
* @internal
64+
*/
65+
public _doLoad(resolved: string) {
66+
try {
67+
if (this.alreadyLoaded.has(resolved)) {
68+
return;
69+
}
70+
4071
/* eslint-disable @typescript-eslint/no-require-imports */
41-
const plugin = require(moduleSpec);
72+
const plugin = require(resolved);
4273
/* eslint-enable */
4374
if (!isPlugin(plugin)) {
44-
throw new ToolkitError(`Module ${moduleSpec} is not a valid plug-in, or has an unsupported version.`);
75+
throw new ToolkitError(`Module ${resolved} is not a valid plug-in, or has an unsupported version.`);
4576
}
4677
if (plugin.init) {
4778
plugin.init(this);
4879
}
80+
81+
this.alreadyLoaded.add(resolved);
4982
} catch (e: any) {
50-
throw ToolkitError.withCause(`Unable to load plug-in '${moduleSpec}'`, e);
83+
throw ToolkitError.withCause(`Unable to load plug-in '${resolved}'`, e);
5184
}
5285

5386
function isPlugin(x: any): x is Plugin {

packages/@aws-cdk/tmp-toolkit-helpers/src/context-providers/index.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ import { TRANSIENT_CONTEXT_KEY } from '../api/context';
1616
import { replaceEnvPlaceholders } from '../api/environment';
1717
import { IO } from '../api/io/private';
1818
import type { IoHelper } from '../api/io/private';
19-
import { PluginHost } from '../api/plugin';
20-
import type { ContextProviderPlugin } from '../api/plugin';
19+
import type { PluginHost, ContextProviderPlugin } from '../api/plugin';
2120
import { ContextProviderError } from '../api/toolkit-error';
2221
import { formatErrorMessage } from '../util';
2322

@@ -72,6 +71,7 @@ export async function provideContextValues(
7271
missingValues: cxschema.MissingContext[],
7372
context: Context,
7473
sdk: SdkProvider,
74+
pluginHost: PluginHost,
7575
ioHelper: IoHelper,
7676
) {
7777
for (const missingContext of missingValues) {
@@ -83,7 +83,7 @@ export async function provideContextValues(
8383

8484
let factory;
8585
if (providerName.startsWith(`${PLUGIN_PROVIDER_PREFIX}:`)) {
86-
const plugin = PluginHost.instance.contextProviderPlugins[providerName.substring(PLUGIN_PROVIDER_PREFIX.length + 1)];
86+
const plugin = pluginHost.contextProviderPlugins[providerName.substring(PLUGIN_PROVIDER_PREFIX.length + 1)];
8787
if (!plugin) {
8888
// eslint-disable-next-line max-len
8989
throw new ContextProviderError(`Unrecognized plugin context provider name: ${missingContext.provider}.`);

packages/@aws-cdk/tmp-toolkit-helpers/src/context-providers/security-groups.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ export class SecurityGroupContextProviderPlugin implements ContextProviderPlugin
6060
}
6161

6262
/**
63-
* TODO: We intend this to be @*internal but a test in aws-cdk depends on it.
64-
* Put the tag back later.
63+
* @internal
6564
*/
6665
export function hasAllTrafficEgress(securityGroup: SecurityGroup) {
6766
let hasAllTrafficCidrV4 = false;

packages/@aws-cdk/tmp-toolkit-helpers/tsconfig.dev.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/tmp-toolkit-helpers/tsconfig.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/context-aware-source.ts

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export class ContextAwareCloudAssemblySource implements ICloudAssemblySource {
103103
assembly.manifest.missing,
104104
this.context,
105105
this.props.services.sdkProvider,
106+
this.props.services.pluginHost,
106107
this.ioHelper,
107108
);
108109

packages/@aws-cdk/toolkit-lib/lib/api/shared-public.ts

+1
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,6 @@ export type {
2020
} from '../../../tmp-toolkit-helpers/src/api/io/io-message';
2121
export type { IIoHost } from '../../../tmp-toolkit-helpers/src/api/io/io-host';
2222
export type { ToolkitAction } from '../../../tmp-toolkit-helpers/src/api/io/toolkit-action';
23+
export { PluginHost } from '../../../tmp-toolkit-helpers/src/api/plugin/plugin';
2324

2425
export * from '../../../tmp-toolkit-helpers/src/payloads';

packages/@aws-cdk/toolkit-lib/lib/toolkit/private/index.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11

22
import type { ICloudAssemblySource } from '../../api/cloud-assembly';
33
import { StackAssembly } from '../../api/cloud-assembly/private';
4-
import type { SdkProvider, IoHelper } from '../../api/shared-private';
4+
import type { SdkProvider, IoHelper, PluginHost } from '../../api/shared-private';
55

66
/**
77
* Helper struct to pass internal services around.
88
*/
99
export interface ToolkitServices {
1010
sdkProvider: SdkProvider;
1111
ioHelper: IoHelper;
12+
pluginHost: PluginHost;
1213
}
1314

1415
/**

0 commit comments

Comments
 (0)