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

feat(pipelines): Add logging options to codeBuildDefaults property #24016

Closed
wants to merge 32 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
eb8d322
add logging property
yo-ga Feb 1, 2023
29862e4
init integration test
yo-ga Feb 1, 2023
b7a33b6
init merge loggings
yo-ga Feb 1, 2023
b62941f
adjust merge policy
yo-ga Feb 5, 2023
1c8bb29
update testing
yo-ga Feb 5, 2023
4bad59a
Merge branch 'main' into feature/22045
yo-ga Feb 5, 2023
ddda736
add readme
yo-ga Feb 5, 2023
643ea84
remove comment
yo-ga Feb 5, 2023
a246aaf
add stack verification description
yo-ga Feb 5, 2023
371947c
Merge branch 'main' into feature/22045
yo-ga Feb 8, 2023
b6eced5
remove comment
yo-ga Feb 8, 2023
eefc182
Merge branch 'main' into feature/22045
yo-ga Feb 9, 2023
99a1f4b
remove unused comment
yo-ga Feb 10, 2023
5294546
Merge remote-tracking branch 'origin/main' into feature/22045
yo-ga Feb 10, 2023
e4a5489
Merge branch 'main' into feature/22045
yo-ga Feb 11, 2023
e727968
refactor: logging class & interface
yo-ga Feb 20, 2023
37dfe9b
Merge branch 'main' into feature/22045
Naumel Feb 27, 2023
068b134
fix: import order
yo-ga Mar 26, 2023
fa62b63
fix: yarn build fix
yo-ga Mar 26, 2023
af0a783
feat: support #23676
yo-ga Mar 26, 2023
05cd3ff
chore: update README
yo-ga Mar 26, 2023
0e970f5
fix: eslint
yo-ga Mar 26, 2023
355f431
Merge branch 'main' into feature/22045
yo-ga Apr 2, 2023
d88ba34
fix: correct merge
yo-ga Apr 5, 2023
972f538
fix: add inte test back
yo-ga Apr 5, 2023
67e6021
use alpha
yo-ga Apr 5, 2023
42663ed
fix import
yo-ga Apr 5, 2023
cc1bce3
update: integ test
yo-ga Apr 5, 2023
66c297b
Merge branch 'main' into feature/22045
yo-ga Apr 5, 2023
3ec34d6
Merge branch 'main' into feature/22045
yo-ga Apr 11, 2023
244159e
fix: resource import
yo-ga Apr 12, 2023
bc80027
Merge branch 'main' into feature/22045
yo-ga Apr 25, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ export class CodeBuildStep extends ShellStep {
*/
readonly timeout?: Duration;

/**
* Information about logs for the build project.
*
* A project can create logs in Amazon CloudWatch Logs, an S3 bucket, or both.
*
* @default - no log configuration is set
*/
readonly logging?: codebuild.LoggingOptions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment above may mean refactoring LoggingOptions instead of building a new interface for this specifically. In fact, that's probably the best path forward. We could improve this experience in general.


private _project?: codebuild.IProject;
private _partialBuildSpec?: codebuild.BuildSpec;
private readonly exportedVariables = new Set<string>();
Expand Down
14 changes: 14 additions & 0 deletions packages/@aws-cdk/pipelines/lib/codepipeline/codepipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,15 @@ export interface CodeBuildOptions {
* @default Duration.hours(1)
*/
readonly timeout?: Duration;

/**
* Information about logs for the build project.
*
* A project can create logs in Amazon CloudWatch Logs, an S3 bucket, or both.
*
* @default - no log configuration is set
*/
readonly logging?: cb.LoggingOptions;
}


Expand Down Expand Up @@ -807,6 +816,11 @@ export class CodePipeline extends PipelineBase {
buildImage: cb.LinuxBuildImage.STANDARD_5_0,
computeType: cb.ComputeType.SMALL,
},
// logging: {
// cloudWatch: {
// enabled: true,
// },
// },
};

const typeBasedCustomizations = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { CodeBuildStep } from '../codebuild-step';
import { CodeBuildOptions } from '../codepipeline';
import { ICodePipelineActionFactory, ProduceActionOptions, CodePipelineActionFactoryResult } from '../codepipeline-action-factory';
import { mergeBuildSpecs } from './buildspecs';
import { mergeLoggings } from './logging';

