From 25aa15e6c1c0d9799123fef8e6ac35f1fbe966da Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 5 Feb 2024 20:02:51 +0100 Subject: [PATCH] network: disallow legacy hostNetwork together with non-default provider Fixes: #13692 Since the introduction of the "host" network provider, the legacy "hostNetwork" setting is intended to be used only in combination with the default network provider (""), but the code did not enforce this. This change adds the required validation checks to throw errors in invalid constellations. These checks are added both in the operator's input validation code and as kubernetes x-validation admission policies in the Cepcluster CRD. Signed-off-by: Michael Adam --- .../charts/rook-ceph/templates/resources.yaml | 2 ++ deploy/examples/crds.yaml | 2 ++ pkg/apis/ceph.rook.io/v1/network.go | 3 ++ pkg/apis/ceph.rook.io/v1/network_test.go | 30 +++++++++++++++++++ pkg/apis/ceph.rook.io/v1/types.go | 1 + 5 files changed, 38 insertions(+) diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index bc4fa1ba0d53..bd4ff3c741f0 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -2282,6 +2282,8 @@ spec: x-kubernetes-validations: - message: at least one network selector must be specified when using multus rule: '!has(self.provider) || (self.provider != ''multus'' || (self.provider == ''multus'' && size(self.selectors) > 0))' + - message: the legacy hostNetwork setting can only be set if the network.provider is set to the empty string + rule: '!has(self.hostNetwork) || self.hostNetwork == false || !has(self.provider) || self.provider == ""' placement: additionalProperties: description: Placement is the placement for an object diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 8c01375060a6..bf773841de34 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -2280,6 +2280,8 @@ spec: x-kubernetes-validations: - message: at least one network selector must be specified when using multus rule: '!has(self.provider) || (self.provider != ''multus'' || (self.provider == ''multus'' && size(self.selectors) > 0))' + - message: the legacy hostNetwork setting can only be set if the network.provider is set to the empty string + rule: '!has(self.hostNetwork) || self.hostNetwork == false || !has(self.provider) || self.provider == ""' placement: additionalProperties: description: Placement is the placement for an object diff --git a/pkg/apis/ceph.rook.io/v1/network.go b/pkg/apis/ceph.rook.io/v1/network.go index 6abd842ee54d..629f30a069d0 100644 --- a/pkg/apis/ceph.rook.io/v1/network.go +++ b/pkg/apis/ceph.rook.io/v1/network.go @@ -39,6 +39,9 @@ func (n *NetworkSpec) IsHost() bool { } func ValidateNetworkSpec(clusterNamespace string, spec NetworkSpec) error { + if spec.HostNetwork && (spec.Provider != NetworkProviderDefault) { + return errors.Errorf(`the legacy hostNetwork setting is only valid with the default network provider ("") and not with '%q'`, spec.Provider) + } if spec.IsMultus() { if len(spec.Selectors) == 0 { return errors.Errorf("at least one network selector must be specified when using the %q network provider", NetworkProviderMultus) diff --git a/pkg/apis/ceph.rook.io/v1/network_test.go b/pkg/apis/ceph.rook.io/v1/network_test.go index 38d4be111ab0..3e3446257f26 100644 --- a/pkg/apis/ceph.rook.io/v1/network_test.go +++ b/pkg/apis/ceph.rook.io/v1/network_test.go @@ -42,6 +42,36 @@ func TestNetworkCephSpecLegacy(t *testing.T) { assert.Equal(t, expected, net) } +func TestValidateNetworkSpec(t *testing.T) { + net := NetworkSpec{ + HostNetwork: true, + Provider: NetworkProviderDefault, + } + err := ValidateNetworkSpec("", net) + assert.NoError(t, err) + + net = NetworkSpec{ + HostNetwork: true, + Provider: NetworkProviderHost, + } + err = ValidateNetworkSpec("", net) + assert.Error(t, err) + + net = NetworkSpec{ + HostNetwork: false, + Provider: NetworkProviderDefault, + } + err = ValidateNetworkSpec("", net) + assert.NoError(t, err) + + net = NetworkSpec{ + HostNetwork: false, + Provider: NetworkProviderHost, + } + err = ValidateNetworkSpec("", net) + assert.NoError(t, err) +} + func TestNetworkCephIsHostLegacy(t *testing.T) { net := NetworkSpec{HostNetwork: true} diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index ddcddc31649f..eb04cd02073c 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -2316,6 +2316,7 @@ type SSSDSidecarAdditionalFile struct { // NetworkSpec for Ceph includes backward compatibility code // +kubebuilder:validation:XValidation:message="at least one network selector must be specified when using multus",rule="!has(self.provider) || (self.provider != 'multus' || (self.provider == 'multus' && size(self.selectors) > 0))" +// +kubebuilder:validation:XValidation:message=`the legacy hostNetwork setting can only be set if the network.provider is set to the empty string`,rule=`!has(self.hostNetwork) || self.hostNetwork == false || !has(self.provider) || self.provider == ""` type NetworkSpec struct { // Provider is what provides network connectivity to the cluster e.g. "host" or "multus". // If the Provider is updated from being empty to "host" on a running cluster, then the operator will automatically fail over all the mons to apply the "host" network settings.