Skip to content

refactor: replace (GVK, String) with KubeResourceId - #267

Merged
drmorr0 merged 1 commit into
mainfrom
drmorr/kube-resource-id
Aug 20, 2026
Merged

refactor: replace (GVK, String) with KubeResourceId#267
drmorr0 merged 1 commit into
mainfrom
drmorr/kube-resource-id

Conversation

@drmorr0

@drmorr0 drmorr0 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description and Rationale

  • Replace all (GVK, String) tuples with an actual KubeResourceId struct: in feat!: update to versioned exported trace format #157 we started tracking GVKs in addition to namespaced-names for all kubernetes resources. These were just getting passed around as tuple values, here we replace those tuples with an actual struct, and update our custom resource trait to allow us to easily get the resource_id for a particular object. This should hopefully clean up some of our code (of particular note, we no longer need the format_gvk_name since this is an impl Display on the KubeResourceId type).

How

  • The complicated/ugly bit of this is implementing the SkResourceExt trait for all T: kubeResource types in addition to DynamicObjects, which require looking up the GVK information at runtime instead of compile time. I had to do some ugly trait juggling in resource.rs with claude's help. Otherwise, it was just a lot of find/replacing.

Test Steps

  • make test passes; some of the mock test fixtures were missing type data that was previously specified directly in the tests, and now has to be computed from the fixture instead, so I fixed those.

Other Notes

  • changes to index.rs are more-or-less getting completely rewritten in the next PR in this stack, so i'm not that worried about performance or any other implications here.

  • I added GVK, KubeResourceId, and SkResourceExt to the sk-core prelude, since it seems like we're using these in alot of places.

  • I'm hoping/expecting that things like .spec() can get added to SkResourceExt so that we can get rid of the dyn_obj_spec/dyn_obj_spec_mut functions in k8s/util.rs.

  • I certify that this PR does not contain any code that has been generated with GitHub Copilot or any other AI-based code generation tool, in accordance with this project's policies.

@drmorr0
drmorr0 force-pushed the drmorr/kube-resource-id branch from 9b67859 to 6280e4e Compare August 17, 2026 16:44
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.20280% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.33%. Comparing base (5e6d043) to head (eb9cb4e).

Files with missing lines Patch % Lines
sk-core/src/k8s/owners.rs 95.00% 1 Missing ⚠️
sk-core/src/k8s/resource.rs 97.82% 1 Missing ⚠️
sk-driver/src/mutation.rs 80.00% 1 Missing ⚠️
sk-tracer/src/store.rs 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #267      +/-   ##
==========================================
+ Coverage   79.12%   79.33%   +0.21%     
==========================================
  Files          61       61              
  Lines        3995     4002       +7     
==========================================
+ Hits         3161     3175      +14     
+ Misses        834      827       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@drmorr0
drmorr0 requested a review from ogorman89 August 17, 2026 16:52
@drmorr0
drmorr0 force-pushed the drmorr/kube-resource-id branch 2 times, most recently from 7f921b3 to 5938c61 Compare August 19, 2026 16:32
@drmorr0
drmorr0 force-pushed the drmorr/kube-resource-id branch from 5938c61 to eb9cb4e Compare August 20, 2026 17:07
@drmorr0
drmorr0 merged commit 558d809 into main Aug 20, 2026
7 checks passed
@drmorr0
drmorr0 deleted the drmorr/kube-resource-id branch August 20, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant