Skip to content

[WIP] feat(triggers): schema-aware match conditions#7566

Open
dejanzele wants to merge 3 commits intomainfrom
feat/cluster-resources-discovery
Open

[WIP] feat(triggers): schema-aware match conditions#7566
dejanzele wants to merge 3 commits intomainfrom
feat/cluster-resources-discovery

Conversation

@dejanzele
Copy link
Copy Markdown
Contributor

@dejanzele dejanzele commented Apr 22, 2026

Summary

  • Pushes CRD OpenAPI schemas from agent to Control Plane and surfaces them in the UI as schema-aware autocomplete for TestTrigger.match[].path.
  • Validates each match-condition path against the canonical regex (MatchPathPattern) and rejects bracket suffixes ([*], [N]) loudly because the expression engine in pkg/expressions can't tokenize them yet — array-path support is a follow-up.
  • Adds the inventory push pipeline: gRPC client + controller that pushes on startup, on a 1h periodic tick, and on CRD informer events (debounced 5s to coalesce helm-install bursts).

Companion PR

Required: testkube-cloud-api PR — receives + caches + serves the snapshot, validates match[] against the schema, and ships the UI. cp-api's go.mod is pinned to this branch's HEAD; the two should land together.

Test plan

  • CI: go test ./pkg/triggers/... ./internal/inventory/... ./pkg/clusterdiscovery/...
  • Local: tilt agent connects, pushes a snapshot, CP receives it, UI shows the picker
  • Demo: build a match condition on Argo Rollout .status.phase, save, observe the trigger fire on a real Rollout transition
  • Type-aware FE validation: .spec.replicas (integer) rejects non-numeric value
  • Bracket path: typing .spec.containers[*].image shows the BE rejection on save

Known gaps (deferred)

  • Array-path matching: backend can't evaluate [*]/[N] yet (validator catches at save time; runtime no-op pinned by TestMatchOnCRDArrayPathsCurrentlyUnsupported)
  • Schemas for built-in K8s types (Pod, Deployment): only CRDs ship a schema today; built-ins fall back to manual path entry

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an agent-side autodiscovery endpoint to enumerate cluster resources (preferred-version GVKs) and annotate each with whether the agent ServiceAccount can list,watch it, for use in upcoming UI resourceRef selection.

Changes:

  • Introduces pkg/clusterdiscovery to list watchable API resources and compute canWatch via SelfSubjectRulesReview.
  • Exposes GET /v1/cluster-resources from the v1 API server and wires in the discoverer from cmd/api-server.
  • Adds unit tests for RBAC rule matching helpers (canWatch, hasVerb).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/clusterdiscovery/discovery.go New discovery + RBAC evaluation logic and response struct.
