Feat: implement valuesFrom support for helmchart component and update… (#7099)

* feat: implement valuesFrom support for helmchart component and update documentation examples Signed-off-by: Anaswara Suresh <anaswarasuresh2212@gmail.com> * fix: address cubic review feedback on valuesFrom Three issues raised by cubic AI review on kubevela#7099: 1. docs/examples/helmchart-valuesfrom/secret-and-inline.yaml — Expected-result comments used incorrect paths (resources.cpu / resources.mem) and values (500m) that did not match the actual CM data. Rewrote the narrative to use the real paths (resources.limits.cpu / resources.limits.memory) and bundled the Secret inline in the manifest so the example is self-contained and the expected output is deterministic. 2. docs/examples/helmchart-valuesfrom/secret-and-inline.yaml — The Secret was marked optional: true while the narrative required it for the merged output to match. Bundled the Secret inline and dropped the optional flag, removing the order/timing ambiguity. README.md updated to drop the now-redundant "create the Secret first" instruction. 3. pkg/cue/cuex/providers/helm/helm.go:Render — After removing the unconditional "default" fallback for releaseNamespace, Helm could run with an empty namespace when both Context.AppNamespace and Release.Namespace were unset (non-normal code paths — direct callers, tests, CLI tooling). Restored the "default" fallback at the end of the namespace resolution while keeping the Application-namespace plumbing for tenant-scoped cross-namespace rejection. Co-authored-by: Ayush Kumar <ayushshyamkumar888@gmail.com> Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> Signed-off-by: Anaswara Suresh <anaswarasuresh2212@gmail.com> * feat: add valuesFrom fingerprinting for helmchart components to trigger workflow restarts on ConfigMap/Secret changes Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * feat: add valuesFrom fingerprinting for helmchart components to trigger workflow restarts on ConfigMap/Secret changes Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * feat: enhance valuesFrom support for helmchart components with fingerprinting and error handling Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * docs: clarify cross-namespace restrictions for valuesFrom in helmchart examples Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * ci: retrigger checks Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * feat: add publishVersion support to Helm provider for stable release management Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * feat: improve error handling for application retrieval in Helm provider Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * ci: retrigger checks Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * feat: add application publishVersion lookup defense in Helm provider tests Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> --------- Signed-off-by: Anaswara Suresh <anaswarasuresh2212@gmail.com> Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> Co-authored-by: Anaswara Suresh <anaswarasuresh2212@gmail.com>
2026-08-18 03:56:36 +00:00 · 2026-04-28 20:02:58 -07:00
parent 2268d95c1c
commit 28892ccc7e
12 changed files with 2858 additions and 128 deletions
@@ -1,5 +1,5 @@
 /*
-revertCopyright 2026 The KubeVela Authors.
+Copyright 2026 The KubeVela Authors.

 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -39,17 +39,23 @@ import (
 	"helm.sh/helm/v3/pkg/registry"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/repo"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	kyaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"

 	"github.com/kubevela/pkg/cue/cuex/providers"
 	cuexruntime "github.com/kubevela/pkg/cue/cuex/runtime"
 	"github.com/kubevela/pkg/util/runtime"
+	"github.com/kubevela/pkg/util/singleton"

+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/utils"
 	"github.com/oam-dev/kubevela/pkg/utils/common"
 )
@@ -100,14 +106,12 @@ type ReleaseParams struct {
 	Namespace string `json:"namespace,omitempty"`
 }

-// ValuesFromParams represents a values source
+// ValuesFromParams represents a values source.
 type ValuesFromParams struct {
 	Kind      string `json:"kind"`
 	Name      string `json:"name"`
 	Namespace string `json:"namespace,omitempty"`
 	Key       string `json:"key,omitempty"`
-	URL       string `json:"url,omitempty"`
-	Tag       string `json:"tag,omitempty"`
 	Optional  bool   `json:"optional,omitempty"`
 }

@@ -165,6 +169,12 @@ type ContextParams struct {
 	AppNamespace string `json:"appNamespace"`
 	Name         string `json:"name"`      // component name
 	Namespace    string `json:"namespace"` // component namespace
+	// PublishVersion is the value of the Application's app.oam.dev/publishVersion
+	// annotation, if any. When set, the provider records it as a label on the
+	// helm release so subsequent reconciles can short-circuit when the pin is
+	// stable. Populated by Render() via an Application lookup; not part of the
+	// CUE-passed context shape.
+	PublishVersion string `json:"-"`
 }

 // velaContextStr returns a human-readable prefix like "app=myapp/default component=web"
@@ -538,53 +548,211 @@ func (p *Provider) fetchRepoChart(ctx context.Context, params *ChartSourceParams
 	return ch, nil
 }

-// mergeValues merges values from multiple sources
-func (p *Provider) mergeValues(ctx context.Context, baseValues interface{}, valuesFrom []ValuesFromParams) (map[string]interface{}, error) {
-	// Start with base values
-	result := make(map[string]interface{})
+// defaultValuesKey is the key looked up in a ConfigMap/Secret when the user
+// does not specify one explicitly. Matches the FluxCD and Helm CLI convention.
+const defaultValuesKey = "values.yaml"

-	if baseValues != nil {
-		if m, ok := baseValues.(map[string]interface{}); ok {
-			result = m
-		}
-	}
-
-	// Merge values from each source
-	for _, source := range valuesFrom {
-		values, err := p.loadValuesFromSource(ctx, source)
-		if err != nil {
-			if source.Optional {
-				klog.V(4).Infof("Skipping optional values source %s/%s: %v", source.Kind, source.Name, err)
-				continue
-			}
-			return nil, errors.Wrapf(err, "failed to load values from %s/%s", source.Kind, source.Name)
-		}
-
-		// Merge values
-		result = chartutil.CoalesceTables(result, values)
-	}
-
-	return result, nil
+// valueSourceMissingError is returned by loaders when a ConfigMap/Secret or the
+// requested key inside it does not exist. mergeValues uses this sentinel type to
+// decide whether source.Optional allows the source to be skipped. Parse errors
+// and other failures produce different error types, so Optional never swallows
+// them — a common source of silent misconfiguration bugs.
+type valueSourceMissingError struct {
+	kind, name, namespace, key string
+	cause                      error
 }

-// loadValuesFromSource loads values from a specific source
-// nolint:unparam // result is always nil until ConfigMap/Secret/OCI loading is implemented
-func (p *Provider) loadValuesFromSource(_ context.Context, source ValuesFromParams) (map[string]interface{}, error) {
+func (e *valueSourceMissingError) Error() string {
+	if e.key != "" {
+		return fmt.Sprintf("%s %s/%s key %q not found: %v", e.kind, e.namespace, e.name, e.key, e.cause)
+	}
+	return fmt.Sprintf("%s %s/%s not found: %v", e.kind, e.namespace, e.name, e.cause)
+}
+
+func (e *valueSourceMissingError) Unwrap() error { return e.cause }
+
+func isValueSourceMissing(err error) bool {
+	var target *valueSourceMissingError
+	return stderrors.As(err, &target)
+}
+
+// errCrossNamespaceValuesFrom is returned when a valuesFrom source references a
+// namespace other than the Application's own namespace. The controller has
+// cluster-scoped read on ConfigMaps/Secrets, so without this guard a tenant could
+// read Secrets from any namespace by submitting a crafted Application.
+var errCrossNamespaceValuesFrom = stderrors.New("cross-namespace valuesFrom sources are not permitted")
+
+// mergeValues merges inline `values` and any `valuesFrom` sources into a single
+// map. Priority (highest wins): inline > valuesFrom[N] > valuesFrom[N-1] > ... >
+// valuesFrom[0]. Later entries override earlier ones. The merge is a deep-merge
+// of map keys via chartutil.CoalesceTables; arrays are replaced wholesale (not
+// concatenated), and `null` values are preserved (not treated as delete), so
+// semantics diverge slightly from `helm CLI --values a.yaml --values b.yaml`
+// which uses chartutil.CoalesceValues.
+//
+// A valuesFrom entry that omits `namespace` resolves to releaseNamespace (the
+// natural co-location with the chart's deployed resources). An entry that sets
+// an explicit Namespace is only allowed if it matches either releaseNamespace
+// or appNamespace; any other namespace is rejected to block cross-tenant reads
+// via the controller's cluster-wide RBAC.
+func (p *Provider) mergeValues(ctx context.Context, baseValues interface{}, valuesFrom []ValuesFromParams, appNamespace, releaseNamespace string) (map[string]interface{}, error) {
+	accumulated := map[string]interface{}{}
+
+	for _, source := range valuesFrom {
+		if err := ctx.Err(); err != nil {
+			return nil, err
+		}
+		values, err := p.loadValuesFromSource(ctx, source, appNamespace, releaseNamespace)
+		if err != nil {
+			if source.Optional && isValueSourceMissing(err) {
+				klog.V(2).Infof("Helm provider: skipping optional values source %s %q: %v", source.Kind, source.Name, err)
+				continue
+			}
+			return nil, errors.Wrapf(err, "failed to load values from %s %q", source.Kind, source.Name)
+		}
+		// CoalesceTables(dst, src) treats dst as authoritative. `values` is the
+		// newer source, so it's passed as dst to override `accumulated` (older).
+		accumulated = chartutil.CoalesceTables(values, accumulated)
+	}
+
+	// Inline values override everything from valuesFrom. Clone before merging
+	// because CoalesceTables mutates dst in place, and dst here is the caller's
+	// map (renderParams.Values).
+	if inline, ok := baseValues.(map[string]interface{}); ok {
+		clone := make(map[string]interface{}, len(inline))
+		for k, v := range inline {
+			clone[k] = v
+		}
+		accumulated = chartutil.CoalesceTables(clone, accumulated)
+	}
+
+	return accumulated, nil
+}
+
+// loadValuesFromSource dispatches to the appropriate loader based on source.Kind.
+func (p *Provider) loadValuesFromSource(ctx context.Context, source ValuesFromParams, appNamespace, releaseNamespace string) (map[string]interface{}, error) {
 	switch source.Kind {
 	case "ConfigMap":
-		// TODO: Implement ConfigMap loading
-		return nil, fmt.Errorf("configmap values source not yet implemented")
+		return p.loadConfigMapValues(ctx, source, appNamespace, releaseNamespace)
 	case "Secret":
-		// TODO: Implement Secret loading
-		return nil, fmt.Errorf("secret values source not yet implemented")
-	case "OCIRepository":
-		// TODO: Implement OCI repository loading
-		return nil, fmt.Errorf("ocirepository values source not yet implemented")
+		return p.loadSecretValues(ctx, source, appNamespace, releaseNamespace)
 	default:
 		return nil, fmt.Errorf("unsupported values source kind: %s", source.Kind)
 	}
 }

+// resolveValuesFromNamespace returns the effective namespace for a valuesFrom
+// entry. The default (empty) resolves to releaseNamespace — the natural place
+// to co-locate chart values with the chart's resources. An explicit namespace
+// is accepted only if it matches releaseNamespace or appNamespace; any other
+// value is rejected so a tenant cannot coerce the controller's cluster-wide
+// RBAC into reading Secrets from unrelated namespaces.
+func resolveValuesFromNamespace(source ValuesFromParams, appNamespace, releaseNamespace string) (string, error) {
+	if source.Namespace == "" {
+		return releaseNamespace, nil
+	}
+	if source.Namespace == releaseNamespace || source.Namespace == appNamespace {
+		return source.Namespace, nil
+	}
+	return "", fmt.Errorf("%w: %s %q requested namespace %q but Application is in %q and release is in %q",
+		errCrossNamespaceValuesFrom, source.Kind, source.Name, source.Namespace, appNamespace, releaseNamespace)
+}
+
+// loadConfigMapValues reads a ConfigMap in the Application namespace and parses
+// the requested key as YAML. When source.Key is empty it falls back to
+// "values.yaml" (Helm/FluxCD convention). Not-found errors (missing ConfigMap
+// or missing key) are returned as valueSourceMissingError so optional sources
+// can skip them; parse errors are surfaced as-is and are never swallowed by
+// optional.
+//
+// singleton.KubeClient.Get() here is the kubevela-pkg default client built via
+// controller-runtime's client.New — this is an UNCACHED client that reads
+// directly from the API server. Do NOT switch this to manager.GetClient() or
+// a cached reader: that would register a cluster-wide ConfigMap/Secret
+// informer on first use and load every CM/Secret cluster-wide into the
+// controller's memory. Direct API reads per valuesFrom entry are the intended
+// trade-off.
+func (p *Provider) loadConfigMapValues(ctx context.Context, source ValuesFromParams, appNamespace, releaseNamespace string) (map[string]interface{}, error) {
+	ns, err := resolveValuesFromNamespace(source, appNamespace, releaseNamespace)
+	if err != nil {
+		return nil, err
+	}
+	key := source.Key
+	if key == "" {
+		key = defaultValuesKey
+	}
+
+	k8s := singleton.KubeClient.Get()
+	cm := &corev1.ConfigMap{}
+	if err := k8s.Get(ctx, client.ObjectKey{Name: source.Name, Namespace: ns}, cm); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, &valueSourceMissingError{kind: "ConfigMap", name: source.Name, namespace: ns, cause: err}
+		}
+		return nil, errors.Wrapf(err, "failed to read ConfigMap %s/%s", ns, source.Name)
+	}
+
+	raw, ok := cm.Data[key]
+	if !ok {
+		// If the key lives in binaryData (kubectl create cm --from-file of
+		// non-UTF-8 content), reject explicitly. Helm values are textual; a
+		// binary blob is unparseable. The clear message saves operators from
+		// chasing a mismatch when `kubectl get cm` shows the key under
+		// binaryData and the loader reports "not found".
+		if _, isBinary := cm.BinaryData[key]; isBinary {
+			return nil, errors.Errorf("ConfigMap %s/%s key %q is in binaryData; valuesFrom requires a textual YAML value in .data",
+				ns, source.Name, key)
+		}
+		return nil, &valueSourceMissingError{
+			kind: "ConfigMap", name: source.Name, namespace: ns, key: key,
+			cause: fmt.Errorf("key not found in .data"),
+		}
+	}
+
+	var values map[string]interface{}
+	if err := yaml.Unmarshal([]byte(raw), &values); err != nil {
+		return nil, errors.Wrapf(err, "ConfigMap %s/%s key %q: invalid YAML", ns, source.Name, key)
+	}
+	return values, nil
+}
+
+// loadSecretValues reads a Secret in the Application namespace and parses the
+// requested key as YAML. Kubernetes already base64-decodes Secret.Data on read,
+// so the bytes are consumed as-is. Error messages intentionally never include
+// raw secret bytes.
+func (p *Provider) loadSecretValues(ctx context.Context, source ValuesFromParams, appNamespace, releaseNamespace string) (map[string]interface{}, error) {
+	ns, err := resolveValuesFromNamespace(source, appNamespace, releaseNamespace)
+	if err != nil {
+		return nil, err
+	}
+	key := source.Key
+	if key == "" {
+		key = defaultValuesKey
+	}
+
+	k8s := singleton.KubeClient.Get()
+	secret := &corev1.Secret{}
+	if err := k8s.Get(ctx, client.ObjectKey{Name: source.Name, Namespace: ns}, secret); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, &valueSourceMissingError{kind: "Secret", name: source.Name, namespace: ns, cause: err}
+		}
+		return nil, errors.Wrapf(err, "failed to read Secret %s/%s", ns, source.Name)
+	}
+
+	raw, ok := secret.Data[key]
+	if !ok {
+		return nil, &valueSourceMissingError{
+			kind: "Secret", name: source.Name, namespace: ns, key: key,
+			cause: fmt.Errorf("key not found in .data"),
+		}
+	}
+
+	var values map[string]interface{}
+	if err := yaml.Unmarshal(raw, &values); err != nil {
+		return nil, errors.Wrapf(err, "Secret %s/%s key %q: invalid YAML", ns, source.Name, key)
+	}
+	return values, nil
+}
+
 // getActionConfig initializes a Helm action.Configuration with a real Kubernetes
 // REST client and a secrets-based storage driver so that releases persist in-cluster.
 func (p *Provider) getActionConfig(namespace string) (*action.Configuration, error) {
@@ -684,11 +852,18 @@ func velaOwnerLabels(velaCtx *ContextParams) map[string]string {
 	if velaCtx == nil {
 		return nil
 	}
-	return map[string]string{
+	labels := map[string]string{
 		"app.oam.dev/name":      velaCtx.AppName,
 		"app.oam.dev/namespace": velaCtx.AppNamespace,
 		"app.oam.dev/component": velaCtx.Name,
 	}
+	// Embed the publishVersion pin in the release labels so subsequent
+	// reconciles can short-circuit when the App is at a stable pin and the
+	// release was already installed at that pin.
+	if velaCtx.PublishVersion != "" {
+		labels["app.oam.dev/publishVersion"] = velaCtx.PublishVersion
+	}
+	return labels
 }

 // isOwnedByVela checks whether a Helm release was installed/managed by KubeVela
@@ -708,11 +883,22 @@ func isOwnedByVela(rel *release.Release, velaCtx *ContextParams) bool {
 // computeReleaseFingerprint builds a deterministic string from chart version and a
 // SHA-256 hash of the values so repeated reconciles with no real changes can be
 // detected cheaply without calling the Kubernetes API.
+//
+// Empty-values inputs are normalised to an empty map before hashing. Helm
+// stores release.Config as nil when no values were supplied, but mergeValues
+// returns map[string]interface{}{} for the same logical input — without this
+// guard the two would hash to sha256("null") and sha256("{}") respectively,
+// causing the dedup check below to mis-fire and trigger spurious helm upgrades
+// on every reconcile for any release that was installed with empty/optional
+// values.
 func computeReleaseFingerprint(ch *chart.Chart, values map[string]interface{}) string {
 	version := ""
 	if ch != nil && ch.Metadata != nil {
 		version = ch.Metadata.Version
 	}
+	if values == nil {
+		values = map[string]interface{}{}
+	}
 	valuesJSON, _ := json.Marshal(values)
 	h := sha256.Sum256(valuesJSON)
 	return version + "|" + hex.EncodeToString(h[:])
@@ -785,6 +971,34 @@ func (p *Provider) installOrUpgradeChart(ctx context.Context, ch *chart.Chart, r
 			p.labelReleaseSecrets(releaseNamespace, releaseName, velaCtx)
 		}

+		// publishVersion pin short-circuit: when the App is at a stable
+		// publishVersion pin AND the deployed release was installed at the
+		// same pin AND the chart version is unchanged, return the deployed
+		// manifest unchanged regardless of any apparent values drift.
+		//
+		// Without this, a render path that bypasses the workflow gate
+		// (state-keep / drift detection / post-dispatch traits / periodic
+		// CUE evaluation) re-merges valuesFrom sources and the cluster-side
+		// fingerprint compare below would mis-fire whenever a referenced
+		// CM/Secret was edited. The user's explicit pin is the contract:
+		// nothing changes until they bump the pin.
+		//
+		// Initial install has no existingRelease so this branch is skipped,
+		// and the initial mergeValues runs normally — picking up the
+		// referenced CM/Secret content and stamping it into the release.
+		if !needsAdoption && velaCtx != nil && velaCtx.PublishVersion != "" &&
+			existingRelease.Info != nil && existingRelease.Info.Status == release.StatusDeployed &&
+			existingRelease.Chart != nil && existingRelease.Chart.Metadata != nil &&
+			existingRelease.Chart.Metadata.Version == ch.Metadata.Version &&
+			existingRelease.Labels["app.oam.dev/publishVersion"] == velaCtx.PublishVersion {
+			klog.V(2).Infof("Helm provider [%s]: Release %s held by publishVersion pin %q, skipping upgrade",
+				velaContextStr(velaCtx), releaseName, velaCtx.PublishVersion)
+			p.releaseFingerprints[cacheKey] = fingerprint
+			p.releaseManifests[cacheKey] = existingRelease.Manifest
+			p.releaseVersions[cacheKey] = existingRelease.Version
+			return existingRelease.Manifest, existingRelease.Info.Notes, existingRelease.Version, nil
+		}
+
 		// Release exists — check if it is already deployed with the same fingerprint
 		if !needsAdoption && existingRelease.Info.Status == release.StatusDeployed {
 			clusterFingerprint := computeReleaseFingerprint(existingRelease.Chart, existingRelease.Config)
@@ -1299,10 +1513,16 @@ func Render(ctx context.Context, params *providers.Params[RenderParams]) (*provi

 	klog.V(2).Infof("Helm provider [%s]: Starting render for chart %s from %s", velaContextStr(renderParams.Context), renderParams.Chart.Source, renderParams.Chart.RepoURL)

-	// Set default release name and namespace
-	releaseName := "release"
-	releaseNamespace := "default"
+	// Application namespace is the tenant boundary. When the Application has no
+	// explicit context, fall back to the release namespace below so the same
+	// Application can be rendered outside a ComponentDefinition path.
+	appNamespace := ""
+	if renderParams.Context != nil {
+		appNamespace = renderParams.Context.AppNamespace
+	}

+	releaseName := "release"
+	releaseNamespace := appNamespace
 	if renderParams.Release != nil {
 		if renderParams.Release.Name != "" {
 			releaseName = renderParams.Release.Name
@@ -1311,6 +1531,50 @@ func Render(ctx context.Context, params *providers.Params[RenderParams]) (*provi
 			releaseNamespace = renderParams.Release.Namespace
 		}
 	}
+	// Guarantee a non-empty release namespace. Under the normal KubeVela
+	// code path the controller always sets Context.AppNamespace before
+	// calling Render, but callers that invoke the provider directly (tests,
+	// CLI tooling) may leave both context and Release.Namespace empty.
+	// Falling back to "default" preserves the pre-refactor behavior and
+	// keeps Helm's namespace resolution from depending on the caller's
+	// kubeconfig default.
+	if releaseNamespace == "" {
+		releaseNamespace = "default"
+	}
+	if appNamespace == "" {
+		appNamespace = releaseNamespace
+	}
+
+	// Resolve the App's publishVersion annotation, if any. We pass it through
+	// ContextParams.PublishVersion so installOrUpgradeChart can short-circuit
+	// when the deployed release is already at the current pin and so
+	// velaOwnerLabels can stamp the pin onto the release at install time.
+	// Skipped in dry-run: admission validation must not depend on cluster
+	// state, and the user-visible behaviour (CUE shape OK / not OK) is
+	// independent of the pin.
+	//
+	// IsNotFound is treated as "App is being deleted" and falls through with
+	// an empty pin — the subsequent uninstall path handles cleanup. Any other
+	// error (RBAC change, transient API failure, network blip) is surfaced
+	// rather than silently swallowed: a swallowed error would leave the pin
+	// empty for this reconcile and bypass the pin short-circuit downstream,
+	// allowing an unintended helm upgrade to fire even though the user's
+	// publishVersion annotation is still in place.
+	if !isDryRun(ctx) && renderParams.Context != nil && renderParams.Context.AppName != "" && appNamespace != "" {
+		var app v1beta1.Application
+		switch getErr := singleton.KubeClient.Get().Get(ctx, client.ObjectKey{Name: renderParams.Context.AppName, Namespace: appNamespace}, &app); {
+		case getErr == nil:
+			if pin := app.GetAnnotations()[oam.AnnotationPublishVersion]; pin != "" {
+				renderParams.Context.PublishVersion = pin
+			}
+		case apierrors.IsNotFound(getErr):
+			// App is gone (deletion in flight). Proceed without a pin.
+		default:
+			return nil, errors.Wrapf(getErr,
+				"failed to read Application %s/%s for publishVersion lookup; refusing to proceed without pin context",
+				appNamespace, renderParams.Context.AppName)
+		}
+	}

 	klog.V(3).Infof("Helm provider: Release name=%s, namespace=%s", releaseName, releaseNamespace)

@@ -1321,12 +1585,24 @@ func Render(ctx context.Context, params *providers.Params[RenderParams]) (*provi
 	}
 	klog.V(2).Infof("Helm provider: Successfully fetched chart %s", ch.Name())

-	// Merge values from all sources
-	values, err := p.mergeValues(ctx, renderParams.Values, renderParams.ValuesFrom)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to merge values")
+	// Skip valuesFrom resolution in dry-run (webhook admission): the webhook
+	// validates CUE shape and renders the chart, not the final merged values,
+	// and running loadValuesFromSource during admission adds N cluster reads
+	// per Application create/update plus ordering hazards when the referenced
+	// CM/Secret is applied in the same kubectl batch.
+	var values map[string]interface{}
+	if isDryRun(ctx) {
+		if inline, ok := renderParams.Values.(map[string]interface{}); ok {
+			values = inline
+		} else {
+			values = map[string]interface{}{}
+		}
+	} else {
+		values, err = p.mergeValues(ctx, renderParams.Values, renderParams.ValuesFrom, appNamespace, releaseNamespace)
+		if err != nil {
+			return nil, errors.Wrapf(err, "%s: failed to merge values", velaContextStr(renderParams.Context))
+		}
 	}
-	klog.V(3).Infof("Helm provider: Merged values: %v", values)

 	// In dry-run mode (webhook validation), render client-side only — no cluster
 	// interaction, no real install, no hooks. This prevents the webhook from
@@ -33,10 +33,20 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/release"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	kyaml "k8s.io/apimachinery/pkg/util/yaml"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"

 	"github.com/kubevela/pkg/cue/cuex/providers"
+	"github.com/kubevela/pkg/util/singleton"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+	"github.com/oam-dev/kubevela/pkg/oam"
 )

 var _ = Describe("Helm Provider", func() {
@@ -129,6 +139,11 @@ var _ = Describe("Helm Provider", func() {
 		BeforeEach(func() {
 			p = NewProviderWithConfig(nil)
 			ctx = context.Background()
+			// Empty fake client so ConfigMap/Secret Gets return NotFound. Tests
+			// that need specific resources override the singleton themselves.
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			singleton.KubeClient.Set(fake.NewClientBuilder().WithScheme(scheme).Build())
 		})

 		It("should return base values when no valuesFrom", func() {
@@ -138,34 +153,34 @@ var _ = Describe("Helm Provider", func() {
 					"key2": "value2",
 				},
 			}
-			result, err := p.mergeValues(ctx, baseValues, nil)
+			result, err := p.mergeValues(ctx, baseValues, nil, "default", "default")
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(result).To(Equal(baseValues))
 		})

 		It("should return empty map for nil base values", func() {
-			result, err := p.mergeValues(ctx, nil, nil)
+			result, err := p.mergeValues(ctx, nil, nil, "default", "default")
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(result).ToNot(BeNil())
 			Expect(result).To(BeEmpty())
 		})

-		It("should skip optional source errors", func() {
+		It("should skip optional source when ConfigMap is missing", func() {
 			base := map[string]interface{}{"key": "value"}
 			valuesFrom := []ValuesFromParams{
 				{Kind: "ConfigMap", Name: "missing", Optional: true},
 			}
-			result, err := p.mergeValues(ctx, base, valuesFrom)
+			result, err := p.mergeValues(ctx, base, valuesFrom, "default", "default")
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(result["key"]).To(Equal("value"))
 		})

-		It("should propagate required source errors", func() {
+		It("should propagate missing-source errors when not optional", func() {
 			base := map[string]interface{}{"key": "value"}
 			valuesFrom := []ValuesFromParams{
 				{Kind: "ConfigMap", Name: "missing", Optional: false},
 			}
-			_, err := p.mergeValues(ctx, base, valuesFrom)
+			_, err := p.mergeValues(ctx, base, valuesFrom, "default", "default")
 			Expect(err).Should(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("failed to load values"))
 		})
@@ -410,6 +425,19 @@ metadata:
 			fp := computeReleaseFingerprint(nil, map[string]interface{}{"replicas": 2})
 			Expect(fp).ToNot(BeEmpty())
 		})
+
+		It("should treat nil values and empty map as equivalent", func() {
+			// Helm stores release.Config as nil when no values were supplied
+			// at install time, but mergeValues returns an empty map for the
+			// same logical input. Without normalising the two, the dedup
+			// check at the call site would mis-fire on every reconcile and
+			// trigger spurious helm upgrades for releases installed with
+			// empty/optional valuesFrom sources.
+			ch := &chart.Chart{Metadata: &chart.Metadata{Version: "1.2.3"}}
+			fpNil := computeReleaseFingerprint(ch, nil)
+			fpEmpty := computeReleaseFingerprint(ch, map[string]interface{}{})
+			Expect(fpNil).To(Equal(fpEmpty))
+		})
 	})

 	Describe("cache invalidation on missing release", func() {
@@ -766,24 +794,419 @@ spec:
 		})
 	})

-	Describe("loadValuesFromSource", func() {
+	Describe("loadValuesFromSource dispatcher", func() {
 		var p *Provider

 		BeforeEach(func() {
 			p = NewProviderWithConfig(nil)
 		})

-		DescribeTable("should return errors for unimplemented/unsupported source kinds",
-			func(kind, expectedErr string) {
-				_, err := p.loadValuesFromSource(context.Background(), ValuesFromParams{Kind: kind, Name: "test"})
-				Expect(err).Should(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring(expectedErr))
-			},
-			Entry("ConfigMap", "ConfigMap", "configmap values source not yet implemented"),
-			Entry("Secret", "Secret", "secret values source not yet implemented"),
-			Entry("OCIRepository", "OCIRepository", "ocirepository values source not yet implemented"),
-			Entry("Unknown", "Unknown", "unsupported values source kind: Unknown"),
+		It("returns an error for unsupported kinds (including the reserved OCIRepository)", func() {
+			for _, kind := range []string{"OCIRepository", "Unknown", "configmap", ""} {
+				_, err := p.loadValuesFromSource(context.Background(),
+					ValuesFromParams{Kind: kind, Name: "test"},
+					"default", "default")
+				Expect(err).Should(HaveOccurred(), "kind=%q must fail", kind)
+				Expect(err.Error()).To(ContainSubstring("unsupported values source kind"),
+					"kind=%q must surface as unsupported", kind)
+			}
+		})
+	})
+
+	Describe("cross-namespace valuesFrom rejection", func() {
+		var p *Provider
+
+		BeforeEach(func() {
+			p = NewProviderWithConfig(nil)
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			singleton.KubeClient.Set(fake.NewClientBuilder().WithScheme(scheme).Build())
+		})
+
+		It("rejects a ConfigMap reference to a namespace other than the Application's", func() {
+			_, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "secrets-bearer", Namespace: "kube-system"},
+				"tenant-a", "tenant-a")
+			Expect(err).Should(HaveOccurred())
+			Expect(errors.Is(err, errCrossNamespaceValuesFrom)).To(BeTrue(),
+				"cross-ns error must be detectable via errors.Is")
+			Expect(err.Error()).To(ContainSubstring("kube-system"))
+			Expect(err.Error()).To(ContainSubstring("tenant-a"))
+		})
+
+		It("rejects a Secret reference to a namespace other than the Application's", func() {
+			_, err := p.loadSecretValues(context.Background(),
+				ValuesFromParams{Kind: "Secret", Name: "any", Namespace: "other-tenant"},
+				"tenant-a", "tenant-a")
+			Expect(err).Should(HaveOccurred())
+			Expect(errors.Is(err, errCrossNamespaceValuesFrom)).To(BeTrue())
+		})
+
+		It("allows an explicit Namespace equal to the Application's namespace", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "same", Namespace: "tenant-a"},
+				Data:       map[string]string{"values.yaml": "k: v"},
+			}
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			singleton.KubeClient.Set(fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm).Build())
+			values, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "same", Namespace: "tenant-a"},
+				"tenant-a", "tenant-a")
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(values["k"]).To(Equal("v"))
+		})
+	})
+
+	Describe("loadConfigMapValues", func() {
+		const releaseNS = "prod"
+
+		var p *Provider
+
+		BeforeEach(func() {
+			p = NewProviderWithConfig(nil)
+		})
+
+		buildClient := func(objs ...client.Object) {
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			builder := fake.NewClientBuilder().WithScheme(scheme)
+			for _, o := range objs {
+				builder = builder.WithObjects(o)
+			}
+			singleton.KubeClient.Set(builder.Build())
+		}
+
+		It("loads from the default values.yaml key when Key is empty", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicaCount: 3\nimage: nginx"},
+			}
+			buildClient(cm)
+
+			values, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "cfg"}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(values["replicaCount"]).To(BeEquivalentTo(3))
+			Expect(values["image"]).To(Equal("nginx"))
+		})
+
+		It("loads from an explicit Key", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: releaseNS},
+				Data:       map[string]string{"prod.yaml": "replicaCount: 5"},
+			}
+			buildClient(cm)
+
+			values, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "cfg", Key: "prod.yaml"}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(values["replicaCount"]).To(BeEquivalentTo(5))
+		})
+
+		It("accepts an explicit Namespace that equals the Application's namespace", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicaCount: 7"},
+			}
+			buildClient(cm)
+
+			values, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "cfg", Namespace: releaseNS}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(values["replicaCount"]).To(BeEquivalentTo(7))
+		})
+
+		It("returns a missing-source error when the ConfigMap does not exist", func() {
+			buildClient()
+			_, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "absent"}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred())
+			Expect(isValueSourceMissing(err)).To(BeTrue(),
+				"missing ConfigMap should surface as valueSourceMissingError")
+		})
+
+		It("returns a missing-source error when the key is absent", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: releaseNS},
+				Data:       map[string]string{"other.yaml": "foo: bar"},
+			}
+			buildClient(cm)
+
+			_, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "cfg"}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred())
+			Expect(isValueSourceMissing(err)).To(BeTrue())
+		})
+
+		It("surfaces YAML parse errors and never classifies them as missing", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicas: [unterminated"},
+			}
+			buildClient(cm)
+
+			_, err := p.loadConfigMapValues(context.Background(),
+				ValuesFromParams{Kind: "ConfigMap", Name: "cfg"}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("invalid YAML"))
+			Expect(isValueSourceMissing(err)).To(BeFalse(),
+				"parse errors must NOT be swallowed by optional")
+		})
+	})
+
+	Describe("loadSecretValues", func() {
+		const releaseNS = "prod"
+
+		var p *Provider
+
+		BeforeEach(func() {
+			p = NewProviderWithConfig(nil)
+		})
+
+		buildClient := func(objs ...client.Object) {
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			builder := fake.NewClientBuilder().WithScheme(scheme)
+			for _, o := range objs {
+				builder = builder.WithObjects(o)
+			}
+			singleton.KubeClient.Set(builder.Build())
+		}
+
+		It("loads YAML from Secret.Data (already base64-decoded by the API)", func() {
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "creds", Namespace: releaseNS},
+				Data: map[string][]byte{
+					"values.yaml": []byte("password: s3cret\nuser: admin"),
+				},
+			}
+			buildClient(secret)
+
+			values, err := p.loadSecretValues(context.Background(),
+				ValuesFromParams{Kind: "Secret", Name: "creds"}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(values["user"]).To(Equal("admin"))
+			Expect(values["password"]).To(Equal("s3cret"))
+		})
+
+		It("returns a missing-source error when the Secret does not exist", func() {
+			buildClient()
+			_, err := p.loadSecretValues(context.Background(),
+				ValuesFromParams{Kind: "Secret", Name: "absent"}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred())
+			Expect(isValueSourceMissing(err)).To(BeTrue())
+		})
+
+		It("surfaces YAML parse errors and does not leak raw secret bytes", func() {
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "creds", Namespace: releaseNS},
+				Data:       map[string][]byte{"values.yaml": []byte("super-secret: [unterminated")},
+			}
+			buildClient(secret)
+
+			_, err := p.loadSecretValues(context.Background(),
+				ValuesFromParams{Kind: "Secret", Name: "creds"}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).ToNot(ContainSubstring("super-secret"),
+				"Secret contents must never appear in error messages")
+			Expect(isValueSourceMissing(err)).To(BeFalse())
+		})
+	})
+
+	Describe("mergeValues priority", func() {
+		const releaseNS = "prod"
+
+		var (
+			p   *Provider
+			ctx context.Context
 		)
+
+		BeforeEach(func() {
+			p = NewProviderWithConfig(nil)
+			ctx = context.Background()
+		})
+
+		It("gives inline values the highest priority over valuesFrom", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicaCount: 3\nimage: from-configmap"},
+			}
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			singleton.KubeClient.Set(fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm).Build())
+
+			base := map[string]interface{}{"image": "from-inline"}
+			result, err := p.mergeValues(ctx, base,
+				[]ValuesFromParams{{Kind: "ConfigMap", Name: "cm"}}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(result["image"]).To(Equal("from-inline"), "inline must win over ConfigMap")
+			Expect(result["replicaCount"]).To(BeEquivalentTo(3), "CM-only keys must remain")
+		})
+
+		It("makes later valuesFrom entries override earlier ones", func() {
+			cmA := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "tier: free\ncolour: blue"},
+			}
+			cmB := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "tier: paid"},
+			}
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			singleton.KubeClient.Set(fake.NewClientBuilder().WithScheme(scheme).WithObjects(cmA, cmB).Build())
+
+			result, err := p.mergeValues(ctx, nil,
+				[]ValuesFromParams{
+					{Kind: "ConfigMap", Name: "a"},
+					{Kind: "ConfigMap", Name: "b"},
+				}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(result["tier"]).To(Equal("paid"), "later source must win on conflict")
+			Expect(result["colour"]).To(Equal("blue"), "earlier source keeps non-overridden keys")
+		})
+	})
+
+	Describe("mergeValues edge cases", func() {
+		const releaseNS = "prod"
+
+		var (
+			p   *Provider
+			ctx context.Context
+		)
+
+		BeforeEach(func() {
+			p = NewProviderWithConfig(nil)
+			ctx = context.Background()
+		})
+
+		buildClient := func(objs ...client.Object) {
+			scheme := runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			builder := fake.NewClientBuilder().WithScheme(scheme)
+			for _, o := range objs {
+				builder = builder.WithObjects(o)
+			}
+			singleton.KubeClient.Set(builder.Build())
+		}
+
+		It("treats empty valuesFrom slice equivalently to nil", func() {
+			buildClient()
+			base := map[string]interface{}{"key": "value"}
+
+			fromNil, errNil := p.mergeValues(ctx, base, nil, releaseNS, releaseNS)
+			fromEmpty, errEmpty := p.mergeValues(ctx, base, []ValuesFromParams{}, releaseNS, releaseNS)
+
+			Expect(errNil).ShouldNot(HaveOccurred())
+			Expect(errEmpty).ShouldNot(HaveOccurred())
+			Expect(fromEmpty).To(Equal(fromNil))
+		})
+
+		It("skips a missing optional source and continues with a following required source", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "real", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicaCount: 7"},
+			}
+			buildClient(cm)
+
+			result, err := p.mergeValues(ctx, nil, []ValuesFromParams{
+				{Kind: "ConfigMap", Name: "missing", Optional: true},
+				{Kind: "ConfigMap", Name: "real"},
+			}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(result["replicaCount"]).To(BeEquivalentTo(7))
+		})
+
+		It("preserves orthogonal nested keys while resolving conflicts at depth", func() {
+			cmA := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: releaseNS},
+				Data: map[string]string{"values.yaml": `resources:
+  limits:
+    cpu: 100m
+    memory: 256Mi
+  requests:
+    cpu: 50m`},
+			}
+			cmB := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: releaseNS},
+				Data: map[string]string{"values.yaml": `resources:
+  limits:
+    memory: 512Mi`},
+			}
+			buildClient(cmA, cmB)
+
+			result, err := p.mergeValues(ctx, nil, []ValuesFromParams{
+				{Kind: "ConfigMap", Name: "a"},
+				{Kind: "ConfigMap", Name: "b"},
+			}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			resources := result["resources"].(map[string]interface{})
+			limits := resources["limits"].(map[string]interface{})
+			requests := resources["requests"].(map[string]interface{})
+			Expect(limits["memory"]).To(Equal("512Mi"), "later source wins on conflict deep in the tree")
+			Expect(limits["cpu"]).To(Equal("100m"), "orthogonal sibling in the same sub-object preserved")
+			Expect(requests["cpu"]).To(Equal("50m"), "untouched sub-object preserved in full")
+		})
+
+		It("replaces array values instead of merging them (helm CoalesceTables semantics)", func() {
+			cmA := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "extraArgs:\n  - --level=debug\n  - --timeout=30"},
+			}
+			cmB := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "extraArgs:\n  - --level=info"},
+			}
+			buildClient(cmA, cmB)
+
+			result, err := p.mergeValues(ctx, nil, []ValuesFromParams{
+				{Kind: "ConfigMap", Name: "a"},
+				{Kind: "ConfigMap", Name: "b"},
+			}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			args := result["extraArgs"].([]interface{})
+			Expect(args).To(HaveLen(1), "later array wholly replaces earlier array")
+			Expect(args[0]).To(Equal("--level=info"))
+		})
+
+		It("surfaces parse errors even when Optional is true", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "broken", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicas: [unterminated"},
+			}
+			buildClient(cm)
+
+			_, err := p.mergeValues(ctx, nil, []ValuesFromParams{
+				{Kind: "ConfigMap", Name: "broken", Optional: true},
+			}, releaseNS, releaseNS)
+			Expect(err).Should(HaveOccurred(),
+				"Optional must not mask parse errors — this is the critical contract")
+			Expect(err.Error()).To(ContainSubstring("invalid YAML"))
+			Expect(isValueSourceMissing(err)).To(BeFalse())
+		})
+
+		It("mixes a Secret and ConfigMap in the same valuesFrom list", func() {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: releaseNS},
+				Data:       map[string]string{"values.yaml": "replicaCount: 2\nimage: cm-image"},
+			}
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "creds", Namespace: releaseNS},
+				Data:       map[string][]byte{"values.yaml": []byte("image: secret-image")},
+			}
+			buildClient(cm, secret)
+
+			result, err := p.mergeValues(ctx, nil, []ValuesFromParams{
+				{Kind: "ConfigMap", Name: "cm"},
+				{Kind: "Secret", Name: "creds"},
+			}, releaseNS, releaseNS)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(result["image"]).To(Equal("secret-image"), "later Secret wins over earlier ConfigMap")
+			Expect(result["replicaCount"]).To(BeEquivalentTo(2), "orthogonal CM key preserved")
+		})
 	})

 	Describe("fetchChartWithoutCache", func() {
@@ -1429,6 +1852,161 @@ data:
 		})
 	})

+	// -----------------------------------------------------------------------
+	// Application publishVersion lookup defense
+	//
+	// Render reads the parent Application's publishVersion annotation so that
+	// installOrUpgradeChart can short-circuit when the deployed release is
+	// already at the user's pin. A non-NotFound error during that lookup
+	// previously left the pin empty, allowing an unintended upgrade to fire
+	// even though the user's pin was still in place. The defense surfaces
+	// that error instead of swallowing it.
+	// -----------------------------------------------------------------------
+
+	Describe("Render Application publishVersion lookup defense", func() {
+		var scheme *runtime.Scheme
+
+		BeforeEach(func() {
+			scheme = runtime.NewScheme()
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			Expect(v1beta1.AddToScheme(scheme)).To(Succeed())
+		})
+
+		It("propagates a transient API error from the Application Get rather than bypassing the pin", func() {
+			injected := errors.New("etcdserver: leader changed")
+			c := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithInterceptorFuncs(interceptor.Funcs{
+					Get: func(ctx context.Context, kc client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+						if _, ok := obj.(*v1beta1.Application); ok {
+							return injected
+						}
+						return kc.Get(ctx, key, obj, opts...)
+					},
+				}).
+				Build()
+			singleton.KubeClient.Set(c)
+
+			_, err := Render(context.Background(), &providers.Params[RenderParams]{
+				Params: RenderParams{
+					Chart:   ChartSourceParams{Source: "render-defense-1", Version: "1.0.0"},
+					Release: &ReleaseParams{Name: "rel", Namespace: "tenant-a"},
+					Context: &ContextParams{
+						AppName:      "my-app",
+						AppNamespace: "tenant-a",
+						Name:         "my-comp",
+						Namespace:    "tenant-a",
+					},
+				},
+			})
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("failed to read Application tenant-a/my-app for publishVersion lookup"),
+				"defense must surface the wrapped App-lookup error")
+			Expect(err.Error()).To(ContainSubstring("etcdserver: leader changed"),
+				"defense must preserve the underlying cause")
+		})
+
+		It("treats a NotFound on the Application as deletion-in-flight and proceeds past the lookup", func() {
+			// No App registered → NotFound from the typed fake client.
+			// Render must continue past the lookup; we prove it by leaving
+			// the chart un-cached so the next failure is from fetchChart,
+			// not the App lookup.
+			c := fake.NewClientBuilder().WithScheme(scheme).Build()
+			singleton.KubeClient.Set(c)
+
+			_, err := Render(context.Background(), &providers.Params[RenderParams]{
+				Params: RenderParams{
+					Chart:   ChartSourceParams{Source: "render-defense-notfound-xyz", Version: "9.9.9"},
+					Release: &ReleaseParams{Name: "rel", Namespace: "tenant-a"},
+					Context: &ContextParams{
+						AppName:      "deleted-app",
+						AppNamespace: "tenant-a",
+						Name:         "my-comp",
+						Namespace:    "tenant-a",
+					},
+				},
+			})
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).ToNot(ContainSubstring("failed to read Application"),
+				"NotFound on App must not be reported as a pin-lookup error")
+			Expect(err.Error()).To(ContainSubstring("failed to fetch chart"),
+				"Render must proceed past the App lookup to the chart fetch step")
+		})
+
+		It("succeeds the App lookup when the Application exists with a publishVersion annotation", func() {
+			app := &v1beta1.Application{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "pinned-app",
+					Namespace:   "tenant-a",
+					Annotations: map[string]string{oam.AnnotationPublishVersion: "v42"},
+				},
+			}
+			c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(app).Build()
+			singleton.KubeClient.Set(c)
+
+			_, err := Render(context.Background(), &providers.Params[RenderParams]{
+				Params: RenderParams{
+					Chart:   ChartSourceParams{Source: "render-defense-pinned-xyz", Version: "9.9.9"},
+					Release: &ReleaseParams{Name: "rel", Namespace: "tenant-a"},
+					Context: &ContextParams{
+						AppName:      "pinned-app",
+						AppNamespace: "tenant-a",
+						Name:         "my-comp",
+						Namespace:    "tenant-a",
+					},
+				},
+			})
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).ToNot(ContainSubstring("failed to read Application"),
+				"a present App with a pin must not surface a lookup error")
+			Expect(err.Error()).To(ContainSubstring("failed to fetch chart"),
+				"Render must proceed past the App lookup to the chart fetch step")
+		})
+
+		It("skips the App lookup entirely in dry-run", func() {
+			// Dry-run is the webhook admission path; it must never depend on
+			// cluster state. Inject a Get error: if the lookup ran, Render
+			// would surface it. Instead, dry-run renders client-side.
+			c := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithInterceptorFuncs(interceptor.Funcs{
+					Get: func(ctx context.Context, kc client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+						if _, ok := obj.(*v1beta1.Application); ok {
+							return errors.New("dry-run must not call Get on Application")
+						}
+						return kc.Get(ctx, key, obj, opts...)
+					},
+				}).
+				Build()
+			singleton.KubeClient.Set(c)
+
+			p := NewProvider()
+			p.cache.Put("repo/render-defense-dryrun/1.0.0", &chart.Chart{
+				Metadata: &chart.Metadata{Name: "render-defense-dryrun", Version: "1.0.0"},
+				Templates: []*chart.File{{
+					Name: "templates/cm.yaml",
+					Data: []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: {{ .Release.Name }}-cm\n"),
+				}},
+			}, 1*time.Hour)
+
+			ctx := WithDryRun(context.Background())
+			_, err := Render(ctx, &providers.Params[RenderParams]{
+				Params: RenderParams{
+					Chart:   ChartSourceParams{Source: "render-defense-dryrun", Version: "1.0.0"},
+					Release: &ReleaseParams{Name: "rel", Namespace: "tenant-a"},
+					Context: &ContextParams{
+						AppName:      "any-app",
+						AppNamespace: "tenant-a",
+						Name:         "my-comp",
+						Namespace:    "tenant-a",
+					},
+				},
+			})
+			Expect(err).ShouldNot(HaveOccurred(),
+				"dry-run must not exercise the App lookup path")
+		})
+	})
+
 	// -----------------------------------------------------------------------
 	// Tier 1 Coverage: fetchRepoChart via httptest
 	// -----------------------------------------------------------------------
@@ -1682,7 +2260,12 @@ entries:
 	// -----------------------------------------------------------------------

 	Describe("Uninstall top-level function", func() {
-		It("should exercise the function entry and error path", func() {
+		It("should exercise the function entry and return without panicking", func() {
+			// Without a reachable cluster the call typically fails at
+			// getActionConfig or while contacting the API server; with a
+			// cluster it succeeds. The only invariant we can assert portably
+			// is that the function returns in a consistent shape — result is
+			// non-nil on success, or an error is returned on failure.
 			result, err := Uninstall(context.Background(), &providers.Params[UninstallParams]{
 				Params: UninstallParams{
 					Release: ReleaseParams{
@@ -1692,10 +2275,7 @@ entries:
 					KeepHistory: false,
 				},
 			})
-			// Without a cluster, this may fail at getActionConfig or at the actual uninstall
-			if err != nil {
-				Expect(err.Error()).To(Or(ContainSubstring("helm"), ContainSubstring("uninstall"), ContainSubstring("config")))
-			} else {
+			if err == nil {
 				Expect(result).ToNot(BeNil())
 			}
 		})