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(context-providers) Enable arguments to be passed as property object #823

Merged
merged 13 commits into from
Oct 22, 2018

Conversation

moofish32
Copy link
Contributor

This is a first draft at updates originally from #425

I am attempting to apply some types to context providers where there were just arrays.
The arrays were good because of ordering, but I've attempted to preserve that with a sort.

I would like to implement a VPC Provider because I find imports for VPCs painful. In addition
to solving a customer problems VPC Provider will stress the ability to return a complex type.

Is this design in the right direction?

TODO - fix commit message this is a BREAKING Change

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

@moofish32
Copy link
Contributor Author

I'll go back and remove the package lock diffs, my apologies.

@moofish32
Copy link
Contributor Author

I need a hint on where this integ test failure is located.

@aws-cdk/aws-autoscaling: A reserved context key ('context.aws:') key was found in /home/travis/build/awslabs/aws-cdk/packages/@aws-cdk/aws-autoscaling/test/cdk.json, it might cause surprising behavior and should be removed.
@aws-cdk/aws-autoscaling: A reserved context key ('context.aws:') key was found in /home/travis/build/awslabs/aws-cdk/packages/@aws-cdk/aws-autoscaling/test/cdk.json, it might cause surprising behavior and should be removed.
@aws-cdk/aws-autoscaling: Need to perform AWS calls for account 12345678, but no credentials found. Tried: default credentials.
@aws-cdk/aws-autoscaling: Error: Command exited with status 1
@aws-cdk/aws-autoscaling:     at exec (/home/travis/build/awslabs/aws-cdk/tools/cdk-integ-tools/lib/integ-helpers.js:102:15)
@aws-cdk/aws-autoscaling:     at IntegrationTest.invoke (/home/travis/build/awslabs/aws-cdk/tools/cdk-integ-tools/lib/integ-helpers.js:50:20)
@aws-cdk/aws-autoscaling:     at <anonymous>
@aws-cdk/aws-autoscaling: Error: cdk-integ-assert exited with error code 1

I can't seem to trace this back through the integ helper.

@moofish32 moofish32 force-pushed the context-provider-fix branch from baf96fa to 1e07618 Compare October 5, 2018 13:12
@moofish32 moofish32 changed the title Context Provider Improvements feat(context-providers) Enable arguments to be passed as property object Oct 5, 2018

let region = process.env.AWS_REGION || process.env.AMAZON_REGION ||
process.env.AWS_DEFAULT_REGION || process.env.AMAZON_DEFAULT_REGION;
process.env.AWS_DEFAULT_REGION || process.env.AMAZON_DEFAULT_REGION;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure why this is autoformatting this way, but I'll fix this.

