Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow legacy hostNetwork together with non-default provider #13693

Merged
merged 2 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .commitlintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"mon",
"monitoring",
"multus",
"network",
"nfs",
"object",
"operator",
Expand Down
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) {
travisn marked this conversation as resolved.
Show resolved Hide resolved
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