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

@@ -474,11 +474,26 @@ func run() error {
 	}
 
 	certProvider := getCertificateProvider()
+
+	// Read proxy configuration from environment variables
+	var proxy *render.Proxy
+	httpProxy := os.Getenv("HTTP_PROXY")
+	httpsProxy := os.Getenv("HTTPS_PROXY")
+	noProxy := os.Getenv("NO_PROXY")
+	if httpProxy != "" || httpsProxy != "" || noProxy != "" {
+		proxy = &render.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:                       proxy,
 	}
 	var cerCfg reconcilerConfigurator
 	if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {
@@ -540,6 +555,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 proxy != nil {
+			setupLog.Info("proxy configuration detected, triggering reconciliation of all ClusterExtensions",
+				"httpProxy", proxy.HTTPProxy, "httpsProxy", proxy.HTTPSProxy, "noProxy", proxy.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 +684,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

@@ -33,6 +33,7 @@ type RegistryV1ManifestProvider struct {
 	CertificateProvider         render.CertificateProvider
 	IsWebhookSupportEnabled     bool
 	IsSingleOwnNamespaceEnabled bool
+	Proxy                       *render.Proxy
 }
 
 func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) {
@@ -70,6 +71,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(render.Proxy{}))
+	}
+
 	if r.IsSingleOwnNamespaceEnabled {
 		configOpts, err := r.extractBundleConfigOptions(&rv1, ext)
 		if err != nil {
@@ -111,6 +120,17 @@ 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 {
+	if r.Proxy == nil {
+		return ""
+	}
+	data, _ := json.Marshal(r.Proxy)
+	hash := sha256.Sum256(data)
+	return fmt.Sprintf("%x", hash[:8])
+}
+
 // 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/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))
 
+		deploymentOpts := []ResourceCreatorOption{
+			WithDeploymentSpec(depSpec.Spec),
+			WithLabels(depSpec.Label),
+		}
+		// 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 = append(deploymentOpts, WithProxy(httpProxy, httpsProxy, noProxy))
 		deploymentResource := CreateDeploymentResource(
 			depSpec.Name,
 			opts.InstallNamespace,
-			WithDeploymentSpec(depSpec.Spec),
-			WithLabels(depSpec.Label),
+			deploymentOpts...,
 		)
 
 		secretInfo := render.CertProvisionerFor(depSpec.Name, opts).GetCertSecretInfo()

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

@@ -115,6 +115,62 @@ func WithMutatingWebhooks(webhooks ...admissionregistrationv1.MutatingWebhook) f
 	}
 }
 
+// WithProxy applies HTTP proxy environment variables to Deployment resources
+func WithProxy(httpProxy, httpsProxy, noProxy string) func(client.Object) {
+	return func(obj client.Object) {
+		switch o := obj.(type) {
+		case *appsv1.Deployment:
+			addProxyEnvVars(httpProxy, httpsProxy, noProxy, o.Spec.Template.Spec.Containers)
+		}
+	}
+}
+
+func addProxyEnvVars(httpProxy, httpsProxy, noProxy string, containers []corev1.Container) {
+	proxyEnvNames := []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"}
+
+	for i := range containers {
+		// First, remove any existing proxy env vars to ensure clean slate
+		// This allows us to remove proxy vars when they're no longer configured
+		var newEnv []corev1.EnvVar
+		if len(containers[i].Env) > 0 {
+			newEnv = make([]corev1.EnvVar, 0, len(containers[i].Env))
+			for _, env := range containers[i].Env {
+				isProxyVar := false
+				for _, proxyName := range proxyEnvNames {
+					if env.Name == proxyName {
+						isProxyVar = true
+						break
+					}
+				}
+				if !isProxyVar {
+					newEnv = append(newEnv, env)
+				}
+			}
+		}
+		containers[i].Env = newEnv
+
+		// Then add the proxy env vars if they're configured
+		if len(httpProxy) > 0 {
+			containers[i].Env = append(containers[i].Env, corev1.EnvVar{
+				Name:  "HTTP_PROXY",
+				Value: httpProxy,
+			})
+		}
+		if len(httpsProxy) > 0 {
+			containers[i].Env = append(containers[i].Env, corev1.EnvVar{
+				Name:  "HTTPS_PROXY",
+				Value: httpsProxy,
+			})
+		}
+		if len(noProxy) > 0 {
+			containers[i].Env = append(containers[i].Env, corev1.EnvVar{
+				Name:  "NO_PROXY",
+				Value: noProxy,
+			})
+		}
+	}
+}
+
 // CreateServiceAccountResource creates a ServiceAccount resource with name 'name', namespace 'namespace', and applying
 // any ServiceAccount related options in opts
 func CreateServiceAccountResource(name string, namespace string, opts ...ResourceCreatorOption) *corev1.ServiceAccount {

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

@@ -6,8 +6,10 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -276,3 +278,60 @@ func Test_WithMutatingWebhook(t *testing.T) {
 		{Name: "wh-two"},
 	}, wh.Webhooks)
 }
+
+func Test_WithProxy(t *testing.T) {
+	depSpec := appsv1.DeploymentSpec{
+		Template: corev1.PodTemplateSpec{
+			Spec: corev1.PodSpec{
+				Containers: []corev1.Container{
+					{
+						Name: "c1",
+						Env: []corev1.EnvVar{
+							{
+								Name:  "TEST",
+								Value: "xxx",
+							},
+						},
+					},
+					{
+						Name: "c2",
+						Env: []corev1.EnvVar{
+							{
+								Name:  "TEST",
+								Value: "xxx",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	depl := generators.CreateDeploymentResource(
+		"test",
+		"test-ns",
+		generators.WithDeploymentSpec(depSpec),
+		generators.WithProxy("http,prox", "https,prox", "no,prox"),
+	)
+
+	expected := []corev1.EnvVar{
+		{
+			Name:  "TEST",
+			Value: "xxx",
+		},
+		{
+			Name:  "HTTP_PROXY",
+			Value: "http,prox",
+		},
+		{
+			Name:  "HTTPS_PROXY",
+			Value: "https,prox",
+		},
+		{
+			Name:  "NO_PROXY",
+			Value: "no,prox",
+		},
+	}
+	assert.Equal(t, expected, depl.Spec.Template.Spec.Containers[0].Env)
+	assert.Equal(t, expected, depl.Spec.Template.Spec.Containers[1].Env)
+}