Remove default admin credential references in DEVELOPER_GUIDE #1415
Conversation
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1415 +/- ##
============================================
- Coverage 84.92% 84.82% -0.10%
Complexity 1274 1274
============================================
Files 167 167
Lines 5186 5186
Branches 491 491
============================================
- Hits 4404 4399 -5
- Misses 573 578 +5
Partials 209 209 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ryan Bogan <[email protected]>
|
k-NN does not use the demo install security script for integration tests, so modifying the integration test setup is not necessary, per opensearch-project/neural-search#551 (comment) |
Signed-off-by: Ryan Bogan <[email protected]>
ryanbogan
changed the title
ryanbogan
requested review from
heemin32,
navneet1v,
VijayanB,
vamshin,
jmazanec15,
naveentatikonda,
junqiu-lei and
martin-gaievski
as code owners
| user = user == null ? "admin" : user | ||
| password = password == null ? "admin" : password |
There was a problem hiding this comment.
Do we need update here? Since I saw 2.12 k-NN build is failed with:
Suite: Test class org.opensearch.knn.index.AdvancedFilteringUseCasesIT
2> REPRODUCE WITH: ./gradlew ':integTest' --tests "org.opensearch.knn.index.AdvancedFilteringUseCasesIT.testFiltering_whenNonNestedKNNAndNestedFilterAndNonNestedFieldWithNestedAndNonNestedFilterQuery_thenSuccess" -Dtests.seed=30AE712710CE3FFB -Dtests.security.manager=false -Dtests.locale=zh-TW -Dtests.timezone=Asia/Almaty -Druntime.java=21
2> org.opensearch.client.ResponseException: method [DELETE], host [https://localhost:9200], URI [/.plugins-ml-config], status line [HTTP/1.1 403 Forbidden]
{"error":{"root_cause":[{"type":"security_exception","reason":"no permissions for [] and User [name=admin, backend_roles=[admin], requestedTenant=null]"}],"type":"security_exception","reason":"no permissions for [] and User [name=admin, backend_roles=[admin], requestedTenant=null]"},"status":403}
There was a problem hiding this comment.
I had that change implemented but based on the comment I linked above the change isn't necessary
There was a problem hiding this comment.
From what I understand k-NN doesn't use demo configuration to setup security. Instead it manually configures the required files/settings. Hence the internal_users.yml is never modified and the old password of admin is picked when cluster is spun-up and the config is written to security index.
Signed-off-by: Ryan Bogan <[email protected]>
There was a problem hiding this comment.
Ty @ryanbogan . I have left a few comments around using place-holder instead of myStrongPassword123!
| user = user == null ? "admin" : user | ||
| password = password == null ? "admin" : password |
There was a problem hiding this comment.
From what I understand k-NN doesn't use demo configuration to setup security. Instead it manually configures the required files/settings. Hence the internal_users.yml is never modified and the old password of admin is picked when cluster is spun-up and the config is written to security index.
Signed-off-by: Ryan Bogan <[email protected]>
ryanbogan
requested review from
DarshitChanpura,
junqiu-lei,
martin-gaievski and
jmazanec15
* Remove default admin credentials Signed-off-by: Ryan Bogan <[email protected]> * Update developer guide Signed-off-by: Ryan Bogan <[email protected]> * Debug Signed-off-by: Ryan Bogan <[email protected]> * Revert build.gradle changes Signed-off-by: Ryan Bogan <[email protected]> * Update developer guide Signed-off-by: Ryan Bogan <[email protected]> * Remove default password in favor of <admin-password> Signed-off-by: Ryan Bogan <[email protected]> --------- Signed-off-by: Ryan Bogan <[email protected]> (cherry picked from commit 0c000ad)
…#1420) * Remove default admin credentials Signed-off-by: Ryan Bogan <[email protected]> (cherry picked from commit 0c000ad) Co-authored-by: Ryan Bogan <[email protected]>
Description
Remove references to admin:admin default security credentials in the developer guide.
Issues Resolved
#1359
Check List
By 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.