Skip to content

Commit

Permalink
fix(codebuild): setting Cache.none() renders nothing in the template
Browse files Browse the repository at this point in the history 
When implementing caching in CodeBuild,
we made the default cache `Cache.none()`,
and, for that reason, do not render anything in the template for that type of cache.
However, that does not work well with the CodeBuild API,
which interprets the lack of a property as the signal to leave it unchanged.
Which means it's not possible currently to disable caching on a Project once it has been enabled once.

Fix this by differentiating between the case of "no Cache has been provided",
and "the none() Cache has been provided".

Closes #18165
  • Loading branch information
@skinny85
skinny85 committed Dec 27, 2021
1 parent a15250e commit 3d91dd1
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 86 deletions.
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-codebuild/lib/cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ export enum LocalCacheMode {
*/
export abstract class Cache {
public static none(): Cache {
return { _toCloudFormation: () => undefined, _bind: () => { return; } };
return {
_toCloudFormation(): CfnProject.ProjectCacheProperty | undefined {
return { type: 'NO_CACHE' };
},
_bind(): void {
},
};
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,10 +1040,10 @@ export class Project extends ProjectBase {
: new NoArtifacts());
const artifactsConfig = artifacts.bind(this, this);

const cache = props.cache || Cache.none();
const cache = props.cache;

// give the caching strategy the option to grant permissions to any required resources
cache._bind(this);
cache?._bind(this);

// Inject download commands for asset if requested
const environmentVariables = props.environmentVariables || {};
Expand Down Expand Up @@ -1085,7 +1085,7 @@ export class Project extends ProjectBase {
// empty will not remove existing encryptionKeys during an update (ref. t/D17810523)
encryptionKey: Lazy.string({ produce: () => this._encryptionKey ? this._encryptionKey.keyArn : 'alias/aws/s3' }),
badgeEnabled: props.badge,
cache: cache._toCloudFormation(),
cache: cache?._toCloudFormation(),
name: this.physicalName,
timeoutInMinutes: props.timeout && props.timeout.toMinutes(),
queuedTimeoutInMinutes: props.queuedTimeout && props.queuedTimeout.toMinutes(),
Expand Down Expand Up @@ -2125,4 +2125,4 @@ export enum ProjectNotificationEvents {

function isBindableBuildImage(x: unknown): x is IBindableBuildImage {
return typeof x === 'object' && !!x && !!(x as any).bind;
}
}
182 changes: 101 additions & 81 deletions packages/@aws-cdk/aws-codebuild/test/project.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { objectLike, ResourcePart, arrayWith } from '@aws-cdk/assert-internal';
import { ABSENT, objectLike, ResourcePart, arrayWith } from '@aws-cdk/assert-internal';
import '@aws-cdk/assert-internal/jest';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
Expand Down Expand Up @@ -292,103 +292,123 @@ describe('BitBucket source', () => {
});
});

test('project with s3 cache bucket', () => {
// GIVEN
const stack = new cdk.Stack();
describe('caching', () => {
test('using Cache.none() results in NO_CACHE in the template', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'SourceBucket'),
path: 'path',
}),
cache: codebuild.Cache.bucket(new s3.Bucket(stack, 'Bucket'), {
prefix: 'cache-prefix',
}),
// WHEN
new codebuild.PipelineProject(stack, 'Project', {
cache: codebuild.Cache.none(),
});

// THEN
expect(stack).toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {
Type: 'NO_CACHE',
Location: ABSENT,
},
});
});

// THEN
expect(stack).toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {
Type: 'S3',
Location: {
'Fn::Join': [
'/',
[
{
'Ref': 'Bucket83908E77',
},
'cache-prefix',
test('project with s3 cache bucket', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'SourceBucket'),
path: 'path',
}),
cache: codebuild.Cache.bucket(new s3.Bucket(stack, 'Bucket'), {
prefix: 'cache-prefix',
}),
});

// THEN
expect(stack).toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {
Type: 'S3',
Location: {
'Fn::Join': [
'/',
[
{
'Ref': 'Bucket83908E77',
},
'cache-prefix',
],
],
],
},
},
},
});
});
});

test('s3 codebuild project with sourceVersion', () => {
// GIVEN
const stack = new cdk.Stack();
test('s3 codebuild project with sourceVersion', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
version: 's3version',
}),
cache: codebuild.Cache.local(codebuild.LocalCacheMode.CUSTOM, codebuild.LocalCacheMode.DOCKER_LAYER,
codebuild.LocalCacheMode.SOURCE),
});
// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
version: 's3version',
}),
cache: codebuild.Cache.local(codebuild.LocalCacheMode.CUSTOM, codebuild.LocalCacheMode.DOCKER_LAYER,
codebuild.LocalCacheMode.SOURCE),
});

// THEN
expect(stack).toHaveResource('AWS::CodeBuild::Project', {
SourceVersion: 's3version',
// THEN
expect(stack).toHaveResource('AWS::CodeBuild::Project', {
SourceVersion: 's3version',
});
});
});

test('project with local cache modes', () => {
// GIVEN
const stack = new cdk.Stack();
test('project with local cache modes', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
}),
cache: codebuild.Cache.local(codebuild.LocalCacheMode.CUSTOM, codebuild.LocalCacheMode.DOCKER_LAYER,
codebuild.LocalCacheMode.SOURCE),
});
// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
}),
cache: codebuild.Cache.local(codebuild.LocalCacheMode.CUSTOM, codebuild.LocalCacheMode.DOCKER_LAYER,
codebuild.LocalCacheMode.SOURCE),
});

// THEN
expect(stack).toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {
Type: 'LOCAL',
Modes: [
'LOCAL_CUSTOM_CACHE',
'LOCAL_DOCKER_LAYER_CACHE',
'LOCAL_SOURCE_CACHE',
],
},
// THEN
expect(stack).toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {
Type: 'LOCAL',
Modes: [
'LOCAL_CUSTOM_CACHE',
'LOCAL_DOCKER_LAYER_CACHE',
'LOCAL_SOURCE_CACHE',
],
},
});
});
});

test('project by default has no cache modes', () => {
// GIVEN
const stack = new cdk.Stack();
test('project by default has no cache modes', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
}),
});
// WHEN
new codebuild.Project(stack, 'Project', {
source: codebuild.Source.s3({
bucket: new s3.Bucket(stack, 'Bucket'),
path: 'path',
}),
});

// THEN
expect(stack).not.toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {},
// THEN
expect(stack).not.toHaveResourceLike('AWS::CodeBuild::Project', {
Cache: {},
});
});
});

Expand Down

0 comments on commit 3d91dd1

Please sign in to comment.