export interface CodeBuildFactoryProps {
/**
Expand Down Expand Up @@ -162,6 +163,7 @@ export class CodeBuildFactory implements ICodePipelineActionFactory {
subnetSelection: step.subnetSelection,
cache: step.cache,
timeout: step.timeout,
logging: step.logging,
}),
});

Expand Down Expand Up @@ -302,6 +304,7 @@ export class CodeBuildFactory implements ICodePipelineActionFactory {
buildSpec: projectBuildSpec,
role: this.props.role,
timeout: projectOptions.timeout,
logging: projectOptions.logging,
});

if (this.props.additionalDependable) {
Expand Down Expand Up @@ -429,6 +432,7 @@ export function mergeCodeBuildOptions(...opts: Array<CodeBuildOptions | undefine
vpc: b.vpc ?? a.vpc,
subnetSelection: b.subnetSelection ?? a.subnetSelection,
timeout: b.timeout ?? a.timeout,
logging: mergeLoggings(a.logging, b.logging),
cache: b.cache ?? a.cache,
};
}
Expand Down
18 changes: 18 additions & 0 deletions packages/@aws-cdk/pipelines/lib/codepipeline/private/logging.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as codebuild from '@aws-cdk/aws-codebuild';

export function mergeLoggings(a: codebuild.LoggingOptions, b?: codebuild.LoggingOptions): codebuild.LoggingOptions | undefined;
export function mergeLoggings(a: codebuild.LoggingOptions | undefined, b: codebuild.LoggingOptions): codebuild.LoggingOptions | undefined;
export function mergeLoggings(a?: codebuild.LoggingOptions, b?: codebuild.LoggingOptions): codebuild.LoggingOptions | undefined;
export function mergeLoggings(a?: codebuild.LoggingOptions, b?: codebuild.LoggingOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add spacing between these for my delicate eyeballs, please.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Granted, with my suggestions above, these might not be needed.

Copy link
Contributor Author

@yo-ga yo-ga Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any suggestion for a coding style?
It seems that other overloading functions are followed like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you know, I didn't even notice that it was an overloaded function because I reacted too quickly to the spacing issue. This is appropriate and my eyeballs will deal, lol.

const cloudWatch = b?.cloudWatch ?? a?.cloudWatch;
const s3 = b?.s3 ?? a?.s3;

if (!cloudWatch && !s3) {
return undefined;
}

return {
cloudWatch: (cloudWatch?.enabled && !cloudWatch?.logGroup) ? undefined : cloudWatch,
s3: s3,
};
}
2 changes: 2 additions & 0 deletions packages/@aws-cdk/pipelines/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"@aws-cdk/aws-events": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/aws-secretsmanager": "0.0.0",
Expand All @@ -82,6 +83,7 @@
"@aws-cdk/aws-events": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/aws-secretsmanager": "0.0.0",
Expand Down
43 changes: 43 additions & 0 deletions packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { Template, Annotations, Match } from '@aws-cdk/assertions';
import * as ccommit from '@aws-cdk/aws-codecommit';
import { Pipeline } from '@aws-cdk/aws-codepipeline';
import * as iam from '@aws-cdk/aws-iam';
import { LogGroup } from '@aws-cdk/aws-logs';
import { Bucket } from '@aws-cdk/aws-s3';
import * as sqs from '@aws-cdk/aws-sqs';
import * as cdk from '@aws-cdk/core';
import { Stack } from '@aws-cdk/core';
Expand Down Expand Up @@ -392,6 +394,47 @@ test('action name is calculated properly if it has cross-stack dependencies', ()
});
});

test('Support logging setting from codeBuildDefaults', () => {
// GIVEN
const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
const bucket = new Bucket(pipelineStack, 'MyLogsBucket');
const logGroup = new LogGroup(pipelineStack, 'LogGroup', {
logGroupName: 'TestLogGroup',
});
new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
codeBuildDefaults: {
logging: {
cloudWatch: {
logGroup: logGroup,
prefix: 'prefix',
enabled: true,
},
s3: {
encrypted: true,
bucket: bucket,
prefix: 'test',
enabled: true,
},
},
},
});

// THEN
const template = Template.fromStack(pipelineStack);
template.hasResourceProperties('AWS::CodeBuild::Project', {
LogsConfig: Match.objectLike({
CloudWatchLogs: Match.objectLike({
Status: 'ENABLED',
StreamName: 'prefix',
}),
S3Logs: Match.objectLike({
EncryptionDisabled: true,
Status: 'ENABLED',
}),
}),
});
});

interface ReuseCodePipelineStackProps extends cdk.StackProps {
reuseCrossRegionSupportStacks?: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"d99681b07ca842e41d76f0e1de1c36e98184be62476be52ea702af467851166d": {
"source": {
"path": "LoggingPipelineStack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "d99681b07ca842e41d76f0e1de1c36e98184be62476be52ea702af467851166d.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Loading