Skip to content

Conversation

@shawkins
Copy link
Contributor

@shawkins shawkins commented Oct 4, 2024

closes: #32110

Calls out more clearly the usage of /opt/keycloak and provides an overview of what our directory structure looks like.

There's a note about the relative paths - I think a separate issue is needed if we want to change all existing relative paths to read as /path instead.

The README.md in our root directory could also be updated to have a link to this guide.

WDYT? @andymunro @vmuzikar @mabartos

@shawkins shawkins marked this pull request as ready for review October 10, 2024 12:02
@shawkins shawkins requested review from a team as code owners October 10, 2024 12:02
Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

@shawkins Thank you for the PR! I think this guide is a nice addition.

The README.md in our root directory could also be updated to have a link to this guide.

I was thinking about if we should have this guide only in the form of the README included in the root of the dist. That's probably the natural place I'd be looking for an info on the dir structure as a user. But then we have the container and the README there is not as accessible. So +1 for having this as a online guide and adding just a link to the README. We could do that as part of this PR even.

There's a note about the relative paths - I think a separate issue is needed if we want to change all existing relative paths to read as /path instead.

I think that's fine the way it is.

I was also wondering about backporting this. @ahus1 Do you think it's a valid action to add a guide in a micro release?

@@ -1,3 +1,4 @@
directory-structure
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not place it so high. The first think user usually needs is configuration and admin user. I'd place it between bootstrap-admin-recovery and containers. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems fine.

Comment on lines 9 to 11
== Installation Locations

`/opt/keycloak` is the root install location for the server in all containerized usage shown for {project_name} including <@links.server id="containers"/> and the Getting Started Guides for Docker, Podman, Kubernetes, and OpenShift - see link:{gettingstarted_link_latest}[{gettingstarted_name_short}].

If you are installing from a zip file then by default there will be an install root directory of `{archivebasename}-{version}`, which can be created anywhere you choose on your filesystem.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd swap this chapter with Directory Structure. It feels to me a bit odd to start a "Directory Structure" guide with a note where KC is placed in the container image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to first establish context about where the directory structure could be found. Would altenative wording help?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd maybe at least swap the two paragraphs to have ZIP instructions first (at a more "natural" distribution).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, made that change.

@shawkins
Copy link
Contributor Author

@shawkins Thank you for the PR! I think this guide is a nice addition.

The README.md in our root directory could also be updated to have a link to this guide.

I was thinking about if we should have this guide only in the form of the README included in the root of the dist. That's probably the natural place I'd be looking for an info on the dir structure as a user. But then we have the container and the README there is not as accessible. So +1 for having this as a online guide and adding just a link to the README. We could do that as part of this PR even.

Ok, I'll add that.

There's a note about the relative paths - I think a separate issue is needed if we want to change all existing relative paths to read as /path instead.

I think that's fine the way it is.

I was also wondering about backporting this. @ahus1 Do you think it's a valid action to add a guide in a micro release?

@shawkins
Copy link
Contributor Author

Let me know if the README link seems ok, or if it's good enough to be implied by the https://www.keycloak.org/guides#server link at the bottom

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@shawkins Can you please create a backport? I think there's no harm in adding docs in a micro.

@vmuzikar vmuzikar merged commit fd89297 into keycloak:main Oct 21, 2024
shawkins added a commit to shawkins/keycloak that referenced this pull request Oct 21, 2024
@shawkins
Copy link
Contributor Author

LGTM, thank you.

@shawkins Can you please create a backport? I think there's no harm in adding docs in a micro.

#34158

shawkins added a commit that referenced this pull request Oct 21, 2024
…#34158)

* fix: adding a server guide on installation location / layout (#33604)

closes: #32110

Signed-off-by: Steve Hawkins <[email protected]>
(cherry picked from commit fd89297)

* adding getting started links

Signed-off-by: Steve Hawkins <[email protected]>

---------

Signed-off-by: Steve Hawkins <[email protected]>
@edewit edewit mentioned this pull request Nov 19, 2024
@edewit edewit mentioned this pull request Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Documentation] - Configuring trusted certificates - Fully specify truststore path

2 participants