From b41d57921521a507f97a83639debc6bd4395e67c Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 19 Aug 2026 18:00:01 +0200 Subject: [PATCH] OCPBUGS-112075: skip proxy for OSImageStream discovery in HyperShift In HyperShift (ExternalTopologyMode), MCC bootstrap runs inside the ignition-server pod on the management cluster but inherits the guest cluster's proxy config. The guest proxy is unreachable from the management cluster, causing OSImageStream discovery to timeout. Add WithoutProxy() to SysContextBuilder and use it in buildSysContextFactory() when ControlPlaneTopology is External. Consolidate the two separate sysCtxFactory creation sites into a single one in Run(), shared by fetchOSImageStream and StreamClassInspector. Co-authored-by: Pablo Acevedo Co-Authored-By: Claude Opus 4.6 Signed-off-by: Juan Manuel Parrilla Madrid --- pkg/controller/bootstrap/bootstrap.go | 30 ++++++--------- pkg/imageutils/sys_context.go | 9 ++++- pkg/imageutils/sys_context_test.go | 55 +++++++++++++++++---------- 3 files changed, 54 insertions(+), 40 deletions(-) diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 55c73f9652..5da3beadb5 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -50,7 +50,7 @@ type Bootstrap struct { // dir used to read pools and user defined machineconfigs. manifestDir string // pull secret file - pullSecretFile string + pullSecretFile string imageStreamFactory osimagestream.ImageStreamFactory inspectorFactory osimagestream.ImagesInspectorFactory } @@ -242,21 +242,15 @@ func (b *Bootstrap) Run(destDir string) error { return fmt.Errorf("error filtering pools: %w", err) } + sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, cconfig.Spec.Infra, imgCfg, icspRules, idmsRules, itmsRules) + // Enable OSImageStreams if the FeatureGate is active. // Previously this also excluded ExternalTopologyMode (HyperShift) because // HyperShift did not yet write stream selection into the synthetic MCP. // Now that HyperShift writes 99_osimagestream.yaml into the MCC template // directory (openshift/hypershift#8792), the guard is no longer needed. if osimagestream.IsFeatureEnabled(fgHandler) { - osImageStream, err = b.fetchOSImageStream( - imageStream, - cconfig, - icspRules, - idmsRules, - itmsRules, - imgCfg, - pullSecret, - osImageStream) + osImageStream, err = b.fetchOSImageStream(sysCtxFactory, imageStream, cconfig, osImageStream) if err != nil { return err } @@ -367,7 +361,6 @@ func (b *Bootstrap) Run(destDir string) error { klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs)) } - sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, imgCfg, icspRules, idmsRules, itmsRules) inspector := osimagestream.NewStreamClassInspector(b.inspectorFactory, sysCtxFactory) fpools, gconfigs, err := render.RunBootstrap(context.TODO(), pools, configs, cconfig, osImageStream, inspector) if err != nil { @@ -498,21 +491,15 @@ func filterPools(pools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfigPool } func (b *Bootstrap) fetchOSImageStream( + sysCtxFactory imageutils.SysContextFactory, imageStream *imagev1.ImageStream, cconfig *mcfgv1.ControllerConfig, - icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, - idmsRules []*apicfgv1.ImageDigestMirrorSet, - itmsRules []*apicfgv1.ImageTagMirrorSet, - imgCfg *apicfgv1.Image, - pullSecret *corev1.Secret, existingOSImageStream *mcfgv1.OSImageStream, ) (*mcfgv1.OSImageStream, error) { ctx, cancel := context.WithTimeout(context.Background(), time.Minute) defer cancel() - sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, imgCfg, icspRules, idmsRules, itmsRules) - factory := b.imageStreamFactory createOpts := osimagestream.CreateOptions{ ExistingOSImageStream: existingOSImageStream, @@ -536,6 +523,7 @@ func (b *Bootstrap) fetchOSImageStream( func buildSysContextFactory( pullSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig, + infra *apicfgv1.Infrastructure, imgCfg *apicfgv1.Image, icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmsRules []*apicfgv1.ImageDigestMirrorSet, @@ -546,6 +534,12 @@ func buildSysContextFactory( WithControllerConfig(cconfig). WithSecret(pullSecret) + // In HCP the proxy config belongs to the data plane cluster and is + // unreachable from the management cluster where this code runs. + if infra != nil && infra.Status.ControlPlaneTopology == apicfgv1.ExternalTopologyMode { + builder.WithoutProxy() + } + registriesConfig, err := imageutils.GenerateRegistriesConfig(imgCfg, icspRules, idmsRules, itmsRules) if err != nil { return nil, fmt.Errorf("failed to generate registries config: %w", err) diff --git a/pkg/imageutils/sys_context.go b/pkg/imageutils/sys_context.go index 53e261da22..919840e137 100644 --- a/pkg/imageutils/sys_context.go +++ b/pkg/imageutils/sys_context.go @@ -30,6 +30,7 @@ type SysContextBuilder struct { secret *corev1.Secret controllerConfig *mcfgv1.ControllerConfig registriesConfig *sysregistriesv2.V2RegistriesConf + skipProxy bool } // NewSysContextBuilder creates a new SysContextBuilder for building SysContext instances. @@ -49,6 +50,12 @@ func (b *SysContextBuilder) WithControllerConfig(cc *mcfgv1.ControllerConfig) *S return b } +// WithoutProxy disables proxy configuration even if the ControllerConfig has one. +func (b *SysContextBuilder) WithoutProxy() *SysContextBuilder { + b.skipProxy = true + return b +} + // WithRegistriesConfig adds custom container registry configuration to the SysContext. // The registries config will be written as a TOML file and used for registry lookups, // mirrors, and pull policies. @@ -157,7 +164,7 @@ func (b *SysContextBuilder) buildRegistries(sysContext *SysContext) error { // Prioritizes HTTPS proxy over HTTP proxy when both are configured. // Returns early if no controller config was provided or no proxy is configured. func (b *SysContextBuilder) buildProxy(sysContext *SysContext) error { - if b.controllerConfig == nil { + if b.controllerConfig == nil || b.skipProxy { return nil } // TODO: Improve when containers-libs is used with https://github.com/containers/container-libs/pull/583 diff --git a/pkg/imageutils/sys_context_test.go b/pkg/imageutils/sys_context_test.go index 33bfe08632..36c39c8d1d 100644 --- a/pkg/imageutils/sys_context_test.go +++ b/pkg/imageutils/sys_context_test.go @@ -320,6 +320,7 @@ func TestSysContextBuilderWithProxy(t *testing.T) { name string httpProxy string httpsProxy string + skipProxy bool expectedScheme string expectedHost string expectedUsername string @@ -376,6 +377,12 @@ func TestSysContextBuilderWithProxy(t *testing.T) { expectedUsername: "user", expectedPassword: "p@ssw0rd!", }, + { + name: "WithoutProxy skips proxy even when configured", + httpsProxy: "https://proxy.example.com:3128", + httpProxy: "http://proxy.example.com:8080", + skipProxy: true, + }, } for _, tc := range testCases { @@ -389,35 +396,41 @@ func TestSysContextBuilderWithProxy(t *testing.T) { }, } - sysCtx, err := NewSysContextBuilder(). + builder := NewSysContextBuilder(). WithSecret(secret). - WithControllerConfig(cc). - Build() + WithControllerConfig(cc) + if tc.skipProxy { + builder.WithoutProxy() + } + + sysCtx, err := builder.Build() require.NoError(t, err, "SysContextBuilder.Build should not fail") require.NotNil(t, sysCtx, "SysContext wrapper should not be nil") require.NotNil(t, sysCtx.SysContext, "Underlying SystemContext should not be nil") - // Check that proxy was set correctly - require.NotNil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should not be nil") - assert.Equal(t, tc.expectedScheme, sysCtx.SysContext.DockerProxyURL.Scheme, "Proxy scheme should match") - assert.Equal(t, tc.expectedHost, sysCtx.SysContext.DockerProxyURL.Host, "Proxy host should match") + if tc.skipProxy { + assert.Nil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should be nil when proxy is skipped") + } else { + require.NotNil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should not be nil") + assert.Equal(t, tc.expectedScheme, sysCtx.SysContext.DockerProxyURL.Scheme, "Proxy scheme should match") + assert.Equal(t, tc.expectedHost, sysCtx.SysContext.DockerProxyURL.Host, "Proxy host should match") - // Check username and password if provided - if tc.expectedUsername != "" { - assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") - assert.Equal(t, tc.expectedUsername, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should match") - } + if tc.expectedUsername != "" { + assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") + assert.Equal(t, tc.expectedUsername, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should match") + } - if tc.expectedPassword != "" { - assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") - password, hasPassword := sysCtx.SysContext.DockerProxyURL.User.Password() - assert.True(t, hasPassword, "Proxy should have password") - assert.Equal(t, tc.expectedPassword, password, "Proxy password should match") - } + if tc.expectedPassword != "" { + assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") + password, hasPassword := sysCtx.SysContext.DockerProxyURL.User.Password() + assert.True(t, hasPassword, "Proxy should have password") + assert.Equal(t, tc.expectedPassword, password, "Proxy password should match") + } - if tc.expectedUsername == "" && tc.expectedPassword == "" { - if sysCtx.SysContext.DockerProxyURL.User != nil { - assert.Empty(t, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should be empty") + if tc.expectedUsername == "" && tc.expectedPassword == "" { + if sysCtx.SysContext.DockerProxyURL.User != nil { + assert.Empty(t, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should be empty") + } } }