Skip to content

Commit

Permalink
(fix): Switch Log archive bucket policy to Org policy (#1051)
Browse files Browse the repository at this point in the history
* Adding code for destination policy update

* Changing logic to use subscriptionFilterRoleArn

* Updating custom resource runtime

* Testing some code for update custom resource

* Trying to force custom resource update

* Updating uuid to force update

* Fixing custom resource again

* Removing uuid from other function

* Fixing uuid()

* Fixing props object

* ADding some logging

* Moving logging

* More logging

* Fixing subscriptionfilterrolearn

* Removing additional log statements

* Reducing Store Outputs SM max concurrency from 50 to 20. Also adding limit increase requests for both CloudWatch Logs and Lambda Concurrency

* Reducing Store Outputs SM max concurrency from 50 to 20. Also adding limit increase requests for both CloudWatch Logs and Lambda Concurrency

* Updating maxconcurrency in one more sm to 20

* Adding extra logging stmts

* Removing additional logging statements

* Moving SM concurrent executions down to 10 from 20 to Mirror Ryan's limits

* Create force-github-actions.txt

* Fixing prettier issues

* Adding if statement around subscriptionFilterRoleArn. If it doesn't exist, don't add Subscription Filter

---------

Co-authored-by: Brian969 <[email protected]>
  • Loading branch information
rycerrat and Brian969 authored Mar 9, 2023
1 parent bed0a62 commit 696adb8
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 23 deletions.
10 changes: 10 additions & 0 deletions src/core/runtime/src/load-limits-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ const LIMITS: { [limitKey: string]: LimitCode } = {
quotaCode: 'L-29A0C5DF',
enabled: false,
},
[Limit.CloudWatchCreateLogStream]: {
serviceCode: 'logs',
quotaCode: 'L-76507CEF',
enabled: true,
},
[Limit.LambdaConcurrentExecutions]: {
serviceCode: 'lambda',
quotaCode: 'L-B99A9384',
enabled: true,
},
};

