-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
[DOCS] Add missing description for transport_client
reserved role
#108633
Conversation
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.
Pinging @elastic/es-docs (Team:Docs) |
Pinging @elastic/es-security (Team:Security) |
"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." |
There was a problem hiding this comment.
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:
"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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
I think we still need to deprecate it. I'll leave the description as is and handle deprecation in a followup PR. |
…#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.
…#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.
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.