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

[DOCS] Add missing description for transport_client reserved role #108633

Conversation

slobodanadamovic
Copy link
Contributor

@slobodanadamovic slobodanadamovic commented May 14, 2024

This PR adds missing role description for the transport_client role, and a test to enforce that all reserved roles are described. The description also serves as self-documentation for roles, thus it is reasonable to make this a requirement for all reserved roles.

Relates to #108422, which included descriptions for other reserved roles.

This commit adds missing role description and a test to enforce that all
reserved roles are described. The description also serves as
self-documentation for roles, thus it's it is reasonable to
make this a requirement for all reserved roles.
@slobodanadamovic slobodanadamovic added >docs General docs changes :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team labels May 14, 2024
@slobodanadamovic slobodanadamovic self-assigned this May 14, 2024
@elasticsearchmachine elasticsearchmachine added v8.15.0 Team:Docs Meta label for docs team labels May 14, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Comment on lines +175 to +178
"Grants the privileges required to access the cluster through the Java Transport Client. "
+ "The Java Transport Client fetches information about the nodes in the cluster using "
+ "the Node Liveness API and the Cluster State API (when sniffing is enabled). "
+ "Assign your users this role if they use the Transport Client."
Copy link
Contributor

Choose a reason for hiding this comment

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

not finding the node liveness api documented anywhere.

does "when sniffing is enabled" modify both the node liveness API + cluster state api? if so:

Suggested change
"Grants the privileges required to access the cluster through the Java Transport Client. "
+ "The Java Transport Client fetches information about the nodes in the cluster using "
+ "the Node Liveness API and the Cluster State API (when sniffing is enabled). "
+ "Assign your users this role if they use the Transport Client."
"Grants the privileges required to access the cluster through the Java Transport Client. "
+ "When sniffing is enabled, the Java Transport Client can fetch information about the nodes in the cluster using "
+ "the Node Liveness API and the Cluster State API. "
+ "Assign your users this role if they use the Transport Client."

Additionally, is it the user that is using the JTC, or is the user being used to auth the JTC?

to clarify, you're intending to refer to the deprecated java transport client, and not the newest java client, correct?

I see that you basically just copied the text from the original description so you can take this all with a grain of salt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. The Java Transport client is deprecated and removed in 8.0. I will check internally if we should also deprecate and remove this role. It must have simply been missed.

I see that you basically just copied the text from the original description so you can take this all with a grain of salt.

True, for completeness, I've simply copied existing role documentation. I can also update existing documentation with your suggestion, but let me first double check if we should keep it.

Copy link
Contributor

Choose a reason for hiding this comment

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

not finding the node liveness api documented anywhere.

Yes, the liveness action doesn't exist anymore. I would avoid mentioning it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does "when sniffing is enabled" modify both the node liveness API + cluster state api?

Just checked this and the Transport Client was periodically checking if the configured list of nodes is alive - even without sniff enabled. So I will leave this description as is, since it is accurate.

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM
The role is deprecated, so I would add that to the description as well.

@slobodanadamovic
Copy link
Contributor Author

The role is deprecated, so I would add that to the description as well.

I think we still need to deprecate it. I'll leave the description as is and handle deprecation in a followup PR.

@slobodanadamovic slobodanadamovic merged commit 0330efb into elastic:main May 16, 2024
20 checks passed
nicktindall pushed a commit to nicktindall/elasticsearch that referenced this pull request May 17, 2024
…#108633)

This PR adds missing role description for the `transport_client role`, 
and a test to enforce that all reserved roles are described. 
The description also serves as self-documentation for roles, 
thus it is reasonable to make this a requirement for all reserved roles.

Relates to elastic#108422, which included descriptions for other reserved roles.
parkertimmins pushed a commit to parkertimmins/elasticsearch that referenced this pull request May 17, 2024
…#108633)

This PR adds missing role description for the `transport_client role`, 
and a test to enforce that all reserved roles are described. 
The description also serves as self-documentation for roles, 
thus it is reasonable to make this a requirement for all reserved roles.

Relates to elastic#108422, which included descriptions for other reserved roles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Docs Meta label for docs team Team:Security Meta label for security team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants