Skip to content

Commit

Permalink
fix: better handle authentication issues (#42)
Browse files Browse the repository at this point in the history
- Show a warning message if the account/region is unknown
- Throw an error if the account is for the wrong

Fixes #35
  • Loading branch information
corymhall committed Jan 17, 2024
1 parent a521528 commit c90937f
Show file tree
Hide file tree
Showing 13 changed files with 17,978 additions and 6,612 deletions.
4 changes: 4 additions & 0 deletions .projen/deps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .projen/tasks.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const project = new GitHubActionTypeScriptProject({
'@actions/tool-cache@^2.0.0',
'fs-extra',
'@aws-sdk/client-cloudformation',
'@aws-sdk/client-sts',
'@smithy/types',
'chalk@^4',
'@aws-sdk/credential-providers',
Expand Down
24,035 changes: 17,445 additions & 6,590 deletions dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 18 additions & 1 deletion src/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export interface StackInfo {
*/
region?: string;

/**
* The account the stack is deployed to
*
* @default - unknown-account
*/
account?: string;

/**
* The lookup role to use
*
Expand Down Expand Up @@ -104,13 +111,23 @@ export class AssemblyManifestReader {
const props = artifact.properties as AwsCloudFormationStackProperties;
const template = fs.readJSONSync(path.resolve(this.directory, props.templateFile));
const env = artifact.environment?.split(/\/\/?/);
let region = env && env.length === 3 ? env[2] : undefined;
const validEnv = env && env.length === 3;
let region: string | undefined;
let account: string | undefined;
if (validEnv) {
region = env[2];
account = env[1];
}
if (region === 'unknown-region') {
region = undefined;
}
if (account === 'unknown-account') {
account = undefined;
}
stacks.push({
content: template,
region,
account,
lookupRole: props.lookupRole,
name: props.stackName ?? artifactId,
});
Expand Down
55 changes: 55 additions & 0 deletions src/diff.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ResourceDifference, ResourceImpact, TemplateDiff, diffTemplate } from '@aws-cdk/cloudformation-diff';
import { CloudFormationClient, GetTemplateCommand, StackNotFoundException } from '@aws-sdk/client-cloudformation';
import { STSClient, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
import { fromTemporaryCredentials } from '@aws-sdk/credential-providers';
import { AwsCredentialIdentityProvider } from '@smithy/types';
import { StackInfo } from './assembly';
Expand Down Expand Up @@ -27,6 +28,15 @@ export interface ChangeDetails {
* Information on any destructive changes
*/
destructiveChanges: DestructiveChange[];

/**
* Whether this stack has an unknown env. If that is the case
* then this returns the environment credentials that are being used
* in the format aws://<account>/<region>
*
* @default - account is known
*/
unknownEnvironment?: string;
}

/**
Expand Down Expand Up @@ -75,11 +85,55 @@ export class StackDiff {
});
}

/**
* Validates the environment of the stack and whether it matches
* the credentials being used by the GitHub action
*
* There are two cases that we care about:
* 1. The stack has an account and region defined _and_ the credentials
* being used by the GitHub action are for a different account and region.
* In this case we throw an error because the stack diff would not be correct
* 2. The stack has an account and region that are _not_ defined. In this case
* We will return true to indicate that the environment is unknown. This will be used
* later to print a warning in the stack diff.
*
* @returns whether or not the environment is unknown
*/
private async validateEnvironment(): Promise<string | undefined> {
let unknownAccount = false;
let unknownRegion = false;
const stsClient = new STSClient({});
const callerIdentity = new GetCallerIdentityCommand({ });
const identity = await stsClient.send(callerIdentity);
const configRegion = await this.client.config.region();
if (!this.stack.region) {
unknownRegion = true;
}
if (!this.stack.account) {
unknownAccount = true;
}
if (this.stack.account && identity.Account !== this.stack.account) {
throw new Error(`Credentials are for account ${identity.Account} but stack is in account ${this.stack.account}`);
}

if (this.stack.region && configRegion !== this.stack.region) {
throw new Error(`Credentials are for region ${configRegion} but stack is in region ${this.stack.region}`);
}

if (unknownAccount || unknownRegion) {
return `aws://${identity.Account}/${configRegion}`;
}

return;
}

/** Performs the diff on the CloudFormation stack
* This reads the existing stack from CFN and then uses the cloudformation-diff
* package to perform the diff and collect additional information on the type of changes
*/
public async diffStack(): Promise<{ diff: TemplateDiff; changes: ChangeDetails }> {
const unknownEnv = await this.validateEnvironment();

const cmd = new GetTemplateCommand({
StackName: this.stack.name,
});
Expand All @@ -95,6 +149,7 @@ export class StackDiff {
try {
const diff = diffTemplate(existingTemplate, this.stack.content);
const changes = this.evaluateDiff(this.stack.name, diff);
changes.unknownEnvironment = unknownEnv;
return {
diff,
changes,
Expand Down
7 changes: 7 additions & 0 deletions src/stage-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class StageProcessor {
prev.stacks.push({
name: curr.name,
lookupRole: curr.lookupRole,
account: curr.account,
region: curr.region,
});
return prev;
Expand Down Expand Up @@ -184,6 +185,12 @@ export class StageProcessor {
'<details><summary>Details</summary>',
'',
]);
if (changes.unknownEnvironment) {
output.push('> [!INFO]\n> ***Unknown Environment*** :information_source:');
output.push('> This stack has an unknown environment which may mean the diff is performed against the wrong environment');
output.push(`> Environmment used ${changes.unknownEnvironment}`);
output.push('');
}
if (changes.destructiveChanges.length) {
output.push('');
output.push('> [!WARNING]\n> ***Destructive Changes*** :bangbang:'),
Expand Down
2 changes: 2 additions & 0 deletions test/assembly.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ describe('cloud assembly manifest reader', () => {
name: 'test-stack',
content: { data: 'data' },
region: 'us-east-1',
account: '1234567891012',
lookupRole: expect.objectContaining({
arn: lookupRoleArn,
}),
Expand All @@ -109,6 +110,7 @@ describe('cloud assembly manifest reader', () => {
{
name: 'SomeStage',
region: undefined,
account: undefined,
stacks: [{
name: 'test-stack2',
content: { data: 'data' },
Expand Down
Loading

0 comments on commit c90937f

Please sign in to comment.