Skip to content

Commit 1f7527b

Browse files
rix0rrrgithub-actions
andauthored
fix(integ-testing): add test that forces cdk-assets to use the Docker credentials helper (#663)
This runs `docker-credential-cdk-asset` (the Docker helper tool) and checks its output to see that it actually usefully does something. - I would have liked to do a full integration between `cdk-assets` and the correct Docker config, but unfortunately this only makes sense for a 2nd registry (not the primary registry we're pushing to). So we would need either: - A fake docker registry; I tried looking for one or faking one via a static HTTP server, but the requirement of signing the image manifest got me stuck. Also, the Docker registry/HTTP server needs to be accessible from the Docker Host, so it can't easily run inside the CodeBuild/GHA container; we would need to set up AWS infra to host this static server which implies S3 + CloudFront or APIGW + Lambda, which starts to get annoying. - A real Docker registry that requires user/pass authentication; but now we have to manage a username and password and security is going to make us rotate it. - An ECR repository; it must be in a different account or region, because the registry name (`<account>.dkr.ecr.<region>.amazonaws.com`) must be different. To make it in a different account or region, we would need to run Atmosphere twice, but that doesn't help when we're running locally or in a non-Atmosphere enabled environment. - I would also have liked to do a test that reads from SecretsManager, but at least the Atmosphere roles don't have permissions to use SecretsManager and I'm not sure what other environments these tests run in, so this test is commented out for now. ## Also in this PR - Features to create and clean up a role in the integ tests. - Properly configure the `cdk-assets` version to test (not just `latest`, that will work in our online builds with a fake NPM repo but won't work locally, for example). This needed some refactoring of the "package sources" code to generalize the classes that were dealing with the `aws-cdk` package to take a package name. - Refactored the assets helper code a bit to pull out some helper functions used to prepare assets. - Fix a bug in `yarn-cling` that allows hoisting conflicting dependencies into certain places if `yarn` has already hoisted them during the proper install. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <github-actions@github.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent ded7ae1 commit 1f7527b

File tree

26 files changed

+1806
-1508
lines changed

26 files changed

+1806
-1508
lines changed

.projenrc.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,14 +1639,15 @@ const cliInteg = configureProject(
16391639
'@aws-sdk/client-sns',
16401640
'@aws-sdk/client-sso',
16411641
'@aws-sdk/client-sts',
1642+
'@aws-sdk/client-secrets-manager',
16421643
'@aws-sdk/credential-providers',
16431644
'@cdklabs/cdk-atmosphere-client',
16441645
'@smithy/util-retry', // smithy packages don't have the same major version as SDK packages
16451646
'@smithy/types', // smithy packages don't have the same major version as SDK packages
16461647
'axios@^1',
16471648
'chalk@^4',
16481649
'fs-extra@^9',
1649-
'glob@^7',
1650+
'glob@^9',
16501651
'make-runnable@^1',
16511652
'mockttp@^3',
16521653
'npm@^10',
@@ -1660,6 +1661,7 @@ const cliInteg = configureProject(
16601661
'jest@^29',
16611662
'jest-junit@^15',
16621663
'ts-jest@^29',
1664+
'proxy-agent',
16631665
'node-pty',
16641666
],
16651667
devDeps: [
@@ -1668,7 +1670,6 @@ const cliInteg = configureProject(
16681670
'@types/semver@^7',
16691671
'@types/yargs@^16',
16701672
'@types/fs-extra@^9',
1671-
'@types/glob@^7',
16721673
],
16731674
bin: {
16741675
'run-suite': 'bin/run-suite',

packages/@aws-cdk-testing/cli-integ/.projen/deps.json

Lines changed: 9 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/cli-integ/.projen/tasks.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/cli-integ/lib/aws.ts

Lines changed: 105 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
import { DeleteRepositoryCommand, ECRClient } from '@aws-sdk/client-ecr';
99
import { ECRPUBLICClient } from '@aws-sdk/client-ecr-public';
1010
import { ECSClient } from '@aws-sdk/client-ecs';
11-
import { IAMClient } from '@aws-sdk/client-iam';
11+
import { CreateRoleCommand, DeleteRoleCommand, DeleteRolePolicyCommand, IAMClient, ListRolePoliciesCommand, PutRolePolicyCommand } from '@aws-sdk/client-iam';
1212
import { LambdaClient } from '@aws-sdk/client-lambda';
1313
import {
1414
S3Client,
@@ -17,27 +17,31 @@ import {
1717
type ObjectIdentifier,
1818
DeleteBucketCommand,
1919
} from '@aws-sdk/client-s3';
20+
import { SecretsManagerClient } from '@aws-sdk/client-secrets-manager';
2021
import { SNSClient } from '@aws-sdk/client-sns';
2122
import { SSOClient } from '@aws-sdk/client-sso';
22-
import { STSClient, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
23+
import { AssumeRoleCommand, STSClient, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
2324
import { fromIni, fromNodeProviderChain } from '@aws-sdk/credential-providers';
24-
import type { AwsCredentialIdentity, AwsCredentialIdentityProvider } from '@smithy/types';
25+
import type { AwsCredentialIdentity, AwsCredentialIdentityProvider, NodeHttpHandlerOptions } from '@smithy/types';
2526
import { ConfiguredRetryStrategy } from '@smithy/util-retry';
27+
2628
interface ClientConfig {
2729
readonly credentials: AwsCredentialIdentityProvider | AwsCredentialIdentity;
2830
readonly region: string;
2931
readonly retryStrategy: ConfiguredRetryStrategy;
32+
readonly requestHandler?: NodeHttpHandlerOptions;
3033
}
3134

3235
export class AwsClients {
33-
public static async forIdentity(region: string, identity: AwsCredentialIdentity, output: NodeJS.WritableStream) {
34-
return new AwsClients(region, output, identity);
36+
public static async forIdentity(randomString: string, region: string, identity: AwsCredentialIdentity, output: NodeJS.WritableStream) {
37+
return new AwsClients(randomString, region, output, identity);
3538
}
3639

37-
public static async forRegion(region: string, output: NodeJS.WritableStream) {
38-
return new AwsClients(region, output);
40+
public static async forRegion(randomString: string, region: string, output: NodeJS.WritableStream) {
41+
return new AwsClients(randomString, region, output);
3942
}
4043

44+
private readonly cleanup: (() => Promise<void>)[] = [];
4145
private readonly config: ClientConfig;
4246

4347
public readonly cloudFormation: CloudFormationClient;
@@ -50,8 +54,11 @@ export class AwsClients {
5054
public readonly iam: IAMClient;
5155
public readonly lambda: LambdaClient;
5256
public readonly sts: STSClient;
57+
public readonly secretsManager: SecretsManagerClient;
5358

54-
constructor(
59+
private constructor(
60+
/** A random string to use for temporary resources, like roles (should preferably match unique test-specific randomString) */
61+
private readonly randomString: string,
5562
public readonly region: string,
5663
private readonly output: NodeJS.WritableStream,
5764
public readonly identity?: AwsCredentialIdentity) {
@@ -60,6 +67,7 @@ export class AwsClients {
6067
region: this.region,
6168
retryStrategy: new ConfiguredRetryStrategy(9, (attempt: number) => attempt ** 500),
6269
};
70+
6371
this.cloudFormation = new CloudFormationClient(this.config);
6472
this.s3 = new S3Client(this.config);
6573
this.ecr = new ECRClient(this.config);
@@ -70,6 +78,22 @@ export class AwsClients {
7078
this.iam = new IAMClient(this.config);
7179
this.lambda = new LambdaClient(this.config);
7280
this.sts = new STSClient(this.config);
81+
this.secretsManager = new SecretsManagerClient(this.config);
82+
}
83+
84+
public addCleanup(cleanup: () => Promise<any>) {
85+
this.cleanup.push(cleanup);
86+
}
87+
88+
public async dispose() {
89+
for (const cleanup of this.cleanup) {
90+
try {
91+
await cleanup();
92+
} catch (e: any) {
93+
this.output.write(`⚠️ Error during cleanup: ${e.message}\n`);
94+
}
95+
}
96+
this.cleanup.splice(0, this.cleanup.length);
7397
}
7498

7599
public async account(): Promise<string> {
@@ -219,6 +243,79 @@ export class AwsClients {
219243
throw e;
220244
}
221245
}
246+
247+
/**
248+
* Create a role that will be cleaned up when the AwsClients object is cleaned up
249+
*/
250+
public async temporaryRole(namePrefix: string, assumeRolePolicyStatements: any[], policyStatements: any[]) {
251+
const response = await this.iam.send(new CreateRoleCommand({
252+
RoleName: `${namePrefix}-${this.randomString}`,
253+
AssumeRolePolicyDocument: JSON.stringify({
254+
Version: '2012-10-17',
255+
Statement: assumeRolePolicyStatements,
256+
}, undefined, 2),
257+
Tags: [
258+
{
259+
Key: 'deleteme',
260+
Value: 'true',
261+
},
262+
],
263+
}));
264+
await this.iam.send(new PutRolePolicyCommand({
265+
RoleName: `${namePrefix}-${this.randomString}`,
266+
PolicyName: 'DefaultPolicy',
267+
PolicyDocument: JSON.stringify({
268+
Version: '2012-10-17',
269+
Statement: policyStatements,
270+
}, undefined, 2),
271+
}));
272+
273+
this.addCleanup(() => this.deleteRole(response.Role!.RoleName!));
274+
275+
return response.Role?.Arn ?? '*CreateRole did not return an ARN*';
276+
}
277+
278+
public async waitForAssumeRole(roleArn: string) {
279+
// Wait until the role has replicated
280+
const deadline = Date.now() + 60_000;
281+
let lastError: Error | undefined;
282+
while (Date.now() < deadline) {
283+
try {
284+
await this.sts.send(new AssumeRoleCommand({
285+
RoleArn: roleArn,
286+
RoleSessionName: 'test-existence',
287+
}));
288+
return;
289+
} catch (e: any) {
290+
lastError = e;
291+
292+
if (e.name === 'AccessDenied') {
293+
continue;
294+
}
295+
296+
throw e;
297+
}
298+
}
299+
300+
throw new Error(`Timed out waiting for role ${roleArn} to become assumable: ${lastError}`);
301+
}
302+
303+
public async deleteRole(name: string) {
304+
const policiesResponse = await this.iam.send(new ListRolePoliciesCommand({
305+
RoleName: name,
306+
}));
307+
308+
for (const policyName of policiesResponse.PolicyNames ?? []) {
309+
await this.iam.send(new DeleteRolePolicyCommand({
310+
RoleName: name,
311+
PolicyName: policyName,
312+
}));
313+
}
314+
315+
await this.iam.send(new DeleteRoleCommand({
316+
RoleName: name,
317+
}));
318+
}
222319
}
223320

224321
export function isStackMissingError(e: Error) {

packages/@aws-cdk-testing/cli-integ/lib/cli/run-suite.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { RunnerLibraryPreinstalledSource } from '../package-sources/library-prei
1111
import type { IRunnerSource, ITestCliSource, ITestLibrarySource } from '../package-sources/source';
1212
import { serializeSources } from '../package-sources/subprocess';
1313

14+
const CLI_PACKAGE_NAME = 'aws-cdk';
15+
const CDK_ASSETS_PACKAGE_NAME = 'cdk-assets';
16+
1417
async function main() {
1518
const args = await yargs
1619
.command('* <SUITENAME>', 'default command', y => y
@@ -41,6 +44,11 @@ async function main() {
4144
alias: 'l',
4245
type: 'string',
4346
})
47+
.options('cdk-assets-version', {
48+
describe: 'cdk-assets version to use.',
49+
alias: 'a',
50+
type: 'string',
51+
})
4452
.option('use-source', {
4553
descripton: 'Use TypeScript packages from the given source repository (or "auto")',
4654
type: 'string',
@@ -105,25 +113,55 @@ async function main() {
105113
const suiteName = args.SUITENAME;
106114

107115
// So many ways to specify this, and with various ways to spell the same flag (o_O)
116+
// Also, some of them depend on each other for convenience.
108117
const cliSource = new UniqueOption<IRunnerSource<ITestCliSource>>('CLI version');
118+
const cdkAssetsSource = new UniqueOption<IRunnerSource<ITestCliSource>>('cdk-assets version');
119+
120+
// Specific CLI version
109121
for (const flagAlias of ['cli-version', 'use-cli-release'] as const) {
110122
if (args[flagAlias]) {
111-
cliSource.set(new RunnerCliNpmSource(args[flagAlias]), `--${flagAlias}`);
123+
cliSource.set(new RunnerCliNpmSource(CLI_PACKAGE_NAME, args[flagAlias]), `--${flagAlias}`);
112124
}
113125
}
126+
127+
// Specific cdk-assets version
128+
if (args['cdk-assets-version']) {
129+
cdkAssetsSource.set(new RunnerCliNpmSource(CDK_ASSETS_PACKAGE_NAME, args['cdk-assets-version']), '--cdk-assets-version');
130+
}
131+
132+
// Specifically use a source location
114133
for (const flagAlias of ['cli-source', 'use-source'] as const) {
115134
if (args[flagAlias]) {
116135
const root = args[flagAlias] === 'auto' ? await autoFindRepoRoot() : args[flagAlias];
117-
cliSource.set(new RunnerCliRepoSource(root), `--${flagAlias}`);
136+
cliSource.set(new RunnerCliRepoSource(CLI_PACKAGE_NAME, root), `--${flagAlias}`);
137+
cdkAssetsSource.set(new RunnerCliRepoSource(CDK_ASSETS_PACKAGE_NAME, root), `--${flagAlias}`);
118138
}
119139
}
140+
141+
// Specifically request that a source location is given, or we didn't find a CLI yet.
142+
// A CLI source is required, so if this fails that's alright.
120143
if (args['auto-source'] || !cliSource.isSet()) {
121-
cliSource.set(new RunnerCliRepoSource(await autoFindRepoRoot()), '--auto-source');
144+
cliSource.set(new RunnerCliRepoSource(CLI_PACKAGE_NAME, await autoFindRepoRoot()), '--auto-source');
145+
}
146+
147+
// If the CLI is taken from the source, and cdk-assets is not set, we can copy the cdk-assets source from the CLI source.
148+
if (!cdkAssetsSource.isSet()) {
149+
const cliSrc = cliSource.assert();
150+
if (cliSrc instanceof RunnerCliRepoSource) {
151+
cdkAssetsSource.set(new RunnerCliRepoSource(CDK_ASSETS_PACKAGE_NAME, cliSrc.repoRoot), 'copied from CLI source');
152+
}
122153
}
123154

155+
// If cdk-assets is still not configured, fall back to the latest version that is available
156+
if (!cdkAssetsSource.isSet()) {
157+
cdkAssetsSource.set(new RunnerCliNpmSource(CDK_ASSETS_PACKAGE_NAME, 'latest'), '--cdk-assets-version not set');
158+
}
159+
160+
// Library source is either the given one, or 'latest' (nice and simple)
124161
const librarySource: IRunnerSource<ITestLibrarySource>
125162
= new RunnerLibraryNpmSource('aws-cdk-lib', args['framework-version'] ? args['framework-version'] : 'latest');
126163

164+
// Toolkit lib source is either the given one, or the one that's being brought by 'package.json' already, or 'latest'
127165
const toolkitLibPackage = '@aws-cdk/toolkit-lib';
128166
let toolkitSource: IRunnerSource<ITestLibrarySource> | undefined;
129167
if (args['toolkit-lib-version']) {
@@ -141,6 +179,7 @@ async function main() {
141179
console.log(` CLI source: ${cliSource.assert().sourceDescription}`);
142180
console.log(` Library source: ${librarySource.sourceDescription}`);
143181
console.log(` Toolkit lib source: ${toolkitSource.sourceDescription}`);
182+
console.log(` cdk-assets source: ${cdkAssetsSource.assert().sourceDescription}`);
144183

145184
if (args.verbose) {
146185
process.env.VERBOSE = '1';
@@ -171,10 +210,15 @@ async function main() {
171210
disposables.push(toolkitLib);
172211
console.log(` Toolkit library: ${toolkitLib.version}`);
173212

213+
const cdkAssets = await cdkAssetsSource.assert().runnerPrepare();
214+
disposables.push(cdkAssets);
215+
console.log(` cdk-assets: ${cdkAssets.version}`);
216+
174217
serializeSources({
175218
cli,
176219
library,
177220
toolkitLib,
221+
cdkAssets,
178222
});
179223

180224
const jestConfig = path.resolve(__dirname, '..', '..', 'resources', 'integ.jest.config.js');

packages/@aws-cdk-testing/cli-integ/lib/npm.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const MINIMUM_VERSION = '3.9';
77
export async function npmMostRecentMatching(packageName: string, range: string) {
88
const output = JSON.parse(await shell(['node', require.resolve('npm'), '--silent', 'view', `${packageName}@${range}`, 'version', '--json'], {
99
show: 'error',
10+
captureStderr: false,
1011
}));
1112

1213
if (typeof output === 'string') {

packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,20 @@ import { addToShellPath, rimraf, shell } from '../shell';
88
export class RunnerCliNpmSource implements IRunnerSource<ITestCliSource> {
99
public readonly sourceDescription: string;
1010

11-
constructor(private readonly range: string) {
11+
constructor(private readonly packageName: string, private readonly range: string) {
1212
this.sourceDescription = `${this.range} (npm)`;
1313
}
1414

1515
public async runnerPrepare(): Promise<IPreparedRunnerSource<ITestCliSource>> {
1616
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tmpcdk'));
1717
fs.mkdirSync(tempDir, { recursive: true });
1818

19-
await shell(['node', require.resolve('npm'), 'install', `aws-cdk@${this.range}`], {
19+
await shell(['node', require.resolve('npm'), 'install', `${this.packageName}@${this.range}`], {
2020
cwd: tempDir,
2121
show: 'error',
2222
outputs: [process.stderr],
2323
});
24-
const installedVersion = await npmQueryInstalledVersion('aws-cdk', tempDir);
24+
const installedVersion = await npmQueryInstalledVersion(this.packageName, tempDir);
2525

2626
return {
2727
version: installedVersion,

0 commit comments

Comments
 (0)