Skip to content

Commit beaf436

Browse files
carboninclaude
authored and
mlorenzofr
committed
Enrich IBI NoProxy with cluster network CIDRs in config image
IBI cluster configuration was forwarding the user-provided Proxy config directly without adding cluster, service, and machine network CIDRs to NoProxy, which would cause in-cluster traffic to be routed through the proxy. Extract BuildNoProxySet into pkg/types so the manifests proxy asset and the IBI config image asset share the same NoProxy building logic. Update tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 68a05fb commit beaf436

5 files changed

Lines changed: 272 additions & 36 deletions

File tree

‎pkg/asset/imagebased/configimage/clusterconfiguration.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"os"
1111
"path/filepath"
12+
"strings"
1213

1314
"github.com/thoas/go-funk"
1415
k8sjson "sigs.k8s.io/json"
@@ -108,7 +109,7 @@ func (cc *ClusterConfiguration) Generate(_ context.Context, dependencies asset.P
108109
Hostname: imageBasedConfig.Config.Hostname,
109110
InfraID: clusterID.InfraID,
110111
KubeadminPasswordHash: pwdHash,
111-
Proxy: installConfig.Config.Proxy,
112+
Proxy: enrichNoProxy(installConfig.Config),
112113
PullSecret: installConfig.Config.PullSecret,
113114
ReleaseRegistry: imageBasedConfig.Config.ReleaseRegistry,
114115
SSHKey: installConfig.Config.SSHKey,
@@ -216,6 +217,25 @@ func (cc *ClusterConfiguration) finish() error {
216217
return nil
217218
}
218219

220+
// enrichNoProxy adds cluster, service, and machine networks to the NoProxy field
221+
// to ensure internal cluster communication bypasses the proxy.
222+
func enrichNoProxy(installConfig *types.InstallConfig) *types.Proxy {
223+
if installConfig.Proxy == nil {
224+
return nil
225+
}
226+
if installConfig.Proxy.NoProxy == "*" {
227+
return installConfig.Proxy
228+
}
229+
230+
set := types.BuildNoProxySet(installConfig)
231+
232+
return &types.Proxy{
233+
HTTPProxy: installConfig.Proxy.HTTPProxy,
234+
HTTPSProxy: installConfig.Proxy.HTTPSProxy,
235+
NoProxy: strings.Join(set.List(), ","),
236+
}
237+
}
238+
219239
func chronyConfWithAdditionalNTPSources(sources []string) string {
220240
content := defaultChronyConf[:]
221241
for _, source := range sources {

‎pkg/asset/imagebased/configimage/clusterconfiguration_test.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func TestClusterConfiguration_Generate(t *testing.T) {
120120
imageBasedConfig(),
121121
},
122122

123-
expectedConfig: clusterConfiguration().proxy(proxy()).build().Config,
123+
expectedConfig: clusterConfiguration().proxy(enrichedProxy()).build().Config,
124124
},
125125
{
126126
name: "valid configuration with additionalTrustBundle, policyAlways without proxy",
@@ -159,7 +159,7 @@ func TestClusterConfiguration_Generate(t *testing.T) {
159159
},
160160

161161
expectedConfig: clusterConfiguration().
162-
proxy(proxy()).
162+
proxy(enrichedProxy()).
163163
additionalTrustBundle(imagebased.AdditionalTrustBundle{
164164
UserCaBundle: testCert,
165165
}).
@@ -183,7 +183,7 @@ func TestClusterConfiguration_Generate(t *testing.T) {
183183
},
184184

185185
expectedConfig: clusterConfiguration().
186-
proxy(proxy()).
186+
proxy(enrichedProxy()).
187187
additionalTrustBundle(imagebased.AdditionalTrustBundle{
188188
UserCaBundle: testCert,
189189
ProxyConfigmapBundle: testCert,
@@ -416,6 +416,17 @@ func proxy() *types.Proxy {
416416
}
417417
}
418418

419+
func enrichedProxy() *types.Proxy {
420+
// This is the proxy after enrichNoProxy() is applied with default test networks:
421+
// MachineNetwork: 10.10.11.0/24, ClusterNetwork: 192.168.111.0/24, ServiceNetwork: 172.30.0.0/16
422+
// ClusterDomain: ocp-ibi.testing.com (from defaultInstallConfig)
423+
return &types.Proxy{
424+
HTTPProxy: "http://10.10.10.11:80",
425+
HTTPSProxy: "http://my-lab-proxy.org:443",
426+
NoProxy: ".cluster.local,.svc,10.10.11.0/24,127.0.0.1,172.30.0.0/16,192.168.111.0/24,api-int.ocp-ibi.testing.com,internal.com,localhost",
427+
}
428+
}
429+
419430
func kubeadminPassword() *password.KubeadminPassword {
420431
return &password.KubeadminPassword{
421432
PasswordHash: []byte("a-password-hash"),

‎pkg/asset/manifests/proxy.go‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ package manifests
33
import (
44
"context"
55
"fmt"
6-
"net/url"
76
"path"
87
"strings"
98

109
"github.com/pkg/errors"
1110
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12-
"k8s.io/apimachinery/pkg/util/sets"
1311
"sigs.k8s.io/yaml"
1412

1513
configv1 "github.com/openshift/api/config/v1"
@@ -121,18 +119,7 @@ func (p *Proxy) Generate(_ context.Context, dependencies asset.Parents) error {
121119
// https://docs.microsoft.com/en-us/azure/virtual-machines/windows/instance-metadata-service
122120
// https://cloud.google.com/compute/docs/storing-retrieving-metadata
123121
func createNoProxy(installConfig *installconfig.InstallConfig) (string, error) {
124-
internalAPIServer, err := url.Parse(getInternalAPIServerURL(installConfig.Config))
125-
if err != nil {
126-
return "", errors.New("failed parsing internal API server when creating Proxy manifest")
127-
}
128-
129-
set := sets.NewString(
130-
"127.0.0.1",
131-
"localhost",
132-
".svc",
133-
".cluster.local",
134-
internalAPIServer.Hostname(),
135-
)
122+
set := types.BuildNoProxySet(installConfig.Config)
136123

137124
platform := installConfig.Config.Platform.Name()
138125

@@ -169,24 +156,6 @@ func createNoProxy(installConfig *installconfig.InstallConfig) (string, error) {
169156
set.Insert("metadata", "metadata.google.internal", "metadata.google.internal.")
170157
}
171158

172-
for _, network := range installConfig.Config.Networking.ServiceNetwork {
173-
set.Insert(network.String())
174-
}
175-
176-
for _, network := range installConfig.Config.Networking.MachineNetwork {
177-
set.Insert(network.CIDR.String())
178-
}
179-
180-
for _, clusterNetwork := range installConfig.Config.Networking.ClusterNetwork {
181-
set.Insert(clusterNetwork.CIDR.String())
182-
}
183-
184-
for _, userValue := range strings.Split(installConfig.Config.Proxy.NoProxy, ",") {
185-
if userValue != "" {
186-
set.Insert(userValue)
187-
}
188-
}
189-
190159
return strings.Join(set.List(), ","), nil
191160
}
192161

‎pkg/types/proxy.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package types
2+
3+
import (
4+
"strings"
5+
6+
"k8s.io/apimachinery/pkg/util/sets"
7+
)
8+
9+
// BuildNoProxySet creates a set of NoProxy entries from install configuration.
10+
// It includes localhost entries (.svc, .cluster.local, 127.0.0.1, localhost),
11+
// all network CIDRs (cluster, service, machine), the internal API server hostname,
12+
// and user-provided NoProxy values.
13+
// The caller can add platform-specific entries as needed.
14+
func BuildNoProxySet(config *InstallConfig) sets.String {
15+
set := sets.NewString(
16+
"127.0.0.1",
17+
"localhost",
18+
".svc",
19+
".cluster.local",
20+
)
21+
22+
for _, network := range config.Networking.ServiceNetwork {
23+
set.Insert(network.String())
24+
}
25+
26+
for _, network := range config.Networking.MachineNetwork {
27+
set.Insert(network.CIDR.String())
28+
}
29+
30+
for _, network := range config.Networking.ClusterNetwork {
31+
set.Insert(network.CIDR.String())
32+
}
33+
34+
// Add internal API server hostname
35+
set.Insert("api-int." + config.ClusterDomain())
36+
37+
if config.Proxy != nil {
38+
for _, userValue := range strings.Split(config.Proxy.NoProxy, ",") {
39+
trimmed := strings.TrimSpace(userValue)
40+
if trimmed != "" {
41+
set.Insert(trimmed)
42+
}
43+
}
44+
}
45+
46+
return set
47+
}

‎pkg/types/proxy_test.go‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
package types
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
9+
"github.com/openshift/installer/pkg/ipnet"
10+
)
11+
12+
func TestBuildNoProxySet(t *testing.T) {
13+
cases := []struct {
14+
name string
15+
config *InstallConfig
16+
expected []string
17+
}{
18+
{
19+
name: "empty networking and no user entries",
20+
config: &InstallConfig{
21+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
22+
BaseDomain: "example.com",
23+
Networking: &Networking{},
24+
},
25+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "api-int.test.example.com", "localhost"},
26+
},
27+
{
28+
name: "service network entries are included",
29+
config: &InstallConfig{
30+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
31+
BaseDomain: "example.com",
32+
Networking: &Networking{
33+
ServiceNetwork: []ipnet.IPNet{
34+
*ipnet.MustParseCIDR("172.30.0.0/16"),
35+
},
36+
},
37+
},
38+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "172.30.0.0/16", "api-int.test.example.com", "localhost"},
39+
},
40+
{
41+
name: "machine network entries are included",
42+
config: &InstallConfig{
43+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
44+
BaseDomain: "example.com",
45+
Networking: &Networking{
46+
MachineNetwork: []MachineNetworkEntry{
47+
{CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")},
48+
},
49+
},
50+
},
51+
expected: []string{".cluster.local", ".svc", "10.0.0.0/16", "127.0.0.1", "api-int.test.example.com", "localhost"},
52+
},
53+
{
54+
name: "cluster network entries are included",
55+
config: &InstallConfig{
56+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
57+
BaseDomain: "example.com",
58+
Networking: &Networking{
59+
ClusterNetwork: []ClusterNetworkEntry{
60+
{CIDR: *ipnet.MustParseCIDR("10.128.0.0/14")},
61+
},
62+
},
63+
},
64+
expected: []string{".cluster.local", ".svc", "10.128.0.0/14", "127.0.0.1", "api-int.test.example.com", "localhost"},
65+
},
66+
{
67+
name: "single user no-proxy entry is included",
68+
config: &InstallConfig{
69+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
70+
BaseDomain: "example.com",
71+
Networking: &Networking{},
72+
Proxy: &Proxy{
73+
NoProxy: "example.com",
74+
},
75+
},
76+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "api-int.test.example.com", "example.com", "localhost"},
77+
},
78+
{
79+
name: "multiple comma-separated user entries are included",
80+
config: &InstallConfig{
81+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
82+
BaseDomain: "example.com",
83+
Networking: &Networking{},
84+
Proxy: &Proxy{
85+
NoProxy: "example.com,internal.corp,192.168.1.0/24",
86+
},
87+
},
88+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "192.168.1.0/24", "api-int.test.example.com", "example.com", "internal.corp", "localhost"},
89+
},
90+
{
91+
name: "user entries with surrounding whitespace are trimmed",
92+
config: &InstallConfig{
93+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
94+
BaseDomain: "example.com",
95+
Networking: &Networking{},
96+
Proxy: &Proxy{
97+
NoProxy: " example.com , internal.corp ",
98+
},
99+
},
100+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "api-int.test.example.com", "example.com", "internal.corp", "localhost"},
101+
},
102+
{
103+
name: "empty segments in user no-proxy are ignored",
104+
config: &InstallConfig{
105+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
106+
BaseDomain: "example.com",
107+
Networking: &Networking{},
108+
Proxy: &Proxy{
109+
NoProxy: "example.com,,internal.corp,",
110+
},
111+
},
112+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "api-int.test.example.com", "example.com", "internal.corp", "localhost"},
113+
},
114+
{
115+
name: "all network types and user entries combined",
116+
config: &InstallConfig{
117+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
118+
BaseDomain: "example.com",
119+
Networking: &Networking{
120+
ServiceNetwork: []ipnet.IPNet{
121+
*ipnet.MustParseCIDR("172.30.0.0/16"),
122+
},
123+
MachineNetwork: []MachineNetworkEntry{
124+
{CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")},
125+
},
126+
ClusterNetwork: []ClusterNetworkEntry{
127+
{CIDR: *ipnet.MustParseCIDR("10.128.0.0/14")},
128+
},
129+
},
130+
Proxy: &Proxy{
131+
NoProxy: "example.com",
132+
},
133+
},
134+
expected: []string{
135+
".cluster.local", ".svc", "10.0.0.0/16", "10.128.0.0/14",
136+
"127.0.0.1", "172.30.0.0/16", "api-int.test.example.com", "example.com", "localhost",
137+
},
138+
},
139+
{
140+
name: "duplicate user entry matching a built-in entry is deduplicated",
141+
config: &InstallConfig{
142+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
143+
BaseDomain: "example.com",
144+
Networking: &Networking{
145+
ServiceNetwork: []ipnet.IPNet{
146+
*ipnet.MustParseCIDR("172.30.0.0/16"),
147+
},
148+
},
149+
Proxy: &Proxy{
150+
NoProxy: "172.30.0.0/16,localhost",
151+
},
152+
},
153+
expected: []string{".cluster.local", ".svc", "127.0.0.1", "172.30.0.0/16", "api-int.test.example.com", "localhost"},
154+
},
155+
{
156+
name: "multiple entries in each network type",
157+
config: &InstallConfig{
158+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
159+
BaseDomain: "example.com",
160+
Networking: &Networking{
161+
ServiceNetwork: []ipnet.IPNet{
162+
*ipnet.MustParseCIDR("172.30.0.0/16"),
163+
*ipnet.MustParseCIDR("fd02::/112"),
164+
},
165+
MachineNetwork: []MachineNetworkEntry{
166+
{CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")},
167+
{CIDR: *ipnet.MustParseCIDR("fd00::/48")},
168+
},
169+
ClusterNetwork: []ClusterNetworkEntry{
170+
{CIDR: *ipnet.MustParseCIDR("10.128.0.0/14")},
171+
{CIDR: *ipnet.MustParseCIDR("fd01::/48")},
172+
},
173+
},
174+
},
175+
expected: []string{
176+
".cluster.local", ".svc", "10.0.0.0/16", "10.128.0.0/14",
177+
"127.0.0.1", "172.30.0.0/16", "api-int.test.example.com",
178+
"fd00::/48", "fd01::/48", "fd02::/112", "localhost",
179+
},
180+
},
181+
}
182+
183+
for _, tc := range cases {
184+
t.Run(tc.name, func(t *testing.T) {
185+
result := BuildNoProxySet(tc.config)
186+
assert.ElementsMatch(t, tc.expected, result.List())
187+
})
188+
}
189+
}

0 commit comments

Comments
 (0)