From bc44e4c1647e0155482aa7e6ca7bc3f10e7dc826 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 30 Jan 2024 17:58:02 +0100 Subject: [PATCH 1/2] ci: add "network" to the allowed commit message prefixes Signed-off-by: Michael Adam (cherry picked from commit c38612c28068c7f8ae3fb490366621aa9c4b5551) --- .commitlintrc.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.commitlintrc.json b/.commitlintrc.json index 070615343..ba22b047f 100644 --- a/.commitlintrc.json +++ b/.commitlintrc.json @@ -28,6 +28,7 @@ "mon", "monitoring", "multus", + "network", "nfs", "object", "operator", From 824626d3e5896eb1b29134d935fb456a88fb1f65 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 5 Feb 2024 20:02:51 +0100 Subject: [PATCH 2/2] 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 (cherry picked from commit 25aa15e6c1c0d9799123fef8e6ac35f1fbe966da) --- .../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 fb347c90b..015c476c7 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -2273,6 +2273,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 89db00ae8..bf84413ec 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -2271,6 +2271,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 6abd842ee..629f30a06 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 38d4be111..3e3446257 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 ddcddc316..eb04cd020 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.