Skip to content

Commit

Permalink
fix(apigateway): rest api deployment does not depend on authorizers (#…
Browse files Browse the repository at this point in the history
…23215)

----

Closes #16554 (formerly #22808)

The Rest API deployment needs to depend on all authorizers attached to the API, so there is a new deployment if any of the authorizers change. This is similar to what is already done for `Method`s. Includes trivial change to integ test.

Note - Because this will change the logical ID of existing deployments, this is technically a breaking change, so I am not sure if it requires a feature flag.

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
cecheta committed Feb 15, 2023
1 parent 4d29610 commit 12e13c1
Show file tree
Hide file tree
Showing 37 changed files with 1,204 additions and 173 deletions.
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-apigateway/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,18 @@ to allow users revert the stage to an old deployment manually.
[Deployment]: https://docs.aws.amazon.com/apigateway/api-reference/resource/deployment/
[Stage]: https://docs.aws.amazon.com/apigateway/api-reference/resource/stage/

In order to also create a new deployment when changes are made to any authorizer attached to the API,
the `@aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId` [feature flag](https://docs.aws.amazon.com/cdk/v2/guide/featureflags.html) can be enabled. This can be set
in the `cdk.json` file.

```json
{
"context": {
"@aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId": true
}
}
```

## Custom Domains

To associate an API with a custom domain, use the `domainName` configuration when
Expand Down
26 changes: 22 additions & 4 deletions packages/@aws-cdk/aws-apigateway/lib/authorizers/cognito.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as cognito from '@aws-cdk/aws-cognito';
import { Duration, Lazy, Names, Stack } from '@aws-cdk/core';
import { Duration, FeatureFlags, Lazy, Names, Stack } from '@aws-cdk/core';
import { APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID } from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { CfnAuthorizer } from '../apigateway.generated';
import { CfnAuthorizer, CfnAuthorizerProps } from '../apigateway.generated';
import { Authorizer, IAuthorizer } from '../authorizer';
import { AuthorizationType } from '../method';
import { IRestApi } from '../restapi';
Expand Down Expand Up @@ -64,18 +65,25 @@ export class CognitoUserPoolsAuthorizer extends Authorizer implements IAuthorize

private restApiId?: string;

private readonly authorizerProps: CfnAuthorizerProps;

constructor(scope: Construct, id: string, props: CognitoUserPoolsAuthorizerProps) {
super(scope, id);

const restApiId = this.lazyRestApiId();
const resource = new CfnAuthorizer(this, 'Resource', {

const authorizerProps = {
name: props.authorizerName ?? Names.uniqueId(this),
restApiId,
type: 'COGNITO_USER_POOLS',
providerArns: props.cognitoUserPools.map(userPool => userPool.userPoolArn),
authorizerResultTtlInSeconds: props.resultsCacheTtl?.toSeconds(),
identitySource: props.identitySource || 'method.request.header.Authorization',
});
};

this.authorizerProps = authorizerProps;

const resource = new CfnAuthorizer(this, 'Resource', authorizerProps);

this.authorizerId = resource.ref;
this.authorizerArn = Stack.of(this).formatArn({
Expand All @@ -96,6 +104,16 @@ export class CognitoUserPoolsAuthorizer extends Authorizer implements IAuthorize
}

this.restApiId = restApi.restApiId;

const addToLogicalId = FeatureFlags.of(this).isEnabled(APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID);

const deployment = restApi.latestDeployment;
if (deployment && addToLogicalId) {
deployment.node.addDependency(this);
deployment.addToLogicalId({
authorizer: this.authorizerProps,
});
}
}

/**
Expand Down
51 changes: 45 additions & 6 deletions packages/@aws-cdk/aws-apigateway/lib/authorizers/lambda.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { Arn, ArnFormat, Duration, Lazy, Names, Stack } from '@aws-cdk/core';
import { Arn, ArnFormat, Duration, FeatureFlags, Lazy, Names, Stack } from '@aws-cdk/core';
import { APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID } from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { CfnAuthorizer } from '../apigateway.generated';
import { CfnAuthorizer, CfnAuthorizerProps } from '../apigateway.generated';
import { Authorizer, IAuthorizer } from '../authorizer';
import { IRestApi } from '../restapi';

Expand Down Expand Up @@ -69,6 +70,8 @@ abstract class LambdaAuthorizer extends Authorizer implements IAuthorizer {

protected restApiId?: string;

protected abstract readonly authorizerProps: CfnAuthorizerProps;

protected constructor(scope: Construct, id: string, props: LambdaAuthorizerProps) {
super(scope, id);

Expand All @@ -90,6 +93,28 @@ abstract class LambdaAuthorizer extends Authorizer implements IAuthorizer {
}

this.restApiId = restApi.restApiId;

const deployment = restApi.latestDeployment;
const addToLogicalId = FeatureFlags.of(this).isEnabled(APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID);

if (deployment && addToLogicalId) {
let functionName;

if (this.handler instanceof lambda.Function) {
// if not imported, attempt to get the function name, which
// may be a token
functionName = (this.handler.node.defaultChild as lambda.CfnFunction).functionName;
} else {
// if imported, the function name will be a token
functionName = this.handler.functionName;
}

deployment.node.addDependency(this);
deployment.addToLogicalId({
authorizer: this.authorizerProps,
authorizerToken: functionName,
});
}
}

/**
Expand Down Expand Up @@ -163,11 +188,14 @@ export class TokenAuthorizer extends LambdaAuthorizer {

public readonly authorizerArn: string;

protected readonly authorizerProps: CfnAuthorizerProps;

constructor(scope: Construct, id: string, props: TokenAuthorizerProps) {
super(scope, id, props);

const restApiId = this.lazyRestApiId();
const resource = new CfnAuthorizer(this, 'Resource', {

const authorizerProps: CfnAuthorizerProps = {
name: props.authorizerName ?? Names.uniqueId(this),
restApiId,
type: 'TOKEN',
Expand All @@ -176,7 +204,11 @@ export class TokenAuthorizer extends LambdaAuthorizer {
authorizerResultTtlInSeconds: props.resultsCacheTtl?.toSeconds(),
identitySource: props.identitySource || 'method.request.header.Authorization',
identityValidationExpression: props.validationRegex,
});
};

this.authorizerProps = authorizerProps;

const resource = new CfnAuthorizer(this, 'Resource', authorizerProps);

this.authorizerId = resource.ref;
this.authorizerArn = Stack.of(this).formatArn({
Expand Down Expand Up @@ -221,6 +253,8 @@ export class RequestAuthorizer extends LambdaAuthorizer {

public readonly authorizerArn: string;

protected readonly authorizerProps: CfnAuthorizerProps;

constructor(scope: Construct, id: string, props: RequestAuthorizerProps) {
super(scope, id, props);

Expand All @@ -229,15 +263,20 @@ export class RequestAuthorizer extends LambdaAuthorizer {
}

const restApiId = this.lazyRestApiId();
const resource = new CfnAuthorizer(this, 'Resource', {

const authorizerProps: CfnAuthorizerProps = {
name: props.authorizerName ?? Names.uniqueId(this),
restApiId,
type: 'REQUEST',
authorizerUri: lambdaAuthorizerArn(props.handler),
authorizerCredentials: props.assumeRole?.roleArn,
authorizerResultTtlInSeconds: props.resultsCacheTtl?.toSeconds(),
identitySource: props.identitySources.map(is => is.toString()).join(','),
});
};

this.authorizerProps = authorizerProps;

const resource = new CfnAuthorizer(this, 'Resource', authorizerProps);

this.authorizerId = resource.ref;
this.authorizerArn = Stack.of(this).formatArn({
Expand Down
54 changes: 54 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/authorizers/cognito.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,58 @@ describe('Cognito Authorizer', () => {

expect(authorizer.authorizerArn.endsWith(`/authorizers/${authorizer.authorizerId}`)).toBeTruthy();
});

test('rest api depends on the authorizer when @aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId is enabled', () => {
const stack = new Stack();
stack.node.setContext('@aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId', true);
const userPool1 = new cognito.UserPool(stack, 'UserPool');

const authorizer = new CognitoUserPoolsAuthorizer(stack, 'Authorizer', {
cognitoUserPools: [userPool1],
});

const restApi = new RestApi(stack, 'Api');

restApi.root.addMethod('ANY', undefined, {
authorizer,
authorizationType: AuthorizationType.COGNITO,
});

const template = Template.fromStack(stack);

const authorizerId = Object.keys(template.findResources('AWS::ApiGateway::Authorizer'))[0];
const deployment = Object.values(template.findResources('AWS::ApiGateway::Deployment'))[0];

expect(deployment.DependsOn).toEqual(expect.arrayContaining([authorizerId]));
});

test('a new deployment is created when a cognito user pool is re-created and @aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId is enabled', () => {
const createApiTemplate = (userPoolId: string) => {
const stack = new Stack();
stack.node.setContext('@aws-cdk/aws-apigateway:authorizerChangeDeploymentLogicalId', true);

const userPool = new cognito.UserPool(stack, userPoolId);

const auth = new CognitoUserPoolsAuthorizer(stack, 'myauthorizer', {
resultsCacheTtl: Duration.seconds(0),
cognitoUserPools: [userPool],
});

const restApi = new RestApi(stack, 'myrestapi');
restApi.root.addMethod('ANY', undefined, {
authorizer: auth,
authorizationType: AuthorizationType.COGNITO,
});

return Template.fromStack(stack);
};

const oldTemplate = createApiTemplate('foo');
const newTemplate = createApiTemplate('bar');

const oldDeploymentId = Object.keys(oldTemplate.findResources('AWS::ApiGateway::Deployment'))[0];
const newDeploymentId = Object.keys(newTemplate.findResources('AWS::ApiGateway::Deployment'))[0];

expect(oldDeploymentId).not.toEqual(newDeploymentId);
});
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "20.0.0",
"version": "22.0.0",
"files": {
"551baa1ebfdea9d8d905ffd1e2e8ac09982d0a49e669c97ad0d8f8c092cb96df": {
"81ccfaff55790eb0a0ba90c4ede5ca2168072939afb21004c5dcb5ca74295b40": {
"source": {
"path": "CognitoUserPoolsAuthorizerInteg.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "551baa1ebfdea9d8d905ffd1e2e8ac09982d0a49e669c97ad0d8f8c092cb96df.json",
"objectKey": "81ccfaff55790eb0a0ba90c4ede5ca2168072939afb21004c5dcb5ca74295b40.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
},
"myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a": {
"myrestapiDeployment419B1464d5146a3a0aa3a9f79024a52930571dc6": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
Expand All @@ -114,6 +114,7 @@
"Description": "Automatically created by the RestApi construct"
},
"DependsOn": [
"myauthorizer23CB99DD",
"myrestapiANY94B0497F"
]
},
Expand All @@ -124,7 +125,7 @@
"Ref": "myrestapi551C8392"
},
"DeploymentId": {
"Ref": "myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a"
"Ref": "myrestapiDeployment419B1464d5146a3a0aa3a9f79024a52930571dc6"
},
"StageName": "prod"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"20.0.0"}
{"version":"22.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "22.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"version": "20.0.0",
"version": "22.0.0",
"testCases": {
"cognito-authorizer/DefaultTest": {
"stacks": [
"CognitoUserPoolsAuthorizerInteg"
],
"assertionStack": "cognito-authorizer/DefaultTest/DeployAssert"
"assertionStack": "cognito-authorizer/DefaultTest/DeployAssert",
"assertionStackName": "cognitoauthorizerDefaultTestDeployAssert4551574C"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
{
"version": "20.0.0",
"version": "22.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
},
"CognitoUserPoolsAuthorizerInteg.assets": {
"type": "cdk:asset-manifest",
"properties": {
Expand All @@ -23,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/551baa1ebfdea9d8d905ffd1e2e8ac09982d0a49e669c97ad0d8f8c092cb96df.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/81ccfaff55790eb0a0ba90c4ede5ca2168072939afb21004c5dcb5ca74295b40.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -72,7 +66,7 @@
"/CognitoUserPoolsAuthorizerInteg/myrestapi/Deployment/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a"
"data": "myrestapiDeployment419B1464d5146a3a0aa3a9f79024a52930571dc6"
}
],
"/CognitoUserPoolsAuthorizerInteg/myrestapi/DeploymentStage.prod/Resource": [
Expand Down Expand Up @@ -104,6 +98,15 @@
"type": "aws:cdk:logicalId",
"data": "CheckBootstrapVersion"
}
],
"myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a": [
{
"type": "aws:cdk:logicalId",
"data": "myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
]
},
"displayName": "CognitoUserPoolsAuthorizerInteg"
Expand Down Expand Up @@ -154,6 +157,12 @@
]
},
"displayName": "cognito-authorizer/DefaultTest/DeployAssert"
},
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
}
}
}
Loading

0 comments on commit 12e13c1

Please sign in to comment.