const dynamodb = new DynamoDB();
Expand Down
13 changes: 11 additions & 2 deletions src/deployments/cdk/src/apps/phase-1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,21 @@ export interface IamPolicyArtifactsOutput {
* - Transit Gateway Peering
* - Create LogGroup required for DNS Logging
*/
export async function deploy({ acceleratorConfig, accountStacks, accounts, context, limiter, outputs }: PhaseInput) {
export async function deploy({
acceleratorConfig,
accountStacks,
accounts,
context,
limiter,
outputs,
organizations,
}: PhaseInput) {
const assignedVpcCidrPools = await loadAssignedVpcCidrPool(context.vpcCidrPoolAssignedTable);
const assignedSubnetCidrPools = await loadAssignedSubnetCidrPool(context.subnetCidrPoolAssignedTable);
const masterAccountKey = acceleratorConfig.getMandatoryAccountKey('master');
const iamConfigs = acceleratorConfig.getIamConfigs();
const masterAccountId = getAccountId(accounts, masterAccountKey);
const rootOrgId = organizations[0].rootOrgId!;
if (!masterAccountId) {
throw new Error(`Cannot find mandatory primary account ${masterAccountKey}`);
}
Expand Down Expand Up @@ -576,10 +585,10 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
// Central Services step 1
await cwlCentralLoggingToS3.step1({
accountStacks,
accounts,
logBucket,
outputs,
config: acceleratorConfig,
rootOrgId,
});

await vpcDeployment.step1({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,18 @@ import path from 'path';

export interface CentralLoggingToS3Step1Props {
accountStacks: AccountStacks;
accounts: Account[];
logBucket: s3.IBucket;
outputs: StackOutput[];
config: c.AcceleratorConfig;
rootOrgId: string;
}

/**
* Enable Central Logging to S3 in "log-archive" account Step 1
*/
export async function step1(props: CentralLoggingToS3Step1Props) {
const { accountStacks, accounts, logBucket, config, outputs } = props;
const { accountStacks, logBucket, config, outputs, rootOrgId } = props;
// Setup for CloudWatch logs storing in logs account
const allAccountIds = accounts.map(account => account.id);
const centralLogServices = config['global-options']['central-log-services'];
const cwlRegionsConfig = config['global-options']['additional-cwl-regions'];
const homeRegion = config['global-options']['central-log-services'].region;
Expand Down Expand Up @@ -112,7 +111,6 @@ export async function step1(props: CentralLoggingToS3Step1Props) {

await cwlSettingsInLogArchive({
scope: logAccountStack,
accountIds: allAccountIds,
bucketArn: logBucket.bucketArn,
shardCount: regionConfig['kinesis-stream-shard-count'],
logStreamRoleArn: cwlLogStreamRoleOutput.roleArn,
Expand All @@ -121,6 +119,7 @@ export async function step1(props: CentralLoggingToS3Step1Props) {
region,
encryptionKey,
homeRegionEncryptionKey,
rootOrgId,
});
}
}
Expand All @@ -131,7 +130,6 @@ export async function step1(props: CentralLoggingToS3Step1Props) {
*/
async function cwlSettingsInLogArchive(props: {
scope: cdk.Construct;
accountIds: string[];
bucketArn: string;
logStreamRoleArn: string;
kinesisStreamRoleArn: string;
Expand All @@ -140,10 +138,10 @@ async function cwlSettingsInLogArchive(props: {
shardCount?: number;
dynamicS3LogPartitioning?: c.S3LogPartition[];
region: string;
rootOrgId: string;
}) {
const {
scope,
accountIds,
bucketArn,
logStreamRoleArn,
kinesisStreamRoleArn,
Expand All @@ -152,6 +150,7 @@ async function cwlSettingsInLogArchive(props: {
region,
encryptionKey,
homeRegionEncryptionKey,
rootOrgId,
} = props;

// Create Kinesis Stream for Logs streaming
Expand All @@ -166,26 +165,29 @@ async function cwlSettingsInLogArchive(props: {
});

const destinationName = createName({
name: 'LogDestination',
name: 'LogDestinationOrg',
suffixLength: 0,
});

const destinatinPolicy = {
const destinationPolicy = {
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Principal: {
AWS: accountIds,
},
Principal: '*',
Action: 'logs:PutSubscriptionFilter',
Resource: `arn:aws:logs:${cdk.Aws.REGION}:${cdk.Aws.ACCOUNT_ID}:destination:${destinationName}`,
Condition: {
StringEquals: {
'aws:PrincipalOrgID': [rootOrgId],
},
},
},
],
};
const destinationPolicyStr = JSON.stringify(destinatinPolicy);
const destinationPolicyStr = JSON.stringify(destinationPolicy);
// Create AWS Logs Destination
const logDestination = new logs.CfnDestination(scope, 'Log-Destination', {
const logDestination = new logs.CfnDestination(scope, 'Log-Destination-Org', {
destinationName,
targetArn: logsStream.streamArn,
roleArn: logStreamRoleArn,
Expand Down Expand Up @@ -269,7 +271,7 @@ async function cwlSettingsInLogArchive(props: {
});

// Store LogDestination ARN in output so that subsequent phases can access the output
new CfnLogDestinationOutput(scope, `CloudWatchCentralLoggingOutput`, {
new CfnLogDestinationOutput(scope, `CloudWatchCentralLoggingOrgOutput`, {
destinationArn: logDestination.attrArn,
destinationName: logDestination.destinationName,
destinationKey: 'CwlCentralLogDestination',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { CentralLoggingSubscriptionFilter } from '@aws-accelerator/custom-resour
import { createName } from '@aws-accelerator/cdk-accelerator/src/core/accelerator-name-generator';
import { LogDestinationOutputFinder } from '@aws-accelerator/common-outputs/src/log-destination';
import { IamRoleOutputFinder } from '@aws-accelerator/common-outputs/src/iam-role';
import { v4 as uuidv4 } from 'uuid';

export interface CentralLoggingToS3Step2Props {
accountStacks: AccountStacks;
Expand Down Expand Up @@ -86,6 +87,7 @@ export async function step2(props: CentralLoggingToS3Step2Props) {
ruleName,
logRetention,
roleArn: subscriptionRoleOutput.roleArn,
uuid: uuidv4(),
});
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/common-outputs/src/limits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum Limit {
CloudFormationStackCount = 'AWS CloudFormation/Stack count',
CloudFormationStackSetPerAdmin = 'AWS CloudFormation/Stack sets per administrator account',
OrganizationsMaximumAccounts = 'AWS Organizations/Maximum accounts',
CloudWatchCreateLogStream = 'AWS CloudWatch Logs/CreateLogStream throttle limit in transactions per second',
LambdaConcurrentExecutions = 'AWS Lambda/Concurrent Executions',
}

export interface LimitOutput {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const handler = async (input: any): Promise<string> => {

const logGroupName = input.detail.requestParameters.logGroupName as string;
const logDestinationArn = process.env.LOG_DESTINATION;
const subscriptionFilterRoleArn = process.env.SUBSCRIPTION_FILTER_ROLE_ARN;
if (!logDestinationArn) {
console.warn(`Log Destination is not parent in env for this account`);
const newLocal = `Log Destination is not parent in env for this account`;
Expand All @@ -40,7 +41,11 @@ export const handler = async (input: any): Promise<string> => {
if (isExcluded(exclusions, logGroupName)) {
return `No Need of Subscription Filter for "${logGroupName}"`;
}
await addSubscriptionFilter(logGroupName, logDestinationArn);

if (subscriptionFilterRoleArn) {
await addSubscriptionFilter(logGroupName, logDestinationArn, subscriptionFilterRoleArn);
}

const logRetention = process.env.LOG_RETENTION;
if (logRetention) {
// Update Log Retention Policy
Expand All @@ -49,7 +54,7 @@ export const handler = async (input: any): Promise<string> => {
return 'SUCCESS';
};

async function addSubscriptionFilter(logGroupName: string, destinationArn: string) {
async function addSubscriptionFilter(logGroupName: string, destinationArn: string, subscriptionFilterRoleArn: string) {
// Adding subscription filter
await throttlingBackOff(() =>
logs
Expand All @@ -58,6 +63,7 @@ async function addSubscriptionFilter(logGroupName: string, destinationArn: strin
logGroupName,
filterName: `${CloudWatchRulePrefix}${logGroupName}`,
filterPattern: '',
roleArn: subscriptionFilterRoleArn,
})
.promise(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface HandlerProperties {
logDestinationArn: string;
globalExclusions?: string[];
logRetention: number;
subscriptionFilterRoleArn?: string;
}

export const handler = errorHandler(onEvent);
Expand Down Expand Up @@ -90,7 +91,7 @@ const isExcluded = (exclusions: string[], logGroupName: string): boolean => {

async function centralLoggingSubscription(event: CloudFormationCustomResourceEvent): Promise<void> {
const properties = (event.ResourceProperties as unknown) as HandlerProperties;
const { logDestinationArn, logRetention } = properties;
const { logDestinationArn, logRetention, subscriptionFilterRoleArn } = properties;
const globalExclusions = properties.globalExclusions || [];
const logGroups = await getLogGroups();
const filterLogGroups = logGroups.filter(lg => !isExcluded(globalExclusions, lg.logGroupName!));
Expand All @@ -108,14 +109,14 @@ async function centralLoggingSubscription(event: CloudFormationCustomResourceEve
await putLogRetentionPolicy(logGroup.logGroupName!, logRetention);
// Add Subscription filter to logGroup
console.log(`Adding subscription filter for ${logGroup.logGroupName}`);
await addSubscriptionFilter(logGroup.logGroupName!, logDestinationArn);
await addSubscriptionFilter(logGroup.logGroupName!, logDestinationArn, subscriptionFilterRoleArn!);
}),
);
}

async function centralLoggingSubscriptionUpdate(event: CloudFormationCustomResourceEvent): Promise<void> {
const properties = (event.ResourceProperties as unknown) as HandlerProperties;
const { logDestinationArn, logRetention } = properties;
const { logDestinationArn, logRetention, subscriptionFilterRoleArn } = properties;
const globalExclusions = properties.globalExclusions || [];
const logGroups = await getLogGroups();
const filterLogGroups = logGroups.filter(lg => !isExcluded(globalExclusions, lg.logGroupName!));
Expand All @@ -142,7 +143,7 @@ async function centralLoggingSubscriptionUpdate(event: CloudFormationCustomResou
await putLogRetentionPolicy(logGroup.logGroupName!, logRetention);
// Add Subscription filter to logGroup
console.log(`Adding subscription filter for ${logGroup.logGroupName}`);
await addSubscriptionFilter(logGroup.logGroupName!, logDestinationArn);
await addSubscriptionFilter(logGroup.logGroupName!, logDestinationArn, subscriptionFilterRoleArn!);
}),
);
}
Expand All @@ -167,7 +168,7 @@ async function removeSubscriptionFilter(logGroupName: string, filterName: string
}
}

async function addSubscriptionFilter(logGroupName: string, destinationArn: string) {
async function addSubscriptionFilter(logGroupName: string, destinationArn: string, subscriptionFilterRoleArn: string) {
try {
// Adding subscription filter
await throttlingBackOff(() =>
Expand All @@ -177,6 +178,7 @@ async function addSubscriptionFilter(logGroupName: string, destinationArn: strin
logGroupName,
filterName: `${CloudWatchRulePrefix}${logGroupName}`,
filterPattern: '',
roleArn: subscriptionFilterRoleArn,
})
.promise(),
);
Expand Down
21 changes: 21 additions & 0 deletions src/lib/custom-resources/logs-add-subscription-filter/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export interface CentralLoggingSubscriptionFilterProps {
ruleName: string;
logRetention: number;
roleArn: string;
uuid: string;
subscriptionFilterRoleArn?: string;
}

/**
Expand All @@ -41,6 +43,24 @@ export class CentralLoggingSubscriptionFilter extends cdk.Construct {
constructor(scope: cdk.Construct, id: string, props: CentralLoggingSubscriptionFilterProps) {
super(scope, id);

// Since this is deployed organization wide, this role is required
// https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/CreateSubscriptionFilter-IAMrole.html
const subscriptionFilterRole = new iam.Role(this, 'SubscriptionFilterRole', {
assumedBy: new iam.ServicePrincipal(cdk.Fn.sub('logs.${AWS::Region}.amazonaws.com')),
description: 'Role used by Subscription Filter to allow access to CloudWatch Destination',
inlinePolicies: {
accessLogEvents: new iam.PolicyDocument({
statements: [
new iam.PolicyStatement({
resources: ['*'],
actions: ['logs:PutLogEvents'],
}),
],
}),
},
});

props.subscriptionFilterRoleArn = subscriptionFilterRole.roleArn;
this.role = iam.Role.fromRoleArn(this, `${resourceType}Role`, props.roleArn);
// Custom Resource to add subscriptin filter to existing logGroups
this.resource = new cdk.CustomResource(this, 'CustomResource', {
Expand All @@ -56,6 +76,7 @@ export class CentralLoggingSubscriptionFilter extends cdk.Construct {
EXCLUSIONS: JSON.stringify(props.globalExclusions),
LOG_DESTINATION: props.logDestinationArn,
LOG_RETENTION: props.logRetention.toString(),
SUBSCRIPTION_FILTER_ROLE_ARN: subscriptionFilterRole.roleArn,
};
const addSubscriptionLambda = this.ensureLambdaFunction(
this.cloudWatchEnventLambdaPath,
Expand Down

0 comments on commit 696adb8

Please sign in to comment.