Add HTTP proxy support for operator deployments

Fixes: OCPBUGS-61082

Add support for HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment
variables in operator deployments. Proxy configuration is read from
operator-controller's environment at startup and injected into all
containers in deployed operator Deployments.

When operator-controller restarts with changed proxy configuration,
existing operator deployments are automatically updated via triggered
reconciliation.

Supports both helm and boxcutter appliers.
Includes unit and e2e tests.

Signed-off-by: Todd Short <tshort@redhat.com>
Assisted-By: Claude

Author: tmshort

Makefile

@@ -243,7 +243,7 @@ test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run a
 
 .PHONY: e2e
 e2e: #EXHELP Run the e2e tests.
-	go test -count=1 -v ./test/e2e/features_test.go
+	$(if $(GODOG_TAGS),go test -timeout=30m -count=1 -v ./test/e2e/features_test.go -args --godog.tags="$(GODOG_TAGS)",go test -timeout=30m -count=1 -v ./test/e2e/features_test.go)
 
 E2E_REGISTRY_NAME := docker-registry
 E2E_REGISTRY_NAMESPACE := operator-controller-e2e

cmd/operator-controller/main.go

@@ -68,6 +68,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/finalizers"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/proxy"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/resolve"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
@@ -219,6 +220,7 @@ func validateMetricsFlags() error {
 	}
 	return nil
 }
+
 func run() error {
 	setupLog.Info("starting up the controller", "version info", version.String())
 
@@ -474,11 +476,17 @@ func run() error {
 	}
 
 	certProvider := getCertificateProvider()
+
+	// Create proxy configuration from environment variables
+	// The fingerprint is calculated once during construction and cached
+	proxyConfig := proxy.NewFromEnv()
+
 	regv1ManifestProvider := &applier.RegistryV1ManifestProvider{
 		BundleRenderer:              registryv1.Renderer,
 		CertificateProvider:         certProvider,
 		IsWebhookSupportEnabled:     certProvider != nil,
 		IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport),
+		Proxy:                       proxyConfig,
 	}
 	var cerCfg reconcilerConfigurator
 	if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {
@@ -540,6 +548,73 @@ func run() error {
 		return err
 	}
 
+	// Add a runnable to trigger reconciliation of all ClusterExtensions on startup.
+	// This ensures existing deployments get updated when proxy configuration changes
+	// (added, modified, or removed).
+	if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
+		// Wait for the cache to sync
+		if !mgr.GetCache().WaitForCacheSync(ctx) {
+			return fmt.Errorf("failed to wait for cache sync")
+		}
+
+		// Always trigger reconciliation on startup to handle proxy config changes
+		if proxyConfig != nil {
+			setupLog.Info("proxy configuration detected, triggering reconciliation of all ClusterExtensions",
+				"httpProxy", proxy.SanitizeURL(proxyConfig.HTTPProxy), "httpsProxy", proxy.SanitizeURL(proxyConfig.HTTPSProxy), "noProxy", proxyConfig.NoProxy)
+		} else {
+			setupLog.Info("no proxy configuration detected, triggering reconciliation to remove proxy vars from existing deployments")
+		}
+
+		extList := &ocv1.ClusterExtensionList{}
+		if err := cl.List(ctx, extList); err != nil {
+			setupLog.Error(err, "failed to list ClusterExtensions for proxy update")
+			return nil // Don't fail startup
+		}
+
+		for i := range extList.Items {
+			ext := &extList.Items[i]
+
+			// Get current and desired proxy hash
+			currentHash, exists := ext.Annotations[proxy.ConfigHashKey]
+			desiredHash := proxyConfig.Fingerprint()
+
+			// Skip if annotation matches desired state
+			if exists && currentHash == desiredHash {
+				continue
+			}
+			// Skip if neither proxy nor annotation exists
+			if !exists && desiredHash == "" {
+				continue
+			}
+
+			// Use Patch instead of Update to avoid resourceVersion conflicts
+			// This ensures the annotation is set even if the ClusterExtension is modified concurrently
+			patch := client.MergeFrom(ext.DeepCopy())
+			if ext.Annotations == nil {
+				ext.Annotations = make(map[string]string)
+			}
+
+			// Delete annotation if no proxy is configured, otherwise set it
+			if desiredHash == "" {
+				delete(ext.Annotations, proxy.ConfigHashKey)
+			} else {
+				ext.Annotations[proxy.ConfigHashKey] = desiredHash
+			}
+
+			if err := cl.Patch(ctx, ext, patch); err != nil {
+				setupLog.Error(err, "failed to set proxy hash on ClusterExtension", "name", ext.Name)
+				// Continue with other ClusterExtensions
+			}
+		}
+
+		setupLog.Info("triggered reconciliation for existing ClusterExtensions", "count", len(extList.Items))
+
+		return nil
+	})); err != nil {
+		setupLog.Error(err, "unable to add startup reconciliation trigger")
+		return err
+	}
+
 	setupLog.Info("starting manager")
 	ctx := ctrl.SetupSignalHandler()
 	if err := mgr.Start(ctx); err != nil {
@@ -625,13 +700,18 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 		ActionClientGetter: acg,
 		RevisionGenerator:  rg,
 	}
