diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go index f4d93d577f8..4e414944129 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go @@ -74,8 +74,8 @@ type Connection struct { // Protocol is the protocol used to connect from client to the konnectivity server. ProxyProtocol ProtocolType - // Transport defines the transport configurations we use to dial to the konnectivity server - // This is required if ProxyProtocol is HTTPConnect or GRPC + // Transport defines the transport configurations we use to dial to the konnectivity server. + // This is required if ProxyProtocol is HTTPConnect or GRPC. // +optional Transport *Transport } @@ -96,6 +96,7 @@ const ( // Transport defines the transport configurations we use to dial to the konnectivity server type Transport struct { // TCP is the TCP configuration for communicating with the konnectivity server via TCP + // ProxyProtocol of GRPC is not supported with TCP transport at the moment // Requires at least one of TCP or UDS to be set // +optional TCP *TCPTransport @@ -128,24 +129,20 @@ type UDSTransport struct { // Only used with TCPTransport type TLSConfig struct { // caBundle is the file location of the CA to be used to determine trust with the konnectivity server. - // Must be absent/empty HTTPConnect using the plain http - // If absent while using the HTTPConnect protocol with HTTPS - // default to system trust roots - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // If absent while TCPTransport.URL is prefixed with https://, default to system trust roots. // +optional CABundle string // clientKey is the file location of the client key to authenticate with the konnectivity server - // Must be absent/empty HTTPConnect using the plain http - // Must be configured for HTTPConnect using the https protocol - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // Must be configured if TCPTransport.URL is prefixed with https:// // +optional ClientKey string // clientCert is the file location of the client certificate to authenticate with the konnectivity server - // Must be absent/empty HTTPConnect using the plain http - // Must be configured for HTTPConnect using the https protocol - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // Must be configured if TCPTransport.URL is prefixed with https:// // +optional ClientCert string } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go index 2fdbcd06036..7b9aacae86f 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go @@ -74,8 +74,8 @@ type Connection struct { // Protocol is the protocol used to connect from client to the konnectivity server. ProxyProtocol ProtocolType `json:"proxyProtocol,omitempty"` - // Transport defines the transport configurations we use to dial to the konnectivity server - // This is required if ProxyProtocol is HTTPConnect or GRPC + // Transport defines the transport configurations we use to dial to the konnectivity server. + // This is required if ProxyProtocol is HTTPConnect or GRPC. // +optional Transport *Transport `json:"transport,omitempty"` } @@ -96,6 +96,7 @@ const ( // Transport defines the transport configurations we use to dial to the konnectivity server type Transport struct { // TCP is the TCP configuration for communicating with the konnectivity server via TCP + // ProxyProtocol of GRPC is not supported with TCP transport at the moment // Requires at least one of TCP or UDS to be set // +optional TCP *TCPTransport `json:"tcp,omitempty"` @@ -120,6 +121,7 @@ type TCPTransport struct { // UDSTransport provides the information to connect to konnectivity server via UDS type UDSTransport struct { // UDSName is the name of the unix domain socket to connect to konnectivity server + // This does not use a unix:// prefix. (Eg: /etc/srv/kubernetes/konnectivity-server/konnectivity-server.socket) UDSName string `json:"udsName,omitempty"` } @@ -127,23 +129,20 @@ type UDSTransport struct { // Only used with TCPTransport type TLSConfig struct { // caBundle is the file location of the CA to be used to determine trust with the konnectivity server. - // Must be absent/empty HTTPConnect using the plain http - // If absent while using the HTTPConnect protocol with HTTPS - // default to system trust roots - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // If absent while TCPTransport.URL is prefixed with https://, default to system trust roots. // +optional CABundle string `json:"caBundle,omitempty"` // clientKey is the file location of the client key to be used in mtls handshakes with the konnectivity server. - // Must be absent/empty HTTPConnect using the plain http - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // Must be configured if TCPTransport.URL is prefixed with https:// // +optional ClientKey string `json:"clientKey,omitempty"` // clientCert is the file location of the client certificate to be used in mtls handshakes with the konnectivity server. - // Must be absent/empty HTTPConnect using the plain http - // Must be configured for HTTPConnect using the https protocol - // Misconfiguration will cause an error + // Must be absent/empty if TCPTransport.URL is prefixed with http:// + // Must be configured if TCPTransport.URL is prefixed with https:// // +optional ClientCert string `json:"clientCert,omitempty"` } diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go index 77e45736373..7c40154e6d5 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go @@ -78,42 +78,70 @@ func ValidateEgressSelectorConfiguration(config *apiserver.EgressSelectorConfigu return allErrs // Treating a nil configuration as valid } for _, service := range config.EgressSelections { - base := field.NewPath("service", "connection") + fldPath := field.NewPath("service", "connection") switch service.Connection.ProxyProtocol { case apiserver.ProtocolDirect: - allErrs = append(allErrs, validateDirectConnection(service.Connection, base)...) + allErrs = append(allErrs, validateDirectConnection(service.Connection, fldPath)...) case apiserver.ProtocolHTTPConnect: - if service.Connection.Transport.TCP != nil && service.Connection.Transport.UDS != nil { - allErrs = append(allErrs, field.Invalid( - base.Child("tcp"), - service.Connection.Transport.TCP, - "TCP and UDS cannot both be set")) - } else if service.Connection.Transport.TCP == nil && service.Connection.Transport.UDS == nil { - allErrs = append(allErrs, field.Required( - base.Child("tcp"), - "One of TCP or UDS must be set")) - } else if service.Connection.Transport.TCP != nil { - allErrs = append(allErrs, validateTCPConnection(service.Connection, base)...) - } else if service.Connection.Transport.UDS != nil { - allErrs = append(allErrs, validateUDSConnection(service.Connection, base)...) - } + allErrs = append(allErrs, validateHTTPConnectTransport(service.Connection.Transport, fldPath)...) case apiserver.ProtocolGRPC: - if service.Connection.Transport.UDS != nil { - allErrs = append(allErrs, validateUDSConnection(service.Connection, base)...) - } else { - allErrs = append(allErrs, field.NotSupported( - base.Child("protocol"), - service.Connection.ProxyProtocol, - []string{"uds"})) - } + allErrs = append(allErrs, validateGRPCTransport(service.Connection.Transport, fldPath)...) default: allErrs = append(allErrs, field.NotSupported( - base.Child("protocol"), + fldPath.Child("protocol"), service.Connection.ProxyProtocol, - []string{"direct", "HTTPConnect", "grpc"})) + []string{ + string(apiserver.ProtocolDirect), + string(apiserver.ProtocolHTTPConnect), + string(apiserver.ProtocolGRPC), + })) } } + return allErrs +} +func validateHTTPConnectTransport(transport *apiserver.Transport, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if transport == nil { + allErrs = append(allErrs, field.Required( + fldPath.Child("transport"), + "transport must be set for HTTPConnect")) + return allErrs + } + + if transport.TCP != nil && transport.UDS != nil { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tcp"), + transport.TCP, + "TCP and UDS cannot both be set")) + } else if transport.TCP == nil && transport.UDS == nil { + allErrs = append(allErrs, field.Required( + fldPath.Child("tcp"), + "One of TCP or UDS must be set")) + } else if transport.TCP != nil { + allErrs = append(allErrs, validateTCPConnection(transport.TCP, fldPath)...) + } else if transport.UDS != nil { + allErrs = append(allErrs, validateUDSConnection(transport.UDS, fldPath)...) + } + return allErrs +} + +func validateGRPCTransport(transport *apiserver.Transport, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if transport == nil { + allErrs = append(allErrs, field.Required( + fldPath.Child("transport"), + "transport must be set for GRPC")) + return allErrs + } + + if transport.UDS != nil { + allErrs = append(allErrs, validateUDSConnection(transport.UDS, fldPath)...) + } else { + allErrs = append(allErrs, field.Required( + fldPath.Child("uds"), + "UDS must be set with GRPC")) + } return allErrs } @@ -129,9 +157,9 @@ func validateDirectConnection(connection apiserver.Connection, fldPath *field.Pa return nil } -func validateUDSConnection(connection apiserver.Connection, fldPath *field.Path) field.ErrorList { +func validateUDSConnection(udsConfig *apiserver.UDSTransport, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if connection.Transport.UDS.UDSName == "" { + if udsConfig.UDSName == "" { allErrs = append(allErrs, field.Invalid( fldPath.Child("udsName"), "nil", @@ -140,68 +168,65 @@ func validateUDSConnection(connection apiserver.Connection, fldPath *field.Path) return allErrs } -func validateTCPConnection(connection apiserver.Connection, fldPath *field.Path) field.ErrorList { +func validateTCPConnection(tcpConfig *apiserver.TCPTransport, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if connection.Transport.TCP.TLSConfig == nil { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig"), - "nil", - "TLSConfig config should be present for HTTPConnect via tcp")) - } else if strings.HasPrefix(connection.Transport.TCP.URL, "https://") { - if connection.Transport.TCP.TLSConfig.CABundle != "" { - if exists, err := path.Exists(path.CheckFollowSymlink, connection.Transport.TCP.TLSConfig.CABundle); exists == false || err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "caBundle"), - connection.Transport.TCP.TLSConfig.CABundle, - "HTTPConnect ca bundle does not exist")) - } - } - if connection.Transport.TCP.TLSConfig.ClientCert == "" { + + if strings.HasPrefix(tcpConfig.URL, "http://") { + if tcpConfig.TLSConfig != nil { allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientCert"), + fldPath.Child("tlsConfig"), "nil", - "HTTPConnect via https requires clientCert")) - } else if exists, err := path.Exists(path.CheckFollowSymlink, connection.Transport.TCP.TLSConfig.ClientCert); exists == false || err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientCert"), - connection.Transport.TCP.TLSConfig.ClientCert, - "HTTPConnect client cert does not exist")) - } - if connection.Transport.TCP.TLSConfig.ClientKey == "" { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientKey"), - "nil", - "HTTPConnect via https requires clientKey")) - } else if exists, err := path.Exists(path.CheckFollowSymlink, connection.Transport.TCP.TLSConfig.ClientKey); exists == false || err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientKey"), - connection.Transport.TCP.TLSConfig.ClientKey, - "HTTPConnect client key does not exist")) - } - } else if strings.HasPrefix(connection.Transport.TCP.URL, "http://") { - if connection.Transport.TCP.TLSConfig.CABundle != "" { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "caBundle"), - connection.Transport.TCP.TLSConfig.CABundle, - "HTTPConnect via http does not support caBundle")) - } - if connection.Transport.TCP.TLSConfig.ClientCert != "" { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientCert"), - connection.Transport.TCP.TLSConfig.ClientCert, - "HTTPConnect via http does not support clientCert")) - } - if connection.Transport.TCP.TLSConfig.ClientKey != "" { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("tlsConfig", "clientKey"), - connection.Transport.TCP.TLSConfig.ClientKey, - "HTTPConnect via http does not support clientKey")) + "TLSConfig config should not be present when using HTTP")) } + } else if strings.HasPrefix(tcpConfig.URL, "https://") { + return validateTLSConfig(tcpConfig.TLSConfig, fldPath) } else { allErrs = append(allErrs, field.Invalid( fldPath.Child("url"), - connection.Transport.TCP.URL, + tcpConfig.URL, "supported connection protocols are http:// and https://")) } return allErrs } + +func validateTLSConfig(tlsConfig *apiserver.TLSConfig, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if tlsConfig == nil { + allErrs = append(allErrs, field.Required( + fldPath.Child("tlsConfig"), + "TLSConfig must be present when using HTTPS")) + return allErrs + } + if tlsConfig.CABundle != "" { + if exists, err := path.Exists(path.CheckFollowSymlink, tlsConfig.CABundle); exists == false || err != nil { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tlsConfig", "caBundle"), + tlsConfig.CABundle, + "TLS config ca bundle does not exist")) + } + } + if tlsConfig.ClientCert == "" { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tlsConfig", "clientCert"), + "nil", + "Using TLS requires clientCert")) + } else if exists, err := path.Exists(path.CheckFollowSymlink, tlsConfig.ClientCert); exists == false || err != nil { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tlsConfig", "clientCert"), + tlsConfig.ClientCert, + "TLS client cert does not exist")) + } + if tlsConfig.ClientKey == "" { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tlsConfig", "clientKey"), + "nil", + "Using TLS requires requires clientKey")) + } else if exists, err := path.Exists(path.CheckFollowSymlink, tlsConfig.ClientKey); exists == false || err != nil { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("tlsConfig", "clientKey"), + tlsConfig.ClientKey, + "TLS client key does not exist")) + } + return allErrs +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go index 0f36bffbd8b..87b8951d697 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go @@ -223,3 +223,206 @@ spec: }) } } + +func TestValidateEgressSelectorConfiguration(t *testing.T) { + testcases := []struct { + name string + expectError bool + contents *apiserver.EgressSelectorConfiguration + }{ + { + name: "direct-valid", + expectError: false, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "master", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolDirect, + }, + }, + }, + }, + }, + { + name: "direct-invalid-transport", + expectError: true, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "master", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolDirect, + Transport: &apiserver.Transport{}, + }, + }, + }, + }, + }, + { + name: "httpconnect-no-https", + expectError: false, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolHTTPConnect, + Transport: &apiserver.Transport{ + TCP: &apiserver.TCPTransport{ + URL: "http://127.0.0.1:8131", + }, + }, + }, + }, + }, + }, + }, + { + name: "httpconnect-https-no-cert-error", + expectError: true, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolHTTPConnect, + Transport: &apiserver.Transport{ + TCP: &apiserver.TCPTransport{ + URL: "https://127.0.0.1:8131", + }, + }, + }, + }, + }, + }, + }, + { + name: "httpconnect-tcp-uds-both-set", + expectError: true, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolHTTPConnect, + Transport: &apiserver.Transport{ + TCP: &apiserver.TCPTransport{ + URL: "http://127.0.0.1:8131", + }, + UDS: &apiserver.UDSTransport{ + UDSName: "/etc/srv/kubernetes/konnectivity/konnectivity-server.socket", + }, + }, + }, + }, + }, + }, + }, + { + name: "httpconnect-uds", + expectError: false, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolHTTPConnect, + Transport: &apiserver.Transport{ + UDS: &apiserver.UDSTransport{ + UDSName: "/etc/srv/kubernetes/konnectivity/konnectivity-server.socket", + }, + }, + }, + }, + }, + }, + }, + { + name: "grpc-https-invalid", + expectError: true, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolGRPC, + Transport: &apiserver.Transport{ + TCP: &apiserver.TCPTransport{ + URL: "http://127.0.0.1:8131", + TLSConfig: &apiserver.TLSConfig{ + CABundle: "", + ClientKey: "", + ClientCert: "", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "grpc-uds", + expectError: false, + contents: &apiserver.EgressSelectorConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "", + APIVersion: "", + }, + EgressSelections: []apiserver.EgressSelection{ + { + Name: "cluster", + Connection: apiserver.Connection{ + ProxyProtocol: apiserver.ProtocolGRPC, + Transport: &apiserver.Transport{ + UDS: &apiserver.UDSTransport{ + UDSName: "/etc/srv/kubernetes/konnectivity/konnectivity-server.socket", + }, + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + errs := ValidateEgressSelectorConfiguration(tc.contents) + if tc.expectError == false && len(errs) != 0 { + t.Errorf("Calling ValidateEgressSelectorConfiguration expected no error, got %v", errs) + } else if tc.expectError == true && len(errs) == 0 { + t.Errorf("Calling ValidateEgressSelectorConfiguration expected error, got no error") + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/egress_selector.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/egress_selector.go index 56d8fcc7a78..d333debefbe 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/egress_selector.go +++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/egress_selector.go @@ -148,6 +148,7 @@ func createConnectTCPDialer(tcpTransport *apiserver.TCPTransport) (utilnet.DialF return nil, fmt.Errorf("failed to append CA cert to the cert pool") } } else { + // Use host's root CA set instead of providing our own certPool = nil } contextDialer := func(ctx context.Context, network, addr string) (net.Conn, error) { @@ -166,13 +167,13 @@ func createConnectTCPDialer(tcpTransport *apiserver.TCPTransport) (utilnet.DialF return contextDialer, nil } -func createConnectUDSDialer(udsName string) (utilnet.DialFunc, error) { +func createConnectUDSDialer(udsConfig *apiserver.UDSTransport) (utilnet.DialFunc, error) { contextDialer := func(ctx context.Context, network, addr string) (net.Conn, error) { - proxyConn, err := net.Dial("unix", udsName) + proxyConn, err := net.Dial("unix", udsConfig.UDSName) if err != nil { - return nil, fmt.Errorf("dialing proxy %q failed: %v", udsName, err) + return nil, fmt.Errorf("dialing proxy %q failed: %v", udsConfig.UDSName, err) } - return tunnelHTTPConnect(proxyConn, udsName, addr) + return tunnelHTTPConnect(proxyConn, udsConfig.UDSName, addr) } return contextDialer, nil } @@ -221,7 +222,7 @@ func NewEgressSelector(config *apiserver.EgressSelectorConfiguration) (*EgressSe case apiserver.ProtocolHTTPConnect: if service.Connection.Transport.UDS != nil { - contextDialer, err := createConnectUDSDialer(service.Connection.Transport.UDS.UDSName) + contextDialer, err := createConnectUDSDialer(service.Connection.Transport.UDS) if err != nil { return nil, fmt.Errorf("failed to create HTTPConnect uds dialer: %v", err) } @@ -233,7 +234,7 @@ func NewEgressSelector(config *apiserver.EgressSelectorConfiguration) (*EgressSe } cs.egressToDialer[name] = contextDialer } else { - return nil, fmt.Errorf("Either TCP or UDP transport must be specified") + return nil, fmt.Errorf("Either a TCP or UDS transport must be specified") } case apiserver.ProtocolGRPC: if service.Connection.Transport.UDS != nil { @@ -244,7 +245,7 @@ func NewEgressSelector(config *apiserver.EgressSelectorConfiguration) (*EgressSe cs.egressToDialer[name] = grpcContextDialer } else { - return nil, fmt.Errorf("Either TCP or UDP transport must be specified") + return nil, fmt.Errorf("UDS transport must be specified for GRPC") } case apiserver.ProtocolDirect: cs.egressToDialer[name] = directDialer