Add option "secureCluster"

This options enforces internal clients to use secure connections and validates that related servers are ready to handle TLS. Also it forces TLS-only mode for native bus transport and HTTPS-only for default role of HTTP proxies. I.e. if it is enabled - only non-default HTTP proxies and any RPC proxies could be not strictly TLS-only. Issue: #285
ytsaurus · Oct 18, 2024 · c1ffc3c · c1ffc3c
1 parent 805bba2
commit c1ffc3c
Show file tree

Hide file tree

Showing 14 changed files with 113 additions and 6 deletions.
diff --git a/api/v1/ytsaurus_types.go b/api/v1/ytsaurus_types.go
@@ -564,6 +564,11 @@ type CommonSpec struct {
 	//+optional
 	NativeTransport *RPCTransportSpec `json:"nativeTransport,omitempty"`
 
+	// Always use TLS/HTTPS for connections inside cluster.
+	//+kubebuilder:default:=false
+	//+optional
+	SecureCluster bool `json:"secureCluster"`
+
 	// Allow prioritizing performance over data safety. Useful for tests and experiments.
 	//+kubebuilder:default:=false
 	//+optional

diff --git a/api/v1/ytsaurus_webhook.go b/api/v1/ytsaurus_webhook.go
@@ -61,6 +61,35 @@ func (r *Ytsaurus) SetupWebhookWithManager(mgr ctrl.Manager) error {
 
 //////////////////////////////////////////////////
 
+func validateCommonSpec(spec *CommonSpec) field.ErrorList {
+	var allErrors field.ErrorList
+
+	path := field.NewPath("spec").Child("nativeTransport")
+	nt := spec.NativeTransport
+	if nt == nil {
+		nt = &RPCTransportSpec{}
+	}
+
+	secretPath := path.Child("tlsSecret")
+	if nt.TLSSecret == nil && nt.TLSRequired {
+		allErrors = append(allErrors, field.Required(secretPath, "TLS certificate for native transport is required"))
+	}
+
+	if spec.SecureCluster {
+		if nt.TLSSecret == nil {
+			allErrors = append(allErrors, field.Required(secretPath, "Secure cluster requires TLS certificate for native transport"))
+		}
+		if !nt.TLSRequired {
+			allErrors = append(allErrors, field.Forbidden(path.Child("tlsRequired"), "Secure cluster must require TLS for native transport"))
+		}
+		if nt.TLSInsecure {
+			allErrors = append(allErrors, field.Forbidden(path.Child("tlsInsecure"), "Secure cluster requires TLS certificate validation"))
+		}
+	}
+
+	return allErrors
+}
+
 func (r *ytsaurusValidator) validateDiscovery(newYtsaurus *Ytsaurus) field.ErrorList {
 	var allErrors field.ErrorList
 
@@ -154,6 +183,21 @@ func (r *ytsaurusValidator) validateHTTPProxies(newYtsaurus *Ytsaurus) field.Err
 		}
 		httpRoles[hp.Role] = true
 
+		if hp.Transport.HTTPSSecret == nil {
+			secretPath := path.Child("transport").Child("httpsSecret")
+			if hp.Transport.DisableHTTP {
+				allErrors = append(allErrors, field.Required(secretPath, "HTTPS-only proxy requires TLS certificate"))
+			}
+			if newYtsaurus.Spec.SecureCluster {
+				allErrors = append(allErrors, field.Required(secretPath, "Secure cluster requires TLS certificate for all HTTP proxies"))
+			}
+		}
+
+		if !hp.Transport.DisableHTTP && newYtsaurus.Spec.SecureCluster && hp.Role == consts.DefaultHTTPProxyRole {
+			insecurePath := path.Child("transport").Child("disableHttp")
+			allErrors = append(allErrors, field.Forbidden(insecurePath, "Secure cluster requires HTTPS-only proxies for default role"))
+		}
+
 		allErrors = append(allErrors, r.validateInstanceSpec(hp.InstanceSpec, path)...)
 	}
 
@@ -177,6 +221,21 @@ func (r *ytsaurusValidator) validateRPCProxies(newYtsaurus *Ytsaurus) field.Erro
 		}
 		rpcRoles[rp.Role] = true
 
