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

Fix and unmute OperatorPrivilegesIT #117218

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Nov 21, 2024

This updates the constants for OperatorPrivilegesIT to include the registered actions that were missing and unmutes the test

Resolves: #102992

This updates the constants for `OperatorPrivilegesIT` to include
the registered actions that were missing and unmutes the test
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v9.0.0 v8.16.2 v8.17.1 labels Nov 21, 2024
@tvernum tvernum requested review from masseyke, dnhatn and a team November 21, 2024 03:33
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Nov 21, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@tvernum
Copy link
Contributor Author

tvernum commented Nov 21, 2024

@masseyke and @dnhatn It looks like actions that you created slipped in while the test was muted.

Can you verify that none of them are supposed to be operator only?

@@ -358,6 +358,7 @@ public class Constants {
"cluster:monitor/nodes/data_tier_usage",
"cluster:monitor/nodes/features",
"cluster:monitor/nodes/hot_threads",
"cluster:monitor/nodes/index_mode_stats",
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This action seems to be only existing in 8.17.+ versions. We should not be backporting it to 8.16.2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for cluster:monitor/xpack/info/logsdb and cluster:monitor/xpack/usage/logsdb.

@@ -488,6 +491,7 @@ public class Constants {
"indices:admin/block/add[s]",
"indices:admin/cache/clear",
"indices:admin/data_stream/lazy_rollover",
"indices:admin/data_stream/reindex",
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this one seems to be only added in 9.0 with a backport pending to 8.18+ (#116780).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah just added yesterday. The backport to 8.18 is coming soon (#117251).

Copy link
Member

Choose a reason for hiding this comment

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

And it will not be backported to 8.17 or earlier.

Copy link
Contributor

@slobodanadamovic slobodanadamovic left a comment

Choose a reason for hiding this comment

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

LGTM (with a note on backport labels)

Thanks for handling this! 👍

@masseyke
Copy link
Member

@masseyke and @dnhatn It looks like actions that you created slipped in while the test was muted.

Can you verify that none of them are supposed to be operator only?

Mine (the data stream reindex one) is not operator-only.

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Tim!

@tvernum
Copy link
Contributor Author

tvernum commented Nov 21, 2024

(with a note on backport labels)

Yeah, it's muted on various branches, and failing on others, so I think I'm going to need to go through each one and work out what action is needed.

@tvernum tvernum merged commit eff0c42 into elastic:main Nov 22, 2024
21 checks passed
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Nov 22, 2024
This updates the constants for `OperatorPrivilegesIT` to include
the registered actions that were missing and unmutes the test

Backport of: elastic#117218
tvernum added a commit that referenced this pull request Nov 22, 2024
This updates the constants for `OperatorPrivilegesIT` to include
the registered actions that were missing and unmutes the test

Backport of: #117218
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Nov 22, 2024
This updates the constants for `OperatorPrivilegesIT` to include
the registered actions that were missing and unmutes the test
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
This updates the constants for `OperatorPrivilegesIT` to include
the registered actions that were missing and unmutes the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v8.16.2 v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] OperatorPrivilegesIT testEveryActionIsEitherOperatorOnlyOrNonOperator failing
5 participants