+	// Get the ManifestProvider to extract proxy fingerprint
+	regv1Provider, ok := c.regv1ManifestProvider.(*applier.RegistryV1ManifestProvider)
+	if !ok {
+		return fmt.Errorf("manifest provider is not of type *applier.RegistryV1ManifestProvider")
+	}
 	ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
 		controllers.HandleFinalizers(c.finalizers),
 		controllers.MigrateStorage(storageMigrator),
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
 		controllers.UnpackBundle(c.imagePuller, c.imageCache),
-		controllers.ApplyBundleWithBoxcutter(appl.Apply),
+		controllers.ApplyBundleWithBoxcutter(appl.Apply, regv1Provider.ProxyFingerprint),
 	}
 
 	baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(c.mgr.GetConfig())

internal/operator-controller/applier/boxcutter.go

@@ -33,6 +33,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/proxy"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
 	"github.com/operator-framework/operator-controller/internal/shared/util/cache"
 )
@@ -485,17 +486,28 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		desiredRevision.Spec.Revision = currentRevision.Spec.Revision
 		desiredRevision.Name = currentRevision.Name
 
-		err := bc.createOrUpdate(ctx, getUserInfo(ext), desiredRevision)
-		switch {
-		case apierrors.IsInvalid(err):
-			// We could not update the current revision due to trying to update an immutable field.
-			// Therefore, we need to create a new revision.
+		// Check if proxy configuration changed by comparing annotations.
+		// The Phases field has CEL immutability validation (self == oldSelf) which prevents
+		// updating nested content like env vars. When proxy config changes, we must create
+		// a new revision rather than attempting to patch the existing one.
+		// Comparing proxy hash annotations is more reliable than comparing phases content,
+		// which has false positives due to serialization differences in unstructured objects.
+		currentProxyHash := currentRevision.Annotations[proxy.ConfigHashKey]
+		desiredProxyHash := desiredRevision.Annotations[proxy.ConfigHashKey]
+		if currentProxyHash != desiredProxyHash {
 			state = StateNeedsUpgrade
-		case err == nil:
-			// inplace patch was successful, no changes in phases
-			state = StateUnchanged
-		default:
-			return false, "", fmt.Errorf("patching %s Revision: %w", desiredRevision.Name, err)
+		} else {
+			err = bc.createOrUpdate(ctx, getUserInfo(ext), desiredRevision)
+			switch {
+			case apierrors.IsInvalid(err):
+				// We could not update the current revision due to trying to update an immutable field.
+				// Therefore, we need to create a new revision.
+				state = StateNeedsUpgrade
+			case err == nil:
+				state = StateUnchanged
+			default:
+				return false, "", fmt.Errorf("patching %s Revision: %w", desiredRevision.Name, err)
+			}
 		}
 	}
 
@@ -530,6 +542,10 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 
 		desiredRevision.Name = fmt.Sprintf("%s-%d", ext.Name, revisionNumber)
 		desiredRevision.Spec.Revision = revisionNumber