+		if rp.Transport.TLSSecret == nil {
+			secretPath := path.Child("transport").Child("tlsSecret")
+			if rp.Transport.TLSRequired {
+				allErrors = append(allErrors, field.Required(secretPath, "TLS-only RPC proxy requires certificate"))
+			}
+			if newYtsaurus.Spec.SecureCluster {
+				allErrors = append(allErrors, field.Required(secretPath, "Secure cluster demands TLS certificate for RPC proxies"))
+			}
+		}
+
+		if rp.Transport.TLSInsecure && newYtsaurus.Spec.SecureCluster {
+			insecurePath := path.Child("transport").Child("tlsInsecure")
+			allErrors = append(allErrors, field.Forbidden(insecurePath, "Secure cluster requires TLS certificate validation"))
+		}
+
 		allErrors = append(allErrors, r.validateInstanceSpec(rp.InstanceSpec, path)...)
 	}
 
@@ -481,6 +540,7 @@ func (r *ytsaurusValidator) validateExistsYtsaurus(ctx context.Context, newYtsau
 func (r *ytsaurusValidator) validateYtsaurus(ctx context.Context, newYtsaurus, oldYtsaurus *Ytsaurus) field.ErrorList {
 	var allErrors field.ErrorList
 
+	allErrors = append(allErrors, validateCommonSpec(&newYtsaurus.Spec.CommonSpec)...)
 	allErrors = append(allErrors, r.validateDiscovery(newYtsaurus)...)
 	allErrors = append(allErrors, r.validatePrimaryMasters(newYtsaurus, oldYtsaurus)...)
 	allErrors = append(allErrors, r.validateSecondaryMasters(newYtsaurus)...)

diff --git a/config/crd/bases/cluster.ytsaurus.tech_remotedatanodes.yaml b/config/crd/bases/cluster.ytsaurus.tech_remotedatanodes.yaml
@@ -953,6 +953,10 @@ spec:
                 type: object
               runtimeClassName:
                 type: string
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               setHostnameAsFqdn:
                 default: true
                 description: SetHostnameAsFQDN indicates whether to set the hostname

diff --git a/config/crd/bases/cluster.ytsaurus.tech_remoteexecnodes.yaml b/config/crd/bases/cluster.ytsaurus.tech_remoteexecnodes.yaml
@@ -1141,6 +1141,10 @@ spec:
                 type: object
               runtimeClassName:
                 type: string
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               setHostnameAsFqdn:
                 default: true
                 description: SetHostnameAsFQDN indicates whether to set the hostname

diff --git a/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml b/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml
@@ -29423,6 +29423,10 @@ spec:
                   - cellTag
                   type: object
                 type: array
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               spyt:
                 properties:
                   sparkVersion:

diff --git a/docs/api.md b/docs/api.md
@@ -241,6 +241,7 @@ _Appears in:_
 | `jobImage` _string_ | Default docker image for user jobs. |  |  |
 | `caBundle` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core)_ | Reference to ConfigMap with trusted certificates: "ca.crt". |  |  |
 | `nativeTransport` _[RPCTransportSpec](#rpctransportspec)_ | Common config for native RPC bus transport. |  |  |
+| `secureCluster` _boolean_ | Always use TLS/HTTPS for connections inside cluster. | false |  |
 | `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false |  |
 | `useIpv6` _boolean_ |  | false |  |
 | `useIpv4` _boolean_ |  | false |  |
@@ -1187,6 +1188,7 @@ _Appears in:_
 | `jobImage` _string_ | Default docker image for user jobs. |  |  |
 | `caBundle` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core)_ | Reference to ConfigMap with trusted certificates: "ca.crt". |  |  |
 | `nativeTransport` _[RPCTransportSpec](#rpctransportspec)_ | Common config for native RPC bus transport. |  |  |
+| `secureCluster` _boolean_ | Always use TLS/HTTPS for connections inside cluster. | false |  |
 | `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false |  |
 | `useIpv6` _boolean_ |  | false |  |
 | `useIpv4` _boolean_ |  | false |  |
@@ -1265,6 +1267,7 @@ _Appears in:_
 | `jobImage` _string_ | Default docker image for user jobs. |  |  |
 | `caBundle` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core)_ | Reference to ConfigMap with trusted certificates: "ca.crt". |  |  |
 | `nativeTransport` _[RPCTransportSpec](#rpctransportspec)_ | Common config for native RPC bus transport. |  |  |
+| `secureCluster` _boolean_ | Always use TLS/HTTPS for connections inside cluster. | false |  |
 | `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false |  |
 | `useIpv6` _boolean_ |  | false |  |
 | `useIpv4` _boolean_ |  | false |  |
@@ -1820,6 +1823,7 @@ _Appears in:_
 | `jobImage` _string_ | Default docker image for user jobs. |  |  |
 | `caBundle` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core)_ | Reference to ConfigMap with trusted certificates: "ca.crt". |  |  |
 | `nativeTransport` _[RPCTransportSpec](#rpctransportspec)_ | Common config for native RPC bus transport. |  |  |
+| `secureCluster` _boolean_ | Always use TLS/HTTPS for connections inside cluster. | false |  |
 | `ephemeralCluster` _boolean_ | Allow prioritizing performance over data safety. Useful for tests and experiments. | false |  |
 | `useIpv6` _boolean_ |  | false |  |
 | `useIpv4` _boolean_ |  | false |  |

diff --git a/pkg/components/chyt.go b/pkg/components/chyt.go
@@ -113,7 +113,7 @@ func (c *Chyt) createInitScript() string {
 func (c *Chyt) createInitChPublicScript() string {
 	script := []string{
 		initJobPrologue,
-		fmt.Sprintf("export YT_PROXY=%v CHYT_CTL_ADDRESS=%v YT_LOG_LEVEL=debug", c.cfgen.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole), c.cfgen.GetStrawberryControllerServiceAddress()),
+		fmt.Sprintf("export YT_PROXY=%v CHYT_CTL_ADDRESS=%v YT_LOG_LEVEL=debug", c.cfgen.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole), c.cfgen.GetStrawberryControllerServiceAddress()),
 		"yt create scheduler_pool --attributes '{name=chyt; pool_tree=default}' --ignore-existing",
 		"yt clickhouse ctl create ch_public || true",
 		"yt clickhouse ctl set-option --alias ch_public pool chyt",
@@ -173,7 +173,7 @@ func (c *Chyt) doSync(ctx context.Context, dry bool) (ComponentStatus, error) {
 		container.Env = []corev1.EnvVar{
 			{
 				Name:  "YT_PROXY",
-				Value: c.cfgen.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole),
+				Value: c.cfgen.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole),
 			},
 			{
 				Name:  "YT_TOKEN",

diff --git a/pkg/components/spyt.go b/pkg/components/spyt.go
@@ -135,7 +135,7 @@ func (s *Spyt) doSync(ctx context.Context, dry bool) (ComponentStatus, error) {
 		container.Env = []corev1.EnvVar{
 			{
 				Name:  "YT_PROXY",
-				Value: s.cfgen.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole),
+				Value: s.cfgen.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole),
 			},
 			{
 				Name:  "YT_TOKEN",

diff --git a/pkg/components/ytsaurus_client.go b/pkg/components/ytsaurus_client.go
@@ -372,11 +372,13 @@ func (yc *YtsaurusClient) doSync(ctx context.Context, dry bool) (ComponentStatus
 		proxy, ok := os.LookupEnv("YTOP_PROXY")
 		disableProxyDiscovery := true
 		if !ok {
+			// FIXME(khlebnikov): Use GetHTTPProxyUrl() when SDK will be ready.
 			proxy = yc.cfgen.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole)
 			disableProxyDiscovery = false
 		}
 		yc.ytClient, err = ythttp.NewClient(&yt.Config{
 			Proxy:                 proxy,
+			UseTLS:                yc.cfgen.UseHTTPSProxy(),
 			Token:                 token,
 			LightRequestTimeout:   &timeout,
 			DisableProxyDiscovery: disableProxyDiscovery,

diff --git a/pkg/ytconfig/generator.go b/pkg/ytconfig/generator.go
@@ -318,7 +318,7 @@ func (g *Generator) GetStrawberryControllerConfig() ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	proxy := g.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole)
+	proxy := g.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole)
 	c.LocationProxies = []string{proxy}
 	c.HTTPLocationAliases = map[string][]string{
 		proxy: []string{g.ytsaurus.Name},
@@ -328,7 +328,7 @@ func (g *Generator) GetStrawberryControllerConfig() ([]byte, error) {
 
 func (g *Generator) GetChytInitClusterConfig() ([]byte, error) {
 	c := getChytInitCluster()
-	c.Proxy = g.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole)
+	c.Proxy = g.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole)
 	return marshallYsonConfig(c)
 }
 
@@ -695,7 +695,7 @@ func (g *Generator) GetUIClustersConfig() ([]byte, error) {
 	c := getUIClusterCarcass()
 	c.ID = g.ytsaurus.Name
 	c.Name = g.ytsaurus.Name
-	c.Proxy = g.GetHTTPProxiesAddress(consts.DefaultHTTPProxyRole)
+	c.Proxy = g.GetHTTPProxyUrl(consts.DefaultHTTPProxyRole)
 	c.Secure = g.ytsaurus.Spec.UI.Secure
 	c.ExternalProxy = g.ytsaurus.Spec.UI.ExternalProxy
 	c.PrimaryMaster.CellTag = g.ytsaurus.Spec.PrimaryMasters.CellTag

diff --git a/pkg/ytconfig/names.go b/pkg/ytconfig/names.go
@@ -120,6 +120,18 @@ func (g *Generator) GetHTTPProxiesAddress(role string) string {
 		g.clusterDomain)
 }
 
+func (g *Generator) UseHTTPSProxy() bool {
+	return g.ytsaurus.Spec.SecureCluster
+}
+
+func (g *Generator) GetHTTPProxyUrl(role string) string {
+	address := g.GetHTTPProxiesAddress(role)
+	if g.ytsaurus.Spec.SecureCluster {
+		return "https://" + address
+	}
+	return "http://" + address
+}
+
 func (g *Generator) GetSchedulerStatefulSetName() string {
 	return g.getName("sch")
 }

diff --git a/ytop-chart/templates/crds/remotedatanodes.cluster.ytsaurus.tech.yaml b/ytop-chart/templates/crds/remotedatanodes.cluster.ytsaurus.tech.yaml
@@ -964,6 +964,10 @@ spec:
                 type: object
               runtimeClassName:
                 type: string
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               setHostnameAsFqdn:
                 default: true
                 description: SetHostnameAsFQDN indicates whether to set the hostname

diff --git a/ytop-chart/templates/crds/remoteexecnodes.cluster.ytsaurus.tech.yaml b/ytop-chart/templates/crds/remoteexecnodes.cluster.ytsaurus.tech.yaml
@@ -1152,6 +1152,10 @@ spec:
                 type: object
               runtimeClassName:
                 type: string
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               setHostnameAsFqdn:
                 default: true
                 description: SetHostnameAsFQDN indicates whether to set the hostname

diff --git a/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml b/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml
@@ -29434,6 +29434,10 @@ spec:
                   - cellTag
                   type: object
                 type: array
+              secureCluster:
+                default: false
+                description: Always use TLS/HTTPS for connections inside cluster.
+                type: boolean
               spyt:
                 properties:
                   sparkVersion: