Skip to content

Commit

Permalink
network: disallow legacy hostNetwork together with non-default provider
Browse files Browse the repository at this point in the history
Fixes: rook#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 <obnox@samba.org>
  • Loading branch information
obnoxxx committed Feb 12, 2024
1 parent c38612c commit 25aa15e
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 0 deletions.
2 changes: 2 additions & 0 deletions deploy/charts/rook-ceph/templates/resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions deploy/examples/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/ceph.rook.io/v1/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions pkg/apis/ceph.rook.io/v1/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
1 change: 1 addition & 0 deletions pkg/apis/ceph.rook.io/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 25aa15e

Please sign in to comment.