From a332db481b37465809dbfa4906e3bc3d0d7feefb Mon Sep 17 00:00:00 2001 From: Ilias Rinis Date: Thu, 28 Sep 2023 13:08:59 +0200 Subject: [PATCH] proxyconfig: ignore changes to certs when HTTP URLs are used in httpsProxy --- .../proxyconfig/proxyconfig_controller.go | 41 +++++++++++++-- .../proxyconfig_controller_test.go | 52 +++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/proxyconfig/proxyconfig_controller.go b/pkg/controllers/proxyconfig/proxyconfig_controller.go index 90d415add4..a2a22e8209 100644 --- a/pkg/controllers/proxyconfig/proxyconfig_controller.go +++ b/pkg/controllers/proxyconfig/proxyconfig_controller.go @@ -8,11 +8,13 @@ import ( "net/http" "net/url" "reflect" + "strings" "sync" "time" "golang.org/x/net/http/httpproxy" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" @@ -74,13 +76,19 @@ func NewProxyConfigChecker( } // sync attempts to connect to route using configured proxy settings and reports any error. -func (p *proxyConfigChecker) sync(ctx context.Context, _ factory.SyncContext) error { +func (p *proxyConfigChecker) sync(ctx context.Context, syncCtx factory.SyncContext) error { proxyConfig := httpproxy.FromEnvironment() if !isProxyConfigured(proxyConfig) { // If proxy is not configured, then it is a no-op. return nil } + if keyMatchesConfigMaps(syncCtx.QueueKey(), p.caConfigMaps) && + strings.HasPrefix(proxyConfig.HTTPSProxy, "http://") { + // HTTP proxy configured in the httpsProxy field; certificates are irrelevant, so skip check + return nil + } + route, err := p.routeLister.Routes(p.routeNamespace).Get(p.routeName) if err != nil { return err @@ -252,8 +260,35 @@ func eventHandler(filterFunc func(obj interface{}) bool, syncCtx factory.SyncCon } func enqueue(obj interface{}, syncCtx factory.SyncContext) { - runtimeObj := obj.(runtime.Object) - for _, key := range factory.DefaultQueueKeysFunc(runtimeObj) { + keys := factory.DefaultQueueKeysFunc(obj.(runtime.Object)) + + switch obj := obj.(type) { + case *corev1.ConfigMap: + key := getConfigMapKey(obj.ObjectMeta.GetNamespace(), obj.ObjectMeta.GetName()) + keys = []string{key} + } + + for _, key := range keys { syncCtx.Queue().Add(key) } } + +func getConfigMapKey(nsName, cmName string) string { + if nsName == "" || cmName == "" { + return factory.DefaultQueueKey + } + + return fmt.Sprintf("key:ConfigMap:%s:%s", nsName, cmName) +} + +func keyMatchesConfigMaps(key string, configMaps map[string][]string) bool { + for ns := range configMaps { + for _, cm := range configMaps[ns] { + if key == getConfigMapKey(ns, cm) { + return true + } + } + } + + return false +} diff --git a/pkg/controllers/proxyconfig/proxyconfig_controller_test.go b/pkg/controllers/proxyconfig/proxyconfig_controller_test.go index b76afe8a63..6b5e6d5118 100644 --- a/pkg/controllers/proxyconfig/proxyconfig_controller_test.go +++ b/pkg/controllers/proxyconfig/proxyconfig_controller_test.go @@ -9,6 +9,7 @@ import ( "reflect" "testing" + "github.com/openshift/library-go/pkg/controller/factory" "golang.org/x/net/http/httpproxy" ) @@ -222,6 +223,57 @@ func Test_checkProxyConfig(t *testing.T) { } } +func Test_getConfigMapKey(t *testing.T) { + tests := []struct { + name string + nsName, cmName string + expectedKey string + }{ + {"empty strings", "", "", factory.DefaultQueueKey}, + {"one empty string", "testns", "", factory.DefaultQueueKey}, + {"one empty string", "", "testcm", factory.DefaultQueueKey}, + {"non-empty strings", "testns", "testcm", "key:ConfigMap:testns:testcm"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := getConfigMapKey(tt.nsName, tt.cmName) + if key != tt.expectedKey { + t.Errorf("wanted %s; got %s", tt.expectedKey, key) + } + }) + } +} + +func Test_keyMatchesConfigMaps(t *testing.T) { + tests := []struct { + name string + key string + configMaps map[string][]string + expectedResult bool + }{ + {"empty map", "somekey", map[string][]string{}, false}, + {"empty configMaps", "somekey", map[string][]string{"testns": {}}, false}, + {"empty key", "", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"ns does not match", "key:ConfigMap:ns3:cm1", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"cm does not match", "key:ConfigMap:ns1:cm3", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"ns and cm do not match", "key:ConfigMap:ns3:cm3", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"match ns1 cm1", "key:ConfigMap:ns1:cm1", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"match ns1 cm2", "key:ConfigMap:ns1:cm2", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"match ns2 cm1", "key:ConfigMap:ns2:cm1", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + {"match ns2 cm2", "key:ConfigMap:ns2:cm2", map[string][]string{"ns1": {"cm1", "cm2"}, "ns2": {"cm1", "cm2"}}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := keyMatchesConfigMaps(tt.key, tt.configMaps) + if result != tt.expectedResult { + t.Errorf("wanted %t; got %t", tt.expectedResult, result) + } + }) + } +} + type workingHTTPRoundTripper struct{} type faultyHTTPRoundTripper struct{}