Skip to content

Commit

Permalink
Merge pull request #338 from openshift-cherrypick-robot/cherry-pick-3…
Browse files Browse the repository at this point in the history 
…25-to-release-4.2

[release-4.2] Fixes Bug 1759253: Adds GCP metadata hostnames to default noProxy
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Oct 8, 2019
2 parents bf92b4e + ea9717a commit 86be17e
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 20 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/proxyconfig/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (r *ReconcileProxyConfig) ValidateProxyConfig(proxyConfig *configv1.ProxySp
if proxyConfig.NoProxy != noProxyWildcard {
for _, v := range strings.Split(proxyConfig.NoProxy, ",") {
v = strings.TrimSpace(v)
errDomain := validation.DomainName(v, false)
errDomain := validation.DomainName(v, true)
_, _, errCIDR := net.ParseCIDR(v)
if errDomain != nil && errCIDR != nil {
return fmt.Errorf("invalid noProxy: %v", v)
Expand Down
4 changes: 4 additions & 0 deletions pkg/util/proxyconfig/no_proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func MergeUserSystemNoProxy(proxy *configv1.Proxy, infra *configv1.Infrastructur
} else {
set.Insert(fmt.Sprintf(".%s.compute.internal", region))
}
case configv1.GCPPlatformType:
// From https://cloud.google.com/vpc/docs/special-configurations add GCP metadata.
// "metadata.google.internal." added due to https://bugzilla.redhat.com/show_bug.cgi?id=1754049
set.Insert("metadata", "metadata.google.internal", "metadata.google.internal.")
}

if len(ic.ControlPlane.Replicas) > 0 {
Expand Down
62 changes: 43 additions & 19 deletions pkg/util/proxyconfig/no_proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,33 @@ func proxyConfigWithNoProxy(noProxy string) *configv1.Proxy {
return proxy
}

func infraConfig(domain, region string) *configv1.Infrastructure {
func infraConfig(platform configv1.PlatformType, domain, region string) *configv1.Infrastructure {
platformStatus := &configv1.PlatformStatus{}
switch platform {
case configv1.AWSPlatformType:
platformStatus = &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
AWS: &configv1.AWSPlatformStatus{
Region: region,
},
}
case configv1.GCPPlatformType:
platformStatus = &configv1.PlatformStatus{
Type: configv1.GCPPlatformType,
GCP: &configv1.GCPPlatformStatus{
Region: region,
},
}
}
return &configv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{
Name: "test-infra",
},
Status: configv1.InfrastructureStatus{
APIServerURL: "https://api." + domain + ":6443",
APIServerInternalURL: "https://api-int." + domain + ":6443",
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
AWS: &configv1.AWSPlatformStatus{
Region: region,
},
},
EtcdDiscoveryDomain: domain,
PlatformStatus: platformStatus,
EtcdDiscoveryDomain: domain,
},
}
}
Expand Down Expand Up @@ -96,10 +108,10 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
want string
wantErr bool
}{
{name: "valid proxy config",
{name: "valid proxy config with aws provider",
args: args{
proxy: proxyConfig(),
infra: infraConfig("test.cluster.com", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "test.cluster.com", "us-west-2"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -108,10 +120,22 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
"etcd-0.test.cluster.com,etcd-1.test.cluster.com,etcd-2.test.cluster.com,localhost",
wantErr: false,
},
{name: "valid proxy config with gcp provider",
args: args{
proxy: proxyConfig(),
infra: infraConfig(configv1.GCPPlatformType, "test.cluster.com", "us-west2"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
want: ".cluster.local,.svc,10.0.0.0/16,10.128.0.0/14,127.0.0.1,169.254.169.254,172.30.0.0/16," +
"api-int.test.cluster.com,api.test.cluster.com,etcd-0.test.cluster.com,etcd-1.test.cluster.com," +
"etcd-2.test.cluster.com,localhost,metadata,metadata.google.internal,metadata.google.internal.",
wantErr: false,
},
{name: "valid proxy config using us-east-1 aws region",
args: args{
proxy: proxyConfig(),
infra: infraConfig("test.cluster.com", "us-east-1"),
infra: infraConfig(configv1.AWSPlatformType, "test.cluster.com", "us-east-1"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -123,7 +147,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "valid proxy config with single user noProxy",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1"),
infra: infraConfig("test.cluster.com", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "test.cluster.com", "us-west-2"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -135,7 +159,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "valid proxy config with multiple user noProxy",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1,.foo.test.com,199.161.0.0/16"),
infra: infraConfig("test.cluster.com", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "test.cluster.com", "us-west-2"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -147,7 +171,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid api server url",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("10.128.0.0/14", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -156,7 +180,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid missing service network",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("10.128.0.0/14", ""),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -165,7 +189,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid missing cluster network",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig(cfgMapKey, cfgMapData),
},
Expand All @@ -174,7 +198,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid empty configmap",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("", "172.30.0.0/16"),
cluster: cfgMap(),
},
Expand All @@ -183,7 +207,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid configmap key",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig("bad-key", cfgMapData),
},
Expand All @@ -192,7 +216,7 @@ func TestMergeUserSystemNoProxy(t *testing.T) {
{name: "invalid configmap data",
args: args{
proxy: proxyConfigWithNoProxy("172.30.0.1."),
infra: infraConfig("^&", "us-west-2"),
infra: infraConfig(configv1.AWSPlatformType, "^&", "us-west-2"),
network: netConfig("", "172.30.0.0/16"),
cluster: cfgMapWithInstallConfig("bad-key", "bad data"),
},
Expand Down

0 comments on commit 86be17e

Please sign in to comment.