Repository navigation
fix(dashboard): drop the resource map ConfigMaps once the map is unreadable - #4789
Conversation
…adable React Query keeps the last successful list when a refetch fails. After the resource map became forbidden or went away, the hook went on parsing that old copy, so the ConfigMaps tab stayed with stale names, and a malformed map kept its old error, instead of the tab going away as it does when the map is unreadable from the start. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe ConfigMap hook ignores resource-map data after 403 or 404 errors. Tests cover the resulting ConfigMaps tab visibility and displayed content. ChangesConfigMap access error handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable issue remains in the supplied evidence; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
europrinter (@yankawai) LGTM. React Query keeps the last successful data after a failed refetch, so the stale names were real, and moving the 403/404 check in front of the parse is the smallest fix for it.
On a trial merge with current main the detail route tests pass, typecheck is clean and lint stays at the three existing warnings. With the hook reverted to main, all four new tests fail, so they pin the fix.
CI hasn't run on this head yet, every workflow is waiting for approval.
…ion ConfigMap to tenants (#4684) ## What this PR does Backport of #4612 to `release-1.6`, together with its follow-up #4789. All three commits are cherry-picked. Two of them needed their `ApplicationDetailPage.test.tsx` cases added by hand, because on this branch that file has none of the virtual machine tab tests they sit next to on main. FoundationDB keeps its connection string in a ConfigMap that tenants may read, and the console had no view for ConfigMaps. The application page gets a ConfigMaps tab for the ConfigMaps named in the application's resource map, with a copy button for each value. Each read uses an exact-name field selector, so no general ConfigMap access is needed. The FoundationDB chart now renders its resource map with monitoring disabled too, and tenant roles may read it. If the resource map stops being readable after a successful read, the tab goes away instead of keeping the old names. ### Validation - All 6 FoundationDB chart tests pass, and so do all 338 console tests in 52 files and the TypeScript check. - Tests cover finding the ConfigMaps through the resource map, reading only the named ones in the active namespace, copying a value, a failed copy, a denied read, malformed resource maps, and a resource map that answers 403 or 404 after a successful read. The four tests for the last case fail without the #4789 commit. ### Screenshots The tab is the same as in #4612, which has a screenshot of it. ### Downstream repositories Walked the trigger map against the diff. It changes a FoundationDB chart template and its test, and the console, with no values, schema, CRD or `hack/` file, and the console is vendored in this repository. No companion repository needs an update. - [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): backport FoundationDB connection ConfigMaps to release-1.6 ```
What this PR does
Follow-up to #4612.
The ConfigMaps tab takes its names from the application's resource map. React Query keeps the last successful list when a refetch fails, so when the resource map became forbidden or went away after a successful read,
useApplicationConfigMapswent on parsing the old copy. The tab stayed with the old names, every listed ConfigMap showed an access error, and a malformed map kept its old error. A map that is unreadable from the start already hid the tab.Now a 403 or 404 on the resource map drops the names and the error whether or not an earlier read succeeded, so the tab goes away in both cases. Any other error is still shown on the tab, as before.
The new tests are a page test with stale resource map data next to a 403, and hook tests that read the map and then refetch it with 403 and with 404, plus a malformed map followed by a 403. All four fail on main and pass here.
pnpm typecheck,pnpm test(451 tests in 62 files) andpnpm lintpass.Screenshots
No visual change. Once the resource map is unreadable, the page looks the same as for an application whose map was never readable, with no ConfigMaps tab. The tab itself is shown in #4612.
Downstream repositories
Walked the trigger map against the diff. It touches only the console hook that reads the resource map and its tests. No chart, values, schema, CRD or
hack/file changes, and the console is vendored in this repository.Release note
Summary by CodeRabbit