Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose use_remote_address #1290

Merged
merged 2 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/200-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ data:
# right side of the x-forwarded-for HTTP header to trust.
trusted-hops-count: "0"

# Configures the connection manager to use the real remote address
# of the client connection when determining internal versus external origin and manipulating various headers.
use-remote-address: "false"

# Specifies the cipher suites for TLS external listener.
# Use ',' separated values like "ECDHE-ECDSA-AES128-GCM-SHA256,ECDHE-ECDSA-CHACHA20-POLY1305"
# The default uses the default cipher suites of the envoy version.
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ const (
// right side of the x-forwarded-for HTTP header to trust.
trustedHopsCount = "trusted-hops-count"

// useRemoteAddress Configure the connection manager to use the real remote address
// of the client connection when determining internal versus external origin and manipulating various headers.
useRemoteAddress = "use-remote-address"

// CipherSuites is the cipher suites for TLS external listener.
cipherSuites = "cipher-suites"
)
Expand Down
5 changes: 5 additions & 0 deletions pkg/config/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func DefaultConfig() *Kourier {
TrustedHopsCount: 0,
CipherSuites: nil,
EnableCryptoMB: false,
UseRemoteAddress: false,
}
}

Expand All @@ -76,6 +77,7 @@ func NewConfigFromMap(configMap map[string]string) (*Kourier, error) {
cm.AsString(clusterCert, &nc.ClusterCertSecret),
cm.AsDuration(IdleTimeoutKey, &nc.IdleTimeout),
cm.AsUint32(trustedHopsCount, &nc.TrustedHopsCount),
cm.AsBool(useRemoteAddress, &nc.UseRemoteAddress),
cm.AsStringSet(cipherSuites, &nc.CipherSuites),
cm.AsBool(enableCryptoMB, &nc.EnableCryptoMB),
asTracing(TracingCollectorFullEndpoint, &nc.Tracing),
Expand Down Expand Up @@ -149,6 +151,9 @@ type Kourier struct {
// TrustedHopsCount configures the number of additional ingress proxy hops from the
// right side of the x-forwarded-for HTTP header to trust.
TrustedHopsCount uint32
// UseRemoteAddress configures the connection manager to use the real remote address
// of the client connection when determining internal versus external origin and manipulating various headers.
UseRemoteAddress bool
// EnableCryptoMB specifies whether Kourier enable CryptoMB private provider to accelerate
// TLS handshake. The default value is "false".
EnableCryptoMB bool
Expand Down
9 changes: 9 additions & 0 deletions pkg/config/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@ func TestKourierConfig(t *testing.T) {
data: map[string]string{
TracingCollectorFullEndpoint: "",
},
}, {
name: "Enable use remote address",
want: &Kourier{
EnableServiceAccessLogging: true,
UseRemoteAddress: true,
},
data: map[string]string{
useRemoteAddress: "true",
},
}}

for _, tt := range configTests {
Expand Down
1 change: 1 addition & 0 deletions pkg/envoy/api/http_connection_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func NewHTTPConnectionManager(routeConfigName string, kourierConfig *config.Kour
},
StreamIdleTimeout: durationpb.New(idleTimeout),
XffNumTrustedHops: kourierConfig.TrustedHopsCount,
UseRemoteAddress: &wrapperspb.BoolValue{Value: kourierConfig.UseRemoteAddress},
}

if enableProxyProtocol {
Expand Down
14 changes: 12 additions & 2 deletions pkg/envoy/api/http_connection_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestNewHTTPConnectionManagerWithoutAccessLogWithoutProxyProtocol(t *testing
}
connManager := NewHTTPConnectionManager("test", &kourierConfig)
assert.Check(t, len(connManager.AccessLog) == 0)
assert.Check(t, connManager.UseRemoteAddress == nil)
assert.Check(t, connManager.UseRemoteAddress.Value == false)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are they mutual exclusive? Should we warn the user? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by that?

Copy link
Contributor Author

@skonto skonto Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean can we use proxy protocol along with use-remote-address, what if both are enabled? I will check if that creates an issue.

Copy link
Contributor Author

@skonto skonto Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not seem to create a problem in gateway logs but don't have an environment to easily check this with traffic.

}

func TestNewHTTPConnectionManagerWithAccessLogWithoutProxyProtocol(t *testing.T) {
Expand All @@ -50,7 +50,7 @@ func TestNewHTTPConnectionManagerWithAccessLogWithoutProxyProtocol(t *testing.T)
IdleTimeout: 0 * time.Second,
}
connManager := NewHTTPConnectionManager("test", &kourierConfig)
assert.Check(t, connManager.UseRemoteAddress == nil)
assert.Check(t, connManager.UseRemoteAddress.Value == false)
accessLog := connManager.AccessLog[0]
accessLogPathAny := accessLog.ConfigType.(*envoy_config_filter_accesslog_v3.AccessLog_TypedConfig).TypedConfig
fileAccesLog := &fileaccesslog.FileAccessLog{}
Expand Down Expand Up @@ -148,3 +148,13 @@ func TestNewHTTPConnectionManagerWithTrustedHops(t *testing.T) {
})
}
}

func TestNewHTTPConnectionManagerWithUseRemoteAddress(t *testing.T) {
kourierConfig := config.Kourier{
EnableServiceAccessLogging: false,
UseRemoteAddress: true,
IdleTimeout: 0 * time.Second,
}
connManager := NewHTTPConnectionManager("test", &kourierConfig)
assert.Check(t, connManager.UseRemoteAddress.Value == true)
}
Loading