Add file permissions for 'cpu,cpuacct' cgroup

[andrross]

This change is required to get CPU stat reporting to work on certain linux distributions.

Related Issues

Resolves #19120

Check List

• Functionality includes testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Summary by CodeRabbit

• Chores

  • Expanded system security permissions to allow broader read access to CPU-related cgroup paths, enabling more comprehensive resource monitoring.

• Documentation

  • Added a changelog entry documenting a fix for negative CPU usage values reported in node statistics.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Added three FilePermission entries to the security policy for additional cgroup read paths, and appended a changelog entry documenting a fix for negative CPU usage values in node stats.

Changes

Cohort / File(s)	Summary
Security Policy Permissions server/src/main/resources/org/opensearch/bootstrap/security.policy	Added three FilePermission entries granting read access to /sys/fs/cgroup/cpu,cpuacct, /sys/fs/cgroup/cpu,cpuacct/-, and /sys/fs/cgroup/cpuset/-.
Changelog CHANGELOG.md	Added a Fixed section entry documenting a fix addressing negative CPU usage values in node stats (_cat/nodes / cluster stats).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Verify the cgroup paths are correct and intended (including the combined cpu,cpuacct controller syntax).
• Confirm changelog entry text and placement match project style.

Poem

🐰
I hopped through trees of cgroup and root,
Found hidden reads and fixed the boot,
Three tiny hops, permissions bestowed,
Now CPU counts dance down the road,
— a rabbit's cheer for stats in tune.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding file permissions for the 'cpu,cpuacct' cgroup, which is the primary modification in the security.policy file.
Description check	✅ Passed	The description explains the purpose (CPU stat reporting on certain Linux distributions), references the linked issue (#19120), confirms testing was added, and includes the required DCO declaration.
Linked Issues check	✅ Passed	The PR adds file permissions for cpu,cpuacct cgroup paths to the security policy, directly addressing the root cause of negative CPU values in node stats reported in issue #19120.
Out of Scope Changes check	✅ Passed	All changes are in-scope: security.policy updates enable cgroup access, and CHANGELOG.md documents the fix. Both directly relate to resolving issue #19120.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

CHANGELOG.md (1)

93-93: LGTM! Clear documentation of the fix.

The changelog entry accurately documents the fix for negative CPU usage values.

Optional: For consistency with some other entries (e.g., line 91), consider adding # before the issue number:

-- Fix negative CPU usage values in node stats ([19120](https://github.com/opensearch-project/OpenSearch/issues/19120))
+- Fix negative CPU usage values in node stats ([#19120](https://github.com/opensearch-project/OpenSearch/issues/19120))

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d69581 and e8d64d6.

📒 Files selected for processing (2)

• CHANGELOG.md (1 hunks)
• server/src/main/resources/org/opensearch/bootstrap/security.policy (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: gradle-check
• GitHub Check: assemble (25, ubuntu-latest)
• GitHub Check: assemble (21, ubuntu-latest)
• GitHub Check: assemble (25, ubuntu-24.04-arm)
• GitHub Check: assemble (25, windows-latest)
• GitHub Check: assemble (21, windows-latest)
• GitHub Check: assemble (21, ubuntu-24.04-arm)
• GitHub Check: Analyze (java)
• GitHub Check: precommit (25, ubuntu-latest)
• GitHub Check: precommit (21, windows-2025, true)
• GitHub Check: precommit (25, ubuntu-24.04-arm)
• GitHub Check: precommit (21, windows-latest)
• GitHub Check: precommit (21, ubuntu-24.04-arm)
• GitHub Check: precommit (21, macos-15)
• GitHub Check: precommit (25, macos-15-intel)
• GitHub Check: precommit (21, ubuntu-latest)
• GitHub Check: precommit (21, macos-15-intel)
• GitHub Check: precommit (25, windows-latest)
• GitHub Check: precommit (25, macos-15)
• GitHub Check: detect-breaking-change

🔇 Additional comments (2)

server/src/main/resources/org/opensearch/bootstrap/security.policy (2)

245-246: LGTM! Essential permissions for cpu,cpuacct combined mount.

These permissions address the reported issue where CPU stats return -1 on distributions (like RHEL 8.10) that mount cpu and cpuacct together as "cpu,cpuacct" instead of separately. The pattern correctly follows the existing convention of granting both base directory and recursive read permissions.

---

247-247: The cpuset permission is correctly configured — base directory permission is not needed.

The review comment misunderstands Java FilePermission semantics. The pattern /sys/fs/cgroup/cpuset/- grants recursive read access to all files and subdirectories under cpuset, which is exactly what the JVM needs to read cpuset.cpus and cpuset.cpus.effective files.

The base permission /sys/fs/cgroup/cpuset (without the - suffix) grants read access only to the directory itself (metadata/listing), not to files within it. Since OpenSearch only needs the JVM to read specific cpuset files nested in subdirectories, the recursive form alone is both necessary and sufficient.

While other cgroup controllers (cpu, cpuacct) include both base and recursive permissions, this appears to be defensive consistency rather than a functional requirement—the recursive permission already covers the required access. No inconsistency or missing permission exists here.

[github-actions]

✅ Gradle check result for e8d64d6: SUCCESS