feat(codebuild): add support for local cache modes

[SanderKnape]

Fixes #1956

---

Pull Request Checklist

• Testing
  • Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
  • CLI change?: coordinate update of integration tests with team
  • cdk-init template change?: coordinated update of integration tests with team
• Docs
  • jsdocs: All public APIs documented
  • README: README and/or documentation topic updated
  • Design: For significant features, design document added to design folder
• Title and Description
  • Change type: title prefixed with fix, feat and module name in parens, which will appear in changelog
  • Title: use lower-case and doesn't end with a period
  • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
• Sensitive Modules (requires 2 PR approvers)
  • IAM Policy Document (in @aws-cdk/aws-iam)
  • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  • Grant APIs (only if not based on official documentation with a reference)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

---

BREAKING CHANGE: the CodeBuild Project cacheBucket and cacheDir properties are removed in favour of the new cache property that allows no caching (default), local caching (new) and S3 caching

[eladb]

Thanks for the contribution�!

Please update README with some details on this feature and use-cases (copy & paste from AWS docs)

[eladb]

The prefix "Local" seems unnecessary

[eladb]

Please add docstrings

[eladb]

One thing that's not clear from this is what causes a reuse of an instance? When is the local cache being utilized.

[eladb]

Any reason why this we shouldn't enable this by default?

@skinny85

[SanderKnape]

This also isn't entirely clear to me to be honest. The only hint regarding this is that "After a build, AWS CodeBuild maintains the cache for some time in anticipation of another build." (https://aws.amazon.com/about-aws/whats-new/2019/02/aws-codebuild-now-supports-local-caching/). My documentation is largely copy/pasted from the official documentation where this also isn't clearly mentioned.

[skinny85]

@Kaixiang-AWS can you weigh in here? Thanks!

[Kaixiang-AWS]

CodeBuild cannot guarantee a reuse of instance. It's a best effort. Imagine you have a build ran and cached on an instance, and then you have 2 builds start at the same time. Only one of them will utilize the cache and the other one will have to run on a new instance which doesn't have cache in it.
Anyway, I think the problem here is we might miss a modeling opportunity if we do it this way. Defining a base cache class and extending it to S3 or local caching might be a more feasible and extensible way. If there are more cache types in the future, in this way we only need to add another class which extends base class and don't need to handle the complicated input validation.

[eladb]

Thanks! Can we put this info into the README?

[SanderKnape]

Done! I also looked into the refactor that @Kaixiang-AWS mentioned. I'm currently not experienced enough with Typescript and/or the CDK to properly implement this. Can we for now merge this PR and create a new issue to refactor it? The feature would at least become available if we merge the PR.

[eladb]

I like @Kaixiang-AWS's suggestion. It's a pattern we've been using we call "union-like classes" and it fits this API perfectly. I rather we spend a bit more time now than introduce a breaking change later.

Here's a sketch:

export interface BucketCacheOptions {
  cacheDir?: string;
}

export enum LocalCacheMode {
  Source = 'LOCAL_SOURCE_CACHE',
	DockerLayer = 'LOCAL_DOCKER_LAYER_CACHE',
	Custom = 'LOCAL_CUSTOM_CACHE',
}

export abstract class Cache {
  public static none(): Cache {
    return { _toCloudFormation: () => undefined };
  }

  public static bucket(bucket: s3.IBucket, options: BucketCacheOptions): Cache {
    const cacheDir = options.cacheDir ? options.cacheDir : Aws.noValue;
    return { _toCloudFormation: () => ({
      type: 'S3',
      location: Fn.join('/', [ bucket.bucketName, cacheDir ])})
    };
  }

  public static local(...modes: LocalCacheMode[]): Cache {
    return {
      _toCloudFormation: () => ({
        type: 'LOCAL',
        modes
      })
    };
  }

  /**  @internal */
  public abstract _toCloudFormation(): codebuild.CfnProject.ProjectCacheProperty | undefined;
}

export interface CommonProjectProps {
  /**
   * @default Cache.none
   */
  readonly cache?: Cache;
}

[SanderKnape]

Looks good, I'll dive into it 👍

[SanderKnape]

Just pushed an update. Small notes:

• Instead of the BucketCacheOptions from @eladb his example, I simply use a prefix string property. This is the same terminology as used in documentation/cloudformation. And I don't see a reason for adding that property to an object - but let me know if that is desired.
• I implemented a _bind() function to allow the S3 caching strategy to grant permissions for the CodeBuild Project to add files to S3

[eladb]

Replace stack with this everywhere

[eladb]

Remove the postfix Cache from these enum members.

[eladb]

2 spaces please

[eladb]

Why do we need to export this ?

[eladb]

I don't know why it's needed really.

[eladb]

Project should implement IGrantable and then you should be able to just do bucket.grantReadWrite(project).

[eladb]

Sorry but please use the the Options API because it is future-compatible. If we end up needing more options, we'll have to break this API. It's also more readable when it's used: Cache.bucket(myBucket, 'foo') as oppose to Cache.bucket(myBucket, { prefix: 'foo' }).

[eladb]

Style: call ._toCloudFormation once, so usually we do this:

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

// ...

  cache: cache._toCloudFormation();

[eladb]

It should be codebuild.Cache

[eladb]

Show an example with two modes to demonstrate that this is a list

[SanderKnape]

Hi @eladb , I've incorporated all your changes - including the last ones about the docs. Can you take another peek?