Skip to content

Commit

Permalink
fix(elasticsearch): domain configured with access policies and a cust…
Browse files Browse the repository at this point in the history
…om kms key fails to deploy (aws#11699)

The problem was that we were missing the necessary kms permissions for the custom resource that applies the access policies.

Fixes aws#11412

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
iliapolo authored Dec 29, 2020
1 parent d18756d commit 245ee6a
Show file tree
Hide file tree
Showing 6 changed files with 777 additions and 2 deletions.
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-elasticsearch/lib/domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,21 @@ export class Domain extends DomainBase implements IDomain {
accessPolicies: accessPolicyStatements,
});

if (props.encryptionAtRest?.kmsKey) {

// https://docs.aws.amazon.com/elasticsearch-service/latest/developerguide/encryption-at-rest.html

// these permissions are documented as required during domain creation.
// while not strictly documented for updates as well, it stands to reason that an update
// operation might require these in case the cluster uses a kms key.
// empircal evidence shows this is indeed required: https://github.com/aws/aws-cdk/issues/11412
accessPolicy.grantPrincipal.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['kms:List*', 'kms:Describe*', 'kms:CreateGrant'],
resources: [props.encryptionAtRest.kmsKey.keyArn],
effect: iam.Effect.ALLOW,
}));
}

accessPolicy.node.addDependency(this.domain);
}
}
Expand Down
43 changes: 41 additions & 2 deletions packages/@aws-cdk/aws-elasticsearch/test/domain.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* eslint-disable jest/expect-expect */
import '@aws-cdk/assert/jest';
import { ResourcePart } from '@aws-cdk/assert';
import * as assert from '@aws-cdk/assert';
import { Metric, Statistic } from '@aws-cdk/aws-cloudwatch';
import { Subnet, Vpc, EbsDeviceVolumeType } from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as logs from '@aws-cdk/aws-logs';
import { App, Stack, Duration, SecretValue } from '@aws-cdk/core';
import { Domain, ElasticsearchVersion } from '../lib';
Expand All @@ -27,6 +28,44 @@ const readWriteActions = [
...writeActions,
];

test('grants kms permissions if needed', () => {

const key = new kms.Key(stack, 'Key');

new Domain(stack, 'Domain', {
version: ElasticsearchVersion.V7_1,
encryptionAtRest: {
kmsKey: key,
},
// so that the access policy custom resource will be used.
useUnsignedBasicAuth: true,
});

const expectedPolicy = {
Statement: [
{
Action: [
'kms:List*',
'kms:Describe*',
'kms:CreateGrant',
],
Effect: 'Allow',
Resource: {
'Fn::GetAtt': [
'Key961B73FD',
'Arn',
],
},
},
],
Version: '2012-10-17',
};

const resources = assert.expect(stack).value.Resources;
expect(resources.AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E.Properties.PolicyDocument).toStrictEqual(expectedPolicy);

});

test('minimal example renders correctly', () => {
new Domain(stack, 'Domain', { version: ElasticsearchVersion.V7_1 });

Expand Down Expand Up @@ -79,7 +118,7 @@ test('can enable version upgrade update policy', () => {
UpdatePolicy: {
EnableVersionUpgrade: true,
},
}, ResourcePart.CompleteDefinition);
}, assert.ResourcePart.CompleteDefinition);
});

describe('log groups', () => {
Expand Down
Loading

0 comments on commit 245ee6a

Please sign in to comment.