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.

Sign up for GitHub

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

Jump to bottom

[Multicast] Add precheck on the encryption mode configurations #5920

Merged
merged 1 commit into from
Jan 30, 2024
Merged

[Multicast] Add precheck on the encryption mode configurations #5920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions cmd/antrea-agent/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,12 @@ func (o *Options) validateFlowExporterConfig() error {
return nil
}

func (o *Options) validateMulticastConfig() error {
if features.DefaultFeatureGate.Enabled(features.Multicast) {
func (o *Options) validateMulticastConfig(encryptionMode config.TrafficEncryptionModeType) error {
if features.DefaultFeatureGate.Enabled(features.Multicast) && o.config.Multicast.Enable {
var err error
if encryptionMode != config.TrafficEncryptionModeNone {
return fmt.Errorf("Multicast feature doesn't work with the current encryption mode '%s'", encryptionMode)
}
if o.config.Multicast.IGMPQueryInterval != "" {
o.igmpQueryInterval, err = time.ParseDuration(o.config.Multicast.IGMPQueryInterval)
if err != nil {
Expand Down Expand Up @@ -584,7 +587,7 @@ func (o *Options) validateK8sNodeOptions() error {
if err := o.validateFlowExporterConfig(); err != nil {
return fmt.Errorf("failed to validate flow exporter config: %v", err)
}
if err := o.validateMulticastConfig(); err != nil {
if err := o.validateMulticastConfig(encryptionMode); err != nil {
return fmt.Errorf("failed to validate multicast config: %v", err)
}
if err := o.validateEgressConfig(encapMode); err != nil {
Expand Down
33 changes: 30 additions & 3 deletions cmd/antrea-agent/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,18 +219,42 @@ func TestOptionsValidateMulticastConfig(t *testing.T) {
tests := []struct {
name string
igmpQueryVersions []int
encryptionMode config.TrafficEncryptionModeType
expectedErr error
expectedVersions []uint8
}{
{
name: "wrong versions",
igmpQueryVersions: []int{1, 3, 4},
encryptionMode: config.TrafficEncryptionModeNone,
expectedErr: fmt.Errorf("igmpQueryVersions should be a subset of [1 2 3]"),
expectedVersions: nil,
},
{
name: "incorrect encryption mode with IPSec",
igmpQueryVersions: []int{1, 2},
encryptionMode: config.TrafficEncryptionModeIPSec,
expectedErr: fmt.Errorf("Multicast feature doesn't work with the current encryption mode 'IPsec'"),
expectedVersions: nil,
},
{
name: "incorrect encryption mode with WireGuard",
igmpQueryVersions: []int{1, 2},
encryptionMode: config.TrafficEncryptionModeWireGuard,
expectedErr: fmt.Errorf("Multicast feature doesn't work with the current encryption mode 'WireGuard'"),
expectedVersions: nil,
},
{
name: "incorrect encryption mode with invalid",
igmpQueryVersions: []int{1, 2},
encryptionMode: config.TrafficEncryptionModeInvalid,
expectedErr: fmt.Errorf("Multicast feature doesn't work with the current encryption mode 'invalid'"),
expectedVersions: nil,
},
{
name: "no error",
igmpQueryVersions: []int{1, 2},
encryptionMode: config.TrafficEncryptionModeNone,
expectedErr: nil,
expectedVersions: []uint8{1, 2},
},
Expand All @@ -240,11 +264,14 @@ func TestOptionsValidateMulticastConfig(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, features.DefaultFeatureGate, features.Multicast, true)()
o := &Options{config: &agentconfig.AgentConfig{
Multicast: agentconfig.MulticastConfig{
Enable: true,
IGMPQueryVersions: tt.igmpQueryVersions},
}}
err := o.validateMulticastConfig()
assert.Equal(t, tt.expectedErr, err)
assert.Equal(t, tt.expectedVersions, o.igmpQueryVersions)
err := o.validateMulticastConfig(tt.encryptionMode)
require.Equal(t, tt.expectedErr, err)
if err != nil {
assert.Equal(t, tt.expectedVersions, o.igmpQueryVersions)
}
})
}
}
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/wireguard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,20 @@ func TestWireGuard(t *testing.T) {
skipIfMissingKernelModule(t, data, node.name, []string{"wireguard"})
}
var previousTrafficEncryptionMode string
var previousMulticastEnabledState bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto: changes to this file can be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may still need this change, otherwise, "E2e tests on a Kind cluster on Linux with all features enabled" may fail as Multicast was enabled before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. It would have been nice to have a comment here explaining that enabling WireGuard requires disabling Multicast, but no big deal, I see that CI jobs have passed already

ac := func(config *agentconfig.AgentConfig) {
previousTrafficEncryptionMode = config.TrafficEncryptionMode
config.TrafficEncryptionMode = "wireguard"
previousMulticastEnabledState = config.Multicast.Enable
config.Multicast.Enable = false
}
if err := data.mutateAntreaConfigMap(nil, ac, false, true); err != nil {
t.Fatalf("Failed to enable WireGuard tunnel: %v", err)
}
defer func() {
ac := func(config *agentconfig.AgentConfig) {
config.TrafficEncryptionMode = previousTrafficEncryptionMode
config.Multicast.Enable = previousMulticastEnabledState
}
if err := data.mutateAntreaConfigMap(nil, ac, false, true); err != nil {
t.Errorf("Failed to disable WireGuard tunnel: %v", err)
Expand Down
Loading