-
Notifications
You must be signed in to change notification settings - Fork 138
GitOps drift: detection completeness + Argo CD API integration + Settings redesign #1138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nadaverell
wants to merge
27
commits into
main
Choose a base branch
from
feature/argo-api-diff
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
8096d83
GitOps drift detection + Argo CD API deep diff
nadaverell e994a35
Drift diff: descend into arrays instead of dumping raw JSON
nadaverell 030848e
ignoreDifferences note: legible for operators, detail for experts
nadaverell 15e78d9
GitOps Resources row: vertically center the status/action cells
nadaverell 3a4a2f6
Settings: two-pane sidebar nav + honest per-section apply semantics
nadaverell 0617c0d
Settings: render My permissions inline, not as a launcher to another …
nadaverell 696433e
Remove stray session scratch files from the branch
nadaverell 5577506
Settings: use Tooltip instead of native title= on nav items
nadaverell 35cdf74
Review fixes: integration-field ownership, array-index ignore rules, …
nadaverell f29c484
Harden Argo CD credential model and fix CodeQL log-injection
nadaverell 83c420c
Settings: add Overview landing tab + clearer integration copy
nadaverell 9c33042
Settings: fix the dialog to a constant height
nadaverell f5edaf4
Settings: consistent tab layout + stronger Argo CD auth copy
nadaverell 24b9ce8
GitOps: show Git commit metadata for the deployed revision (Argo API)
nadaverell fb67519
GitOps: flag a broken Argo CD repo connection as an issue
nadaverell ae07134
GitOps repo health: carry Argo's raw connection error for diagnosis
nadaverell a9ea424
Merge origin/main into feature/argo-api-diff
nadaverell 4a4cc02
Argo: clearer context-mismatch error + normalize '_' namespace
nadaverell a98fb3b
Argo: persist the auto-discovery token's context binding
nadaverell e47bb03
Settings: honest footer + clearer server-launch copy
nadaverell bbab5e5
Settings: detect the Argo CD CLI session and offer it, don't ask
nadaverell b1cc62d
Argo: don't offer a loopback CLI session (port-forward artifact)
nadaverell 260ad86
GitOps: a detail page's own root node isn't a portal to itself
nadaverell f3357d1
GitOps: collapse a ComparisonError's derivative noise into one diagnosis
nadaverell 4e9240a
Merge remote-tracking branch 'origin/main' into feature/argo-api-diff
nadaverell 562629c
Argo: keep the cross-cluster guard intact on failed-connect rollback …
nadaverell 951918a
GitOps: only offer the Argo diff when it can actually be served, inli…
nadaverell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,254 @@ | ||
| package argocd | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "log" | ||
| "sort" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/client-go/kubernetes" | ||
|
|
||
| pfpkg "github.com/skyhook-io/radar/pkg/portforward" | ||
| ) | ||
|
|
||
| const serverLabelSelector = "app.kubernetes.io/name=argocd-server" | ||
|
|
||
| type candidate struct { | ||
| namespace string | ||
| name string | ||
| scheme string | ||
| port int | ||
| targetPort int | ||
| } | ||
|
|
||
| func (c candidate) clusterURL() string { | ||
| return fmt.Sprintf("%s://%s.%s.svc:%d", c.scheme, c.name, c.namespace, c.port) | ||
| } | ||
|
|
||
| // discoverCandidates lists Services labeled app.kubernetes.io/name=argocd-server | ||
| // across all namespaces. Sorted with the conventional "argocd" namespace first | ||
| // so multi-install clusters probe the default install before exotic ones. | ||
| func discoverCandidates(ctx context.Context, client kubernetes.Interface) ([]candidate, error) { | ||
| svcs, err := client.CoreV1().Services(metav1.NamespaceAll).List(ctx, metav1.ListOptions{ | ||
| LabelSelector: serverLabelSelector, | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var out []candidate | ||
| for i := range svcs.Items { | ||
| if c, ok := pickPort(svcs.Items[i]); ok { | ||
| out = append(out, c) | ||
| } | ||
| } | ||
| sort.Slice(out, func(i, j int) bool { | ||
| if (out[i].namespace == "argocd") != (out[j].namespace == "argocd") { | ||
| return out[i].namespace == "argocd" | ||
| } | ||
| if out[i].namespace != out[j].namespace { | ||
| return out[i].namespace < out[j].namespace | ||
| } | ||
| return out[i].name < out[j].name | ||
| }) | ||
| return out, nil | ||
| } | ||
|
|
||
| // pickPort selects the service port to target: https (443) preferred, http | ||
| // (80) fallback, else the first declared port. | ||
| func pickPort(svc corev1.Service) (candidate, bool) { | ||
| var httpsPort, httpPort *corev1.ServicePort | ||
| for i := range svc.Spec.Ports { | ||
| p := &svc.Spec.Ports[i] | ||
| switch { | ||
| case p.Port == 443 || strings.EqualFold(p.Name, "https"): | ||
| if httpsPort == nil { | ||
| httpsPort = p | ||
| } | ||
| case p.Port == 80 || strings.EqualFold(p.Name, "http"): | ||
| if httpPort == nil { | ||
| httpPort = p | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pick := httpsPort | ||
| scheme := "https" | ||
| if pick == nil { | ||
| pick = httpPort | ||
| scheme = "http" | ||
| } | ||
| if pick == nil { | ||
| if len(svc.Spec.Ports) == 0 { | ||
| return candidate{}, false | ||
| } | ||
| pick = &svc.Spec.Ports[0] | ||
| scheme = "http" | ||
| } | ||
|
|
||
| return candidate{ | ||
| namespace: svc.Namespace, | ||
| name: svc.Name, | ||
| scheme: scheme, | ||
| port: int(pick.Port), | ||
| targetPort: resolveTargetPort(pick), | ||
| }, true | ||
| } | ||
|
|
||
| // resolveTargetPort returns the container port for port-forwarding, which | ||
| // bypasses the Service (e.g., service:443 → container:8080). | ||
| func resolveTargetPort(p *corev1.ServicePort) int { | ||
| if p.TargetPort.IntVal > 0 { | ||
| return int(p.TargetPort.IntVal) | ||
| } | ||
| return int(p.Port) | ||
| } | ||
|
|
||
| // discover finds a reachable Argo CD API server: | ||
| // 1. Every candidate at its in-cluster Service address (works in-cluster, or | ||
| // when the user's machine can route to cluster DNS). | ||
| // 2. Out-of-cluster only: port-forward candidates in priority order, probing | ||
| // https then http on the forwarded port (argocd-server serves both on one | ||
| // port via cmux). | ||
| // | ||
| // The port-forward is owned by this package rather than internal/portforward: | ||
| // that package manages a single shared forward for the metrics stack | ||
| // (Prometheus/traffic), and starting an argocd-server forward through it | ||
| // would tear down the active metrics forward. | ||
| func (m *Manager) discover(ctx context.Context, snap probeSnapshot) (string, error) { | ||
| // The client is captured in the snapshot, NOT read live — so a context | ||
| // switch that swaps the live client mid-probe can't redirect this token's | ||
| // discovery at a different cluster's argocd-server. | ||
| k8sc := snap.k8sClient | ||
| if k8sc == nil { | ||
| return "", fmt.Errorf("%w: no Kubernetes client available for discovery", ErrUnreachable) | ||
| } | ||
|
|
||
| cands, err := discoverCandidates(ctx, k8sc) | ||
| if err != nil { | ||
| return "", fmt.Errorf("%w: listing argocd-server services: %v", ErrUnreachable, err) | ||
| } | ||
| if len(cands) == 0 { | ||
| return "", fmt.Errorf("%w: no Service labeled %s found in cluster — set an explicit Argo CD URL", ErrUnreachable, serverLabelSelector) | ||
| } | ||
|
|
||
| for _, cand := range cands { | ||
| addr := cand.clusterURL() | ||
| if m.probeEndpoint(ctx, addr, snap) == nil { | ||
| log.Printf("[argocd] Discovered Argo CD at %s (service %s/%s)", addr, cand.namespace, cand.name) | ||
| return addr, nil | ||
| } | ||
| } | ||
|
|
||
| if m.inCluster() { | ||
| return "", fmt.Errorf("%w: argocd-server service found but not reachable in-cluster (if it serves a self-signed certificate, set argoCdInsecureTls)", ErrUnreachable) | ||
| } | ||
|
|
||
| var lastErr error | ||
| for _, cand := range cands { | ||
| log.Printf("[argocd] No candidate reachable in-cluster, starting port-forward to %s/%s...", cand.namespace, cand.name) | ||
| fwd, pfErr := m.startPortForward(ctx, snap, cand.namespace, cand.name, cand.targetPort) | ||
| if pfErr != nil { | ||
| lastErr = fmt.Errorf("port-forward to %s/%s failed: %w", cand.namespace, cand.name, pfErr) | ||
| continue | ||
| } | ||
|
|
||
| connected := "" | ||
| for _, scheme := range []string{"https", "http"} { | ||
| addr := fmt.Sprintf("%s://localhost:%d", scheme, fwd.localPort) | ||
| if m.probeEndpoint(ctx, addr, snap) == nil { | ||
| connected = addr | ||
| break | ||
| } | ||
| } | ||
| if connected != "" { | ||
| m.mu.Lock() | ||
| if m.staleLocked(snap) { | ||
| // A config change (SetConfig/Reset) superseded this probe while | ||
| // we were forwarding. Don't install the forward — the caller | ||
| // will discard the whole result; leaving it would dangle a | ||
| // forward to a target the user has moved away from. | ||
| m.mu.Unlock() | ||
| fwd.stop() | ||
| return "", errStaleProbe | ||
| } | ||
| old := m.forward | ||
| m.forward = fwd | ||
| m.mu.Unlock() | ||
| if old != nil { | ||
| old.stop() | ||
| } | ||
| log.Printf("[argocd] Connected to %s/%s via port-forward at %s", cand.namespace, cand.name, connected) | ||
| return connected, nil | ||
| } | ||
|
|
||
| fwd.stop() | ||
| lastErr = fmt.Errorf("argocd-server at %s/%s not responding after port-forward (if it serves a self-signed certificate, set argoCdInsecureTls)", cand.namespace, cand.name) | ||
| } | ||
|
|
||
| return "", fmt.Errorf("%w: %v", ErrUnreachable, lastErr) | ||
| } | ||
|
|
||
| type activeForward struct { | ||
| localPort int | ||
| stopCh chan struct{} | ||
| cancel context.CancelFunc | ||
| stopOnce sync.Once | ||
| } | ||
|
|
||
| func (f *activeForward) stop() { | ||
| f.stopOnce.Do(func() { | ||
| f.cancel() | ||
| close(f.stopCh) | ||
| }) | ||
| } | ||
|
|
||
| func (m *Manager) startPortForward(ctx context.Context, snap probeSnapshot, namespace, service string, targetPort int) (*activeForward, error) { | ||
| // Use the snapshot's client/config (frozen at probe start), not the live | ||
| // ones — otherwise a context switch mid-probe would dial the forward through | ||
| // the NEW cluster, sending this probe's token to a different cluster's Argo. | ||
| client := snap.k8sClient | ||
| cfg := snap.k8sConfig | ||
| if client == nil || cfg == nil { | ||
| return nil, errors.New("kubernetes client not initialized") | ||
| } | ||
|
|
||
| podName, err := pfpkg.FindPodForService(ctx, client, namespace, service) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("find pod for service %s/%s: %w", namespace, service, err) | ||
| } | ||
| localPort, err := pfpkg.FindFreePort() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("find free port: %w", err) | ||
| } | ||
|
|
||
| stopCh := make(chan struct{}) | ||
| readyCh := make(chan struct{}) | ||
| pfCtx, cancel := context.WithCancel(context.Background()) | ||
| fwd := &activeForward{localPort: localPort, stopCh: stopCh, cancel: cancel} | ||
|
|
||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| errCh <- pfpkg.RunPortForward(pfCtx, client, cfg, namespace, podName, localPort, targetPort, stopCh, readyCh) | ||
| }() | ||
|
|
||
| select { | ||
| case <-readyCh: | ||
| log.Printf("[argocd] Port-forward ready: localhost:%d -> %s/%s:%d", localPort, namespace, service, targetPort) | ||
| return fwd, nil | ||
| case err := <-errCh: | ||
| fwd.stop() | ||
| return nil, fmt.Errorf("port-forward failed: %w", err) | ||
| case <-time.After(10 * time.Second): | ||
| fwd.stop() | ||
| return nil, errors.New("port-forward timed out") | ||
| case <-ctx.Done(): | ||
| fwd.stop() | ||
| return nil, ctx.Err() | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Named Service targetPort mis-forwarded
Low Severity
Out-of-cluster discovery port-forwards using
resolveTargetPort, which falls back to the Service port whenTargetPortis a name (not an integer). Forwarding the Service port onto the pod often fails, so auto-discovery can incorrectly report Argo CD unreachable.Reviewed by Cursor Bugbot for commit 8096d83. Configure here.