refactor(interceptor): extract shared proxy handler for testability

Extract building the handler chain for better testability and refactor
the interceptor/main.go file to make it cleaner.

Changes:
- Extract BuildProxyHandler for shared use by production and tests
- Move TLS configuration logic to dedicated tls_config.go and add tests
- Replace main_test.go with proxy_test.go using shared builder
- Simplify config parsing to return values instead of pointers
- Remove deprecated DeploymentCachePollIntervalMS config option

Signed-off-by: Vincent Link <vlink@redhat.com>

Author: linkvt

interceptor/config/serving.go

@@ -24,11 +24,6 @@ type Serving struct {
 	// ConfigMapCacheRsyncPeriod is the time interval
 	// for the configmap informer to rsync the local cache.
 	ConfigMapCacheRsyncPeriod time.Duration `envconfig:"KEDA_HTTP_SCALER_CONFIG_MAP_INFORMER_RSYNC_PERIOD" default:"60m"`
-	// Deprecated: The interceptor has an internal process that periodically fetches the state
-	// of deployment that is running the servers it forwards to.
-	//
-	// This is the interval (in milliseconds) representing how often to do a fetch
-	DeploymentCachePollIntervalMS int `envconfig:"KEDA_HTTP_DEPLOYMENT_CACHE_POLLING_INTERVAL_MS" default:"250"`
 	// The interceptor has an internal process that periodically fetches the state
 	// of endpoints that is running the servers it forwards to.
 	//
@@ -55,10 +50,10 @@ type Serving struct {
 	LogRequests bool `envconfig:"KEDA_HTTP_LOG_REQUESTS" default:"false"`
 }
 
-// Parse parses standard configs using envconfig and returns a pointer to the
-// newly created config. Returns nil and a non-nil error if parsing failed
-func MustParseServing() *Serving {
-	ret := new(Serving)
-	envconfig.MustProcess("", ret)
+// MustParseServing parses standard configs using envconfig and returns the
+// newly created config. It panics if parsing fails.
+func MustParseServing() Serving {
+	var ret Serving
+	envconfig.MustProcess("", &ret)
 	return ret
 }

interceptor/config/timeouts.go

@@ -52,10 +52,10 @@ func (t Timeouts) DefaultBackoff() wait.Backoff {
 	}
 }
 
-// MustParseTimeouts parses standard configs using envconfig and returns a pointer to the
+// MustParseTimeouts parses standard configs using envconfig and returns the
 // newly created config. It panics if parsing fails.
-func MustParseTimeouts() *Timeouts {
-	ret := new(Timeouts)
-	envconfig.MustProcess("", ret)
+func MustParseTimeouts() Timeouts {
+	var ret Timeouts
+	envconfig.MustProcess("", &ret)
 	return ret
 }

interceptor/config/tracing.go

@@ -12,10 +12,10 @@ type Tracing struct {
 	Exporter string `envconfig:"OTEL_EXPORTER_OTLP_TRACES_PROTOCOL" default:"console"`
 }
 
-// Parse parses standard configs using envconfig and returns a pointer to the
-// newly created config. Returns nil and a non-nil error if parsing failed
-func MustParseTracing() *Tracing {
-	ret := new(Tracing)
-	envconfig.MustProcess("", ret)
+// MustParseTracing parses standard configs using envconfig and returns the
+// newly created config. It panics if parsing fails.
+func MustParseTracing() Tracing {
+	var ret Tracing
+	envconfig.MustProcess("", &ret)
 	return ret
 }

interceptor/config/validate.go

@@ -2,30 +2,10 @@ package config
 
 import (
 	"fmt"
-	"os"
 	"time"
-
-	"github.com/go-logr/logr"
 )
 
-func Validate(srvCfg *Serving, timeoutsCfg Timeouts, lggr logr.Logger) error {
-	// TODO(jorturfer): delete this for v0.9.0
-	_, deploymentEnvExist := os.LookupEnv("KEDA_HTTP_DEPLOYMENT_CACHE_POLLING_INTERVAL_MS")
-	_, endpointsEnvExist := os.LookupEnv("KEDA_HTTP_ENDPOINTS_CACHE_POLLING_INTERVAL_MS")
-	if deploymentEnvExist && endpointsEnvExist {
-		return fmt.Errorf(
-			"%s and %s are mutual exclusive",
-			"KEDA_HTTP_DEPLOYMENT_CACHE_POLLING_INTERVAL_MS",
-			"KEDA_HTTP_ENDPOINTS_CACHE_POLLING_INTERVAL_MS",
-		)
-	}
-	if deploymentEnvExist && !endpointsEnvExist {
-		srvCfg.EndpointsCachePollIntervalMS = srvCfg.DeploymentCachePollIntervalMS
-		srvCfg.DeploymentCachePollIntervalMS = 0
-		lggr.Info("WARNING: KEDA_HTTP_DEPLOYMENT_CACHE_POLLING_INTERVAL_MS has been deprecated in favor of KEDA_HTTP_ENDPOINTS_CACHE_POLLING_INTERVAL_MS and wil be removed for v0.9.0")
-	}
-	// END TODO
-
+func Validate(srvCfg Serving, timeoutsCfg Timeouts) error {
 	endpointsCachePollInterval := time.Duration(srvCfg.EndpointsCachePollIntervalMS) * time.Millisecond
 	if timeoutsCfg.WorkloadReplicas < endpointsCachePollInterval {
 		return fmt.Errorf(

interceptor/handler/upstream.go

@@ -22,11 +22,11 @@ var (
 
 type Upstream struct {
 	roundTripper   http.RoundTripper
-	tracingCfg     *config.Tracing
+	tracingCfg     config.Tracing
 	shouldFailover bool
 }
 
-func NewUpstream(roundTripper http.RoundTripper, tracingCfg *config.Tracing, shouldFailover bool) *Upstream {
+func NewUpstream(roundTripper http.RoundTripper, tracingCfg config.Tracing, shouldFailover bool) *Upstream {
 	return &Upstream{
 		roundTripper:   roundTripper,
 		tracingCfg:     tracingCfg,

interceptor/handler/upstream_test.go

@@ -260,7 +260,7 @@ func TestForwarderSuccess(t *testing.T) {
 	timeouts := defaultTimeouts()
 	dialCtxFunc := retryDialContextFunc(timeouts, timeouts.DefaultBackoff())
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	uh := NewUpstream(rt, &config.Tracing{}, false)
+	uh := NewUpstream(rt, config.Tracing{}, false)
 	uh.ServeHTTP(res, req)
 
 	r.True(
@@ -304,7 +304,7 @@ func TestForwarderHeaderTimeout(t *testing.T) {
 	r.NoError(err)
 	req = util.RequestWithStream(req, originURL)
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	uh := NewUpstream(rt, &config.Tracing{}, false)
+	uh := NewUpstream(rt, config.Tracing{}, false)
 	uh.ServeHTTP(res, req)
 
 	forwardedRequests := hdl.IncomingRequests()
@@ -354,7 +354,7 @@ func TestForwarderWaitsForSlowOrigin(t *testing.T) {
 	r.NoError(err)
 	req = util.RequestWithStream(req, originURL)
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	uh := NewUpstream(rt, &config.Tracing{}, false)
+	uh := NewUpstream(rt, config.Tracing{}, false)
 	uh.ServeHTTP(res, req)
 	// wait for the goroutine above to finish, with a little cusion
 	ensureSignalBeforeTimeout(originWaitCh, originDelay*2)
@@ -377,7 +377,7 @@ func TestForwarderConnectionRetryAndTimeout(t *testing.T) {
 	r.NoError(err)
 	req = util.RequestWithStream(req, noSuchURL)
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	uh := NewUpstream(rt, &config.Tracing{}, false)
+	uh := NewUpstream(rt, config.Tracing{}, false)
 
 	start := time.Now()
 	uh.ServeHTTP(res, req)
@@ -431,7 +431,7 @@ func TestForwardRequestRedirectAndHeaders(t *testing.T) {
 	r.NoError(err)
 	req = util.RequestWithStream(req, srvURL)
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	uh := NewUpstream(rt, &config.Tracing{}, false)
+	uh := NewUpstream(rt, config.Tracing{}, false)
 	uh.ServeHTTP(res, req)
 	r.Equal(301, res.Code)
 	r.Equal("abc123.com", res.Header().Get("Location"))
@@ -488,7 +488,7 @@ func TestUpstreamPreservesXForwardedHeaders(t *testing.T) {
 			}
 
 			// Configure the Upstream and send a dummy request
-			upstream := NewUpstream(http.DefaultTransport, &config.Tracing{}, false)
+			upstream := NewUpstream(http.DefaultTransport, config.Tracing{}, false)
 
 			req := httptest.NewRequest("GET", "/test", nil)
 			if tt.forwardedFor != "" {
@@ -604,7 +604,7 @@ func serveHTTP(w http.ResponseWriter, r *http.Request) {
 	timeouts := defaultTimeouts()
 	dialCtxFunc := retryDialContextFunc(timeouts, timeouts.DefaultBackoff())
 	rt := newRoundTripper(dialCtxFunc, timeouts.ResponseHeader)
-	upstream := NewUpstream(rt, &config.Tracing{Enabled: true}, false)
+	upstream := NewUpstream(rt, config.Tracing{Enabled: true}, false)
 
 	upstream.ServeHTTP(w, r)
 }