@@ -24,10 +24,17 @@ export type CXRequest = ListStacksRequest | SynthesizeRequest;
* Represents a missing piece of context.
* (should have been an interface, but jsii still doesn't have support for structs).
*/
export interface MissingContext {
export interface ContextProviderProps {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

after reading this again (a few days later). I think I'm ready to switch this back to MissingContext.

export interface MissingContext {
  provider: string;
  props: {
    account: string;
    region: string;
    [key: string]: string;
  };
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please.

@moofish32 moofish32 force-pushed the context-provider-fix branch from 1e07618 to a9110d1 Compare October 7, 2018 13:51
* Add Hosted Zone Provider
* Enabling complex filter types for context provider keys
@moofish32 moofish32 force-pushed the context-provider-fix branch from a9110d1 to f6c1ec2 Compare October 7, 2018 23:41
@moofish32
Copy link
Contributor Author

@rix0rrr @eladb looking for a little feedback on the direction here.

My end goal is to enable a significantly improved capability for VPC Import. Right now you have to know the vpcIds and all the subnetIds in order to use a VpcPlacementStrategy.

Instead of having do something like

export class MyStack extends cdk.Stack {
    constructor(parent: cdk.App, name: string, props?: cdk.StackProps) {
        super(parent, name, props);
        const vpc: ec2.VpcNetworkRef = ec2.VpcNetwork.import(this, 'MyVPC', {
            vpcId: new ec2.VPCId(props.vpcId),
           availabilityZones: props.availabilityZones,
           publicSubnetIds: props.subnetIds,
           isolatedSubnetIds: [],
        });
       // create VpcPlacementStrategy
       // create my asg or other resources (e.g. lambda, eks, ecs etc)
       // I would rather 
      const vpc: ec2.VpcNetworkRef = ec2.VpcProvider(this, { vpcId: 'vpc-xxxxxxxx' });
      // or
      const vpc2: ec2.VpcNetworkRef = ec2.VpcProvider(this, { cidrBlock: '192.168.0.0/16'});
      // essentially enable the filters you see here: 
      // https://docs.aws.amazon.com/cli/latest/reference/ec2/describe-vpcs.html
   }
}

In addition to Vpc we would also enable the AMI look up feature that would not rely on the ssm parameter method we have now. Obviously the type checking and casting behavior would all be hidden from the user, but we have to use some any values to enable this code behind the scenes.

Most important question I have is: Is this a good feature to spend time implementing and am I headed in the right direction?

@eladb
Copy link
Contributor

eladb commented Oct 8, 2018

@rix0rrr can you please take a look at this?

@@ -24,10 +24,17 @@ export type CXRequest = ListStacksRequest | SynthesizeRequest;
* Represents a missing piece of context.
* (should have been an interface, but jsii still doesn't have support for structs).
*/
export interface MissingContext {
export interface ContextProviderProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please.

provider: string;
scope: string[];
args: string[];
account: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to use a dict anyway, then shouldn't account and region also just go into that dict?

}

public get key(): string {
const account = this.account;
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I'd prefer it if you put all the different string fragments into an array and called .join(':') on them. Simplifies this method a bunch. So something like this:

const parts = [this.provider, this.account, this.region].concat(this.propsToArray());
return parts.join(':');

In fact, I wonder if we shouldn't just add region and account to the props object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we move account and region into props, do we care if those are just sorted like all other props or should I pull them to the front and filter them out? Personally, I am fine with them sorted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes me too.


if (account == null || region == null) {
return undefined;
private objectToString(obj: any): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

The function name lies. It doesn't return a string at all.

Also, since it doesn't access this. at all, no need for it to be a member/class function. You can move it to a free function.

switch (typeof obj[key]) {
case 'object': {
const childObjStrs = this.objectToString(obj[key]);
const qualifiedChildStr = childObjStrs.map( child => (`${key}${child}`)).join(':');
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like this needs a separator: ${key}.${child} or something.

Also, I'd be perfectly happy disallow this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok - so I started by blocking this. However, I have found it very useful to allow tags to be used in providers (from other frameworks that provide similar function). Both AMIs and VPCs are great examples where tags will be useful. So I would prefer to add the separator and keep the use case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool.


constructor(private context: Construct) {
constructor(private context: Construct, provider: string, props: {[key: string]: any} = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you declare the arguments as:

constructor(..., private readonly provider: string, private readonly props: {[key: string]: any} = {}) {
}

You don't have to declare the member variables above and don't have to assign them either, that happens automatically.

import { Mode, SDK } from './api';
import { debug } from './logging';
import { Settings } from './settings';

export interface ContextProviderPlugin {
getValue(scope: string[], args: string[]): Promise<any>;
getValue(filter: cxapi.ContextProviderProps): Promise<any>;
Copy link
Contributor

Choose a reason for hiding this comment

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

"filter"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about provider: cxapi.MissingContext instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

After moving account and region into the dictdionary, is there a reason to pass the entire ContextProviderProps object anymore?

We can just do:

getValue(args: {[key: string]: any})

right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh - a little different than what I first thought, but let me try that.

const props: cxapi.HostedZoneProviderProps = filter.props;
const domainName = props.domainName;
const privateZone: boolean = !!props.privateZone;
const vpcId =
Copy link
Contributor

Choose a reason for hiding this comment

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

??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah -- this looks like a coding on a plane bonus bug ✈️ 🐛

Should we expand the scope of this PR and find a away to integration test these?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not opposed to it, but I have no clever ideas on how to do it that wouldn't take a long time to build and/or make tests complicated to run.

For example, if we're going to test the actual service call, we'd need AWS credentials and network access on every build--something we can (mostly) avoid now.

You might potentially consider mocking/stubbing the external APIs, OR having a test that you run manually, and a signature (like a hash of the file) to assert that you manually ran the test on the latest version of the source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't fully looked at this yet, but perhaps you can tell me I'm walking into a big thing.

My thought was to keep this as simple as possible. Enable cdk-integ to be able to run more than one stack. Create resources in the first stack and use providers to find those resources in the second stack. Then sanitize the cdk.json file with the appropriate region/account information such that AWS API credentials are not required after the developer runs the integration test locally.

This definitely isn't simple, but would that work and not be a nightmare to implement?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's definitely a bit out of the mold of our current integ tests.

  • You can't comfortable write this test in aws-cdk package (I don't think), it has to go in one of the library packages. Which one?
  • How are we going to specify the order between multiple stacks in cdk-integ? We have explicitly punted on that because there's no way to automatically infer it, and no place to specify it.
  • We don't check in cdk.json usually, and in fact cdk-integ automatically deletes it. To be able to run an offline test we should keep that file; but preferably not all files in all cases, so only some instances of that file...

return candidateZones[0];
}

private filterZones(r53: AWS.Route53, zones: AWS.Route53.HostedZone[], privateZone: boolean, vpcId: string | undefined): AWS.Route53.HostedZone[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

async

if (vpcId) {
const vpcZones: AWS.Route53.HostedZone[] = [];
for (const zone of candidates) {
r53.getHostedZone({Id: zone.Id}, (err, data) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not going to do what you want, because it's going to be asynchronous.

Just add an await.

  * Add Hosted Zone Provider
  * Add ability to pass dictionary to MissingContext
@moofish32
Copy link
Contributor Author

moofish32 commented Oct 10, 2018

It's definitely a bit out of the mold of our current integ tests.

You can't comfortable write this test in aws-cdk package (I don't think), it has to go in one of the library packages. Which one?
How are we going to specify the order between multiple stacks in cdk-integ? We have explicitly punted on that because there's no way to automatically infer it, and no place to specify it.
We don't check in cdk.json usually, and in fact cdk-integ automatically deletes it. To be able to run an offline test we should keep that file; but preferably not all files in all cases, so only some instances of that file...

I understand what you are saying. I did think the integ tests would need a new type provider.ssm.ts instead of integ.<L2>.ts in the test folder. However, looking at cdk-integ this is bigger than I can do in this PR. The other question is does the team think this is worth it?

Should we move the providers into the @aws-cdk packages in this?
AvailabilityZone -> @aws-cdk/aws-ec2
SSM -> @aws-cdk/aws-ssm
HostedZone -> @aws-cdk/aws-route53

I'll need to do some more manual testing as well.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 10, 2018

Should we move the providers into the @aws-cdk packages in this?
No, they're definitely part of the toolkit.

For now, we don't really have a better answer than manual testing. This is admittedly not great and we're aware of this and thinking about it, but this is the state of the world.

public get key(): string {
const propStrings: string[] = propsToArray({
...this.props,
...{account: this.account, region: this.region},
Copy link
Contributor

Choose a reason for hiding this comment

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

Literals don't need a splat. Could just be:

{
   ...this.props,
   account: this.account,
   region: this.region
}

this.stack.reportMissingContext(key, { provider, scope, args });
this.stack.reportMissingContext(this.key, {
provider: this.provider,
props: { ...this.props, ...{region: this.region, account: this.account} },
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid this duplication, I'm thinking it might even be worth adding a private getter for the "full props", or embed {account, region} into the object at creation time.

scope: string[];
args: string[];
props: {
account: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe account and region are still potentially undefined (for providers that don't require them--even though we don't have any right now), so add a ? to them.

};
}

export interface HostedZoneProviderProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this should be defined next to the HostedZoneProvider class.

I don't feel comfortable using one struct for both the API properties and defining the wire protocol, because they're distinct things. It's probably okay to have an implicit protocol on the wire (or define 2 structs for 2 distinct purposes).

if (vpcId) {
const vpcZones: AWS.Route53.HostedZone[] = [];
for (const zone of candidates) {
await r53.getHostedZone({Id: zone.Id}, (err, data) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I should have been more specific here. I'm not sure whether this works or not, but it sure looks weird to me. I actually meant doing the following:

const data = await r53.getHostedZone({ Id: zone. Id }).promise();
// No need to check for err, just look at data afterwards

It's the same pattern as for the listHostedZonesByName call you're doing in the other function, and basically the way all SDK calls should look.

The .promise() at the end is a weird convention that the SDK has in there (they could have made it so it works without it, but they didn't). It makes it possible to await this API call.

for (const zone of candidates) {
await r53.getHostedZone({Id: zone.Id}, (err, data) => {
if (err) {
throw new Error(err.message);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it matters because you're going to be rewriting this call sequyence anyway, but throwing an Error in an async callback is almost never what you want to be doing.

In an async callback your call stack is gone: you're basically a couple of stack frames deep in an event loop, and throwing an error is basically only going to abort your callback and the rest of the failure is lost. To pull the thread all the way through, you'll generally call another callback with the error (boooh) or report the error via a promise (yay).

But the await syntax takes care of all of that for you.

constructor(
private readonly context: Construct,
private readonly provider: string,
private readonly props: {[key: string]: any} = {}) {
this.stack = Stack.find(context);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we just go like this in the constructor:

this.props.account = this.stack.env.account;
this.props.region = this.stack.env.region;

And then in all other places we just use this.props.

// if scope is undefined, this is probably a test mode, so we just
// return the default value
if (!scope) {
this.context.addError(formatMissingScopeError(provider, args));
if (!this.account || !this.region) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Interestingly, this now means something else. Because previously there was a distinction between "not having a scope" and "not having an account/region", and now there isn't anymore.

This only comes into play if we have context providers that don't need both an account and a region, which we don't have right now anyway so I'm okay with this change and we can always fix it once we do. But it's something to consider.

@moofish32
Copy link
Contributor Author

moofish32 commented Oct 15, 2018

@rix0rrr -- I used this to test the provider:

#!/usr/bin/env node
import r53 = require('@aws-cdk/aws-route53');
import ec2 = require('@aws-cdk/aws-ec2');
import cdk = require('@aws-cdk/cdk');

class TestLatestCdkStack extends cdk.Stack {
  constructor(parent: cdk.App, name: string, props?: cdk.StackProps) {
    super(parent, name, props);
    const vpc = new ec2.VpcNetwork(this, 'MyVpc', {
      subnetConfiguration: [
        {
          cidrMask: 24,
          name: 'Public',
          subnetType: ec2.SubnetType.Public,
        }
      ]
    });

    new r53.PrivateHostedZone(this, 'PrivateZone', {
      zoneName: 'vpc.internal',
      vpc,
    });

    new r53.PublicHostedZone(this, 'PublicZone', {
      zoneName: 'tdfa.xyz',
      comment: 'CDK created',
    });

  }
}

class TestLatestHosted extends cdk.Stack {
  constructor(parent: cdk.App, name: string, props?: cdk.StackProps) {
    super(parent, name, props);
    new r53.PublicHostedZone(this, 'PublicZone', {
      zoneName: 'tdfi.xyz',
      comment: 'CDK created',
    });
    const hostedZoneProps: r53.HostedZoneRefProps = new r53.HostedZoneProvider( this, { domainName: 'tdfa.xyz', }).findHostedZone();
    const zone = r53.HostedZoneRef.import(this, 'Imported', hostedZoneProps);
    new cdk.Output(this, 'Test', {
      value: zone.hostedZoneId,
    });
    const privateZoneProps: r53.HostedZoneRefProps = new r53.HostedZoneProvider( this, {
      domainName: 'vpc.internal',
      privateZone: true,
      vpcId: '<your vpc id>', // I should automate this
    }).findHostedZone();
    const privateZone = r53.HostedZoneRef.import(this, 'ImportedPrivate', privateZoneProps);
    new r53.TXTRecord(zone, 'txt', {
      recordName: 'mytxt',
      recordValue: 'special text message for validation',
    });
    new r53.TXTRecord(privateZone, 'privateTxt', {
      recordName: 'mytxt',
      recordValue: 'special text message for validation',
    });
  }

}
const app = new cdk.App();

new TestLatestHosted(app, 'TestHosted');
new TestLatestCdkStack(app, 'TestLatestCdkStack');

app.run();

I moved the provider (non AWS SDK side -- do we have a name for the split?) into the route53 module because I wanted to return the data structure for r53.HostedZoneRef.import( ... ). Sorry to make some changes this late in the process (specifically I've changed the function name to findHostedZone). I'm not sure if we should make this a generic name or a specific name per provider?

if (!stack) {
throw new Error(`${providerDescription}: construct must be in a stack`);
}
public getStringListValue(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can dry this function up by passing a function here. I don't think I can use generics here. I suppose I could extract this to a non exported function and pass everything into it, but I think just passing a validateType(value: any): boolean{} will probably do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually that is also blocked by JSII ... I'm not sure we can really DRY this up much then?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine as-is.

/**
* Return the hosted zone meeting the filter
*/
public findHostedZone(): HostedZoneRefProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that you have this, might even make sense to add a convenience import function on this class as well:

  public findAndImport(parent: cdk.Construct, id: string): HostedZoneRef {
    return HostedZoneRef.import(parent, id, this.findHostedZone());
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea 👍 I will add.

@@ -4,6 +4,11 @@ import { Construct } from './core/construct';
const AVAILABILITY_ZONES_PROVIDER = 'availability-zones';
const SSM_PARAMETER_PROVIDER = 'ssm';

export interface ContextProviderProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't believe I didn't trip over this before. I don't think this translates into JSII. You're going to have to define it as

Suggested change
export interface ContextProviderProps {
type ContextProviderProps = {[key: string]: any};

That means we can't enforce the presence of account and region, but those will have to be runtime checks. And it's still feasible to have context providers without those parameters.

if (!stack) {
throw new Error(`${providerDescription}: construct must be in a stack`);
}
public getStringListValue(
Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine as-is.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 17, 2018

Looks good! Final touches and then we can get it in.

@moofish32
Copy link
Contributor Author

@rix0rrr -- I believe I have implemented the final changes, trying to fix that conflict with the last push. Anything else you see from me? I believe this is breaking, but open to trying to make this more backwards compatible if we need to.

Copy link
Contributor Author

@moofish32 moofish32 left a comment

Choose a reason for hiding this comment

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

Found a couple of issues to resolve

}
}

export interface SSMParameterProviderProps {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need docs here

packages/@aws-cdk/cdk/lib/context.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/contextplugins.ts Show resolved Hide resolved
@rix0rrr rix0rrr merged commit 1626c37 into aws:master Oct 22, 2018
rix0rrr pushed a commit that referenced this pull request Oct 26, 2018
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade
your installation the CDK Toolkit to the matching version:

```shell
$ npm i -g aws-cdk
$ cdk --version
0.14.0 (build ...)
```

Bug Fixes
=========

* remove CloudFormation property renames ([#973](#973)) ([3f86603](3f86603)), closes [#852](#852)
* **aws-ec2:** fix retention of all egress traffic rule ([#998](#998)) ([b9d5b43](b9d5b43)), closes [#987](#987)
* **aws-s3-deployment:** avoid deletion during update using physical ids ([#1006](#1006)) ([bca99c6](bca99c6)), closes [#981](#981) [#981](#981)
* **cloudformation-diff:** ignore changes to DependsOn ([#1005](#1005)) ([3605f9c](3605f9c)), closes [#274](#274)
* **cloudformation-diff:** track replacements ([#1003](#1003)) ([a83ac5f](a83ac5f)), closes [#1001](#1001)
* **docs:** fix EC2 readme for "natgatway" configuration ([#994](#994)) ([0b1e7cc](0b1e7cc))
* **docs:** updates to contribution guide ([#997](#997)) ([b42e742](b42e742))
* **iam:** Merge multiple principals correctly ([#983](#983)) ([3fc5c8c](3fc5c8c)), closes [#924](#924) [#916](#916) [#958](#958)

Features
=========

* add construct library for Application AutoScaling ([#933](#933)) ([7861c6f](7861c6f)), closes [#856](#856) [#861](#861) [#640](#640) [#644](#644)
* add HostedZone context provider ([#823](#823)) ([1626c37](1626c37))
* **assert:** haveResource lists failing properties ([#1016](#1016)) ([7f6f3fd](7f6f3fd))
* **aws-cdk:** add CDK app version negotiation ([#988](#988)) ([db4e718](db4e718)), closes [#891](#891)
* **aws-codebuild:** Introduce a CodePipeline test Action. ([#873](#873)) ([770f9aa](770f9aa))
* **aws-sqs:** Add grantXxx() methods ([#1004](#1004)) ([8c90350](8c90350))
* **core:** Pre-concatenate Fn::Join ([#967](#967)) ([33c32a8](33c32a8)), closes [#916](#916) [#958](#958)

BREAKING CHANGES
=========

* DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling.
* CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names.
* You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
@rix0rrr rix0rrr mentioned this pull request Oct 26, 2018
rix0rrr added a commit that referenced this pull request Oct 26, 2018
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade
your installation the CDK Toolkit to the matching version:

```shell
$ npm i -g aws-cdk
$ cdk --version
0.14.0 (build ...)
```

Bug Fixes
=========

* remove CloudFormation property renames ([#973](#973)) ([3f86603](3f86603)), closes [#852](#852)
* **aws-ec2:** fix retention of all egress traffic rule ([#998](#998)) ([b9d5b43](b9d5b43)), closes [#987](#987)
* **aws-s3-deployment:** avoid deletion during update using physical ids ([#1006](#1006)) ([bca99c6](bca99c6)), closes [#981](#981) [#981](#981)
* **cloudformation-diff:** ignore changes to DependsOn ([#1005](#1005)) ([3605f9c](3605f9c)), closes [#274](#274)
* **cloudformation-diff:** track replacements ([#1003](#1003)) ([a83ac5f](a83ac5f)), closes [#1001](#1001)
* **docs:** fix EC2 readme for "natgatway" configuration ([#994](#994)) ([0b1e7cc](0b1e7cc))
* **docs:** updates to contribution guide ([#997](#997)) ([b42e742](b42e742))
* **iam:** Merge multiple principals correctly ([#983](#983)) ([3fc5c8c](3fc5c8c)), closes [#924](#924) [#916](#916) [#958](#958)

Features
=========

* add construct library for Application AutoScaling ([#933](#933)) ([7861c6f](7861c6f)), closes [#856](#856) [#861](#861) [#640](#640) [#644](#644)
* add HostedZone context provider ([#823](#823)) ([1626c37](1626c37))
* **assert:** haveResource lists failing properties ([#1016](#1016)) ([7f6f3fd](7f6f3fd))
* **aws-cdk:** add CDK app version negotiation ([#988](#988)) ([db4e718](db4e718)), closes [#891](#891)
* **aws-codebuild:** Introduce a CodePipeline test Action. ([#873](#873)) ([770f9aa](770f9aa))
* **aws-sqs:** Add grantXxx() methods ([#1004](#1004)) ([8c90350](8c90350))
* **core:** Pre-concatenate Fn::Join ([#967](#967)) ([33c32a8](33c32a8)), closes [#916](#916) [#958](#958)

BREAKING CHANGES
=========

* DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling.
* CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names.
* You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
jonparker pushed a commit to jonparker/aws-cdk that referenced this pull request Oct 29, 2018
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade
your installation the CDK Toolkit to the matching version:

```shell
$ npm i -g aws-cdk
$ cdk --version
0.14.0 (build ...)
```

Bug Fixes
=========

* remove CloudFormation property renames ([aws#973](aws#973)) ([3f86603](aws@3f86603)), closes [aws#852](aws#852)
* **aws-ec2:** fix retention of all egress traffic rule ([aws#998](aws#998)) ([b9d5b43](aws@b9d5b43)), closes [aws#987](aws#987)
* **aws-s3-deployment:** avoid deletion during update using physical ids ([aws#1006](aws#1006)) ([bca99c6](aws@bca99c6)), closes [aws#981](aws#981) [aws#981](aws#981)
* **cloudformation-diff:** ignore changes to DependsOn ([aws#1005](aws#1005)) ([3605f9c](aws@3605f9c)), closes [aws#274](aws#274)
* **cloudformation-diff:** track replacements ([aws#1003](aws#1003)) ([a83ac5f](aws@a83ac5f)), closes [aws#1001](aws#1001)
* **docs:** fix EC2 readme for "natgatway" configuration ([aws#994](aws#994)) ([0b1e7cc](aws@0b1e7cc))
* **docs:** updates to contribution guide ([aws#997](aws#997)) ([b42e742](aws@b42e742))
* **iam:** Merge multiple principals correctly ([aws#983](aws#983)) ([3fc5c8c](aws@3fc5c8c)), closes [aws#924](aws#924) [aws#916](aws#916) [aws#958](aws#958)

Features
=========

* add construct library for Application AutoScaling ([aws#933](aws#933)) ([7861c6f](aws@7861c6f)), closes [aws#856](aws#856) [aws#861](aws#861) [aws#640](aws#640) [aws#644](aws#644)
* add HostedZone context provider ([aws#823](aws#823)) ([1626c37](aws@1626c37))
* **assert:** haveResource lists failing properties ([aws#1016](aws#1016)) ([7f6f3fd](aws@7f6f3fd))
* **aws-cdk:** add CDK app version negotiation ([aws#988](aws#988)) ([db4e718](aws@db4e718)), closes [aws#891](aws#891)
* **aws-codebuild:** Introduce a CodePipeline test Action. ([aws#873](aws#873)) ([770f9aa](aws@770f9aa))
* **aws-sqs:** Add grantXxx() methods ([aws#1004](aws#1004)) ([8c90350](aws@8c90350))
* **core:** Pre-concatenate Fn::Join ([aws#967](aws#967)) ([33c32a8](aws@33c32a8)), closes [aws#916](aws#916) [aws#958](aws#958)

BREAKING CHANGES
=========

* DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling.
* CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names.
* You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants