Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): cdk import errors with 'S3 error: Access Denied' #31727

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions packages/aws-cdk/lib/api/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { deployStack, DeployStackResult, destroyStack, DeploymentMethod } from '
import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
import { HotswapMode } from './hotswap/common';
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, RootTemplateWithNestedStacks } from './nested-stack-helpers';
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries, stabilizeStack } from './util/cloudformation';
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries, stabilizeStack, uploadStackTemplateAssets } from './util/cloudformation';
import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
import { StackEventPoller } from './util/cloudformation/stack-event-poller';
import { RollbackChoice } from './util/cloudformation/stack-status';
Expand Down Expand Up @@ -292,13 +292,6 @@ interface AssetOptions {
*/
readonly stack: cxapi.CloudFormationStackArtifact;

/**
* Name of the toolkit stack, if not the default name.
*
* @default 'CDKToolkit'
*/
readonly toolkitStackName?: string;

/**
* Execution role for the building.
*
Expand Down Expand Up @@ -426,14 +419,29 @@ export class Deployments {
const { stackSdk, resolvedEnvironment, envResources } = await this.prepareSdkFor(stackArtifact, undefined, Mode.ForReading);
const cfn = stackSdk.cloudFormation();

await uploadStackTemplateAssets(stackArtifact, this);

// Upload the template, if necessary, before passing it to CFN
const builder = new AssetManifestBuilder();
const cfnParam = await makeBodyParameter(
stackArtifact,
resolvedEnvironment,
new AssetManifestBuilder(),
builder,
envResources,
stackSdk);

// If the `makeBodyParameter` before this added assets, make sure to publish them before
// calling the API.
const addedAssets = builder.toManifest(stackArtifact.assembly.directory);
for (const entry of addedAssets.entries) {
await this.buildSingleAsset('no-version-validation', addedAssets, entry, {
stack: stackArtifact,
});
await this.publishSingleAsset(addedAssets, entry, {
stack: stackArtifact,
});
}

const response = await cfn.getTemplateSummary(cfnParam).promise();
if (!response.ResourceIdentifierSummaries) {
debug('GetTemplateSummary API call did not return "ResourceIdentifierSummaries"');
Expand Down Expand Up @@ -805,16 +813,22 @@ export class Deployments {

/**
* Build a single asset from an asset manifest
*
* If an assert manifest artifact is given, the bootstrap stack version
* will be validated according to the constraints in that manifest artifact.
* If that is not necessary, `'no-version-validation'` can be passed.
*/
// eslint-disable-next-line max-len
public async buildSingleAsset(assetArtifact: cxapi.AssetManifestArtifact, assetManifest: AssetManifest, asset: IManifestEntry, options: BuildStackAssetsOptions) {
public async buildSingleAsset(assetArtifact: cxapi.AssetManifestArtifact | 'no-version-validation', assetManifest: AssetManifest, asset: IManifestEntry, options: BuildStackAssetsOptions) {
const { resolvedEnvironment, envResources } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);

await this.validateBootstrapStackVersion(
options.stack.stackName,
assetArtifact.requiresBootstrapStackVersion,
assetArtifact.bootstrapStackVersionSsmParameter,
envResources);
if (assetArtifact !== 'no-version-validation') {
await this.validateBootstrapStackVersion(
options.stack.stackName,
assetArtifact.requiresBootstrapStackVersion,
assetArtifact.bootstrapStackVersionSsmParameter,
envResources);
}

const publisher = this.cachedPublisher(assetManifest, resolvedEnvironment, options.stackName);
await publisher.buildEntry(asset);
Expand Down
56 changes: 33 additions & 23 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ export type PrepareChangeSetOptions = {
sdkProvider: SdkProvider;
stream: NodeJS.WritableStream;
parameters: { [name: string]: string | undefined };
toolkitStackName?: string;
resourcesToImport?: ResourcesToImport;
}

Expand Down Expand Up @@ -342,12 +341,14 @@ export async function createDiffChangeSet(options: PrepareChangeSetOptions): Pro
}

/**
* Returns all file entries from an AssetManifestArtifact. This is used in the
* `uploadBodyParameterAndCreateChangeSet` function to find all template asset files to build and publish.
* Returns all file entries from an AssetManifestArtifact that look like templates.
*
* This is used in the `uploadBodyParameterAndCreateChangeSet` function to find
* all template asset files to build and publish.
*
* Returns a tuple of [AssetManifest, FileManifestEntry[]]
*/
function fileEntriesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtifact): [AssetManifest, FileManifestEntry[]] {
function templatesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtifact): [AssetManifest, FileManifestEntry[]] {
const assets: (FileManifestEntry)[] = [];
const fileName = artifact.file;
const assetManifest = AssetManifest.fromFile(fileName);
Expand All @@ -365,25 +366,7 @@ function fileEntriesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtif

async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise<DescribeChangeSetOutput | undefined> {
try {
for (const artifact of options.stack.dependencies) {
// Skip artifact if it is not an Asset Manifest Artifact
if (!cxapi.AssetManifestArtifact.isAssetManifestArtifact(artifact)) {
continue;
}

// Build and publish each file entry of the Asset Manifest Artifact:
const [assetManifest, file_entries] = fileEntriesFromAssetManifestArtifact(artifact);
for (const entry of file_entries) {
await options.deployments.buildSingleAsset(artifact, assetManifest, entry, {
stack: options.stack,
toolkitStackName: options.toolkitStackName,
});
await options.deployments.publishSingleAsset(assetManifest, entry, {
stack: options.stack,
toolkitStackName: options.toolkitStackName,
});
}
}
await uploadStackTemplateAssets(options.stack, options.deployments);
const preparedSdk = (await options.deployments.prepareSdkWithDeployRole(options.stack));

const bodyParameter = await makeBodyParameter(
Expand Down Expand Up @@ -419,6 +402,33 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
}
}

/**
* Uploads the assets that look like templates for this CloudFormation stack
*
* This is necessary for any CloudFormation call that needs the template, it may need
* to be uploaded to an S3 bucket first. We have to follow the instructions in the
* asset manifest, because technically that is the only place that knows about
* bucket and assumed roles and such.
*/
export async function uploadStackTemplateAssets(stack: cxapi.CloudFormationStackArtifact, deployments: Deployments) {
for (const artifact of stack.dependencies) {
// Skip artifact if it is not an Asset Manifest Artifact
if (!cxapi.AssetManifestArtifact.isAssetManifestArtifact(artifact)) {
continue;
}

const [assetManifest, file_entries] = templatesFromAssetManifestArtifact(artifact);
for (const entry of file_entries) {
await deployments.buildSingleAsset(artifact, assetManifest, entry, {
stack,
});
await deployments.publishSingleAsset(assetManifest, entry, {
stack,
});
}
}
}

async function createChangeSet(options: CreateChangeSetOptions): Promise<DescribeChangeSetOutput> {
await cleanupOldChangeset(options.changeSetName, options.stack.stackName, options.cfn);

Expand Down
4 changes: 0 additions & 4 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ export class CdkToolkit {
parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
resourcesToImport,
stream,
toolkitStackName: options.toolkitStackName,
});
} else {
debug(`the stack '${stack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`);
Expand Down Expand Up @@ -246,7 +245,6 @@ export class CdkToolkit {
await this.props.deployments.buildSingleAsset(assetNode.assetManifestArtifact, assetNode.assetManifest, assetNode.asset, {
stack: assetNode.parentStack,
roleArn: options.roleArn,
toolkitStackName: options.toolkitStackName,
stackName: assetNode.parentStack.stackName,
});
};
Expand All @@ -255,7 +253,6 @@ export class CdkToolkit {
await this.props.deployments.publishSingleAsset(assetNode.assetManifest, assetNode.asset, {
stack: assetNode.parentStack,
roleArn: options.roleArn,
toolkitStackName: options.toolkitStackName,
stackName: assetNode.parentStack.stackName,
});
};
Expand Down Expand Up @@ -1030,7 +1027,6 @@ export class CdkToolkit {
await graph.removeUnnecessaryAssets(assetNode => this.props.deployments.isSingleAssetPublished(assetNode.assetManifest, assetNode.asset, {
stack: assetNode.parentStack,
roleArn: options.roleArn,
toolkitStackName: options.toolkitStackName,
stackName: assetNode.parentStack.stackName,
}));
}
Expand Down
Loading