feat(helm): edit values when upgrading a release

Add an optional "Adjust your values" step to the Helm release upgrade
dialog. It is collapsed by default, so routine upgrades are unchanged
(pick a version, confirm). When expanded, it pre-fills an editor with the
release's current user-supplied values, lets the user edit them, preview a
manifest-level dry-run diff against the target chart version, and upgrade
with exactly those values.

Backend: extract loadTargetChart from upgradeWith and share it across the
upgrade and preview paths so both resolve the target version identically.
PreviewValuesChange and the upgrade stream accept an optional target
{version, repository}; the edit path applies values WYSIWYG (ResetValues)
while the untouched no-edit path keeps ResetThenReuseValues.

Frontend: reuse YamlEditor and ValuesDiffPreview (now portaled to body so
it layers over the dialog instead of being trapped by the drawer's
transformed ancestor). Invalid YAML disables the confirm action via a new
ConfirmDialog confirmDisabled prop, derived from the same parser used at
submit. Flux-managed releases never reach this dialog and are unaffected.

Closes #1113

Claude-Session: https://claude.ai/code/session_01SpbCXPyqThe723pBDTJDnn

Author: hisco

internal/helm/client.go

@@ -26,6 +26,7 @@ import (
 	"github.com/skyhook-io/radar/pkg/helmhistory"
 
 	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/registry"
@@ -2315,6 +2316,32 @@ func (c *Client) UpgradeAsUser(namespace, name, targetVersion, repositoryName st
 	return c.upgradeWith(actionConfig, name, targetVersion, repositoryName, noop)
 }
 
+// UpgradeWithValuesProgress upgrades a release to a target version applying the
+// supplied user values (WYSIWYG), reporting progress via a channel.
+func (c *Client) UpgradeWithValuesProgress(namespace, name, targetVersion, repositoryName string, newValues map[string]any, progressCh chan<- InstallProgress) error {
+	sendProgress := progressSender(progressCh)
+	sendProgress("preparing", fmt.Sprintf("Getting current release %s...", name), "")
+
+	actionConfig, err := c.getActionConfig(namespace)
+	if err != nil {
+		return err
+	}
+	return c.upgradeWithValues(actionConfig, name, targetVersion, repositoryName, newValues, sendProgress)
+}
+
+// UpgradeWithValuesProgressAsUser upgrades a release to a target version applying
+// the supplied user values with K8s impersonation and progress reporting.
+func (c *Client) UpgradeWithValuesProgressAsUser(namespace, name, targetVersion, repositoryName string, newValues map[string]any, username string, groups []string, progressCh chan<- InstallProgress) error {
+	sendProgress := progressSender(progressCh)
+	sendProgress("preparing", fmt.Sprintf("Getting current release %s...", name), "")
+
+	actionConfig, err := c.getActionConfigForUser(namespace, username, groups)
+	if err != nil {
+		return err
+	}
+	return c.upgradeWithValues(actionConfig, name, targetVersion, repositoryName, newValues, sendProgress)
+}
+
 func progressSender(progressCh chan<- InstallProgress) func(phase, message, detail string) {
 	return func(phase, message, detail string) {
 		if progressCh == nil {
@@ -2335,16 +2362,11 @@ func (c *Client) upgradeWith(actionConfig *action.Configuration, name, targetVer
 		return fmt.Errorf("failed to get current release: %w", err)
 	}
 
-	chartName := rel.Chart.Metadata.Name
-	sendProgress("resolving", fmt.Sprintf("Finding %s version %s in repositories...", chartName, targetVersion), "")
-
-	chartPath, resolvedRepo, err := c.resolveUpgradeChartPath(chartName, targetVersion, repositoryName, chartSourceHosts(rel.Chart.Metadata.Home, rel.Chart.Metadata.Sources))
+	targetChart, err := c.chartForUpgradeTarget(actionConfig, rel, targetVersion, repositoryName, sendProgress)
 	if err != nil {
 		return err
 	}
 
-	sendProgress("downloading", fmt.Sprintf("Downloading %s-%s from %s...", chartName, targetVersion, resolvedRepo), chartPath)
-
 	// Create upgrade action — don't use Wait=true because Radar already
 	// shows real-time resource status via SSE. Waiting blocks the dialog
 	// for minutes with zero feedback; users can monitor the rollout in the UI.
@@ -2357,49 +2379,105 @@ func (c *Client) upgradeWith(actionConfig *action.Configuration, name, targetVer
 	// for keys a newer chart added (a cross-version upgrade footgun).
 	upgradeAction.ResetThenReuseValues = true
 
+	sendProgress("upgrading", fmt.Sprintf("Applying %s %s...", rel.Chart.Metadata.Name, targetVersion), "")
+
+	// Run the upgrade
+	_, err = upgradeAction.Run(name, targetChart, rel.Config)
+	if err != nil {
+		return fmt.Errorf("upgrade failed: %w", err)
+	}
+
+	sendProgress("complete", fmt.Sprintf("Successfully upgraded %s to %s", name, targetVersion), "")
+	return nil
+}
+
+// upgradeWithValues upgrades a release to a target chart version applying exactly
+// the supplied user values (WYSIWYG — ResetValues, no merge with prior overrides).
+// The plain upgradeWith path keeps ResetThenReuseValues for the blind carry-over case.
+func (c *Client) upgradeWithValues(actionConfig *action.Configuration, name, targetVersion, repositoryName string, newValues map[string]any, sendProgress func(phase, message, detail string)) error {
+	getAction := action.NewGet(actionConfig)
+	rel, err := getAction.Run(name)
+	if err != nil {
+		return fmt.Errorf("failed to get current release: %w", err)
+	}
+
+	targetChart, err := c.chartForUpgradeTarget(actionConfig, rel, targetVersion, repositoryName, sendProgress)
+	if err != nil {
+		return err
+	}
+
+	upgradeAction := action.NewUpgrade(actionConfig)
+	upgradeAction.Namespace = rel.Namespace
+	upgradeAction.Timeout = 120 * time.Second
+	upgradeAction.ResetValues = true // WYSIWYG: apply only the edited values
+
+	sendProgress("upgrading", fmt.Sprintf("Applying %s %s...", rel.Chart.Metadata.Name, targetVersion), "")
+
+	_, err = upgradeAction.Run(name, targetChart, newValues)
+	if err != nil {
+		return fmt.Errorf("upgrade failed: %w", err)
+	}
+
+	sendProgress("complete", fmt.Sprintf("Successfully upgraded %s to %s", name, targetVersion), "")
+	return nil
+}
+
+func (c *Client) chartForUpgradeTarget(actionConfig *action.Configuration, rel *release.Release, targetVersion, repositoryName string, sendProgress func(phase, message, detail string)) (*chart.Chart, error) {
+	if targetVersion == "" || targetVersion == rel.Chart.Metadata.Version {
+		return rel.Chart, nil
+	}
+	return c.loadTargetChart(actionConfig, rel, targetVersion, repositoryName, sendProgress)
+}
+
+// loadTargetChart resolves, downloads and loads the chart for a target upgrade
+// version, refusing a silent chart-swap (the resolved source must publish the
+// same chart the release runs). Shared by the upgrade and preview paths so they
+// resolve the target version identically.
+func (c *Client) loadTargetChart(actionConfig *action.Configuration, rel *release.Release, targetVersion, repositoryName string, sendProgress func(phase, message, detail string)) (*chart.Chart, error) {
+	chartName := rel.Chart.Metadata.Name
+	sendProgress("resolving", fmt.Sprintf("Finding %s version %s in repositories...", chartName, targetVersion), "")
+
+	chartPath, resolvedRepo, err := c.resolveUpgradeChartPath(chartName, targetVersion, repositoryName, chartSourceHosts(rel.Chart.Metadata.Home, rel.Chart.Metadata.Sources))
+	if err != nil {
+		return nil, err
+	}
+
+	sendProgress("downloading", fmt.Sprintf("Downloading %s-%s from %s...", chartName, targetVersion, resolvedRepo), chartPath)
+
 	// Use ChartPathOptions to locate/download the chart
-	client := action.NewInstall(actionConfig)
-	client.Version = targetVersion
+	locate := action.NewInstall(actionConfig)
+	locate.Version = targetVersion
 
 	// OCI pulls need a registry client on the action; Radar's action config
 	// doesn't carry one by default. Wire it from the user's helm registry login.
 	if registry.IsOCI(chartPath) {
 		rc, err := c.newRegistryClientConcrete()
 		if err != nil {
-			return fmt.Errorf("failed to build OCI registry client: %w", err)
+			return nil, fmt.Errorf("failed to build OCI registry client: %w", err)
 		}
-		client.SetRegistryClient(rc)
+		locate.SetRegistryClient(rc)
 	}
 
-	cp, err := client.ChartPathOptions.LocateChart(chartPath, c.settings)
+	cp, err := locate.ChartPathOptions.LocateChart(chartPath, c.settings)
 	if err != nil {
-		return fmt.Errorf("failed to locate chart: %w", err)
+		return nil, fmt.Errorf("failed to locate chart: %w", err)
 	}
 
 	sendProgress("loading", "Loading chart...", cp)
 
-	chart, err := loader.Load(cp)
+	targetChart, err := loader.Load(cp)
 	if err != nil {
-		return fmt.Errorf("failed to load chart: %w", err)
+		return nil, fmt.Errorf("failed to load chart: %w", err)
 	}
 
 	// Refuse a silent chart-swap: the resolved source must publish the SAME chart
 	// the release runs, not merely a chart at the same version. Matters most for
 	// OCI prefix probing, where "<prefix>/<chartName>" is derived, not asserted.
-	if chart.Metadata != nil && chart.Metadata.Name != chartName {
-		return fmt.Errorf("resolved chart is %q but release %q runs chart %q — refusing to swap charts", chart.Metadata.Name, name, chartName)
-	}
-
-	sendProgress("upgrading", fmt.Sprintf("Applying %s %s...", chartName, targetVersion), "")
-
-	// Run the upgrade
-	_, err = upgradeAction.Run(name, chart, rel.Config)
-	if err != nil {
-		return fmt.Errorf("upgrade failed: %w", err)
+	if targetChart.Metadata != nil && targetChart.Metadata.Name != chartName {
+		return nil, fmt.Errorf("resolved chart is %q but release %q runs chart %q — refusing to swap charts", targetChart.Metadata.Name, rel.Name, chartName)
 	}
 
-	sendProgress("complete", fmt.Sprintf("Successfully upgraded %s to %s", name, targetVersion), "")
-	return nil
+	return targetChart, nil
 }
 
 type chartPathCandidate struct {
@@ -2689,13 +2767,26 @@ func (c *Client) batchCheckUpgrades(namespace, username string, groups []string)
 	return result, nil
 }
 
-// PreviewValuesChange previews the effect of new values on a release via dry-run
-func (c *Client) PreviewValuesChange(namespace, name string, newValues map[string]any) (*ValuesPreviewResponse, error) {
+// PreviewValuesChange previews the effect of new values on a release via dry-run.
+// When targetVersion is non-empty and differs from the running chart version, the
+// dry-run renders against that target chart instead of the current chart.
+func (c *Client) PreviewValuesChange(namespace, name string, newValues map[string]any, targetVersion, repositoryName string) (*ValuesPreviewResponse, error) {
 	actionConfig, err := c.getActionConfig(namespace)
 	if err != nil {
 		return nil, err
 	}
+	return c.previewValuesChangeWith(actionConfig, name, newValues, targetVersion, repositoryName)
+}
 
+func (c *Client) PreviewValuesChangeAsUser(namespace, name string, newValues map[string]any, targetVersion, repositoryName string, username string, groups []string) (*ValuesPreviewResponse, error) {
+	actionConfig, err := c.getActionConfigForUser(namespace, username, groups)
+	if err != nil {
+		return nil, err
+	}
+	return c.previewValuesChangeWith(actionConfig, name, newValues, targetVersion, repositoryName)
+}
+
+func (c *Client) previewValuesChangeWith(actionConfig *action.Configuration, name string, newValues map[string]any, targetVersion, repositoryName string) (*ValuesPreviewResponse, error) {
 	// Get the current release
 	getAction := action.NewGet(actionConfig)
 	rel, err := getAction.Run(name)
@@ -2713,6 +2804,12 @@ func (c *Client) PreviewValuesChange(namespace, name string, newValues map[strin
 	// Get current manifest
 	currentManifest := rel.Manifest
 
+	noop := func(phase, message, detail string) {}
+	previewChart, err := c.chartForUpgradeTarget(actionConfig, rel, targetVersion, repositoryName, noop)
+	if err != nil {
+		return nil, err
+	}
+
 	// Perform a dry-run upgrade with the new values
 	upgradeAction := action.NewUpgrade(actionConfig)
 	upgradeAction.Namespace = rel.Namespace
@@ -2721,7 +2818,7 @@ func (c *Client) PreviewValuesChange(namespace, name string, newValues map[strin
 	upgradeAction.ResetValues = true // Use only the provided values, don't merge
 
 	// Run the dry-run upgrade
-	newRel, err := upgradeAction.Run(name, rel.Chart, newValues)
+	newRel, err := upgradeAction.Run(name, previewChart, newValues)
 	if err != nil {
 		return nil, fmt.Errorf("failed to preview values change: %w", err)
 	}

internal/helm/client_test.go

@@ -637,6 +637,22 @@ func TestGetValuesWithAllValuesKeepsComputedWhenUserValuesReadFails(t *testing.T
 	}
 }
 
+func TestChartForUpgradeTargetReusesReleaseChartForSameVersion(t *testing.T) {
+	client := testHelmClientWithRepoConfigOnly(t)
+	rel := helmTestRelease("argo-cd", "demo", 1, release.StatusDeployed, "deployed")
+	rel.Chart.Metadata.Version = "9.5.11"
+
+	got, err := client.chartForUpgradeTarget(nil, rel, "9.5.11", "missing-repo", func(phase, message, detail string) {
+		t.Fatalf("same-version chart selection should not resolve/download chart, got progress %q %q %q", phase, message, detail)
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got != rel.Chart {
+		t.Fatal("chartForUpgradeTarget returned a different chart, want current release chart")
+	}
+}
+
 func TestDiffResourceRefs(t *testing.T) {
 	left := []ResourceRef{
 		{APIVersion: "apps/v1", Kind: "Deployment", Namespace: "demo", Name: "cart"},

internal/helm/handlers.go

@@ -2,6 +2,7 @@ package helm
 
 import (
 	"encoding/json"
+	"io"
 	"log"
 	"net/http"
 	"strconv"
@@ -32,6 +33,20 @@ func userCreds(r *http.Request) (string, []string) {
 	return "", nil
 }
 
+func decodeOptionalApplyValuesRequest(body io.Reader) (map[string]any, error) {
+	if body == nil {
+		return nil, nil
+	}
+	var req ApplyValuesRequest
+	if err := json.NewDecoder(body).Decode(&req); err != nil {
+		if err == io.EOF {
+			return nil, nil
+		}
+		return nil, err
+	}
+	return req.Values, nil
+}
+
 // Handlers provides HTTP handlers for Helm endpoints
 type Handlers struct {
 	// resolveNamespaces maps a request to the namespaces a Helm list should
@@ -717,6 +732,12 @@ func (h *Handlers) handleUpgradeStream(w http.ResponseWriter, r *http.Request) {
 	}
 	repositoryName := r.URL.Query().Get("repository")
 
+	editedValues, err := decodeOptionalApplyValuesRequest(r.Body)
+	if err != nil {
+		writeError(w, http.StatusBadRequest, "invalid request body: "+err.Error())
+		return
+	}
+
 	// Set up SSE headers
 	w.Header().Set("Content-Type", "text/event-stream")
 	w.Header().Set("Cache-Control", "no-cache")
@@ -732,9 +753,21 @@ func (h *Handlers) handleUpgradeStream(w http.ResponseWriter, r *http.Request) {
 	progressCh := make(chan InstallProgress, 10)
 	defer close(progressCh)
 
+	auth.AuditLog(r, namespace, name)
 	resultCh := make(chan error, 1)
 	go func() {
-		if user := auth.UserFromContext(r.Context()); user != nil {
+		user := auth.UserFromContext(r.Context())
+		// Use != nil, not len > 0: an explicit empty map ({}) means
+		// "clear all my overrides" and must NOT fall back to carry-over.
+		if editedValues != nil {
+			if user != nil {
+				resultCh <- client.UpgradeWithValuesProgressAsUser(namespace, name, version, repositoryName, editedValues, user.Username, user.Groups, progressCh)
+				return
+			}
+			resultCh <- client.UpgradeWithValuesProgress(namespace, name, version, repositoryName, editedValues, progressCh)
+			return
+		}
+		if user != nil {
 			resultCh <- client.UpgradeWithProgressAsUser(namespace, name, version, repositoryName, user.Username, user.Groups, progressCh)
 			return
 		}
@@ -806,7 +839,8 @@ func (h *Handlers) handlePreviewValues(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	preview, err := client.PreviewValuesChange(namespace, name, req.Values)
+	username, groups := userCreds(r)
+	preview, err := client.PreviewValuesChangeAsUser(namespace, name, req.Values, req.Version, req.Repository, username, groups)
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, err.Error())
 		return

internal/helm/handlers_test.go

@@ -3,8 +3,11 @@ package helm
 import (
 	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/skyhook-io/radar/internal/auth"
@@ -83,6 +86,48 @@ func TestRequireCloudRole(t *testing.T) {
 	}
 }
 
+func TestDecodeOptionalApplyValuesRequest(t *testing.T) {
+	cases := []struct {
+		name    string
+		body    string
+		hasBody bool
+		want    map[string]any
+		wantErr bool
+	}{
+		{name: "nil body"},
+		{name: "empty body", hasBody: true},
+		{name: "explicit empty values stays non nil", body: `{"values":{}}`, hasBody: true, want: map[string]any{}},
+		{name: "populated values", body: `{"values":{"replicaCount":2}}`, hasBody: true, want: map[string]any{"replicaCount": float64(2)}},
+		{name: "invalid json", body: `{"values":`, hasBody: true, wantErr: true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var body io.Reader
+			if tc.hasBody {
+				body = strings.NewReader(tc.body)
+			}
+
+			got, err := decodeOptionalApplyValuesRequest(body)
+			if tc.wantErr {
+				if err == nil {
+					t.Fatal("expected error")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("decodeOptionalApplyValuesRequest returned error: %v", err)
+			}
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Fatalf("values = %#v, want %#v", got, tc.want)
+			}
+			if tc.want != nil && got == nil {
+				t.Fatal("values = nil, want explicit non-nil map")
+			}
+		})
+	}
+}
+
 // TestSensitiveHelmHandlers_GateOnViewer asserts that every Helm
 // handler we believe is gated actually 403s a Cloud viewer with
 // error_code=cloud_role_insufficient. The unit test above