Prevent MatchingActiveLeases selector from shadowing label selector

[NickCao]

Fixes-Issue: #164

Summary by CodeRabbit

• Refactor

  • Standardized lease filtering to use explicit label selectors across controllers and services for more consistent lease listing.
  • Consolidated list/filtering logic and aligned error handling with the new selector flow.

• Chores

  • Updated end-to-end workflow to use a branch-specific e2e runner and pass the PR base ref into the job.

[coderabbitai]

Walkthrough

Refactors MatchingActiveLeases to accept and return a labels.Selector (instead of producing a client.ListOption), and updates callers to wrap the returned selector with client.MatchingLabelsSelector or place it in ListOptions.LabelSelector. Imports updated where needed across controllers, services, and e2e workflow reference changed.

Changes

Cohort / File(s)	Summary of Changes
Selector API refactor internal/controller/lease.go	Changed MatchingActiveLeases signature to func(prev labels.Selector) labels.Selector; now augments and returns the provided selector instead of producing a client.ListOption.
Controller callers update internal/controller/exporter_controller.go, internal/controller/lease_controller.go	Replaced MatchingActiveLeases() usage with client.MatchingLabelsSelector{Selector: MatchingActiveLeases(labels.Everything())}; added k8s.io/apimachinery/pkg/labels import where needed.
Service/client callers update internal/service/client/v1/client_service.go, internal/service/controller_service.go	Updated List options to compose selectors via controller.MatchingActiveLeases(...); moved the resulting selector into ListOptions.LabelSelector or wrapped it with client.MatchingLabelsSelector; adjusted imports and error path accordingly.
CI workflow .github/workflows/e2e.yaml	Switched e2e action reference to jumpstarter-dev/jumpstarter-e2e@lease-selector and added jumpstarter-ref: ${{ github.event.pull_request.base.ref }} input.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller as Service/Controller
  participant Selector as MatchingActiveLeases
  participant K8s as Kubernetes API

  Note over Caller: Compose selector and list active leases
  Caller->>Selector: MatchingActiveLeases(labels.Everything())
  Selector-->>Caller: labels.Selector (augmented with active-lease requirement)
  Caller->>K8s: List(... LabelSelector=selector ...)
  K8s-->>Caller: LeaseList (filtered)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Add Lease related methods to client service #115 — Related changes to MatchingActiveLeases signature and its callers.
• Testing/Linting release-* stable branches #133 — Related changes to the e2e workflow usage of the PR base branch / jumpstarter-ref.

Suggested labels

backport release-0.6

Suggested reviewers

• mangelajo
• bennyz

Poem

I twitch my whiskers and tweak a rule,
I stitch selectors from a leafy pool.
From Everything() I hop, then bind,
Only active leases shall I find.
Hopping through code — tidy and spry, 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the primary change—preventing the MatchingActiveLeases helper from overriding an existing label selector—which accurately reflects the core update in this pull request.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch lease-selector

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[mangelajo]

but isn't this going to filter on the labels of the lease itself? should we label the leases with our selector too?

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

.github/workflows/e2e.yaml (1)

19-21: Make jumpstarter-ref robust for workflow_dispatch.

${{ github.event.pull_request.base.ref }} is undefined on manual runs. Provide a sane fallback (e.g., github.base_ref or main) to avoid empty input.

       with:
         controller-ref: ${{ github.ref }}
         # use the matching branch on the jumpstarter repo
-        jumpstarter-ref: ${{ github.event.pull_request.base.ref }}
+        jumpstarter-ref: ${{ github.event.pull_request.base.ref || github.base_ref || 'main' }}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 51d8101 and 8885f21.

📒 Files selected for processing (1)

• .github/workflows/e2e.yaml (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: deploy-kind
• GitHub Check: tests
• GitHub Check: e2e-tests (ubuntu-24.04-arm)
• GitHub Check: e2e-tests (ubuntu-24.04)
• GitHub Check: e2e-tests-28d6b1cc3b49ab9ae176918ab9709a2e2522c97e
• GitHub Check: lint-go

🔇 Additional comments (1)

.github/workflows/e2e.yaml (1)

17-17: Pin jumpstarter-e2e Action to fixed commit SHA.

-      - uses: jumpstarter-dev/jumpstarter-e2e@lease-selector
+      - uses: jumpstarter-dev/jumpstarter-e2e@28bdf0aa84c0f08b86e2d6daaf8ac29b11f769ba