feat(auth): configure Cryostat to hook in to OpenShift RBAC for fine-grained permissions - #337
andrewazores wants to merge 2 commits into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe chart adds namespace-scoped OpenShift RBAC configuration. It updates OAuth proxy access-review defaults and configures RBAC mode and access-token forwarding based on authentication settings. Helm tests cover the supported combinations. ChangesOpenShift authentication configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/cryostat/templates/cryostat_deployment.yaml (1)
189-201: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAlign the chart with an application version that supports these RBAC variables.
The default
quay.io/cryostat/cryostat:4.3.0-snapshotimage has no Cryostat implementation for these variables. They cannot selectPERMISSIVEor cluster-wide access reviews. Update the application image and contract together, then test the deployed behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/cryostat/templates/cryostat_deployment.yaml` around lines 189 - 201, The chart’s RBAC environment variables require an application image version that implements their contract. Update the default Cryostat image reference to a compatible version, ensure the RBAC mode and namespace-scoping values in the deployment template match that application’s supported behavior, and verify the rendered/deployed chart supports permissive and cluster-wide access-review configurations.
🧹 Nitpick comments (1)
charts/cryostat/tests/cryostat_deployment_test.yaml (1)
1560-1577: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd coverage for the changed access-review defaults.
These tests cover access-token forwarding only. Add an OpenShift case that verifies the rendered proxy arguments use
resource=pods, an emptysubresource, andverb=get, and do not use the oldcreate pods/execcombination.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/cryostat/tests/cryostat_deployment_test.yaml` around lines 1560 - 1577, Add a test alongside the existing OpenShift auth-proxy cases that enables OpenShift authentication and asserts the rendered cryostat-authproxy arguments use resource=pods, an empty subresource, and verb=get, while confirming the old create pods/exec combination is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@charts/cryostat/templates/cryostat_deployment.yaml`:
- Around line 189-201: The chart’s RBAC environment variables require an
application image version that implements their contract. Update the default
Cryostat image reference to a compatible version, ensure the RBAC mode and
namespace-scoping values in the deployment template match that application’s
supported behavior, and verify the rendered/deployed chart supports permissive
and cluster-wide access-review configurations.
---
Nitpick comments:
In `@charts/cryostat/tests/cryostat_deployment_test.yaml`:
- Around line 1560-1577: Add a test alongside the existing OpenShift auth-proxy
cases that enables OpenShift authentication and asserts the rendered
cryostat-authproxy arguments use resource=pods, an empty subresource, and
verb=get, while confirming the old create pods/exec combination is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 8a2bb271-0ca3-4dfd-9055-dfba7ac57702
📒 Files selected for processing (5)
charts/cryostat/README.mdcharts/cryostat/templates/cryostat_deployment.yamlcharts/cryostat/tests/cryostat_deployment_test.yamlcharts/cryostat/values.schema.jsoncharts/cryostat/values.yaml
4bbd943 to
977a60a
Compare
Related to cryostatio/cryostat#1733
Depends on cryostatio/cryostat#1734
See cryostatio/cryostat-operator#1428
Configures the Cryostat container for
OPENSHIFTRBAC mode (ie perform additional fine-grained access control checks - see linked Cryostat issue and PR) when OpenShift SSO integration is enabled and Basic auth is not enabled, orBASICRBAC mode (ie assume proxy has authenticated and authorized user, just enforce that the incoming request is being received from the proxy) when auth proxy Basic/htpasswd mode is enabled, or else allow the defaultPERMISSIVERBAC mode (ie accept all requests).The
openshift-oauth-proxyAccess Review/Token Review is dropped fromcreate pods/execto simplyget pods, which is a much lower bar - however, Cryostat itself will expect users to havecreate pods/execpermissions, so in the default configuration the effective permissions required of a user do not change. Users can customize Cryostat's RBAC permissions using the Helm chart's extra env vars configuration to selectively map Cryostat API resources to different k8s RBAC roles as needed.How to manually test:
quay.io/andrewazores/cryostat:4.3.0-fine-grained-permissions-5as the core image.cryostatandapps1.cryostatnamespace with OpenShift OAuth proxy enabled:crchere, log out from the admin account (or use an incognito/private browsing window) and log in as the developer user. The developer should have no access to Cryostat.oc adm policy add-role-to-user -n cryostat view developerto grant the developer read access to the Cryostat installation namespace. The developer should now be able to log in to Cryostat, but should not be able to actually do anything as they are only passing the basic auth proxy access check but failing all fine-grained RBAC checks.core.config.extra.envVars:pods/exec:createis the default mapping so these permission mappings are redundant but included for clarity.pods:getis a lower privilege which the developer'sviewrole grants.oc adm policy add-role-to-user -n cryostat admin developer. Cryostat caches access review decisions for 1 minute, so wait up to 1 minute after invoking this. The developer user should now be able to perform any Cryostat action.oc adm policy remove-role-from-user -n cryostat admin developer, wait another 1 minute, and the developer user should now be back to readonly privileges.