pkg/clusterdiscovery/discovery_test.go Unit tests for core permission-matching helpers.
internal/app/api/v1/server.go Registers the new /cluster-resources v1 route and adds optional discoverer field.
internal/app/api/v1/cluster_resources.go Implements the Fiber handler for listing cluster resources.
cmd/api-server/main.go Wires ClusterDiscoverer into the v1 API instance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +13 to +23
func (s *TestkubeAPI) ListClusterResourcesHandler() fiber.Handler {
return func(c *fiber.Ctx) error {
if s.ClusterDiscoverer == nil {
return s.Error(c, http.StatusNotImplemented, fmt.Errorf("cluster discovery is not configured on this instance"))
}
resources, err := s.ClusterDiscoverer.List(c.Context())
if err != nil {
return s.Error(c, http.StatusBadGateway, fmt.Errorf("cluster discovery: %w", err))
}
return c.JSON(resources)
}
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a new public API endpoint, but there are no handler-level tests covering the HTTP behavior (e.g., 501 when ClusterDiscoverer is nil, 502 on discovery error, and successful JSON response). Other handlers in this package have tests, so adding a small test file for this endpoint would help prevent regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +89
func (d *Discoverer) effectiveWatchRules(ctx context.Context) ([]authzv1.ResourceRule, error) {
review := &authzv1.SelfSubjectRulesReview{
Spec: authzv1.SelfSubjectRulesReviewSpec{Namespace: metav1.NamespaceDefault},
}
result, err := d.authz.AuthorizationV1().SelfSubjectRulesReviews().Create(ctx, review, metav1.CreateOptions{})
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SelfSubjectRulesReviewSpec.Namespace is hard-coded to metav1.NamespaceDefault, so canWatch results for namespaced resources may be incorrect whenever the agent ServiceAccount’s permissions differ between namespaces (e.g., when Testkube runs outside default). Consider plumbing the agent’s configured namespace into Discoverer (or the List call) and using that namespace for the SelfSubjectRulesReview, so the UI reflects the actual watchability in the namespace where triggers run.

Copilot uses AI. Check for mistakes.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR adds GET /v1/cluster-resources to the agent, enumerating every GVK exposed by the cluster and tagging each with canWatch (computed via SelfSubjectRulesReview) to back a planned UI dropdown for TestTrigger/WorkflowTrigger resource selection.

  • P1 — Status.Incomplete not checked: If the cluster's authorizer cannot enumerate all RBAC rules, SelfSubjectRulesReview sets Incomplete: true but the code silently proceeds with a partial rule set. Resources the agent can actually watch will appear as canWatch: false, causing the UI to surface phantom RBAC gaps.
  • P2 — Hardcoded "default" namespace: The SSRR is issued against metav1.NamespaceDefault rather than the agent's own namespace, so RoleBindings scoped to (e.g.) the testkube namespace are not reflected in the result.

Confidence Score: 4/5

Safe to merge with low risk, but the Incomplete flag omission can silently mislead the UI about RBAC permissions on clusters with certain authorizer configurations.

One P1 finding: SelfSubjectRulesReview.Status.Incomplete is never checked, which means on clusters using webhook or mixed authorizers the canWatch field can be systematically wrong. The endpoint is informational (UI dropdown) so no data loss occurs, but users would get incorrect guidance about RBAC gaps. The fix is a small targeted guard.

pkg/clusterdiscovery/discovery.go — effectiveWatchRules function needs Incomplete check and namespace parameter.

Important Files Changed

Filename Overview
pkg/clusterdiscovery/discovery.go Core discovery logic — Status.Incomplete from SelfSubjectRulesReview is never checked (can silently produce incorrect canWatch results), and the namespace is hardcoded to "default" rather than the agent's own namespace.
internal/app/api/v1/cluster_resources.go Thin handler — nil guard for ClusterDiscoverer returns 501, errors return 502, otherwise proxies to discovery.List. Correct.
internal/app/api/v1/server.go Adds ClusterDiscoverer field to TestkubeAPI and registers /cluster-resources route. Clean integration.
cmd/api-server/main.go Wires clusterdiscovery.New(clientset) into the API struct before Init(). Straightforward and correct.
pkg/clusterdiscovery/discovery_test.go Unit tests cover canWatch and hasVerb helpers including wildcard verbs, wildcard groups/resources, and negative cases. Good coverage of the pure logic.

Sequence Diagram

sequenceDiagram
    participant UI
    participant AgentAPI as Agent API /v1/cluster-resources
    participant Discoverer as clusterdiscovery.Discoverer
    participant K8sDiscovery as K8s Discovery API
    participant K8sAuthz as K8s AuthZ API

    UI->>AgentAPI: GET /cluster-resources
    AgentAPI->>Discoverer: List(ctx)
    Discoverer->>K8sDiscovery: ServerPreferredResources()
    K8sDiscovery-->>Discoverer: []APIResourceList (partial ok)
    Discoverer->>K8sAuthz: Create SelfSubjectRulesReview ns=default
    K8sAuthz-->>Discoverer: ResourceRules (Status.Incomplete unchecked)
    Discoverer->>Discoverer: filter subresources and non-watchable kinds
    Discoverer->>Discoverer: canWatch per GVK
    Discoverer-->>AgentAPI: []Resource with CanWatch
    AgentAPI-->>UI: 200 JSON array
Loading

Reviews (1): Last reviewed commit: "feat(agent): add GET /v1/cluster-resourc..." | Re-trigger Greptile

Comment on lines +85 to +93
func (d *Discoverer) effectiveWatchRules(ctx context.Context) ([]authzv1.ResourceRule, error) {
review := &authzv1.SelfSubjectRulesReview{
Spec: authzv1.SelfSubjectRulesReviewSpec{Namespace: metav1.NamespaceDefault},
}
result, err := d.authz.AuthorizationV1().SelfSubjectRulesReviews().Create(ctx, review, metav1.CreateOptions{})
if err != nil {
return nil, fmt.Errorf("self subject rules review: %w", err)
}
return result.Status.ResourceRules, nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Status.Incomplete silently ignored — canWatch can produce false negatives

SelfSubjectRulesReview sets Status.Incomplete = true when the authorizer cannot enumerate all rules (e.g. webhook authorizers, RBAC combined with Node authorizer edge cases). The code never inspects this field, so partial rule sets are treated as complete. In that scenario every resource whose RBAC rule was omitted will appear as canWatch: false in the UI, even though the agent actually has the permission. Users would then be told to add RBAC that they already have.

result, err := d.authz.AuthorizationV1().SelfSubjectRulesReviews().Create(ctx, review, metav1.CreateOptions{})
if err != nil {
    return nil, fmt.Errorf("self subject rules review: %w", err)
}
if result.Status.Incomplete {
    // Surface the incompleteness; callers can decide whether to trust the partial result.
    return nil, fmt.Errorf("self subject rules review is incomplete (authorizer cannot enumerate rules): %s", result.Status.EvaluationError)
}
return result.Status.ResourceRules, nil

Alternatively, expose Incomplete on the response so the UI can show a disclaimer rather than blocking the whole endpoint.

Comment on lines +86 to +88
review := &authzv1.SelfSubjectRulesReview{
Spec: authzv1.SelfSubjectRulesReviewSpec{Namespace: metav1.NamespaceDefault},
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Hardcoded default namespace for RBAC check may produce wrong canWatch results

SelfSubjectRulesReview is namespace-scoped: it returns what the caller can do in that namespace. Using metav1.NamespaceDefault ("default") is correct for ClusterRoleBindings (which apply everywhere), but it misses any RoleBindings the agent has in its own namespace (e.g. testkube). For namespace-scoped resources, permissions from a testkube-namespace RoleBinding would not appear in the result, causing those resources to show canWatch: false even if the agent has them.

Consider passing the agent's own namespace (available as the namespace config field) instead.

Suggested change
review := &authzv1.SelfSubjectRulesReview{
Spec: authzv1.SelfSubjectRulesReviewSpec{Namespace: metav1.NamespaceDefault},
}
review := &authzv1.SelfSubjectRulesReview{
Spec: authzv1.SelfSubjectRulesReviewSpec{Namespace: d.namespace},
}

@dejanzele dejanzele force-pushed the feat/cluster-resources-discovery branch 2 times, most recently from 0ee289a to 4ba2117 Compare April 22, 2026 22:23
@dejanzele dejanzele force-pushed the feat/cluster-resources-discovery branch from 4ba2117 to 17e9743 Compare April 27, 2026 23:38
@dejanzele dejanzele force-pushed the feat/cluster-resources-discovery branch from 17e9743 to 25eb15f Compare April 27, 2026 23:46
@dejanzele dejanzele changed the title feat(agent): add GET /v1/cluster-resources for UI autodiscovery [WIP] feat(triggers): schema-aware match conditions Apr 27, 2026
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.

2 participants