Remove deprecated operator status fields#127
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBumps package/chart to 0.7.9, removes deprecated PostgresPolicy status fields (planned_sql, planned_sql_truncated, last_reconcile_time), teaches CRDs that spec.schemas/roles/retirements are keyed map-lists, defaults omitted approval to Manual, and updates reconciler, tests, docs, samples, CI, and release notes. ChangesPostgresPolicy CRD Deprecation & Cleanup
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6399d53550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| version: 0.7.9 | ||
| appVersion: "0.7.9" |
There was a problem hiding this comment.
With the chart bumped to 0.7.9, a v0.7.9 tag will pass the Helm workflow's chart/tag check and the chart will default the operator image tag to .Chart.AppVersion, but the Rust workspace in Cargo.toml is still 0.7.8 and the release workflow publishes/builds from that version. In a 0.7.9 release this leaves the Helm chart pointing at 0.7.9 while the packaged binaries/crates still report or publish as 0.7.8, and cargo publish can fail if 0.7.8 is already published.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
CHANGELOG.md (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign changelog release header with bumped chart/app version.
The changelog entries added in this PR are under
0.7.8, butcharts/pgroles-operator/Chart.yamlis bumped to0.7.9. Please align the release number to avoid ambiguous release notes.🤖 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 `@CHANGELOG.md` at line 8, The changelog release header currently reads "## [0.7.8] - 2026-06-04" but the chart/app version was bumped to 0.7.9; update the header in CHANGELOG.md to "## [0.7.9] - 2026-06-04" (or the correct release date if different) so the CHANGELOG entry matches the bumped Chart version referenced in charts/pgroles-operator/Chart.yaml.
🧹 Nitpick comments (1)
crates/pgroles-operator/src/crd.rs (1)
3759-3774: ⚡ Quick winStrengthen the CRD map-list test with field-scoped assertions.
This test currently relies on global substring checks, so it can still pass even if one specific list (
schemas/roles/retirements) loses itsx-kubernetes-list-*metadata.Suggested refactor
#[test] fn postgres_policy_crd_marks_named_lists_as_maps() { let crd = PostgresPolicy::crd(); - let yaml = serde_yaml::to_string(&crd).expect("CRD should serialize to YAML"); - - for field_name in ["schemas", "roles"] { - assert!( - yaml.contains(&format!("{field_name}:\n")), - "CRD should contain spec.{field_name}" - ); - } - assert!(yaml.contains("x-kubernetes-list-type: map")); - assert!(yaml.contains("x-kubernetes-list-map-keys:")); - assert!(yaml.contains("- name")); - assert!(yaml.contains("- role")); + let v = serde_yaml::to_value(&crd).expect("CRD should serialize to value"); + let spec_props = &v["spec"]["versions"][0]["schema"]["openAPIV3Schema"]["properties"]["spec"]["properties"]; + + for (field, key) in [("schemas", "name"), ("roles", "name"), ("retirements", "role")] { + let schema = &spec_props[field]; + assert_eq!(schema["x-kubernetes-list-type"], "map", "spec.{field} should be map-list"); + assert_eq!(schema["x-kubernetes-list-map-keys"][0], key, "spec.{field} should be keyed by {key}"); + } }🤖 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 `@crates/pgroles-operator/src/crd.rs` around lines 3759 - 3774, The test postgres_policy_crd_marks_named_lists_as_maps uses global substring checks; change it to scope assertions to each field by locating the index of the field header produced by PostgresPolicy::crd() (e.g., find yaml.find(&format!("{field_name}:\n"))), then assert that the slice after that index contains "x-kubernetes-list-type: map" and "x-kubernetes-list-map-keys:", and also assert the expected map key for that field (for "schemas" assert the following slice contains "- name", for "roles" assert it contains "- role", and include any other named list like "retirements" with its expected key) so each named list is independently verified.
🤖 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 `@CHANGELOG.md`:
- Line 8: The changelog release header currently reads "## [0.7.8] - 2026-06-04"
but the chart/app version was bumped to 0.7.9; update the header in CHANGELOG.md
to "## [0.7.9] - 2026-06-04" (or the correct release date if different) so the
CHANGELOG entry matches the bumped Chart version referenced in
charts/pgroles-operator/Chart.yaml.
---
Nitpick comments:
In `@crates/pgroles-operator/src/crd.rs`:
- Around line 3759-3774: The test postgres_policy_crd_marks_named_lists_as_maps
uses global substring checks; change it to scope assertions to each field by
locating the index of the field header produced by PostgresPolicy::crd() (e.g.,
find yaml.find(&format!("{field_name}:\n"))), then assert that the slice after
that index contains "x-kubernetes-list-type: map" and
"x-kubernetes-list-map-keys:", and also assert the expected map key for that
field (for "schemas" assert the following slice contains "- name", for "roles"
assert it contains "- role", and include any other named list like "retirements"
with its expected key) so each named list is independently verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba00f5e5-113c-4e01-b0fc-7976cf99b6ff
📒 Files selected for processing (7)
CHANGELOG.mdcharts/pgroles-operator/Chart.yamlcharts/pgroles-operator/crds/postgrespolicies.pgroles.io.yamlcrates/pgroles-operator/src/crd.rscrates/pgroles-operator/src/reconciler.rsdocs/src/pages/docs/operator.mdk8s/crd.yaml
💤 Files with no reviewable changes (1)
- crates/pgroles-operator/src/reconciler.rs
1 participant
Summary
Closes #73
Closes #126
Validation
Summary by CodeRabbit
Added
approval: auto.Changed
spec.approvalis omitted.Removed
Chores