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

Typo in enabling-ssl.adoc #3177

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Typo in enabling-ssl.adoc #3177

wants to merge 1 commit into from

Conversation

temalcode
Copy link

Correcting keystore and key trust store file paths in the examples.

https://issues.apache.org/jira/browse/SOLR-XXXXX

Description

Correcting SSL keystore and SSL trust store file paths in the examples.

Solution

Added root directory to Linux and C:\ to windows configuration.

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

Correcting keystore file paths in the examples.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 11, 2025
@temalcode temalcode changed the title Update enabling-ssl.adoc Typo in enabling-ssl.adoc Feb 11, 2025
Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

LGTM.

@janhoy
Copy link
Contributor

janhoy commented Feb 13, 2025

Sorry, but I cannot see that this is a bug or why this change would be any better than the default in the docs. The docs contain the same values as commented in our solr.in.sh/cmd file, and the paths are resolved relative to solr root dir. I.e. if you place your keystore files in solr/server/etc/ then this works. Of course, if you place your keystore elsewhere you need to modify those values, but I cannot see why /etc/ would be a better default?

@temalcode
Copy link
Author

temalcode commented Feb 13, 2025

In Linux:
The current docs path set to etc/ (without the root directory) not /etc/. I got an error (running using solr.sh) even the keystore files are stored on /etc/

In Windows:
There is no root directory and a etc folder in Windows. That's why I changed it to C:\

@epugh
Copy link
Contributor

epugh commented Feb 13, 2025

I had interpreted it the same as @temalcode the you would want a / to have the /etc dir? And the c:\ would be a good practive for the windows version?

@janhoy
Copy link
Contributor

janhoy commented Feb 13, 2025

The doc is just an example. Perhaps what’s missing is to say that the path is relative to solr install dir and can/should be modified to where user wants to place the certs. Since these certs don’t exist already, one location is not more «correct» than another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants