From c4ff70f26e052070fb7a57171c91587f7b0a49d3 Mon Sep 17 00:00:00 2001 From: Yuedong Wu Date: Tue, 7 Apr 2026 18:19:25 +0800 Subject: [PATCH 1/3] Add proxy utility for env var injection into workloads CAPI provider workloads need HTTP_PROXY/HTTPS_PROXY/NO_PROXY env vars when the cluster is behind a proxy, otherwise they cannot reach external endpoints. Add pkg/util/proxy.go that reads the cluster-wide Proxy CR status and injects proxy env vars into Deployment containers. --- pkg/util/proxy.go | 161 ++++++++++++++++++++++ pkg/util/proxy_test.go | 301 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 462 insertions(+) create mode 100644 pkg/util/proxy.go create mode 100644 pkg/util/proxy_test.go diff --git a/pkg/util/proxy.go b/pkg/util/proxy.go new file mode 100644 index 0000000000..d11263f4bb --- /dev/null +++ b/pkg/util/proxy.go @@ -0,0 +1,161 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package util + +import ( + "context" + "fmt" + + configv1 "github.com/openshift/api/config/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + proxyResourceName = "cluster" +) + +// GetProxy returns the cluster-wide Proxy resource. +func GetProxy(ctx context.Context, cl client.Reader) (*configv1.Proxy, error) { + proxy := &configv1.Proxy{} + + if err := cl.Get(ctx, client.ObjectKey{Name: proxyResourceName}, proxy); err != nil { + return nil, fmt.Errorf("failed to get proxy %q: %w", proxyResourceName, err) + } + + return proxy, nil +} + +// ProxyEnvVars converts Proxy Status fields to environment variables. +// Only non-empty fields are included. +// Returns nil if all proxy fields are empty. +func ProxyEnvVars(proxy *configv1.Proxy) []corev1.EnvVar { + if proxy == nil { + return nil + } + + var envVars []corev1.EnvVar + + if proxy.Status.HTTPProxy != "" { + envVars = append(envVars, corev1.EnvVar{ + Name: "HTTP_PROXY", + Value: proxy.Status.HTTPProxy, + }) + } + + if proxy.Status.HTTPSProxy != "" { + envVars = append(envVars, corev1.EnvVar{ + Name: "HTTPS_PROXY", + Value: proxy.Status.HTTPSProxy, + }) + } + + if proxy.Status.NoProxy != "" { + envVars = append(envVars, corev1.EnvVar{ + Name: "NO_PROXY", + Value: proxy.Status.NoProxy, + }) + } + + return envVars +} + +// InjectProxyEnvVarsIntoUnstructured injects proxy environment variables into +// all containers and initContainers of Deployment unstructured objects. +// Skips if an env var with the same name already exists. No-op for other +// kinds or if envVars is empty. +func InjectProxyEnvVarsIntoUnstructured(obj *unstructured.Unstructured, envVars []corev1.EnvVar) error { + if len(envVars) == 0 { + return nil + } + + if obj.GetKind() != "Deployment" { + return nil + } + + containerPaths := [][]string{ + {"spec", "template", "spec", "containers"}, + {"spec", "template", "spec", "initContainers"}, + } + + for _, path := range containerPaths { + if err := injectEnvVarsAtPath(obj, path, envVars); err != nil { + return err + } + } + + return nil +} + +func injectEnvVarsAtPath(obj *unstructured.Unstructured, path []string, envVars []corev1.EnvVar) error { + containers, found, err := unstructured.NestedSlice(obj.Object, path...) + if err != nil || !found { + return nil //nolint:nilerr + } + + for i, c := range containers { + container, ok := c.(map[string]interface{}) + if !ok { + continue + } + + if err := injectEnvVarsIntoContainer(container, i, envVars); err != nil { + return err + } + + containers[i] = container + } + + if err := unstructured.SetNestedSlice(obj.Object, containers, path...); err != nil { + return fmt.Errorf("setting containers at %v: %w", path, err) + } + + return nil +} + +func injectEnvVarsIntoContainer(container map[string]interface{}, index int, envVars []corev1.EnvVar) error { + existingEnv, _, _ := unstructured.NestedSlice(container, "env") + + // Build a set of existing env var names to avoid duplicates. + names := make(map[string]bool, len(existingEnv)) + for _, e := range existingEnv { + if envMap, ok := e.(map[string]interface{}); ok { + if name, ok := envMap["name"].(string); ok { + names[name] = true + } + } + } + + // Append new env vars directly as unstructured maps, preserving + // existing entries (including any valueFrom fields) untouched. + for _, ev := range envVars { + if names[ev.Name] { + continue + } + + existingEnv = append(existingEnv, map[string]interface{}{ + "name": ev.Name, + "value": ev.Value, + }) + } + + if err := unstructured.SetNestedSlice(container, existingEnv, "env"); err != nil { + return fmt.Errorf("setting env vars on container %d: %w", index, err) + } + + return nil +} diff --git a/pkg/util/proxy_test.go b/pkg/util/proxy_test.go new file mode 100644 index 0000000000..f8891e4559 --- /dev/null +++ b/pkg/util/proxy_test.go @@ -0,0 +1,301 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package util + +import ( + "testing" + + . "github.com/onsi/gomega" + + configv1 "github.com/openshift/api/config/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestProxyEnvVars(t *testing.T) { + for _, tc := range []struct { + name string + proxy *configv1.Proxy + wantLen int + wantNil bool + wantVars map[string]string + }{ + { + name: "all fields set", + proxy: &configv1.Proxy{ + Status: configv1.ProxyStatus{ + HTTPProxy: "http://proxy:3128", + HTTPSProxy: "https://proxy:3129", + NoProxy: ".cluster.local", + }, + }, + wantLen: 3, + wantVars: map[string]string{ + "HTTP_PROXY": "http://proxy:3128", + "HTTPS_PROXY": "https://proxy:3129", + "NO_PROXY": ".cluster.local", + }, + }, + { + name: "only HTTPProxy set", + proxy: &configv1.Proxy{ + Status: configv1.ProxyStatus{ + HTTPProxy: "http://proxy:3128", + }, + }, + wantLen: 1, + wantVars: map[string]string{ + "HTTP_PROXY": "http://proxy:3128", + }, + }, + { + name: "empty proxy", + proxy: &configv1.Proxy{}, + wantNil: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + envVars := ProxyEnvVars(tc.proxy) + if tc.wantNil { + g.Expect(envVars).To(BeNil()) + return + } + + g.Expect(envVars).To(HaveLen(tc.wantLen)) + + for _, ev := range envVars { + g.Expect(ev.Value).To(Equal(tc.wantVars[ev.Name]), "env var %s", ev.Name) + } + }) + } +} + +func TestInjectProxyEnvVarsIntoUnstructured(t *testing.T) { + proxyEnvVars := []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + {Name: "HTTPS_PROXY", Value: "https://proxy:3129"}, + {Name: "NO_PROXY", Value: ".cluster.local"}, + } + + for _, tc := range []struct { + name string + obj *unstructured.Unstructured + envVars []corev1.EnvVar + wantEnvCount int // per container + wantContainers int // expected number of containers with env vars + wantInitEnvCount int // per init container, 0 means don't check + wantInitContainers int + wantSkipInspect bool + wantEnvValues map[string]string // optional: verify specific env var values + wantValueFromNames []string // optional: verify valueFrom fields are preserved + }{ + { + name: "ConfigMap (non-workload) is a no-op", + obj: makeUnstructuredConfigMap("test-cm"), + envVars: proxyEnvVars, + wantSkipInspect: true, + }, + { + name: "existing env var preserved and not duplicated", + obj: makeUnstructuredWorkload([]string{"mgr"}, map[string]string{"HTTP_PROXY": "custom"}, nil), + envVars: proxyEnvVars, + wantEnvCount: 3, // 1 existing (preserved) + 2 new + wantContainers: 1, + wantEnvValues: map[string]string{"HTTP_PROXY": "custom"}, + }, + { + name: "empty env vars slice is a no-op", + obj: makeUnstructuredWorkload([]string{"mgr"}, nil, nil), + envVars: nil, + wantSkipInspect: true, + }, + { + name: "container with existing non-proxy env preserved", + obj: makeUnstructuredWorkload([]string{"mgr"}, map[string]string{"FOO": "bar"}, nil), + envVars: proxyEnvVars, + wantEnvCount: 4, // 1 existing + 3 proxy + wantContainers: 1, + wantEnvValues: map[string]string{"FOO": "bar"}, + }, + { + name: "multiple containers all get env vars", + obj: makeUnstructuredWorkload([]string{"a", "b"}, nil, nil), + envVars: proxyEnvVars, + wantEnvCount: 3, + wantContainers: 2, + }, + { + name: "init containers get env vars", + obj: makeUnstructuredWorkload([]string{"main"}, nil, []string{"init"}), + envVars: proxyEnvVars, + wantEnvCount: 3, + wantContainers: 1, + wantInitEnvCount: 3, + wantInitContainers: 1, + }, + { + name: "existing valueFrom env var is preserved", + obj: makeUnstructuredWorkloadWithValueFrom("mgr", "SECRET_KEY", "my-secret", "key"), + envVars: proxyEnvVars, + wantEnvCount: 4, // 1 valueFrom + 3 proxy + wantContainers: 1, + wantValueFromNames: []string{"SECRET_KEY"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + err := InjectProxyEnvVarsIntoUnstructured(tc.obj, tc.envVars) + g.Expect(err).NotTo(HaveOccurred()) + + if tc.wantSkipInspect { + return + } + + containers, _, _ := unstructured.NestedSlice(tc.obj.Object, "spec", "template", "spec", "containers") + g.Expect(containers).To(HaveLen(tc.wantContainers)) + + for _, c := range containers { + container := c.(map[string]interface{}) + env, _, _ := unstructured.NestedSlice(container, "env") + g.Expect(env).To(HaveLen(tc.wantEnvCount)) + + for wantName, wantValue := range tc.wantEnvValues { + g.Expect(env).To(ContainElement(SatisfyAll( + HaveKeyWithValue("name", wantName), + HaveKeyWithValue("value", wantValue), + ))) + } + + for _, wantName := range tc.wantValueFromNames { + g.Expect(env).To(ContainElement(SatisfyAll( + HaveKeyWithValue("name", wantName), + HaveKey("valueFrom"), + Not(HaveKey("value")), + ))) + } + } + + if tc.wantInitContainers > 0 { + initContainers, _, _ := unstructured.NestedSlice(tc.obj.Object, "spec", "template", "spec", "initContainers") + g.Expect(initContainers).To(HaveLen(tc.wantInitContainers)) + + for _, c := range initContainers { + container := c.(map[string]interface{}) + env, _, _ := unstructured.NestedSlice(container, "env") + g.Expect(env).To(HaveLen(tc.wantInitEnvCount)) + } + } + }) + } +} + +// Test helpers + +func makeUnstructuredWorkload(containerNames []string, existingEnv map[string]string, initContainerNames []string) *unstructured.Unstructured { + envList := make([]interface{}, 0, len(existingEnv)) + for k, v := range existingEnv { + envList = append(envList, map[string]interface{}{"name": k, "value": v}) + } + + containers := make([]interface{}, len(containerNames)) + for i, name := range containerNames { + c := map[string]interface{}{ + "name": name, + "image": "registry.example.com/test:latest", + } + if i == 0 && len(envList) > 0 { + c["env"] = envList + } + + containers[i] = c + } + + podSpec := map[string]interface{}{ + "containers": containers, + } + + if len(initContainerNames) > 0 { + initContainers := make([]interface{}, len(initContainerNames)) + for i, name := range initContainerNames { + initContainers[i] = map[string]interface{}{ + "name": name, + "image": "registry.example.com/init:latest", + } + } + + podSpec["initContainers"] = initContainers + } + + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test", "namespace": "default"}, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": podSpec, + }, + }, + }, + } +} + +func makeUnstructuredWorkloadWithValueFrom(containerName, envName, secretName, secretKey string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test", "namespace": "default"}, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": containerName, + "image": "registry.example.com/test:latest", + "env": []interface{}{ + map[string]interface{}{ + "name": envName, + "valueFrom": map[string]interface{}{ + "secretKeyRef": map[string]interface{}{ + "name": secretName, + "key": secretKey, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +} + +func makeUnstructuredConfigMap(name string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{"name": name, "namespace": "default"}, + "data": map[string]interface{}{"key": "value"}, + }, + } +} From 97f07fe76ce7bd5289b61073ab23aa306414f0be Mon Sep 17 00:00:00 2001 From: Yuedong Wu Date: Tue, 7 Apr 2026 18:19:26 +0800 Subject: [PATCH 2/3] Add WithProxyConfig option to revision generator Proxy env vars must be part of the rendered revision so that content ID reflects proxy changes and triggers a new rollout. Add a RevisionRenderOption that injects proxy env vars into Deployment containers during rendering. --- pkg/revisiongenerator/revision.go | 27 ++++++++++++--- pkg/revisiongenerator/revision_test.go | 47 ++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/pkg/revisiongenerator/revision.go b/pkg/revisiongenerator/revision.go index 75cf5ba289..ed0e20fb78 100644 --- a/pkg/revisiongenerator/revision.go +++ b/pkg/revisiongenerator/revision.go @@ -26,6 +26,7 @@ import ( operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" operatorv1alpha1ac "github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -95,13 +96,13 @@ type renderedRevision struct { var _ RenderedRevision = &renderedRevision{} // NewRenderedRevision creates a new RenderedRevision from a list of provider image manifests. -func NewRenderedRevision(profiles []providerimages.ProviderImageManifests, opts ...revisionRenderOption) (RenderedRevision, error) { +func NewRenderedRevision(profiles []providerimages.ProviderImageManifests, opts ...RevisionRenderOption) (RenderedRevision, error) { return newRenderedRevision(profiles, opts...) } // newRenderedRevision implements NewRenderedRevision. It exists to return a // concrete type for internal use. -func newRenderedRevision(profiles []providerimages.ProviderImageManifests, opts ...revisionRenderOption) (*renderedRevision, error) { +func newRenderedRevision(profiles []providerimages.ProviderImageManifests, opts ...RevisionRenderOption) (*renderedRevision, error) { cfg := &revisionRenderConfig{} for _, opt := range opts { opt(cfg) @@ -246,21 +247,31 @@ func buildRevisionName(releaseVersion, contentID string, index int64) operatorv1 type revisionRenderConfig struct { objectCollectors []RevisionObjectCollector + proxyEnvVars []corev1.EnvVar } -type revisionRenderOption func(*revisionRenderConfig) +// RevisionRenderOption configures how a revision is rendered. +type RevisionRenderOption func(*revisionRenderConfig) // RevisionObjectCollector is a function that will be called for each object in // the rendered revision. type RevisionObjectCollector func(obj unstructured.Unstructured) // WithObjectCollectors adds object collectors to the revision render config. -func WithObjectCollectors(collectors ...RevisionObjectCollector) revisionRenderOption { +func WithObjectCollectors(collectors ...RevisionObjectCollector) RevisionRenderOption { return func(opts *revisionRenderConfig) { opts.objectCollectors = append(opts.objectCollectors, collectors...) } } +// WithProxyConfig adds proxy environment variables to inject into Deployment +// containers during revision rendering. +func WithProxyConfig(envVars []corev1.EnvVar) RevisionRenderOption { + return func(opts *revisionRenderConfig) { + opts.proxyEnvVars = envVars + } +} + // NewInstallerRevisionFromAPI creates an InstallerRevision by matching the // components in an API revision against the provided provider profiles and // rendering the matched manifests. The revision name and index are taken @@ -271,7 +282,7 @@ func WithObjectCollectors(collectors ...RevisionObjectCollector) revisionRenderO func NewInstallerRevisionFromAPI( apiRev operatorv1alpha1.ClusterAPIInstallerRevision, providerProfiles []providerimages.ProviderImageManifests, - opts ...revisionRenderOption, + opts ...RevisionRenderOption, ) (InstallerRevision, error) { matched := make([]providerimages.ProviderImageManifests, len(apiRev.Components)) @@ -347,6 +358,12 @@ func newRenderedComponent(providerProfile *providerimages.ProviderImageManifests unstructured = transformObject(unstructured, component.name) + if len(cfg.proxyEnvVars) > 0 { + if err := util.InjectProxyEnvVarsIntoUnstructured(&unstructured, cfg.proxyEnvVars); err != nil { + return nil, fmt.Errorf("error injecting proxy env vars: %w", err) + } + } + for _, collector := range cfg.objectCollectors { collector(unstructured) } diff --git a/pkg/revisiongenerator/revision_test.go b/pkg/revisiongenerator/revision_test.go index fddd50b499..69454fd9e6 100644 --- a/pkg/revisiongenerator/revision_test.go +++ b/pkg/revisiongenerator/revision_test.go @@ -24,6 +24,9 @@ import ( . "github.com/onsi/gomega" operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/openshift/cluster-capi-operator/pkg/providerimages" "github.com/openshift/cluster-capi-operator/pkg/test" ) @@ -403,6 +406,50 @@ data: }) } +func TestWithProxyConfig(t *testing.T) { + proxyEnvVars := []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + {Name: "HTTPS_PROXY", Value: "https://proxy:3129"}, + {Name: "NO_PROXY", Value: ".cluster.local"}, + } + + t.Run("proxy config changes contentID", func(t *testing.T) { + g := NewWithT(t) + + deployYAML := test.DeploymentYAML("mgr") + idWithout := contentIDForProfiles(g, profile(t, "p1", "img1", "default", deployYAML)) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "p1", "img1", "default", deployYAML)}, + WithProxyConfig(proxyEnvVars), + ))(g) + idWith := must(rev.ContentID())(g) + + g.Expect(idWith).NotTo(Equal(idWithout)) + }) + + t.Run("proxy env vars injected into Deployment objects", func(t *testing.T) { + g := NewWithT(t) + + deployYAML := test.DeploymentYAML("mgr") + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "p1", "img1", "default", deployYAML)}, + WithProxyConfig(proxyEnvVars), + ))(g) + + objs := rev.Components()[0].Objects() + g.Expect(objs).To(HaveLen(1)) + + u := objs[0].(*unstructured.Unstructured) + containers, _, _ := unstructured.NestedSlice(u.Object, "spec", "template", "spec", "containers") + g.Expect(containers).To(HaveLen(1)) + + container := containers[0].(map[string]interface{}) + env, _, _ := unstructured.NestedSlice(container, "env") + g.Expect(env).To(HaveLen(3)) + }) +} + func TestComponents(t *testing.T) { t.Run("returns correct component count and names", func(t *testing.T) { g := NewWithT(t) From cfd0b6d0696f108d0fc90aabda68d18ea24d4249 Mon Sep 17 00:00:00 2001 From: Yuedong Wu Date: Tue, 7 Apr 2026 18:19:26 +0800 Subject: [PATCH 3/3] Wire proxy config into revision and installer controllers Controllers need to read the Proxy CR and pass it to the revision generator so provider workloads get proxy env vars at deploy time. Both controllers now fetch the Proxy CR and forward env vars via WithProxyConfig. The revision controller watches Proxy to re-reconcile when proxy config changes. --- pkg/controllers/installer/helpers_test.go | 59 ++++++++++++- .../installer/installer_controller.go | 17 +++- .../installer/installer_controller_test.go | 71 ++++++++++++++++ .../installer/revision_reconciler.go | 10 ++- pkg/controllers/revision/helpers_test.go | 7 ++ .../revision/revision_controller.go | 20 ++++- .../revision/revision_controller_test.go | 84 +++++++++++++++++++ pkg/controllers/revision/suite_test.go | 9 ++ 8 files changed, 269 insertions(+), 8 deletions(-) diff --git a/pkg/controllers/installer/helpers_test.go b/pkg/controllers/installer/helpers_test.go index a9bbe2e85a..aa02ca4d96 100644 --- a/pkg/controllers/installer/helpers_test.go +++ b/pkg/controllers/installer/helpers_test.go @@ -26,6 +26,8 @@ import ( "github.com/onsi/gomega/types" configv1 "github.com/openshift/api/config/v1" operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -244,6 +246,13 @@ func setupProviderProfiles() { // It uses revisiongenerator to compute the content ID, then writes via status update. func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1.ClusterAPIInstallerRevision { GinkgoHelper() + return addRevisionWithOpts(ctx, nil, providerNames...) +} + +// addRevisionWithOpts is like addRevision but accepts render options (e.g. WithProxyConfig) +// so the content ID matches what the controller computes. +func addRevisionWithOpts(ctx context.Context, opts []revisiongenerator.RevisionRenderOption, providerNames ...string) operatorv1alpha1.ClusterAPIInstallerRevision { + GinkgoHelper() // Get current ClusterAPI to determine revision index. clusterAPI := &operatorv1alpha1.ClusterAPI{} @@ -255,7 +264,7 @@ func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1. profiles := lookupProfiles(providerNames...) // Render the revision to compute the correct content ID. - rendered, err := revisiongenerator.NewRenderedRevision(profiles) + rendered, err := revisiongenerator.NewRenderedRevision(profiles, opts...) Expect(err).NotTo(HaveOccurred()) revisionIndex := int64(len(clusterAPI.Status.Revisions) + 1) @@ -304,7 +313,7 @@ func lookupProfiles(names ...string) []providerimages.ProviderImageManifests { return profiles } -// createFixtures creates ClusterAPI and ClusterOperator singletons. +// createFixtures creates ClusterAPI, ClusterOperator, and Proxy singletons. func createFixtures(ctx context.Context) { GinkgoHelper() @@ -321,6 +330,12 @@ func createFixtures(ctx context.Context) { Expect(cl.Create(ctx, clusterAPIObj)).To(Succeed()) cleanupObjs = append(cleanupObjs, clusterAPIObj) + proxyObj := &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + Expect(cl.Create(ctx, proxyObj)).To(Succeed()) + cleanupObjs = append(cleanupObjs, proxyObj) + clusterOperatorObj := &configv1.ClusterOperator{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"}, } @@ -328,17 +343,22 @@ func createFixtures(ctx context.Context) { cleanupObjs = append(cleanupObjs, clusterOperatorObj) } -// createFixturesWithoutClusterAPI creates only the ClusterOperator (not ClusterAPI). +// createFixturesWithoutClusterAPI creates only the ClusterOperator and Proxy (not ClusterAPI). func createFixturesWithoutClusterAPI(ctx context.Context) { GinkgoHelper() + proxyObj := &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + Expect(cl.Create(ctx, proxyObj)).To(Succeed()) + clusterOperatorObj := &configv1.ClusterOperator{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"}, } Expect(cl.Create(ctx, clusterOperatorObj)).To(Succeed()) DeferCleanup(func(ctx context.Context) { - deleteAndWait(ctx, clusterOperatorObj) + deleteAndWait(ctx, proxyObj, clusterOperatorObj) }) } @@ -408,6 +428,37 @@ func getRelatedObjects(ctx context.Context) []configv1.ObjectReference { return co.Status.RelatedObjects } +// makeDeploymentAvailable waits for the named Deployment to exist and then +// sets its Available condition to True. This is a common pattern when a test +// revision includes a Deployment that the controller probes. +func makeDeploymentAvailable(ctx context.Context, name, namespace string) { + GinkgoHelper() + + deploy := &appsv1.Deployment{} + deploy.SetName(name) + deploy.SetNamespace(namespace) + + Eventually(func() error { + return cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy) + }). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(Succeed()) + + Eventually(kWithCtx(ctx).UpdateStatus(deploy, func() { + deploy.Status.Conditions = []appsv1.DeploymentCondition{ + { + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + Reason: "MinimumReplicasAvailable", + }, + } + })). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(Succeed()) +} + // waitForRevision waits for the given revision to be applied and the controller // to report Progressing=False. func waitForRevision(ctx context.Context, revision operatorv1alpha1.RevisionName) { diff --git a/pkg/controllers/installer/installer_controller.go b/pkg/controllers/installer/installer_controller.go index 832d903201..7ad4df4ffe 100644 --- a/pkg/controllers/installer/installer_controller.go +++ b/pkg/controllers/installer/installer_controller.go @@ -219,7 +219,22 @@ func (c *InstallerController) reconcile(ctx context.Context, log logr.Logger) op return opresult.WaitingOnExternal("ClusterAPI revisions") } - revisionReconciler := newRevisionReconciler(c, log) + // Read cluster-wide proxy configuration + var renderOpts []revisiongenerator.RevisionRenderOption + + proxy, err := util.GetProxy(ctx, c.client) + if err != nil { + return opresult.Error(fmt.Errorf("fetching proxy: %w", err)) + } + + if envVars := util.ProxyEnvVars(proxy); len(envVars) > 0 { + log.Info("Injecting proxy configuration into provider manifests", + "httpProxy", proxy.Status.HTTPProxy, "httpsProxy", proxy.Status.HTTPSProxy, "noProxy", proxy.Status.NoProxy) + + renderOpts = append(renderOpts, revisiongenerator.WithProxyConfig(envVars)) + } + + revisionReconciler := newRevisionReconciler(c, log, renderOpts...) reconciledRevision, messages, errs := revisionReconciler.reconcile(ctx, clusterAPI.Status.Revisions) // Write relatedObjects via non-SSA merge patch so the SSA conditions diff --git a/pkg/controllers/installer/installer_controller_test.go b/pkg/controllers/installer/installer_controller_test.go index f847214e1c..41519c9a22 100644 --- a/pkg/controllers/installer/installer_controller_test.go +++ b/pkg/controllers/installer/installer_controller_test.go @@ -36,6 +36,7 @@ import ( "github.com/openshift/cluster-capi-operator/pkg/operatorstatus" "github.com/openshift/cluster-capi-operator/pkg/revisiongenerator" "github.com/openshift/cluster-capi-operator/pkg/test" + "github.com/openshift/cluster-capi-operator/pkg/util" ) func getConfigMap(ctx context.Context, name string) (*corev1.ConfigMap, error) { @@ -649,6 +650,76 @@ var _ = Describe("InstallerController", Serial, func() { }) }) +var _ = Describe("InstallerController proxy", Serial, func() { + BeforeEach(func(ctx context.Context) { + createFixtures(ctx) + }, defaultNodeTimeout) + + AfterEach(func(ctx context.Context) { + emptyRevision := addEmptyRevision(ctx) + waitForRevision(ctx, emptyRevision.Name) + }, defaultNodeTimeout) + + It("injects proxy env vars into deployed Deployments", func(ctx context.Context) { + // Configure proxy before adding revision. + proxy := &configv1.Proxy{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).To(Succeed()) + proxy.Status = configv1.ProxyStatus{ + HTTPProxy: "http://proxy:3128", + HTTPSProxy: "https://proxy:3129", + NoProxy: ".cluster.local", + } + Expect(cl.Status().Update(ctx, proxy)).To(Succeed()) + + // The revision content ID must match what the controller computes, + // which includes proxy injection into Deployment manifests. + proxyEnvVars := util.ProxyEnvVars(proxy) + renderOpts := []revisiongenerator.RevisionRenderOption{ + revisiongenerator.WithProxyConfig(proxyEnvVars), + } + + revision := addRevisionWithOpts(ctx, renderOpts, providerDeployment) + makeDeploymentAvailable(ctx, deploymentName, "default") + waitForRevision(ctx, revision.Name) + + // Re-read the Deployment and verify proxy env vars. + deploy := &appsv1.Deployment{} + deploy.SetName(deploymentName) + deploy.SetNamespace("default") + Expect(cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy)).To(Succeed()) + + env := deploy.Spec.Template.Spec.Containers[0].Env + Expect(env).To(ContainElement(SatisfyAll( + HaveField("Name", "HTTP_PROXY"), + HaveField("Value", "http://proxy:3128"), + ))) + Expect(env).To(ContainElement(SatisfyAll( + HaveField("Name", "HTTPS_PROXY"), + HaveField("Value", "https://proxy:3129"), + ))) + Expect(env).To(ContainElement(SatisfyAll( + HaveField("Name", "NO_PROXY"), + HaveField("Value", ".cluster.local"), + ))) + }, defaultNodeTimeout) + + It("does not inject proxy env vars when proxy is empty", func(ctx context.Context) { + // Proxy is created with empty status by createFixtures. + revision := addRevision(ctx, providerDeployment) + makeDeploymentAvailable(ctx, deploymentName, "default") + waitForRevision(ctx, revision.Name) + + deploy := &appsv1.Deployment{} + deploy.SetName(deploymentName) + deploy.SetNamespace("default") + Expect(cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy)).To(Succeed()) + + for _, ev := range deploy.Spec.Template.Spec.Containers[0].Env { + Expect(ev.Name).NotTo(BeElementOf("HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY")) + } + }, defaultNodeTimeout) +}) + var _ = Describe("InstallerController without ClusterAPI", Serial, func() { It("reports WaitingOnExternal when ClusterAPI does not exist", func(ctx context.Context) { createFixturesWithoutClusterAPI(ctx) diff --git a/pkg/controllers/installer/revision_reconciler.go b/pkg/controllers/installer/revision_reconciler.go index 4cb4a6ff27..97c15ebc31 100644 --- a/pkg/controllers/installer/revision_reconciler.go +++ b/pkg/controllers/installer/revision_reconciler.go @@ -109,9 +109,10 @@ type revisionReconciler struct { collectedNonNSObjects sets.Set[collectedObjectRef] // intermediate storage crdGKResourceMapping map[schema.GroupKind]string // CRD GK → resource relatedObjects sets.Set[configv1.ObjectReference] // kept for backward compatibility + renderOpts []revisiongenerator.RevisionRenderOption } -func newRevisionReconciler(installerController *InstallerController, log logr.Logger) *revisionReconciler { +func newRevisionReconciler(installerController *InstallerController, log logr.Logger, renderOpts ...revisiongenerator.RevisionRenderOption) *revisionReconciler { return &revisionReconciler{ InstallerController: installerController, log: log, @@ -119,6 +120,7 @@ func newRevisionReconciler(installerController *InstallerController, log logr.Lo collectedNonNSObjects: sets.New[collectedObjectRef](), crdGKResourceMapping: make(map[schema.GroupKind]string), relatedObjects: sets.New[configv1.ObjectReference](), + renderOpts: renderOpts, } } @@ -136,8 +138,12 @@ func (r *revisionReconciler) reconcile(ctx context.Context, revisions []operator // Convert all API revisions upfront so that collectObjects (and thus // relatedObjects) is fully populated before reconciliation begins. + opts := make([]revisiongenerator.RevisionRenderOption, 0, 1+len(r.renderOpts)) + opts = append(opts, revisiongenerator.WithObjectCollectors(r.collectObjects)) + opts = append(opts, r.renderOpts...) + converted := util.SliceMap(revisions, func(apiRev operatorv1alpha1.ClusterAPIInstallerRevision) convertedRevision { - rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, revisiongenerator.WithObjectCollectors(r.collectObjects)) + rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, opts...) if err != nil { err = fmt.Errorf("error creating installer revision from API revision %s: %w", apiRev.Name, reconcile.TerminalError(err)) } diff --git a/pkg/controllers/revision/helpers_test.go b/pkg/controllers/revision/helpers_test.go index 74a0bae892..dc6776fcc6 100644 --- a/pkg/controllers/revision/helpers_test.go +++ b/pkg/controllers/revision/helpers_test.go @@ -152,6 +152,13 @@ func createFixtures(ctx context.Context, opts ...fixturesOption) { cleanupObjs = append(cleanupObjs, clusterAPI) } + // Create Proxy singleton + proxyObj := &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + Expect(cl.Create(ctx, proxyObj)).To(Succeed()) + cleanupObjs = append(cleanupObjs, proxyObj) + // Create ClusterOperator singleton clusterOperator = &configv1.ClusterOperator{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"}, diff --git a/pkg/controllers/revision/revision_controller.go b/pkg/controllers/revision/revision_controller.go index 2147564d6f..52bb287f82 100644 --- a/pkg/controllers/revision/revision_controller.go +++ b/pkg/controllers/revision/revision_controller.go @@ -142,7 +142,22 @@ func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revis // Build ordered component list from provider metadata providerComponents := r.buildComponentList(infra.Status.PlatformStatus.Type) - revision, err := revisiongenerator.NewRenderedRevision(providerComponents) + // Read cluster-wide proxy configuration + var opts []revisiongenerator.RevisionRenderOption + + proxy, err := util.GetProxy(ctx, r.Client) + if err != nil { + return nil, opresult.ErrorP(fmt.Errorf("fetching proxy: %w", err)) + } + + if envVars := util.ProxyEnvVars(proxy); len(envVars) > 0 { + ctrl.LoggerFrom(ctx).Info("Injecting proxy configuration into provider manifests", + "httpProxy", proxy.Status.HTTPProxy, "httpsProxy", proxy.Status.HTTPSProxy, "noProxy", proxy.Status.NoProxy) + + opts = append(opts, revisiongenerator.WithProxyConfig(envVars)) + } + + revision, err := revisiongenerator.NewRenderedRevision(providerComponents, opts...) if err != nil { return nil, opresult.ErrorP(fmt.Errorf("error creating rendered revision: %w", err)) } @@ -301,6 +316,9 @@ func (r *RevisionController) SetupWithManager(mgr ctrl.Manager) error { }, }), ). + Watches(&configv1.Proxy{}, + handler.EnqueueRequestsFromMapFunc(toClusterAPI), + ). Complete(r) if err != nil { return fmt.Errorf("failed to create controller: %w", err) diff --git a/pkg/controllers/revision/revision_controller_test.go b/pkg/controllers/revision/revision_controller_test.go index 1894067a60..e6e09a260b 100644 --- a/pkg/controllers/revision/revision_controller_test.go +++ b/pkg/controllers/revision/revision_controller_test.go @@ -46,6 +46,7 @@ var ( updatedProviderImgs []providerimages.ProviderImageManifests nonMatchingProviderImgs []providerimages.ProviderImageManifests invalidAnnotationProviderImgs []providerimages.ProviderImageManifests + deploymentProviderImgs []providerimages.ProviderImageManifests ) const ( @@ -390,6 +391,89 @@ var _ = Describe("RevisionController", Serial, func() { }, defaultNodeTimeout) }) +var _ = Describe("RevisionController proxy", Serial, func() { + var ( + mgr *managerWrapper + ) + + BeforeEach(func(ctx context.Context) { + createFixtures(ctx) + + // Use provider images with Deployment manifests so proxy injection + // changes the content ID (proxy env vars are only injected into + // Deployments/DaemonSets, not ConfigMaps). + mgr = newManagerWrapper(deploymentProviderImgs) + + DeferCleanup(func(ctx context.Context) { + mgr.stop() + }) + + waitForProgressingFalse(ctx) + }, defaultNodeTimeout) + + It("creates a new revision when proxy configuration is added", func(ctx context.Context) { + // Capture the initial revision (created with no proxy). + initialClusterAPI := &operatorv1alpha1.ClusterAPI{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, initialClusterAPI)).To(Succeed()) + Expect(initialClusterAPI.Status.Revisions).To(HaveLen(1)) + originalContentID := initialClusterAPI.Status.Revisions[0].ContentID + + // Update the Proxy status to inject proxy env vars. + proxy := &configv1.Proxy{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).To(Succeed()) + proxy.Status = configv1.ProxyStatus{ + HTTPProxy: "http://proxy:3128", + HTTPSProxy: "https://proxy:3129", + NoProxy: ".cluster.local", + } + Expect(cl.Status().Update(ctx, proxy)).To(Succeed()) + + // Wait for a second revision to appear. + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", HaveLen(2))) + + // The new revision should have a different contentID. + newRev := latestRevision(clusterAPI.Status.Revisions) + Expect(newRev.ContentID).NotTo(Equal(originalContentID)) + Expect(newRev.Revision).To(Equal(int64(2))) + }, defaultNodeTimeout) + + It("creates a new revision when proxy configuration is removed", func(ctx context.Context) { + // First, set a proxy so the initial revision includes proxy env vars. + proxy := &configv1.Proxy{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).To(Succeed()) + proxy.Status = configv1.ProxyStatus{ + HTTPProxy: "http://proxy:3128", + HTTPSProxy: "https://proxy:3129", + NoProxy: ".cluster.local", + } + Expect(cl.Status().Update(ctx, proxy)).To(Succeed()) + + // Wait for the proxy-aware revision (revision 2). + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", HaveLen(2))) + + proxyContentID := latestRevision(clusterAPI.Status.Revisions).ContentID + + // Remove the proxy configuration. + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).To(Succeed()) + proxy.Status = configv1.ProxyStatus{} + Expect(cl.Status().Update(ctx, proxy)).To(Succeed()) + + // Wait for a third revision to appear. + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", HaveLen(3))) + + // The new revision should have a different contentID from the proxy revision. + newRev := latestRevision(clusterAPI.Status.Revisions) + Expect(newRev.ContentID).NotTo(Equal(proxyContentID)) + Expect(newRev.Revision).To(Equal(int64(3))) + }, defaultNodeTimeout) +}) + var _ = Describe("RevisionController waiting states", Serial, func() { Context("when Infrastructure PlatformStatus is nil", func() { BeforeEach(func(ctx context.Context) { diff --git a/pkg/controllers/revision/suite_test.go b/pkg/controllers/revision/suite_test.go index c60be4c6de..8d3c74c671 100644 --- a/pkg/controllers/revision/suite_test.go +++ b/pkg/controllers/revision/suite_test.go @@ -125,6 +125,15 @@ func setupProviderFixtures() { )). Build(), } + + // Provider images that include a Deployment manifest, used for proxy injection tests. + deploymentProviderImgs = []providerimages.ProviderImageManifests{ + test.NewProviderImageManifests(tb, "core"). + WithContentID("core-deploy-content-id"). + WithImageRef("registry.example.com/core@sha256:5555555555555555555555555555555555555555555555555555555555555555"). + WithManifests(test.DeploymentYAML("core-deploy")). + Build(), + } } func kWithCtx(ctx context.Context) komega.Komega {