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 --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,26 @@ func run() error {
 	}
 
 	certProvider := getCertificateProvider()
+
+	// Read proxy configuration from environment variables
+	var proxyConfig *proxy.Proxy
+	httpProxy := os.Getenv("HTTP_PROXY")
+	httpsProxy := os.Getenv("HTTPS_PROXY")
+	noProxy := os.Getenv("NO_PROXY")
+	if httpProxy != "" || httpsProxy != "" || noProxy != "" {
+		proxyConfig = &proxy.Proxy{
+			HTTPProxy:  httpProxy,
+			HTTPSProxy: httpsProxy,
+			NoProxy:    noProxy,
+		}
+	}
+
 	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 +557,50 @@ 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]
+			// Trigger reconciliation by adding an annotation
+			if ext.Annotations == nil {
+				ext.Annotations = make(map[string]string)
+			}
+			ext.Annotations["olm.operatorframework.io/proxy-reconcile"] = time.Now().Format(time.RFC3339)
+			if err := cl.Update(ctx, ext); err != nil {
+				setupLog.Error(err, "failed to trigger reconciliation for 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 +686,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

@@ -485,17 +485,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[labels.ProxyConfigHashKey]
+		desiredProxyHash := desiredRevision.Annotations[labels.ProxyConfigHashKey]
+		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 +541,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) {
@@ -70,6 +72,14 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
 		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
+	if r.Proxy != nil {
+		opts = append(opts, render.WithProxy(*r.Proxy))
+	} else {
+		opts = append(opts, render.WithProxy(proxy.Proxy{}))
+	}
+
 	if r.IsSingleOwnNamespaceEnabled {
 		configOpts, err := r.extractBundleConfigOptions(&rv1, ext)
 		if err != nil {
@@ -111,6 +121,12 @@ 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.
+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

@@ -96,7 +96,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 +105,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[labels.ProxyConfigHashKey] = 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/labels/labels.go

@@ -44,4 +44,9 @@ const (
 	// that were created during migration from Helm releases. This label is used
 	// to distinguish migrated revisions from those created by normal Boxcutter operation.
 	MigratedFromHelmKey = "olm.operatorframework.io/migrated-from-helm"
+
+	// ProxyConfigHashKey is the annotation key used to record a hash of the proxy configuration
+	// (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) that was used when generating the revision manifests.
+	// This allows detection of proxy configuration changes that require creating a new revision.
+	ProxyConfigHashKey = "olm.operatorframework.io/proxy-config-hash"
 )

internal/operator-controller/proxy/proxy.go

@@ -0,0 +1,66 @@
+// Package proxy defines HTTP proxy configuration types used across applier implementations.
+package proxy
+
+import (
+	"crypto/sha256"
+	"encoding/json"
+	"fmt"
+	"net/url"
+)
+
+// 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 returns a stable 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 ""
+	}
+	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])
+}
+
+// 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.
+func SanitizeURL(proxyURL string) string {
+	if proxyURL == "" {
+		return ""
+	}
+
+	u, err := url.Parse(proxyURL)
+	if err != nil {
+		// If we can't parse it, return as-is (might be a hostname or other format)
+		return proxyURL
+	}
+
+	// If there's no user info, return as-is
+	if u.User == nil {
+		return proxyURL
+	}
+
+	// Remove user info and return sanitized URL
+	u.User = nil
+	return u.String()
+}

internal/operator-controller/rukpak/render/registryv1/generators/generators.go

@@ -88,11 +88,23 @@ func BundleCSVDeploymentGenerator(rv1 *bundle.RegistryV1, opts render.Options) (
 		// See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/deployment.go#L177-L180
 		depSpec.Spec.RevisionHistoryLimit = ptr.To(int32(1))
 
+		// Always call WithProxy to ensure proxy env vars are managed correctly
+		// When proxy is nil, this will remove any existing proxy env vars
+		var httpProxy, httpsProxy, noProxy string
+		if opts.Proxy != nil {
+			httpProxy = opts.Proxy.HTTPProxy
+			httpsProxy = opts.Proxy.HTTPSProxy
+			noProxy = opts.Proxy.NoProxy
+		}
+		deploymentOpts := []ResourceCreatorOption{
+			WithDeploymentSpec(depSpec.Spec),
+			WithLabels(depSpec.Label),
+			WithProxy(httpProxy, httpsProxy, noProxy),
+		}
 		deploymentResource := CreateDeploymentResource(
 			depSpec.Name,
 			opts.InstallNamespace,
-			WithDeploymentSpec(depSpec.Spec),
-			WithLabels(depSpec.Label),
+			deploymentOpts...,
 		)
 
 		secretInfo := render.CertProvisionerFor(depSpec.Name, opts).GetCertSecretInfo()