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") + } } }