Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default admin credential references in DEVELOPER_GUIDE #1415

Merged
merged 6 commits into from
Jan 25, 2024
Merged

Remove default admin credential references in DEVELOPER_GUIDE #1415

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8dca144
Remove default admin credentials
ryanbogan Jan 24, 2024
e5e2f39
Update developer guide
ryanbogan Jan 24, 2024
e6a01eb
Debug
ryanbogan Jan 24, 2024
2d39a50
Revert build.gradle changes
ryanbogan Jan 25, 2024
02958d3
Update developer guide
ryanbogan Jan 25, 2024
76de267
Remove default password in favor of <admin-password>
ryanbogan Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions DEVELOPER_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,12 @@ curl localhost:9200

Additionally, it is also possible to run a cluster with security enabled:
```shell script
./gradlew run -Dsecurity.enabled=true -Dhttps=true -Duser=admin -Dpassword=admin
./gradlew run -Dsecurity.enabled=true -Dhttps=true -Duser=admin -Dpassword=myStrongPassword123!
ryanbogan marked this conversation as resolved.
Show resolved Hide resolved
```

By default, if `-Dsecurity.enabled=true` is passed the following defaults will be used: `https=true`, `user=admin` and
`password=admin`.

Then, to access the cluster, we can run
ryanbogan marked this conversation as resolved.
Show resolved Hide resolved
```bash
curl https://localhost:9200 --insecure -u admin:admin
curl https://localhost:9200 --insecure -u admin:myStrongPassword123!
ryanbogan marked this conversation as resolved.
Show resolved Hide resolved

{
"name" : "integTest-0",
Expand All @@ -301,7 +298,6 @@ curl https://localhost:9200 --insecure -u admin:admin
},
"tagline" : "The OpenSearch Project: https://opensearch.org/"
}
```

### Run Multi-node Cluster Locally

Expand Down Expand Up @@ -331,7 +327,7 @@ Integration tests can be run with remote cluster. For that run the following com
In case remote cluster is secured it's possible to pass username and password with the following command:

```
./gradlew :integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="integTest-0" -Dhttps=true -Duser=admin -Dpassword=admin
./gradlew :integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="integTest-0" -Dhttps=true -Duser=admin -Dpassword=myStrongPassword123!
ryanbogan marked this conversation as resolved.
Show resolved Hide resolved
```

### Debugging
Expand Down
2 changes: 2 additions & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,14 @@ integTest {
var is_https = System.getProperty("https")
var user = System.getProperty("user")
var password = System.getProperty("password")

if (System.getProperty("security.enabled") != null) {
// If security is enabled, set is_https/user/password defaults
is_https = is_https == null ? "true" : is_https
user = user == null ? "admin" : user
password = password == null ? "admin" : password
Comment on lines 343 to 344
Copy link
Member

@junqiu-lei junqiu-lei Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}

https://build.ci.opensearch.org/blue/rest/organizations/jenkins/pipelines/integ-test/runs/7131/nodes/96/steps/456/log/?start=0

https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test/detail/integ-test/7131/pipeline/96

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had that change implemented but based on the comment I linked above the change isn't necessary

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

systemProperty("https", is_https)
systemProperty("user", user)
systemProperty("password", password)
Expand Down
Loading