feat(opensearchservice): When a Domain has enforceHttps true, set the connections defaultPort#20602
feat(opensearchservice): When a Domain has enforceHttps true, set the connections defaultPort#20602mergify[bot] merged 5 commits intoaws:mainfrom SamStephens:main
Conversation
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.
The title of one of the tests should say "connections throws if domain is *not* placed inside a vpc".
|
|
corymhall
left a comment
There was a problem hiding this comment.
@SamStephens does it make sense to also set the default port if enforceHttps is not set? I can't find much information on ports with opensearch except for this which mentions only 80 & 443.
thoughts?
|
corymhall said:
Sorry, I should have explained my rational here. My take is that if Also, PR linting is failing because I've not made a change to a README file. I'm not sure what to do about this, as this is such a minor feature I'm not sure it belongs in the Opensearch Service README, which doesn't even describe the enforceHttps option, let alone this minor consequence of it. |
corymhall
left a comment
There was a problem hiding this comment.
Sorry, I should have explained my rational here. My take is that if enforceHttps is false, the cluster listens on ports 80 and 443, so it doesn't make sense to have a singular default port.
gotcha, that makes sense.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thanks @corymhall ! |
… connections defaultPort (aws#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 aws#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*
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:
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.
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license