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.

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 admin:admin default and update instructions for demo setup #5887

Merged
merged 28 commits into from
Feb 1, 2024

Conversation

derek-ho
Copy link
Contributor

@derek-ho derek-ho commented Dec 15, 2023

Description

Starting with the 2.12 release, admin will no longer be set as the default password when running the install demo configuration script. Instead, it will require user input (via an environment variable) to set the initial admin password, otherwise the script will fail. This PR updates the documentation to remove references of admin:admin, as well as updates some instructions on how to set this variable for the different distributions.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].
Related to #5946

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Derek Ho <dxho@amazon.com>
@derek-ho
Copy link
Contributor Author

derek-ho commented Dec 15, 2023

@prudhvigodithi / @rishabh6788 / @DarshitChanpura adding you folks as tech reviewers, especially on the install files to double check that this is feasible/makes sense

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

We should also Docker and Helm documentation. This can also be done in a follow-up PR that address the changes made to demo configuration.

@@ -40,10 +40,10 @@ This guide assumes that you are comfortable working from the Linux command line
1. From the CLI, install using `dpkg`.
```bash
# x64
sudo dpkg -i opensearch-{{site.opensearch_version}}-linux-x64.deb
sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=< Admin password > dpkg -i opensearch-{{site.opensearch_version}}-linux-x64.deb
Copy link
Member

Choose a reason for hiding this comment

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

we should add a comment about why this password is required.

Copy link
Member

Choose a reason for hiding this comment

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

This applies to all updated distributions

export OPENSEARCH_INITIAL_ADMIN_PASSWORD=< Admin password >
```
{% include copy.html %}

1. Run the OpenSearch startup script with the security demo configuration.
Copy link
Member

Choose a reason for hiding this comment

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

this step should be numbered as 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of them are 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is correct, I believe the numbers are handled on the website side.

set OPENSEARCH_INITIAL_ADMIN_PASSWORD=< Admin password >
```
{% include copy.html %}

1. Run the batch script.
Copy link
Member

Choose a reason for hiding this comment

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

this step should be numbered as 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of them are numbered as 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

Markdown automatically will number these in sequence. So if you put "1. 1. 1.", the output will be "1. 2. 3"

Signed-off-by: Derek Ho <dxho@amazon.com>
Signed-off-by: Derek Ho <dxho@amazon.com>
@derek-ho derek-ho marked this pull request as ready for review December 18, 2023 15:25
Copy link
Collaborator

@stephen-crawford stephen-crawford left a comment

Choose a reason for hiding this comment

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

A couple small comments but this seems good to me!


# arm64
sudo dpkg -i opensearch-{{site.opensearch_version}}-linux-arm64.deb
sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=< Admin password> dpkg -i opensearch-{{site.opensearch_version}}-linux-arm64.deb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing space inside ">".

@@ -267,7 +267,7 @@ To associate requests with tasks for better tracking, you can provide a `X-Opaqu
Usage:

```bash
curl -i -H "X-Opaque-Id: 111111" "https://localhost:9200/_tasks" -u 'admin:admin' --insecure
curl -i -H "X-Opaque-Id: 111111" "https://localhost:9200/_tasks" -u 'admin:< Admin password >' --insecure
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will want to ask @kolchfa-aws etc. about the style guidelines for these placeholders. I am not sure whether leading/trailing spaces should be trimmed or whether underscores should be used instead of spaces in between the terms.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the late reply, just saw this. Generally we don't use spaces within angular brackets and separate the words with a hyphen or underscore so <custom-admin-password> looks good to me.


```yml
- name: OPENSEARCH_INITIAL_ADMIN_PASSWORD
# value: < Admin password >
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this meant to be commented out?

