Repository navigation
feat(components): dynamic k8s crd status detector and jsonpath query engine - #581
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new StatusDetector component in pkg/components/bridge to dynamically inspect third-party Kubernetes CRD statuses using a dynamic client and JSONPath evaluation. Feedback highlights a critical issue where the default readiness check and associated test cases utilize JSONPath filter expressions (e.g., [?()]), which are unsupported by k8s.io/client-go/util/jsonpath and will cause runtime parsing errors. It is recommended to programmatically extract the 'Ready' condition to ensure compatibility and robustness.
1b05302 to
9d4b7d4
Compare
88682a7 to
6f2c010
Compare
6f2c010 to
59d22aa
Compare
59d22aa to
68647e8
Compare
32e6262 to
673ed63
Compare
5f14960 to
5b862b4
Compare
5b862b4 to
49008e4
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a dynamic third-party status detector and JSONPath evaluator for XMC components in pkg/components/bridge/. The feedback highlights a potential typed-nil interface panic and performance overhead in EvaluateJSONPath due to repeated JSONPath parsing, missing support for json.Number in extractObservedGeneration, and a redundant client call in DetectStatus.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a dynamic third-party status detector in pkg/components/bridge to query Kubernetes custom resources and evaluate their readiness, endpoints, and connection secrets using JSONPath. The review feedback identifies several critical issues: first, jsonpath.JSONPath is not thread-safe, meaning the current caching mechanism will cause data races under concurrent execution; second, ResolveGVR needs to track whether a resource is namespaced to prevent querying cluster-scoped resources within a namespace; and finally, ToBridgeWrapperRef is missing mappings for custom JSONPath configuration fields from the protobuf definition.
7f3bc62 to
a1f7922
Compare
4a005e1 to
b4e0918
Compare
6eaf903 to
c61e464
Compare
c61e464 to
01f4e15
Compare
01f4e15 to
aebc8d1
Compare
aebc8d1 to
2f1d727
Compare
There was a problem hiding this comment.
🤖 Incremental PR Approval
PR author is a code owner and changes since last approval (4 lines) are within the 10-line grace delta.
Preserving prior approval by @janzak-dev on commit 4f52966.
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
…engine Implement production K8sStatusDetector using client-go dynamic client and deferred discovery RESTMapper. Adds automatic GroupVersionResource (GVR) resolution, normalized JSONPath query evaluation, status condition heuristics, and resilient missing-CRD handling. TAG=agy
…r handling, candidates, and test fixtures TAG=agy
…, and enforce tolerant staleness guard
…ohibit secret status probing TAG=agy
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit e74b2a0.
Summary
Implements the dynamic Kubernetes CRD status detector engine and JSONPath query evaluation engine for third-party custom resources in XMC (Layer 6, stacked on top of PR #708).
This PR builds directly upon the
StatusDetectorcontract established in PR #708 by introducing the productionK8sStatusDetectorimplementation usingk8s.io/client-go/dynamicandk8s.io/client-go/restmapper. It provides automatic GroupVersionResource (GVR) mapping, resilient handling for uninstalled CRDs (meta.IsNoMatchError), normalized JSONPath evaluation (EvaluateJSONPath), and status condition heuristics (status.conditions[type=Ready]).Proposed Changes
Concrete Dynamic K8s Status Detector (
K8sStatusDetector) (pkg/components/bridge/k8s_status_detector.go):StatusDetectorinterface using client-godynamic.Interfaceandmeta.RESTMapper.ResolveGVRwith fallback tometa.UnsafeGuessKindToResource.Found=False, Ready=Falsewith descriptive messages when a CRD is uninstalled or when a custom resource does not exist (apierrors.IsNotFound).Normalized JSONPath Query Engine (
EvaluateJSONPath) (pkg/components/bridge/k8s_status_detector.go):NormalizeJSONPathandEvaluateJSONPathto safely query unstructured objects.{.status.conditions[?(@.type=="Ready")].status},status.phase,$[0]).deriveSiblingMessagePath): Automatically inspects sibling.messagefields on failing condition paths.Status Extraction & Readiness Heuristics (
ExtractResourceStatus) (pkg/components/bridge/k8s_status_detector.go):Running,Ready,Active).Daemon Controller Dynamic Wiring (
cmd/xmc_bridge/main.go):discoveryClientandrestmapper.NewDeferredDiscoveryRESTMapper.bridge.NewK8sStatusDetector(dynamicClient, mapper)intoBridgeProber.Unit & Dynamic Fake Testing (
pkg/components/bridge/k8s_status_detector_test.go):cmp.Diff(want, got)comparisons.Key Contracts & Invariants
Assumptions & Scope Boundary
Verification & Testing
bazel test //cmd/xmc_bridge/... //pkg/components/bridge/...passed cleanly.bazel test --@io_bazel_rules_go//go/config:race //pkg/components/bridge:bridge_test //cmd/xmc_bridge:xmc_bridge_testpassed (0 race conditions).buildifier,gofmt,golangci-lint,addlicense).TAG=agy
CONV=fa5e1d3a-e399-45eb-a73c-ef8413122857