feat(columns + views): REV, kubectl-parity audit, k9s-style views config#271
Open
janosmiko wants to merge 22 commits into
Open
feat(columns + views): REV, kubectl-parity audit, k9s-style views config#271janosmiko wants to merge 22 commits into
janosmiko wants to merge 22 commits into
Conversation
- Deployment: fix int64 silent-drop on Up-to-date/Available; add Unavailable (suppressed when zero) - StatefulSet: add Up-to-date, Update Strategy, Current Revision, Update Revision - DaemonSet: add REV, Current, Up-to-date, Available, Misscheduled (suppressed when zero); add container images - ReplicaSet: add Desired, REV; add container images; signature updated to receive obj - Extract intFromMap helper for dual int64/float64 field reads
- Job: fix int64/float64 silent-drop on Completions/Succeeded/Failed; add Active (suppressed when zero), Duration (start→completion or now), Status (Complete/Failed/Suspended/Running) on ti.Status, REV from obj - CronJob: add Active (len of status.active, suppressed when empty), REV from obj; update both signatures to accept obj as first map arg - Add jobDuration and jobStatus helpers; update client_populate.go callers
- Flux (all kinds): Interval from spec.interval - Kustomization: Source (<kind>/<name>) and Path columns - GitRepository/HelmRepository/OCIRepository/Bucket: URL column - HelmChart: Chart column - Argo Application/ApplicationSet: Project from spec.project - Argo Application: Health (status.health.status) and Sync Status (status.sync.status) - cert-manager Certificate: DNS Names (joined) and Issuer from spec.issuerRef.name All columns conditional — emitted only when the underlying field is non-empty.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds numeric REV sorting and unit tests; stores raw Kubernetes objects on items and refactors populate helpers to accept the full object; extracts and emits REV, Progressing, workload/job/cronjob columns, HPA lastScaleTime and External metrics, Node Unschedulable, IngressClass Controller/Parameters, and PriorityClass Value/Preemption; adds Flux/Argo/cert-manager enrichments; implements a views system with JSONPath column specs, config bridging from resource_columns, runtime application of view columns, and comprehensive tests. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@internal/k8s/client_populate_misc.go`:
- Around line 26-31: The current code unconditionally appends a Parameters entry
to ti.Columns using scope/kind/name which can produce "//" placeholders when
those fields are empty; change the logic in the block that handles
spec["parameters"] (the params map) to only append a
model.KeyValue{Key:"Parameters", Value:...} when at least one of scope, kind or
name is non-empty, and construct the Value string by joining only the non-empty
parts (or otherwise skip adding the column entirely) so you never emit a pure
placeholder like "//".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c07f185b-8561-4ea5-bd26-69fa20bc4599
📒 Files selected for processing (14)
internal/app/tabs_compare.gointernal/app/tabs_compare_rev_test.gointernal/k8s/client_populate.gointernal/k8s/client_populate_argo.gointernal/k8s/client_populate_coverage_test.gointernal/k8s/client_populate_ext.gointernal/k8s/client_populate_ext_test.gointernal/k8s/client_populate_flux.gointernal/k8s/client_populate_gitops_enrichments_test.gointernal/k8s/client_populate_hpa.gointernal/k8s/client_populate_kinds.gointernal/k8s/client_populate_misc.gointernal/k8s/client_populate_workloads.gointernal/k8s/client_populate_workloads_rev_test.go
Wire the new views: config surface into YAML loading. Add Views fields to configFile and clusterConfig; populate ConfigViews and ConfigClusterViews in applyConfigMaps. Compile errors per view produce a logger.Warn and skip that entry — consistent with other invalid-config handling patterns in the codebase.
views supersedes resource_columns with per-resource column and sort configuration keyed by GVR or Kind. Documents column spec syntax, flag modifiers, sort direction defaults, and per-cluster overrides.
janosmiko
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/config-reference.md`:
- Around line 225-227: The fenced code block containing
"NAME[:.jsonpath][|flag]*" is unlabeled and triggers markdownlint MD040; update
that block to include a language identifier (e.g., change the opening fence from
``` to ```text) so it reads "```text" before the line "NAME[:.jsonpath][|flag]*"
to satisfy the linter.
In `@internal/app/views_apply_test.go`:
- Line 55: The tests call ui.BuildView(&ui.ConfigView{Columns:
[]string{"X:.foo"}}) and discard the returned error, which can mask parsing
regressions; update each test invocation (the calls that assign to v, _ :=
BuildView) to capture the error (e.g., v, err := ui.BuildView(...)) and
assert/fail on non-nil err (use t.Fatalf or equivalent/assert helper) before
using v so failures surface immediately; retain the same ui.ConfigView and
variable names (v, err) and apply the same change to all occurrences referenced
in the test file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97b5141f-3a81-47df-a527-2ad0fff5fc2d
📒 Files selected for processing (21)
docs/config-example.yamldocs/config-reference.mdinternal/app/app_sort.gointernal/app/app_sort_view_test.gointernal/app/update_bookmarks.gointernal/app/update_bookmarks_session.gointernal/app/update_navigation.gointernal/app/update_resources_loaded.gointernal/app/views_apply.gointernal/app/views_apply_test.gointernal/k8s/client_populate.gointernal/model/item_raw_test.gointernal/model/types.gointernal/ui/config_apply.gointernal/ui/config_load.gointernal/ui/config_views_bridge_test.gointernal/ui/config_views_load_test.gointernal/ui/views.gointernal/ui/views_jsonpath.gointernal/ui/views_jsonpath_test.gointernal/ui/views_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@internal/ui/views_columns_for_kind_test.go`:
- Around line 32-33: The test currently ignores the error returned by BuildView
which can hide parser regressions; update both occurrences (where v, _ :=
BuildView(&ConfigView{Columns: []string{"Name", "X:.foo"}}) and the similar call
at lines 45-46) to capture the error (e.g., v, err := BuildView(...)) and fail
the test immediately if err != nil (use t.Fatalf or require.NoError) before
assigning ConfigViews = map[string]*View{"pod": v}; this ensures BuildView
errors surface in the test for functions/types BuildView, ConfigView,
ConfigViews and View.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 104821d2-1527-4c56-a6fe-5361913f352f
📒 Files selected for processing (2)
internal/ui/config.gointernal/ui/views_columns_for_kind_test.go
Comment on lines
+32
to
+33
| v, _ := BuildView(&ConfigView{Columns: []string{"Name", "X:.foo"}}) | ||
| ConfigViews = map[string]*View{"pod": v} |
There was a problem hiding this comment.
Do not ignore BuildView errors in tests.
These calls currently discard err, which can mask parser regressions and make failures harder to diagnose. Fail fast when view construction fails.
Proposed fix
- v, _ := BuildView(&ConfigView{Columns: []string{"Name", "X:.foo"}})
+ v, err := BuildView(&ConfigView{Columns: []string{"Name", "X:.foo"}})
+ if err != nil {
+ t.Fatal(err)
+ }- globalView, _ := BuildView(&ConfigView{Columns: []string{"Name", "Age"}})
- prodView, _ := BuildView(&ConfigView{Columns: []string{"Name", "X:.foo", "Y:.bar"}})
+ globalView, err := BuildView(&ConfigView{Columns: []string{"Name", "Age"}})
+ if err != nil {
+ t.Fatal(err)
+ }
+ prodView, err := BuildView(&ConfigView{Columns: []string{"Name", "X:.foo", "Y:.bar"}})
+ if err != nil {
+ t.Fatal(err)
+ }Also applies to: 45-46
🤖 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 `@internal/ui/views_columns_for_kind_test.go` around lines 32 - 33, The test
currently ignores the error returned by BuildView which can hide parser
regressions; update both occurrences (where v, _ :=
BuildView(&ConfigView{Columns: []string{"Name", "X:.foo"}}) and the similar call
at lines 45-46) to capture the error (e.g., v, err := BuildView(...)) and fail
the test immediately if err != nil (use t.Fatalf or require.NoError) before
assigning ConfigViews = map[string]*View{"pod": v}; this ensures BuildView
errors surface in the test for functions/types BuildView, ConfigView,
ConfigViews and View.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes GitHub issue #262. Two phases shipped together on this branch.
Phase 1 — REV column + column audit sweep
REV column
metadata.resourceVersionon Deployment, StatefulSet, DaemonSet, ReplicaSet, Job, CronJob.case "REV"incomparePrimaryColumn).Audit-driven additions
Unavailable; fixed int64 silent-drop onUp-to-date/AvailableUp-to-date,Update Strategy,Current/Update RevisionCurrent,Up-to-date,Available,Misscheduled,Images,REVDesired,Images,REVActive,Duration,ti.Ready,ti.Status; int64 fix;REVActive,REVProgressing(only whengeneration > observedGeneration)Interval(all kinds);Source/Path(Kustomization);URL(GitRepo/HelmRepo/OCIRepo/Bucket);Chart(HelmChart)Health,Sync Status,ProjectProjectDNS Names,IssuerController,Parameters(suppressed when sub-fields empty — CodeRabbit fix)Value,Preemption PolicyLast Scale Time, External metric type (spec + status)UnschedulablePhase 2 — k9s-style
views:configA unified top-level
views:config (global + per-cluster) keyed by GVR primary, Kind fallback (case-insensitive).What it does
NAME:.jsonpathevaluated withk8s.io/client-go/util/jsonpath(kubectl-flavored).desc, everything else toascwhen colon omitted.|R(right-align),|T(humanize timestamp),|W(wide-only) — case-insensitive, stackable.Migration
resource_columns:continues to work — bridged intoviews:at config load with one deprecationlogger.Warn.views:wins when both define the same key.Error handling
Internal changes
model.Item.Rawretains the sourcemap[string]anyso JSONPath can evaluate against it on every refresh.internal/ui/:View,ResolvedColumn,ColumnSpec,ResourceRef,ParseColumnSpec,BuildView,ResolveView,EvalJSONPath,CompileJSONPath,EvalCompiled.updateResourcesLoadedMain), and five sort-seeding sites covering all real Kind-entry paths.Test plan
go test -race ./...green (16 commits, all TDD)golangci-lint run ./...cleanParameters: "//"placeholder suppressed)views:with IMAGE JSONPath column + REV:desc sort; verify column shows, sort persists across navigation, per-cluster override wins.Out of scope (potential Phase 3)
resource_columns(deprecation only).extends: pod).