+		// Clear server-added metadata fields from previous patch operation
+		desiredRevision.SetResourceVersion("")
+		desiredRevision.SetUID("")
+		desiredRevision.SetManagedFields(nil)
 
 		if err = bc.garbageCollectOldRevisions(ctx, prevRevisions); err != nil {
 			return false, "", fmt.Errorf("garbage collecting old revisions: %w", err)

internal/operator-controller/applier/provider.go

@@ -14,6 +14,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/config"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/proxy"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
@@ -33,6 +34,7 @@ type RegistryV1ManifestProvider struct {
 	CertificateProvider         render.CertificateProvider
 	IsWebhookSupportEnabled     bool
 	IsSingleOwnNamespaceEnabled bool
+	Proxy                       *proxy.Proxy
 }
 
 func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) {
@@ -68,6 +70,9 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
 
 	opts := []render.Option{
 		render.WithCertificateProvider(r.CertificateProvider),
+		// Always include proxy option to ensure manifests reflect current proxy state
+		// When r.Proxy is nil, this ensures proxy env vars are removed from manifests
+		render.WithProxy(r.Proxy),
 	}
 
 	if r.IsSingleOwnNamespaceEnabled {
@@ -111,6 +116,13 @@ func (r *RegistryV1ManifestProvider) extractBundleConfigOptions(rv1 *bundle.Regi
 	return opts, nil
 }
 
+// ProxyFingerprint returns a stable hash of the proxy configuration.
+// This is used to detect when proxy settings change and a new revision is needed.
+// The hash is calculated once during proxy construction and cached in the Proxy itself.
+func (r *RegistryV1ManifestProvider) ProxyFingerprint() string {
+	return r.Proxy.Fingerprint()
+}
+
 // RegistryV1HelmChartProvider creates a Helm-Chart from a registry+v1 bundle and its associated ClusterExtension
 type RegistryV1HelmChartProvider struct {
 	ManifestProvider ManifestProvider

internal/operator-controller/controllers/boxcutter_reconcile_steps.go

@@ -32,6 +32,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/proxy"
 )
 
 type BoxcutterRevisionStatesGetter struct {
@@ -96,7 +97,7 @@ func MigrateStorage(m StorageMigrator) ReconcileStepFunc {
 	}
 }
 
-func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error)) ReconcileStepFunc {
+func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error), proxyFingerprint func() string) ReconcileStepFunc {
 	return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
 		l := log.FromContext(ctx)
 		revisionAnnotations := map[string]string{
@@ -105,6 +106,10 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e
 			labels.BundleVersionKey:   state.resolvedRevisionMetadata.Version,
 			labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image,
 		}
+		// Add proxy config fingerprint to detect when proxy settings change
+		if fp := proxyFingerprint(); fp != "" {
+			revisionAnnotations[proxy.ConfigHashKey] = fp
+		}
 		objLbls := map[string]string{
 			labels.OwnerKindKey: ocv1.ClusterExtensionKind,
 			labels.OwnerNameKey: ext.GetName(),

internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go

@@ -135,6 +135,8 @@ func TestApplyBundleWithBoxcutter(t *testing.T) {
 
 			stepFunc := ApplyBundleWithBoxcutter(func(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _, _ map[string]string) (bool, string, error) {
 				return true, "", nil
+			}, func() string {
+				return "" // empty proxy fingerprint for tests
 			})
 			result, err := stepFunc(ctx, state, ext)
 			require.NoError(t, err)

internal/operator-controller/proxy/proxy.go

@@ -0,0 +1,126 @@
+// Package proxy defines HTTP proxy configuration types used across applier implementations.
+package proxy
+
+import (
+	"crypto/sha256"
+	"encoding/json"
+	"fmt"
+	"net/url"
+	"os"
+	"strings"
+)
+
+const (
+	// ConfigHashKey is the annotation key used to record the proxy configuration hash.
+	// This annotation is set on both ClusterExtensions and ClusterExtensionRevisions.
+	// When the hash changes on a ClusterExtension, it triggers reconciliation.
+	// Comparing hashes between ClusterExtension and its current revision determines
+	// if a new revision is needed due to proxy configuration changes.
+	ConfigHashKey = "olm.operatorframework.io/proxy-config-hash"
+)
+
+// Proxy holds HTTP proxy configuration values that are applied to rendered resources.
+// These values are typically set as environment variables on generated Pods to enable
+// operators to function correctly in environments that require HTTP proxies for outbound
+// connections.
+type Proxy struct {
+	// HTTPProxy is the HTTP proxy URL (e.g., "http://proxy.example.com:8080").
+	// An empty value means no HTTP proxy is configured.
+	HTTPProxy string
+	// HTTPSProxy is the HTTPS proxy URL (e.g., "https://proxy.example.com:8443").
+	// An empty value means no HTTPS proxy is configured.
+	HTTPSProxy string
+	// NoProxy is a comma-separated list of hosts, domains, or CIDR ranges that should
+	// bypass the proxy (e.g., "localhost,127.0.0.1,.cluster.local").
+	// An empty value means all traffic will use the proxy (if configured).
+	NoProxy string
+	// fingerprint is a cached hash of the proxy configuration, calculated once during construction.
+	// This is used to detect when proxy settings change and a new revision is needed.
+	fingerprint string
+}
+
+// NewFromEnv creates a new Proxy from environment variables.
+// Returns nil if no proxy environment variables are set.
+// The fingerprint is calculated once during construction and cached.
+func NewFromEnv() *Proxy {
+	httpProxy := os.Getenv("HTTP_PROXY")
+	httpsProxy := os.Getenv("HTTPS_PROXY")
+	noProxy := os.Getenv("NO_PROXY")
+
+	// If no proxy variables are set, return nil
+	if httpProxy == "" && httpsProxy == "" && noProxy == "" {
+		return nil
+	}
+
+	p := &Proxy{
+		HTTPProxy:  httpProxy,
+		HTTPSProxy: httpsProxy,
+		NoProxy:    noProxy,
+	}
+
+	// Calculate and cache the fingerprint
+	p.fingerprint = calculateFingerprint(p)
+
+	return p
+}
+
+// calculateFingerprint computes a stable hash of the proxy configuration.
+func calculateFingerprint(p *Proxy) string {
+	if p == nil {
+		return ""
+	}
+	data, err := json.Marshal(p)
+	if err != nil {
+		// This should never happen for a simple struct with string fields,
+		// but return empty string if it does
+		return ""
+	}
+	hash := sha256.Sum256(data)
+	return fmt.Sprintf("%x", hash[:8])
+}
+
+// Fingerprint returns the cached hash of the proxy configuration.
+// This is used to detect when proxy settings change and a new revision is needed.
+// Returns an empty string if the proxy is nil.
+func (p *Proxy) Fingerprint() string {
+	if p == nil {
+		return ""
+	}
+	return p.fingerprint
+}
+
+// SanitizeURL removes credentials from a proxy URL for safe logging.
+// Returns the original string if it's not a valid URL or doesn't contain credentials.
+// If the string contains @ but credentials can't be parsed out, returns a redacted version.
+func SanitizeURL(proxyURL string) string {
+	if proxyURL == "" {
+		return ""
+	}
+
+	u, err := url.Parse(proxyURL)
+	if err != nil {
+		// If we can't parse it, check if it might contain credentials (user:pass@host pattern)
+		// If so, redact it to avoid leaking credentials in logs
+		if strings.Contains(proxyURL, "@") {
+			return "<redacted>"
+		}
+		// Otherwise return as-is (might be a hostname or other format without credentials)
+		return proxyURL
+	}
+
+	// If there's user info, remove it and return sanitized URL
+	if u.User != nil {
+		u.User = nil
+		return u.String()
+	}
+
+	// If no user info was parsed but the string contains @, it might be a schemelessly-formatted
+	// URL like "user:pass@host:port" which url.Parse doesn't recognize as having credentials.
+	// Redact it to be safe.
+	if strings.Contains(proxyURL, "@") {
+		return "<redacted>"
+	}
+
+	// No credentials detected, return as-is
+	return proxyURL
+}