Update Default Password for OS 2.12 #707
Conversation
nhtruong
force-pushed
the
default-password
branch
from
988bbe6 to
03b09fa
Compare
| @@ -5,10 +5,4 @@ ARG opensearch_path=/usr/share/opensearch | |||
| ARG opensearch_yml=$opensearch_path/config/opensearch.yml | |||
|
|
|||
| ARG SECURE_INTEGRATION | |||
|
|
|||
There was a problem hiding this comment.
Is it possible to replace this health-check with something else?
There was a problem hiding this comment.
This was added to this repo to solve a flaky spec issue myself when I first started. No other client repo has this. From what I've observed so far, it doesn't really help. If it happens again, I'm better equiped to find a better solution. This also adds complexity to the CI workflows esp now that HEALTHCHECK has to be aware of the OS version.
| return major === 2 && minor >= 12; | ||
| } | ||
|
|
||
| function createSecuredClient() { |
There was a problem hiding this comment.
where would this be used?
There was a problem hiding this comment.
This is meant to be used in integration tests only. It's a helper function inside test folder.
- Updated secured test suite to use strong password when OS 2.12 or up is detected - Removed autoheal - Simplified client creation step for the guides Signed-off-by: Theo Truong <[email protected]>
nhtruong
force-pushed
the
default-password
branch
from
1f90740 to
d34bcdb
Compare
nhtruong
requested review from
ananzh,
kavilla,
dblock,
VachaShah,
harshavamsi and
timursaurus
as code owners
|
This is ready for review but we won't be able to merged this till 2.12.x is released on Docker repo as all workflows using |
| @@ -6,10 +6,11 @@ In this guide, we will look at some advanced index actions that are not covered | |||
| Let's create a client instance, and an index named `movies`: | |||
| ```javascript | |||
| const { Client } = require('@opensearch-project/opensearch'); | |||
|
|
|||
| const client = new Client({ | |||
There was a problem hiding this comment.
why not point to createSecureClient() here?
There was a problem hiding this comment.
Beecause that's a helper function for our test suites only. The function creates a client using admin:admin or admin:myStrongPassword123! depending on the OPENSEARCH_VERSION env. Users should not use that function because their password will be different.
There was a problem hiding this comment.
ahh I see. ty for explanation!!
| return major === 2 && minor >= 12; | ||
| } | ||
|
|
||
| function createSecuredClient() { |
There was a problem hiding this comment.
where would this be used?
| ``` | ||
|
|
||
| Let's create a client instance to access this cluster: | ||
| Let's create a client instance to access an OpenSearch cluster: |
There was a problem hiding this comment.
Should we skip mentioning how to start a cluster here?
There was a problem hiding this comment.
I think we should. These guides should focus on OpenSearch features, not how to setup a cluster. We can have another guide to cover how to set up a cluster, but that's already covered in the Readme and especially the OpenSearch website itself.
Signed-off-by: Theo Truong <[email protected]>
|
@nhtruong Now that 2.12 is released this should be unblocked. |
|
@DarshitChanpura All workflows pass now 🥳 |
Description
Prepare repo for the advent of OS 2.12 where
admin:adminis no longer an accepted credential.Issues Resolved
Closes #699
Check List
yarn run lintdoesn't show any errorsBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.