From b687bc40a70d4a4b288419610211d31663ff226a Mon Sep 17 00:00:00 2001 From: Sayali Gaikawad Date: Thu, 14 Mar 2024 15:24:10 -0700 Subject: [PATCH 1/3] Enable TLS protocol when cert is provided and port number is 443 Signed-off-by: Sayali Gaikawad --- README.md | 2 +- lib/infra/infra-stack.ts | 36 ++++++++++++++-------- package-lock.json | 4 +-- package.json | 2 +- test/opensearch-cluster-cdk.test.ts | 46 +++++++++++++++++++++++++++-- 5 files changed, 72 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index ae4d64968c3..684f30c7f86 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ In order to deploy both the stacks the user needs to provide a set of required a | customRoleArn | Optional | string | User provided IAM role arn to be used as ec2 instance profile. `-c customRoleArn=arn:aws:iam:::role/`| | customConfigFiles | Optional | string | You can provide an entire config file to be overwritten or added to OpenSearch and OpenSearch Dashboards. Pass string in the form of JSON with key as local path to the config file to read from and value as file on the server to overwrite/add. Note that the values in the JSON needs to have prefix of `opensearch` or `opensearch-dashboards`. Example: `-c customConfigFiles='{"opensearch-config/config.yml": "opensearch/config/opensearch-security/config.yml", "opensearch-config/role_mapping.yml":"opensearch/config/opensearch-security/roles_mapping.yml", "/roles.yml": "opensearch/config/opensearch-security/roles.yml"}'` | | enableMonitoring | Optional | boolean | Boolean flag to enable monitoring and alarms for Infra Stack. See [InfraStackMonitoring class](./lib/monitoring/alarms.ts) for more details. Defaults to false e.g., `--context enableMonitoring=true` | -| certificateArn | Optional | string | Add ACM certificate to the listener. e.g., `--context certificateArn=arn:1234`| +| certificateArn | Optional | string | Add ACM certificate to the any listener (OpenSearch or OpenSearch-Dashboards) whose port is mapped to 443. e.g., `--context certificateArn=arn:1234`| | mapOpensearchPortTo | Optional | integer | Load balancer port number to map to OpenSearch. e.g., `--context mapOpensearchPortTo=8440` Defaults to 80 when security is disabled and 443 when security is enabled | | mapOpensearchDashboardsPortTo | Optional | integer | Load balancer port number to map to OpenSearch-Dashboards. e.g., `--context mapOpensearchDashboardsPortTo=443` Always defaults to 8443 | diff --git a/lib/infra/infra-stack.ts b/lib/infra/infra-stack.ts index 6b1bf4908cd..18b96da140c 100644 --- a/lib/infra/infra-stack.ts +++ b/lib/infra/infra-stack.ts @@ -198,6 +198,7 @@ export class InfraStack extends Stack { constructor(scope: Stack, id: string, props: InfraProps) { super(scope, id, props); + let opensearchListener: NetworkListener; let dashboardsListener: NetworkListener; let managerAsgCapacity: number; let dataAsgCapacity: number; @@ -412,14 +413,17 @@ export class InfraStack extends Stack { this.opensearchPortMapping = parseInt(opensearchPortMap, 10); } - const opensearchListener = nlb.addListener('opensearch', { - port: this.opensearchPortMapping, - protocol: Protocol.TCP, - }); - if (!this.securityDisabled && !this.minDistribution) { - if (certificateArn !== 'undefined') { - opensearchListener.addCertificates('cert', [ListenerCertificate.fromArn(certificateArn)]); - } + if ((!this.securityDisabled && !this.minDistribution && this.opensearchPortMapping === 443 && certificateArn !== 'undefined')) { + opensearchListener = nlb.addListener('opensearch', { + port: this.opensearchPortMapping, + protocol: Protocol.TLS, + certificates: [ListenerCertificate.fromArn(certificateArn)], + }); + } else { + opensearchListener = nlb.addListener('opensearch', { + port: this.opensearchPortMapping, + protocol: Protocol.TCP, + }); } const opensearchDashboardsPortMap = `${props?.mapOpensearchDashboardsPortTo ?? scope.node.tryGetContext('mapOpensearchDashboardsPortTo')}`; @@ -430,10 +434,18 @@ export class InfraStack extends Stack { } if (this.dashboardsUrl !== 'undefined') { - dashboardsListener = nlb.addListener('dashboards', { - port: this.opensearchDashboardsPortMapping, - protocol: Protocol.TCP, - }); + if ((!this.securityDisabled && !this.minDistribution && this.opensearchDashboardsPortMapping === 443 && certificateArn !== 'undefined')) { + dashboardsListener = nlb.addListener('dashboards', { + port: this.opensearchDashboardsPortMapping, + protocol: Protocol.TLS, + certificates: [ListenerCertificate.fromArn(certificateArn)], + }); + } else { + dashboardsListener = nlb.addListener('dashboards', { + port: this.opensearchDashboardsPortMapping, + protocol: Protocol.TCP, + }); + } } if (this.singleNodeCluster) { diff --git a/package-lock.json b/package-lock.json index 3f951777467..dc36c712cb4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@opensearch-project/opensearch-cluster-cdk", - "version": "1.2.0", + "version": "1.2.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@opensearch-project/opensearch-cluster-cdk", - "version": "1.2.0", + "version": "1.2.1", "dependencies": { "@typescript-eslint/eslint-plugin": "^4.31.1", "@typescript-eslint/parser": "^4.31.1", diff --git a/package.json b/package.json index de96bcda955..e9f4103820d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@opensearch-project/opensearch-cluster-cdk", - "version": "1.2.0", + "version": "1.2.1", "bin": { "cdk_v2": "bin/app.js" }, diff --git a/test/opensearch-cluster-cdk.test.ts b/test/opensearch-cluster-cdk.test.ts index e2ddd60dc18..a68fd5f52d5 100644 --- a/test/opensearch-cluster-cdk.test.ts +++ b/test/opensearch-cluster-cdk.test.ts @@ -895,19 +895,58 @@ test('Test certificate addition and port mapping', () => { infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { Port: 8440, Protocol: 'TCP', + }); + infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { + Port: 443, + Protocol: 'TLS', Certificates: [ { CertificateArn: 'arn:1234', }, ], }); +}); + +test('Test default port mapping', () => { + const app = new App({ + context: { + securityDisabled: false, + minDistribution: false, + distributionUrl: 'www.example.com', + cpuArch: 'x64', + singleNodeCluster: false, + dashboardsUrl: 'www.example.com', + distVersion: '1.0.0', + serverAccessType: 'ipv4', + restrictServerAccessTo: 'all', + }, + }); + + // WHEN + const networkStack = new NetworkStack(app, 'opensearch-network-stack', { + env: { account: 'test-account', region: 'us-east-1' }, + }); + + // @ts-ignore + const infraStack = new InfraStack(app, 'opensearch-infra-stack', { + vpc: networkStack.vpc, + securityGroup: networkStack.osSecurityGroup, + env: { account: 'test-account', region: 'us-east-1' }, + }); + + // THEN + const infraTemplate = Template.fromStack(infraStack); infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { Port: 443, Protocol: 'TCP', }); + infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { + Port: 8443, + Protocol: 'TCP', + }); }); -test('Test default port mapping', () => { +test('Ignore cert and TLS protocol if none of the ports map to 443', () => { const app = new App({ context: { securityDisabled: false, @@ -919,6 +958,9 @@ test('Test default port mapping', () => { distVersion: '1.0.0', serverAccessType: 'ipv4', restrictServerAccessTo: 'all', + certificateArn: 'arn:1234', + mapOpensearchPortTo: '8440', + mapOpensearchDashboardsPortTo: '8443', }, }); @@ -937,7 +979,7 @@ test('Test default port mapping', () => { // THEN const infraTemplate = Template.fromStack(infraStack); infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { - Port: 443, + Port: 8440, Protocol: 'TCP', }); infraTemplate.hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { From 326bc9857f8af1823b73d078262b81bf7de0b24f Mon Sep 17 00:00:00 2001 From: Sayali Gaikawad Date: Thu, 14 Mar 2024 15:39:20 -0700 Subject: [PATCH 2/3] Format Signed-off-by: Sayali Gaikawad --- lib/infra/infra-stack.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/infra/infra-stack.ts b/lib/infra/infra-stack.ts index 18b96da140c..384644b95c2 100644 --- a/lib/infra/infra-stack.ts +++ b/lib/infra/infra-stack.ts @@ -413,7 +413,7 @@ export class InfraStack extends Stack { this.opensearchPortMapping = parseInt(opensearchPortMap, 10); } - if ((!this.securityDisabled && !this.minDistribution && this.opensearchPortMapping === 443 && certificateArn !== 'undefined')) { + if (!this.securityDisabled && !this.minDistribution && this.opensearchPortMapping === 443 && certificateArn !== 'undefined') { opensearchListener = nlb.addListener('opensearch', { port: this.opensearchPortMapping, protocol: Protocol.TLS, @@ -434,7 +434,7 @@ export class InfraStack extends Stack { } if (this.dashboardsUrl !== 'undefined') { - if ((!this.securityDisabled && !this.minDistribution && this.opensearchDashboardsPortMapping === 443 && certificateArn !== 'undefined')) { + if (!this.securityDisabled && !this.minDistribution && this.opensearchDashboardsPortMapping === 443 && certificateArn !== 'undefined') { dashboardsListener = nlb.addListener('dashboards', { port: this.opensearchDashboardsPortMapping, protocol: Protocol.TLS, From bb8b02be4c423fe5f7276457dfde747ff8b3eb85 Mon Sep 17 00:00:00 2001 From: Sayali Gaikawad Date: Thu, 14 Mar 2024 16:41:17 -0700 Subject: [PATCH 3/3] Add error Signed-off-by: Sayali Gaikawad --- lib/infra/infra-stack.ts | 20 +++++++++------ test/opensearch-cluster-cdk.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/lib/infra/infra-stack.ts b/lib/infra/infra-stack.ts index 384644b95c2..8d0b1cd557d 100644 --- a/lib/infra/infra-stack.ts +++ b/lib/infra/infra-stack.ts @@ -403,6 +403,8 @@ export class InfraStack extends Stack { }); const opensearchPortMap = `${props?.mapOpensearchPortTo ?? scope.node.tryGetContext('mapOpensearchPortTo')}`; + const opensearchDashboardsPortMap = `${props?.mapOpensearchDashboardsPortTo ?? scope.node.tryGetContext('mapOpensearchDashboardsPortTo')}`; + if (opensearchPortMap === 'undefined') { if (!this.securityDisabled && !this.minDistribution) { this.opensearchPortMapping = 443; @@ -413,6 +415,17 @@ export class InfraStack extends Stack { this.opensearchPortMapping = parseInt(opensearchPortMap, 10); } + if (opensearchDashboardsPortMap === 'undefined') { + this.opensearchDashboardsPortMapping = 8443; + } else { + this.opensearchDashboardsPortMapping = parseInt(opensearchDashboardsPortMap, 10); + } + + if (this.opensearchPortMapping === this.opensearchDashboardsPortMapping) { + throw new Error('OpenSearch and OpenSearch-Dashboards cannot be mapped to the same port! Please provide different port numbers.' + + ` Current mapping is OpenSearch:${this.opensearchPortMapping} OpenSearch-Dashboards:${this.opensearchDashboardsPortMapping}`); + } + if (!this.securityDisabled && !this.minDistribution && this.opensearchPortMapping === 443 && certificateArn !== 'undefined') { opensearchListener = nlb.addListener('opensearch', { port: this.opensearchPortMapping, @@ -426,13 +439,6 @@ export class InfraStack extends Stack { }); } - const opensearchDashboardsPortMap = `${props?.mapOpensearchDashboardsPortTo ?? scope.node.tryGetContext('mapOpensearchDashboardsPortTo')}`; - if (opensearchDashboardsPortMap === 'undefined') { - this.opensearchDashboardsPortMapping = 8443; - } else { - this.opensearchDashboardsPortMapping = parseInt(opensearchDashboardsPortMap, 10); - } - if (this.dashboardsUrl !== 'undefined') { if (!this.securityDisabled && !this.minDistribution && this.opensearchDashboardsPortMapping === 443 && certificateArn !== 'undefined') { dashboardsListener = nlb.addListener('dashboards', { diff --git a/test/opensearch-cluster-cdk.test.ts b/test/opensearch-cluster-cdk.test.ts index a68fd5f52d5..d38edc4a520 100644 --- a/test/opensearch-cluster-cdk.test.ts +++ b/test/opensearch-cluster-cdk.test.ts @@ -987,3 +987,42 @@ test('Ignore cert and TLS protocol if none of the ports map to 443', () => { Protocol: 'TCP', }); }); + +test('Throw error on duplicate ports', () => { + const app = new App({ + context: { + securityDisabled: false, + minDistribution: false, + distributionUrl: 'www.example.com', + cpuArch: 'x64', + singleNodeCluster: false, + dashboardsUrl: 'www.example.com', + distVersion: '1.0.0', + serverAccessType: 'ipv4', + restrictServerAccessTo: 'all', + certificateArn: 'arn:1234', + mapOpensearchPortTo: '8443', + }, + }); + // WHEN + try { + // WHEN + const networkStack = new NetworkStack(app, 'opensearch-network-stack', { + env: { account: 'test-account', region: 'us-east-1' }, + }); + + // @ts-ignore + const infraStack = new InfraStack(app, 'opensearch-infra-stack', { + vpc: networkStack.vpc, + securityGroup: networkStack.osSecurityGroup, + env: { account: 'test-account', region: 'us-east-1' }, + }); + + // eslint-disable-next-line no-undef + fail('Expected an error to be thrown'); + } catch (error) { + expect(error).toBeInstanceOf(Error); + // @ts-ignore + expect(error.message).toEqual('OpenSearch and OpenSearch-Dashboards cannot be mapped to the same port! Please provide different port numbers. Current mapping is OpenSearch:8443 OpenSearch-Dashboards:8443'); + } +});