Skip to content

Openapi: adding privileges #4434

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

Merged
merged 7 commits into from
Jun 9, 2025
Merged

Openapi: adding privileges #4434

merged 7 commits into from
Jun 9, 2025

Conversation

l-trotta
Copy link
Contributor

Closes #4303.

There was no desired output in the original issue, so I'm not sure if this is the format expected. Let me know and I'll change it!

@l-trotta l-trotta requested a review from lcawl May 30, 2025 16:07
@l-trotta l-trotta requested a review from a team as a code owner May 30, 2025 16:07
@l-trotta l-trotta added the skip-backport This pull request should not be backported label May 30, 2025
@l-trotta l-trotta marked this pull request as draft May 30, 2025 16:09
@l-trotta
Copy link
Contributor Author

validation failing because this adds a new field to the openapi output, will be fixed in case that's how we want to add the information

@lcawl
Copy link
Contributor

lcawl commented Jun 3, 2025

My two cents is that of the three options I mentioned in #4303 (comment), "an addendum tacked onto the beginning or end of the operation's "description" field" is likely the best current option since it aligns with what Kibana's doing. Otherwise I think if we went the extension route we'd need a more open-ended structure since the privileges in the Kibana and Cloud API docs would need different options than "cluster-privileges" and "index-privileges".

@georgewallace
Copy link

Coming into this one a little late but I do agree with @lcawl if we already have a team doing this a different way then we should follow suit to stay consistent. I do think long term keeping these as separate fields makes the most sense though. @lcawl is this what you are talking about?
Screenshot 2025-06-03 at 8 37 59 PM

@l-trotta l-trotta force-pushed the privileges-in-openapi branch from d42aa76 to 813a7d3 Compare June 5, 2025 16:24
@l-trotta
Copy link
Contributor Author

l-trotta commented Jun 5, 2025

@lcawl @georgewallace updated so that privileges are now in the description! please help me with the output format because I'm really not a fan of this one ^^"

@georgewallace
Copy link

@lcawl curious your thoughts but if we follow suite with what Kibana is doing would be something like

[Required authorization] index-privilege: monitor, cluster-privilege: monitor

@lcawl
Copy link
Contributor

lcawl commented Jun 5, 2025

Could we format it into more complete phrases? For example:

## Required authorization

 * Index privileges: `blah`, `blah`, `blah`
 * Cluster privileges: `blah`

Or if there's concern about including markdown, then maybe:

Required authorization includes: `blah`, `blah`, `blah` index privileges and `blah` cluster privileges.

@georgewallace
Copy link

@lcawl I like the first one. It doesnt necessarily match with what Kibana is doing (Get an agent action status) but I think that is ok as your first one likes much better. The Operation summary name is already an H2, for this sake should it be an h3 instead? I see inconsistencies on the pages now so H2 should be fine but just in case.

@l-trotta l-trotta force-pushed the privileges-in-openapi branch from 0c34ce9 to 3a5d0ce Compare June 6, 2025 10:29
@l-trotta l-trotta marked this pull request as ready for review June 6, 2025 10:29
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Left some code-improvement suggestions.

@l-trotta l-trotta force-pushed the privileges-in-openapi branch from 3a5d0ce to b07c082 Compare June 6, 2025 14:46
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM

@l-trotta l-trotta force-pushed the privileges-in-openapi branch from b07c082 to bb4f1f4 Compare June 9, 2025 08:37
@l-trotta l-trotta merged commit 5096167 into main Jun 9, 2025
8 checks passed
@l-trotta l-trotta deleted the privileges-in-openapi branch June 9, 2025 08:42
Copy link
Contributor

The backport to 9.0 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.0 9.0
# Navigate to the new working tree
cd .worktrees/backport-9.0
# Create a new branch
git switch --create backport-4434-to-9.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 509616749de15d76a72c4ae8937c7faaeb34292d
# Push it to GitHub
git push --set-upstream origin backport-4434-to-9.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.0

Then, create a pull request where the base branch is 9.0 and the compare/head branch is backport-4434-to-9.0.

Copy link
Contributor

The backport to 8.19 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.19 8.19
# Navigate to the new working tree
cd .worktrees/backport-8.19
# Create a new branch
git switch --create backport-4434-to-8.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 509616749de15d76a72c4ae8937c7faaeb34292d
# Push it to GitHub
git push --set-upstream origin backport-4434-to-8.19
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.19

Then, create a pull request where the base branch is 8.19 and the compare/head branch is backport-4434-to-8.19.

Copy link
Contributor

The backport to 8.18 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.18 8.18
# Navigate to the new working tree
cd .worktrees/backport-8.18
# Create a new branch
git switch --create backport-4434-to-8.18
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 509616749de15d76a72c4ae8937c7faaeb34292d
# Push it to GitHub
git push --set-upstream origin backport-4434-to-8.18
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.18

Then, create a pull request where the base branch is 8.18 and the compare/head branch is backport-4434-to-8.18.

l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
l-trotta added a commit that referenced this pull request Jun 11, 2025
* adding privileges to openapi export

* privileges in description

* update output format

* rebase

* restore recent changes, better rusting

* rebase
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.

Security privileges are missing from Elasticsearch API documentation
4 participants