Skip to content

Commit

Permalink
feat(opensearchservice): When a Domain has enforceHttps true, set the…
Browse files Browse the repository at this point in the history
… connections defaultPort (#20602)

For an Opensearch domain, if enforceHttps is enabled, set the defaultPort for the connections object of the domain, as we know it communicates over 443 in this scenario.

I also took the liberty of correcting a test title while I was in the code, I hope it's okay coupling that fix with this change.

closes #16251

----

### All Submissions:

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

### Adding new Unconventional Dependencies:

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

### New Features

It doesn't seem that an integration test is a good fit here, as this simply changes/simplifies how you'll create a connection to a Domain. I'm happy to be corrected however and will add one if asked.

* [ ] 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
SamStephens authored Jun 13, 2022
1 parent f8b6896 commit a6fe2cb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-opensearchservice/lib/domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,11 @@ export class Domain extends DomainBase implements IDomain, ec2.IConnectable {
vpc: props.vpc,
description: `Security group for domain ${this.node.id}`,
})];
this._connections = new ec2.Connections({ securityGroups });
if (props.enforceHttps) {
this._connections = new ec2.Connections({ securityGroups, defaultPort: ec2.Port.tcp(443) });
} else {
this._connections = new ec2.Connections({ securityGroups });
}
}

// If VPC options are supplied ensure that the number of subnets matches the number AZ
Expand Down
30 changes: 28 additions & 2 deletions packages/@aws-cdk/aws-opensearchservice/test/domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { Match, Template } from '@aws-cdk/assertions';
import * as acm from '@aws-cdk/aws-certificatemanager';
import { Metric, Statistic } from '@aws-cdk/aws-cloudwatch';
import { Vpc, EbsDeviceVolumeType, SecurityGroup } from '@aws-cdk/aws-ec2';
import { Vpc, EbsDeviceVolumeType, Port, SecurityGroup } 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';
Expand Down Expand Up @@ -31,7 +31,7 @@ const readWriteActions = [

const defaultVersion = EngineVersion.OPENSEARCH_1_0;

test('connections throws if domain is placed inside a vpc', () => {
test('connections throws if domain is not placed inside a vpc', () => {

expect(() => {
new Domain(stack, 'Domain', {
Expand Down Expand Up @@ -109,6 +109,32 @@ test('default subnets and security group when vpc is used', () => {

});

test('connections has no default port if enforceHttps is false', () => {

const vpc = new Vpc(stack, 'Vpc');
const domain = new Domain(stack, 'Domain', {
version: defaultVersion,
vpc,
enforceHttps: false,
});

expect(domain.connections.defaultPort).toBeUndefined();

});

test('connections has default port 443 if enforceHttps is true', () => {

const vpc = new Vpc(stack, 'Vpc');
const domain = new Domain(stack, 'Domain', {
version: defaultVersion,
vpc,
enforceHttps: true,
});

expect(domain.connections.defaultPort).toEqual(Port.tcp(443));

});

test('default removalpolicy is retain', () => {
new Domain(stack, 'Domain', {
version: defaultVersion,
Expand Down

0 comments on commit a6fe2cb

Please sign in to comment.