export OPENSEARCH_INITIAL_ADMIN_PASSWORD=< Admin password >
```
{% include copy.html %}

1. Run the OpenSearch startup script with the security demo configuration.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is correct, I believe the numbers are handled on the website side.

@hdhalter hdhalter added the 4 - Doc review PR: Doc review in progress label Dec 18, 2023
@hdhalter hdhalter added this to the v2.12 milestone Dec 21, 2023
@hdhalter
Copy link
Contributor

@derek-ho , @scrawfor99 - I'll add the 2.12 label for now because you said "and likely to be backported to 2.12 release". We'll check in with you to confirm before 2.12 is released. Thanks!

@derek-ho
Copy link
Contributor Author

@hdhalter yes this is planned for 2.12.0 release. Can we get a tech/doc review for these changes to ensure that we have supporting documentation released in the 2.12.0 release? Thanks!

@hdhalter
Copy link
Contributor

@hdhalter yes this is planned for 2.12.0 release. Can we get a tech/doc review for these changes to ensure that we have supporting documentation released in the 2.12.0 release? Thanks!

Thanks, @derek-ho. @Naarcha-AWS is assigned to do the doc review.

_about/quickstart.md Outdated Show resolved Hide resolved
Naarcha-AWS and others added 2 commits January 23, 2024 10:08
Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@Naarcha-AWS Please see my comments and changes and tag me when complete. I'd like to reread line 114 in the first file before approving. Thanks!

_about/quickstart.md Outdated Show resolved Hide resolved
_about/quickstart.md Outdated Show resolved Hide resolved
_about/quickstart.md Outdated Show resolved Hide resolved
_about/quickstart.md Outdated Show resolved Hide resolved
Co-authored-by: Nathan Bower <nbower@amazon.com>
Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
_about/quickstart.md Outdated Show resolved Hide resolved
Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
@Naarcha-AWS Naarcha-AWS added 6 - Done but waiting to merge PR: The work is done and ready to merge and removed 5 - Editorial review PR: Editorial review in progress labels Jan 23, 2024
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

LGTM

@hdhalter
Copy link
Contributor

@DarshitChanpura - Do we need to modify step 6? It says to enter admin/admin: https://opensearch.org/docs/latest/security/authentication-backends/saml/#docker-example.

@DarshitChanpura
Copy link
Member

DarshitChanpura commented Jan 29, 2024

@DarshitChanpura - Do we need to modify step 6? It says to enter admin/admin: https://opensearch.org/docs/latest/security/authentication-backends/saml/#docker-example.

Ahh..great catch. We can modify this, but then we will also need to modify the steps above to point to 2.12 instead of 2.8. if we plan to leave it as 2.8 we should leave it as admin.

@hdhalter
Copy link
Contributor

hdhalter commented Jan 29, 2024

@DarshitChanpura - Do we need to modify step 6? It says to enter admin/admin: https://opensearch.org/docs/latest/security/authentication-backends/saml/#docker-example.

Ahh..great catch. We can modify this, but then we will also need to modify the steps above to point to 2.12 instead of 2.8. if we plan to leave it as 2.8 we should leave it as admin.

Let's update. We can add a variable for the version, and "<admin user name/password>" will still apply to all versions. @Naarcha-AWS - can you update this? Thanks.

@Naarcha-AWS
Copy link
Collaborator

@DarshitChanpura - Do we need to modify step 6? It says to enter admin/admin: https://opensearch.org/docs/latest/security/authentication-backends/saml/#docker-example.

Ahh..great catch. We can modify this, but then we will also need to modify the steps above to point to 2.12 instead of 2.8. if we plan to leave it as 2.8 we should leave it as admin.

Let's update. We can add a variable for the version, and "<admin user name/password>" will still apply to all versions. @Naarcha-AWS - can you update this? Thanks.

Made this change in a separate PR: #6279

@hdhalter hdhalter added the release-notes PR: Include this PR in the automated release notes label Jan 29, 2024
@hdhalter hdhalter merged commit 6af6650 into opensearch-project:main Feb 1, 2024
3 checks passed
@hdhalter hdhalter added 3 - Done Issue is done/complete and removed 6 - Done but waiting to merge PR: The work is done and ready to merge labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete release-notes PR: Include this PR in the automated release notes v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants