Skip to content

Commit 40a3137

Browse files
authored
refactor(toolkit-lib): remove duplicate SdkLogger and use copy of interface (#467)
We had two version of an `SdkLogger` that logs to an IoHost. This was because historically they lived in different packages. Align both versions to use the CLI one - because that is definitely safe and battle tested. Additionally we move away from using the `Logger` interface provided by `@smithy/types`. This is in preparation of fully removing this dependency. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 73994cf commit 40a3137

File tree

10 files changed

+35
-178
lines changed

10 files changed

+35
-178
lines changed

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/awscli-compatible.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import { createCredentialChain, fromEnv, fromIni, fromNodeProviderChain } from '
33
import { MetadataService } from '@aws-sdk/ec2-metadata-service';
44
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
55
import { loadSharedConfigFiles } from '@smithy/shared-ini-file-loader';
6-
import type { AwsCredentialIdentityProvider, Logger } from '@smithy/types';
6+
import type { AwsCredentialIdentityProvider } from '@smithy/types';
77
import * as promptly from 'promptly';
88
import { makeCachingProvider } from './provider-caching';
99
import { ProxyAgentProvider } from './proxy-agent';
10+
import type { ISdkLogger } from './sdk-logger';
1011
import type { SdkHttpOptions } from './types';
1112
import { AuthenticationError } from '../../toolkit/toolkit-error';
1213
import { IO, type IoHelper } from '../io/private';
@@ -25,9 +26,9 @@ const DEFAULT_TIMEOUT = 300000;
2526
export class AwsCliCompatible {
2627
private readonly ioHelper: IoHelper;
2728
private readonly requestHandler: NodeHttpHandlerOptions;
28-
private readonly logger?: Logger;
29+
private readonly logger?: ISdkLogger;
2930

30-
public constructor(ioHelper: IoHelper, requestHandler: NodeHttpHandlerOptions, logger?: Logger) {
31+
public constructor(ioHelper: IoHelper, requestHandler: NodeHttpHandlerOptions, logger?: ISdkLogger) {
3132
this.ioHelper = ioHelper;
3233
this.requestHandler = requestHandler;
3334
this.logger = logger;
@@ -267,7 +268,7 @@ function shouldPrioritizeEnv() {
267268

268269
export interface CredentialChainOptions {
269270
readonly profile?: string;
270-
readonly logger?: Logger;
271+
readonly logger?: ISdkLogger;
271272
}
272273

273274
export async function makeRequestHandler(ioHelper: IoHelper, options: SdkHttpOptions = {}): Promise<NodeHttpHandlerOptions> {

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk-logger.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
import { inspect, format } from 'util';
2-
import type { Logger } from '@smithy/types';
32
import { replacerBufferWithInfo } from '../../util';
43
import type { IoHelper } from '../io/private';
54
import { IO } from '../io/private';
65

7-
export class SdkToCliLogger implements Logger {
6+
export interface ISdkLogger {
7+
trace?: (...content: any[]) => void;
8+
debug: (...content: any[]) => void;
9+
info: (...content: any[]) => void;
10+
warn: (...content: any[]) => void;
11+
error: (...content: any[]) => void;
12+
}
13+
14+
export class IoHostSdkLogger implements ISdkLogger {
815
private readonly ioHelper: IoHelper;
916

1017
public constructor(ioHelper: IoHelper) {

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk-provider.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { EnvironmentUtils, UNKNOWN_ACCOUNT, UNKNOWN_REGION } from '@aws-cdk/cx-a
55
import type { AssumeRoleCommandInput } from '@aws-sdk/client-sts';
66
import { fromTemporaryCredentials } from '@aws-sdk/credential-providers';
77
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
8-
import type { AwsCredentialIdentityProvider, Logger } from '@smithy/types';
8+
import type { AwsCredentialIdentityProvider } from '@smithy/types';
99
import { AwsCliCompatible } from './awscli-compatible';
1010
import { cached } from './cached';
1111
import { CredentialPlugins } from './credential-plugins';
@@ -16,6 +16,7 @@ import { AuthenticationError } from '../../toolkit/toolkit-error';
1616
import { formatErrorMessage } from '../../util';
1717
import { IO, type IoHelper } from '../io/private';
1818
import { PluginHost, Mode } from '../plugin';
19+
import type { ISdkLogger } from './sdk-logger';
1920

2021
export type AssumeRoleAdditionalOptions = Partial<Omit<AssumeRoleCommandInput, 'ExternalId' | 'RoleArn'>>;
2122

@@ -102,7 +103,7 @@ export class SdkProvider {
102103
private readonly plugins;
103104
private readonly requestHandler: NodeHttpHandlerOptions;
104105
private readonly ioHelper: IoHelper;
105-
private readonly logger?: Logger;
106+
private readonly logger?: ISdkLogger;
106107

107108
public constructor(
108109
defaultCredentialProvider: AwsCredentialIdentityProvider,
@@ -547,5 +548,5 @@ export interface SdkProviderServices {
547548
/**
548549
* An SDK logger
549550
*/
550-
readonly logger?: Logger;
551+
readonly logger?: ISdkLogger;
551552
}

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/sdk.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,12 @@ import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
340340
import { Upload } from '@aws-sdk/lib-storage';
341341
import { getEndpointFromInstructions } from '@smithy/middleware-endpoint';
342342
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
343-
import type { AwsCredentialIdentityProvider, Logger } from '@smithy/types';
343+
import type { AwsCredentialIdentityProvider } from '@smithy/types';
344344
import { ConfiguredRetryStrategy } from '@smithy/util-retry';
345345
import type { WaiterResult } from '@smithy/util-waiter';
346346
import { AccountAccessKeyCache } from './account-cache';
347347
import { cachedAsync } from './cached';
348+
import type { ISdkLogger } from './sdk-logger';
348349
import type { Account } from './sdk-provider';
349350
import { traceMemberMethods } from './tracing';
350351
import { defaultCliUserAgent } from './user-agent';
@@ -385,7 +386,7 @@ export interface ConfigurationOptions {
385386
requestHandler: NodeHttpHandlerOptions;
386387
retryStrategy: ConfiguredRetryStrategy;
387388
customUserAgent: string;
388-
logger?: Logger;
389+
logger?: ISdkLogger;
389390
s3DisableBodySigning?: boolean;
390391
computeChecksums?: boolean;
391392
}
@@ -562,7 +563,7 @@ export class SDK {
562563

563564
public readonly config: ConfigurationOptions;
564565

565-
protected readonly logger?: Logger;
566+
protected readonly logger?: ISdkLogger;
566567

567568
private readonly accountCache;
568569

@@ -590,7 +591,7 @@ export class SDK {
590591
region: string,
591592
requestHandler: NodeHttpHandlerOptions,
592593
ioHelper: IoHelper,
593-
logger?: Logger,
594+
logger?: ISdkLogger,
594595
) {
595596
const debugFn = async (msg: string) => ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(msg));
596597
this.accountCache = new AccountAccessKeyCache(AccountAccessKeyCache.DEFAULT_PATH, debugFn);

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/tracing.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Logger } from '@smithy/types';
1+
import type { ISdkLogger } from './sdk-logger';
22

33
let ENABLED = false;
44
let INDENT = 0;
@@ -10,7 +10,7 @@ export function setSdkTracing(enabled: boolean) {
1010
/**
1111
* Method decorator to trace a single static or member method, any time it's called
1212
*/
13-
export function callTrace(fn: string, className?: string, logger?: Logger) {
13+
export function callTrace(fn: string, className?: string, logger?: ISdkLogger) {
1414
if (!ENABLED || !logger) {
1515
return;
1616
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
export * from './io-host-wrappers';
2-
export * from './sdk-logger';
32
export * from './io-helper';
43
export * from './level-priority';
54
export * from './span';

packages/@aws-cdk/toolkit-lib/lib/api/io/private/sdk-logger.ts

Lines changed: 0 additions & 152 deletions
This file was deleted.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { patternsArrayForWatch } from '../actions/watch/private';
3636
import { BaseCredentials, type SdkConfig } from '../api/aws-auth';
3737
import { makeRequestHandler } from '../api/aws-auth/awscli-compatible';
3838
import type { SdkProviderServices } from '../api/aws-auth/private';
39-
import { SdkProvider } from '../api/aws-auth/private';
39+
import { SdkProvider, IoHostSdkLogger } from '../api/aws-auth/private';
4040
import { Bootstrapper } from '../api/bootstrap';
4141
import type { ICloudAssemblySource } from '../api/cloud-assembly';
4242
import { CachedCloudAssembly, StackSelectionStrategy } from '../api/cloud-assembly';
@@ -47,7 +47,7 @@ import { Deployments } from '../api/deployments';
4747
import { DiffFormatter } from '../api/diff';
4848
import type { IIoHost, IoMessageLevel, ToolkitAction } from '../api/io';
4949
import type { IoHelper } from '../api/io/private';
50-
import { asIoHelper, asSdkLogger, IO, SPAN, withoutColor, withoutEmojis, withTrimmedWhitespace } from '../api/io/private';
50+
import { asIoHelper, IO, SPAN, withoutColor, withoutEmojis, withTrimmedWhitespace } from '../api/io/private';
5151
import { CloudWatchLogEventMonitor, findCloudWatchLogGroups } from '../api/logs-monitor';
5252
import { PluginHost } from '../api/plugin';
5353
import { AmbiguityError, ambiguousMovements, findResourceMovements, formatAmbiguousMappings, formatTypedMappings, fromManifestAndExclusionList, resourceMappings } from '../api/refactoring';
@@ -182,7 +182,7 @@ export class Toolkit extends CloudAssemblySourceBuilder {
182182
const services: SdkProviderServices = {
183183
ioHelper,
184184
requestHandler: await makeRequestHandler(ioHelper, this.props.sdkConfig?.httpOptions),
185-
logger: asSdkLogger(ioHelper),
185+
logger: new IoHostSdkLogger(ioHelper),
186186
pluginHost: this.pluginHost,
187187
};
188188

packages/@aws-cdk/toolkit-lib/test/api/aws-auth/sdk-logger.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
import { formatSdkLoggerContent, SdkToCliLogger } from '../../../lib/api/aws-auth/private';
1+
import { formatSdkLoggerContent, IoHostSdkLogger } from '../../../lib/api/aws-auth/private';
22

3-
describe(SdkToCliLogger, () => {
3+
describe(IoHostSdkLogger, () => {
44
const ioHost = {
55
notify: jest.fn(),
66
requestResponse: jest.fn(),
77
};
8-
const logger = new SdkToCliLogger(ioHost as any);
8+
const logger = new IoHostSdkLogger(ioHost as any);
99

1010
beforeEach(() => {
1111
ioHost.notify.mockReset();
1212
});
1313

14-
test.each(['trace', 'debug'] as Array<keyof SdkToCliLogger>)('%s method does not call notify', (method) => {
14+
test.each(['trace', 'debug'] as Array<keyof IoHostSdkLogger>)('%s method does not call notify', (method) => {
1515
logger[method]('SDK Logger test message');
1616
expect(ioHost.notify).not.toHaveBeenCalled();
1717
});
1818

19-
test.each(['info', 'warn', 'error'] as Array<keyof SdkToCliLogger>)('%s method logs to notify', (method) => {
19+
test.each(['info', 'warn', 'error'] as Array<keyof IoHostSdkLogger>)('%s method logs to notify', (method) => {
2020
logger[method]('SDK Logger test message');
2121
expect(ioHost.notify).toHaveBeenCalledWith(expect.objectContaining({
2222
level: 'trace',

packages/aws-cdk/lib/cli/cli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { Configuration } from './user-configuration';
1111
import * as version from './version';
1212
import { ToolkitError } from '../../../@aws-cdk/toolkit-lib';
1313
import { asIoHelper, IO } from '../../../@aws-cdk/toolkit-lib/lib/api/io/private';
14-
import { SdkProvider, SdkToCliLogger, setSdkTracing } from '../api/aws-auth';
14+
import { SdkProvider, IoHostSdkLogger, setSdkTracing } from '../api/aws-auth';
1515
import type { BootstrapSource } from '../api/bootstrap';
1616
import { Bootstrapper } from '../api/bootstrap';
1717
import type { DeploymentMethod } from '../api/deployments';
@@ -123,7 +123,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
123123
proxyAddress: argv.proxy,
124124
caBundlePath: argv['ca-bundle-path'],
125125
}),
126-
logger: new SdkToCliLogger(asIoHelper(ioHost, ioHost.currentAction as any)),
126+
logger: new IoHostSdkLogger(asIoHelper(ioHost, ioHost.currentAction as any)),
127127
pluginHost: GLOBAL_PLUGIN_HOST,
128128
});
129129

0 commit comments

Comments
 (0)