Repository navigation
fix(foundationdb): let tenant roles read the connection ConfigMap - #4148
Aleksei Sviridkin (lexfrei) merged 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe FoundationDB dashboard resourcemap now includes the release ConfigMap and tenant read-only RBAC resources. A Helm test suite validates the rendered resources, RBAC subjects, monitoring behavior, and document counts. ChangesFoundationDB dashboard access
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change lets tenant users discover and read the release FoundationDB connection ConfigMap with narrowly scoped read-only access. The rendered RBAC and resource-map behavior are covered by Helm tests, with no remaining actionable merge risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
746dd97 to
f72f2d0
Compare
scooby87
left a comment
There was a problem hiding this comment.
LGTM (independent cozy-review pass).
RBAC scope is correct and minimal: namespace-scoped Role, resourceNames pinned to {{ .Release.Name }}-config, verbs limited to get/list/watch, bound at the use access level via the standard tenant helper. No wildcards, no cross-tenant leak. It is a 1:1 copy of the established postgres/clickhouse dashboard-resources pattern.
Verified: helm unittest 6/6; Role and RoleBinding render unconditionally (outside the monitoring guard), which is right since connection config must not depend on monitoring. Phase 5b clean — additive RBAC only, no name collisions on upgrade.
The operator stores the cluster file in the <release>-config ConfigMap, but the dashboard resource map did not list it and no RBAC granted tenant subjects access to it, so a tenant could not retrieve the connection string for its own FoundationDB at all. Add the ConfigMap to the dashboard resource map and create a Role/RoleBinding scoped to exactly that ConfigMap (get/list/watch) for the tenant use-level subjects. Add helm unittest coverage. Signed-off-by: Yan Bondarenko <[email protected]>
f72f2d0 to
9ee2a23
Compare
|
Successfully created backport PR for |
#4612) ## What this PR does Fixes #4611. Since #4148 a tenant may read the `<release>-config` ConfigMap that holds the FoundationDB cluster file, but the console has no view for ConfigMaps, so the connection string was still reachable only with kubectl. Chart: - `<release>-resourcemap` is rendered regardless of `monitoring.enabled`, because the console now reads it; - the tenant Role grants `get`, `list` and `watch` on `<release>-resourcemap` next to `<release>-config`, still by `resourceNames`, with no wider ConfigMap access. Console: - the application page gets a ConfigMaps tab when the application's resource map lists at least one ConfigMap in the same namespace; - each listed ConfigMap is read and watched with a `metadata.name` field selector, which RBAC checks against `resourceNames`, so a tenant never needs `list` on all ConfigMaps; - values are shown as plain text with a copy button, the cluster file needs no decoding; - a resource map that is missing or forbidden hides the tab, a listed ConfigMap that cannot be read shows the API error instead of looking empty, and a malformed map or an invalid name is reported. The tab is generic, but today only FoundationDB ships a resource map, so it is the only application that gets it. #3490 keeps `monitoring.enabled` as the switch for the resource map and asks whether the map should render unconditionally. This PR answers that for FoundationDB by rendering it always, so the wording in #3490 would need to follow if both land. Tests: `helm unittest` for the chart (6 tests), `tsc --noEmit`, `vitest run` (438 tests in 60 files) and `eslint` on the changed files pass. The new tests cover reading only the listed names in the active namespace, the watch, copying, a clipboard failure, a read denial, and malformed maps. The chart renders with `monitoring.enabled` true and false. ### Screenshots The ConfigMaps tab of a FoundationDB application, rendered by the console from this branch against a local stub API with synthetic data:  ### Downstream repositories Walked the trigger map against the diff: it changes a FoundationDB chart template and its test, and the console. No `values.yaml`, `values.schema.json`, README, CRD or `hack/` file is touched. The website FoundationDB page is generated from the chart README and says nothing about connecting, and the Terraform provider does not read these ConfigMaps. The console is vendored in this repository. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(dashboard, foundationdb): the application page shows the FoundationDB connection ConfigMap with the cluster file to tenants who may read it ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application details now include a ConfigMaps tab when configuration maps are available. View their values and copy them to the clipboard. * The tab displays loading, empty, unavailable, and error states. * **Bug Fixes** * The resource map remains available when monitoring is disabled, and its access permissions are included. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
A tenant cannot connect to its own FoundationDB: the fdb-kubernetes-operator writes the cluster file into the
<release>-configConfigMap, but tenant subjects have no RBAC to read it and the dashboard resource map does not expose it, so there is no way to obtain the connection string without cluster-admin access.This PR:
<release>-configto the dashboard resource map;useaccess level (via the existing cozy-lib helper);make test, 6 tests).Out of scope: FoundationDB currently exposes no Service, so clients still need in-cluster reachability to the coordinator pod IPs from the cluster file — that is a separate topic.
Summary by CodeRabbit