Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 81 additions & 1 deletion cmd/operator-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -219,6 +220,7 @@ func validateMetricsFlags() error {
}
return nil
}

func run() error {
setupLog.Info("starting up the controller", "version info", version.String())

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
Expand Down
36 changes: 26 additions & 10 deletions internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions internal/operator-controller/applier/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
126 changes: 126 additions & 0 deletions internal/operator-controller/proxy/proxy.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading
Loading