Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions webhooks/virtualmachine/mutation/virtualmachine_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,8 @@ func setDefaultNetworkInterfaceNetwork(
}

mutated := false
// Fill in any empty fields with that should be there based on the network
// provider. The validation webhook will fail for invalid combinations.
if ifaceNetwork.APIVersion == "" {
ifaceNetwork.APIVersion = networkRef.APIVersion
mutated = true
Expand Down Expand Up @@ -553,39 +555,42 @@ func AddDefaultNetworkInterface(
client ctrlclient.Client,
vm *vmopv1.VirtualMachine) bool {

// Continue to support this ad-hoc v1a1 annotation. I don't think need or want to have this annotation
// in v1a2: Disabled mostly already covers it. We could map between the two for version conversion, but
// they do mean slightly different things, and kind of complicated to know what to do like if the annotation
// is removed.
if _, ok := vm.Annotations[v1alpha1.NoDefaultNicAnnotation]; ok {
if vm.Spec.Network == nil || len(vm.Spec.Network.Interfaces) == 0 {
return false
}
}

if vm.Spec.Network != nil && vm.Spec.Network.Disabled {
return false
}

ok, networkRef := getDefaultNetworkRef(ctx, client)
if !ok {
// TODO(BV): This really can't fail but will need to change for provider migrated namespaces.
return false
}

if vm.Spec.Network == nil {
vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{}
}
if vm.Spec.Network == nil || len(vm.Spec.Network.Interfaces) == 0 {
// Continue to support this ad-hoc v1a1 annotation. I don't think need or want to have
// this annotation in v1a2: Disabled mostly already covers it. We could map between the
// two for version conversion, but they do mean slightly different things, and kind of
// complicated to know what to do like if the annotation is removed.
if _, ok := vm.Annotations[v1alpha1.NoDefaultNicAnnotation]; ok {
return false
}

var updated bool
if len(vm.Spec.Network.Interfaces) == 0 {
if vm.Spec.Network != nil && vm.Spec.Network.Disabled {
return false
}

if vm.Spec.Network == nil {
vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{}
}

// Add the default interface.
vm.Spec.Network.Interfaces = []vmopv1.VirtualMachineNetworkInterfaceSpec{
{
Name: defaultInterfaceName,
Network: &networkRef,
},
}
updated = true
} else {

return true
}

var updated bool
if vm.Spec.Network != nil {
for i := range vm.Spec.Network.Interfaces {
if ok := setDefaultNetworkInterfaceNetwork(vm, i, networkRef); ok {
updated = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,42 @@ func unitTestsMutating() {
})
})

When("Disabled is true", func() {
BeforeEach(func() {
pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) {
config.NetworkProviderType = pkgcfg.NetworkProviderTypeVDS
})
})

BeforeEach(func() {
ctx.vm.Spec.Network.Disabled = true
})

It("Should not add default network interface", func() {
Expect(mutation.AddDefaultNetworkInterface(&ctx.WebhookRequestContext, ctx.Client, ctx.vm)).To(BeFalse())
Expect(ctx.vm.Spec.Network.Interfaces).To(BeEmpty())
})

Context("Interfaces is not empty", func() {
BeforeEach(func() {
ctx.vm.Spec.Network.Interfaces = []vmopv1.VirtualMachineNetworkInterfaceSpec{
{
Name: "eth0",
},
}
})

It("back fills Network ref", func() {
Expect(mutation.AddDefaultNetworkInterface(&ctx.WebhookRequestContext, ctx.Client, ctx.vm)).To(BeTrue())
Expect(ctx.vm.Spec.Network.Interfaces).Should(HaveLen(1))
Expect(ctx.vm.Spec.Network.Interfaces[0].Name).Should(Equal("eth0"))
Expect(ctx.vm.Spec.Network.Interfaces[0].Network).ShouldNot(BeNil())
Expect(ctx.vm.Spec.Network.Interfaces[0].Network.Kind).Should(Equal("Network"))
Expect(ctx.vm.Spec.Network.Interfaces[0].Network.APIVersion).Should(Equal("netoperator.vmware.com/v1alpha1"))
})
})
})

When("NoNetwork annotation is set", func() {
BeforeEach(func() {
pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) {
Expand Down
76 changes: 72 additions & 4 deletions webhooks/virtualmachine/validation/virtualmachine_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

netopv1alpha1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1"
vpcv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"

vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha6"
vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha6/common"
"github.com/vmware-tanzu/vm-operator/api/v1alpha6/sysprep"
ncpv1alpha1 "github.com/vmware-tanzu/vm-operator/external/ncp/api/v1alpha1"
"github.com/vmware-tanzu/vm-operator/pkg/builder"
pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants"
Expand Down Expand Up @@ -1050,21 +1052,85 @@ var macAddressSupportNetworkGroups = []string{
vpcv1alpha1.GroupVersion.Group,
}

type networkProviderValidation struct {
group string
apiVersion string
kinds []string
}

var networkProviderValidations = map[pkgcfg.NetworkProviderType]networkProviderValidation{
pkgcfg.NetworkProviderTypeVDS: {
group: netopv1alpha1.SchemeGroupVersion.Group,
apiVersion: netopv1alpha1.SchemeGroupVersion.String(),
kinds: []string{"Network"},
},
pkgcfg.NetworkProviderTypeNSXT: {
group: ncpv1alpha1.SchemeGroupVersion.Group,
apiVersion: ncpv1alpha1.SchemeGroupVersion.String(),
kinds: []string{"VirtualNetwork"},
},
pkgcfg.NetworkProviderTypeVPC: {
group: vpcv1alpha1.GroupVersion.Group,
apiVersion: vpcv1alpha1.SchemeGroupVersion.String(),
kinds: []string{"Subnet", "SubnetSet"},
},
}

func (v validator) validateNetworkInterfaceNetworkRef(
ctx *pkgctx.WebhookRequestContext,
interfacePath *field.Path,
networkGV schema.GroupVersion,
networkKind string) field.ErrorList {

var allErrs field.ErrorList

networkProvider := pkgcfg.FromContext(ctx).NetworkProviderType
supported, ok := networkProviderValidations[networkProvider]
if !ok {
// No supported for this provider type (e.g., Named network provider).
return allErrs
}

if networkGV.Group != "" && networkGV.Group != supported.group {
// We don't care about the version for anything but show the user provided
// APIVersion since that is the field the group comes from. For supported,
// just show the version we're importing which is OK'ish since none of the
// providers have moved past v1a1.
allErrs = append(allErrs, field.NotSupported(
interfacePath.Child("network", "apiVersion"),
networkGV.String(),
[]string{supported.apiVersion}))
}

if networkKind != "" && !slices.Contains(supported.kinds, networkKind) {
allErrs = append(allErrs, field.NotSupported(
interfacePath.Child("network", "kind"),
networkKind,
supported.kinds))
}

return allErrs
}

//nolint:gocyclo
func (v validator) validateNetworkInterfaceSpec(
ctx *pkgctx.WebhookRequestContext,
interfacePath *field.Path,
interfaceSpec vmopv1.VirtualMachineNetworkInterfaceSpec,
vmName string) field.ErrorList {

var allErrs field.ErrorList
var networkIfCRName string
var networkAPIVersion string
var networkName string
var (
allErrs field.ErrorList
networkIfCRName string
networkAPIVersion string
networkName string
networkKind string
)

if interfaceSpec.Network != nil {
networkAPIVersion = interfaceSpec.Network.APIVersion
networkName = interfaceSpec.Network.Name
networkKind = interfaceSpec.Network.Kind
}

var networkGV schema.GroupVersion
Expand All @@ -1077,6 +1143,8 @@ func (v validator) validateNetworkInterfaceSpec(
}
}

allErrs = append(allErrs, v.validateNetworkInterfaceNetworkRef(ctx, interfacePath, networkGV, networkKind)...)

// The networkInterface CR name ("vmName-networkName-interfaceName" or "vmName-interfaceName") needs to be a DNS1123 Label
if networkName != "" {
networkIfCRName = fmt.Sprintf("%s-%s-%s", vmName, networkName, interfaceSpec.Name)
Expand Down
Loading
Loading