Skip to content

Commit 6004d19

Browse files
stephenfinstephenfin
committed
OCPBUGS-62892: openstack: Handle empty MAPO network subnets
Signed-off-by: Stephen Finucane <[email protected]>
1 parent 06a1f5a commit 6004d19

File tree

3 files changed

+91
-74
lines changed

3 files changed

+91
-74
lines changed

‎pkg/conversion/capi2mapi/openstack.go‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -354,33 +354,36 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1.
354354

355355
mapoNetwork := mapiv1alpha1.NetworkParam{}
356356

357-
// We have already asserted that .Network is non-nil in the caller
358-
359-
switch {
360-
case capoPort.Network.ID != nil:
361-
mapoNetwork.UUID = *capoPort.Network.ID
362-
case capoPort.Network.Filter != nil:
363-
mapoNetwork.Filter = mapiv1alpha1.Filter{
364-
// DeprecatedAdminStateUp is deprecated and ignored by MAPO so we don't set it
365-
// DeprecatedLimit is deprecated and ignored by MAPO so we don't set it
366-
// DeprecatedMarker is deprecated and ignored by MAPO so we don't set it
367-
// DeprecatedShared is deprecated and ignored by MAPO so we don't set it
368-
// DeprecatedSortKey is deprecated and ignored by MAPO so we don't set it
369-
// DeprecatedSortDir is deprecated and ignored by MAPO so we don't set it
370-
// DeprecatedStatus is deprecated and ignored by MAPO so we don't set it
371-
Description: capoPort.Network.Filter.Description,
372-
// ID is deprecated and covered by UUID on the parent NetworkParam so we don't set it
373-
Name: capoPort.Network.Filter.Name,
374-
NotTags: joinTags(capoPort.Network.Filter.NotTags),
375-
NotTagsAny: joinTags(capoPort.Network.Filter.NotTagsAny),
376-
ProjectID: capoPort.Network.Filter.ProjectID,
377-
Tags: joinTags(capoPort.Network.Filter.Tags),
378-
TagsAny: joinTags(capoPort.Network.Filter.TagsAny),
379-
// TenantID is deprecated and covered by ProjectID so we don't set it
357+
if capoPort.Network != nil {
358+
switch {
359+
case capoPort.Network != nil && capoPort.Network.ID != nil:
360+
mapoNetwork.UUID = *capoPort.Network.ID
361+
case capoPort.Network != nil && capoPort.Network.Filter != nil:
362+
mapoNetwork.Filter = mapiv1alpha1.Filter{
363+
// DeprecatedAdminStateUp is deprecated and ignored by MAPO so we don't set it
364+
// DeprecatedLimit is deprecated and ignored by MAPO so we don't set it
365+
// DeprecatedMarker is deprecated and ignored by MAPO so we don't set it
366+
// DeprecatedShared is deprecated and ignored by MAPO so we don't set it
367+
// DeprecatedSortKey is deprecated and ignored by MAPO so we don't set it
368+
// DeprecatedSortDir is deprecated and ignored by MAPO so we don't set it
369+
// DeprecatedStatus is deprecated and ignored by MAPO so we don't set it
370+
Description: capoPort.Network.Filter.Description,
371+
// ID is deprecated and covered by UUID on the parent NetworkParam so we don't set it
372+
Name: capoPort.Network.Filter.Name,
373+
NotTags: joinTags(capoPort.Network.Filter.NotTags),
374+
NotTagsAny: joinTags(capoPort.Network.Filter.NotTagsAny),
375+
ProjectID: capoPort.Network.Filter.ProjectID,
376+
Tags: joinTags(capoPort.Network.Filter.Tags),
377+
TagsAny: joinTags(capoPort.Network.Filter.TagsAny),
378+
// TenantID is deprecated and covered by ProjectID so we don't set it
379+
}
380+
default:
381+
// TODO(OSASINFRA-3779): Add VAP to prevent usage of the below field.
382+
errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "A network must be referenced by a UUID or filter"))
380383
}
381-
default:
384+
} else if len(capoPort.FixedIPs) == 0 {
382385
// TODO(OSASINFRA-3779): Add VAP to prevent usage of the below field.
383-
errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "A network must be referenced by a UUID or filter"))
386+
errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "A network must be referenced by a UUID or filter, else a subnet must be referenced"))
384387
}
385388

386389
if capoPort.DisablePortSecurity != nil {
@@ -397,8 +400,13 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1.
397400
}
398401

399402
mapoSubnet := mapiv1alpha1.SubnetParam{
400-
UUID: *capoFixedIP.Subnet.ID,
401-
Filter: mapiv1alpha1.SubnetFilter{
403+
UUID: *capoFixedIP.Subnet.ID,
404+
PortTags: capoPort.Tags,
405+
// PortSecurity is deprecated and ignored by MAPO so we don't set it here
406+
}
407+
408+
if capoFixedIP.Subnet.Filter != nil {
409+
mapoSubnet.Filter = mapiv1alpha1.SubnetFilter{
402410
CIDR: capoFixedIP.Subnet.Filter.CIDR,
403411
Description: capoFixedIP.Subnet.Filter.Description,
404412
GatewayIP: capoFixedIP.Subnet.Filter.GatewayIP,
@@ -412,10 +420,9 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1.
412420
ProjectID: capoFixedIP.Subnet.Filter.ProjectID,
413421
Tags: joinTags(capoFixedIP.Subnet.Filter.Tags),
414422
TagsAny: joinTags(capoFixedIP.Subnet.Filter.TagsAny),
415-
},
416-
PortTags: capoPort.Tags,
417-
// PortSecurity is deprecated and ignored by MAPO so we don't set it here
423+
}
418424
}
425+
419426
mapoSubnets[i] = mapoSubnet
420427
}
421428

@@ -593,7 +600,7 @@ func convertCAPOPortOptsToMAPO(fldPath *field.Path, capoPorts []openstackv1.Port
593600
warnings := []string{}
594601

595602
for i, capoPort := range capoPorts {
596-
if capoPort.Network != nil && capoPort.Network.Filter != nil {
603+
if (capoPort.Network != nil && capoPort.Network.Filter != nil) || (capoPort.Network == nil && len(capoPort.FixedIPs) > 0 && capoPort.FixedIPs[0].Subnet != nil) {
597604
mapoNetwork, warns, errs := convertCAPOPortOptsToMAPONetwork(fldPath.Index(i), capoPort)
598605
mapoNetworks = append(mapoNetworks, mapoNetwork)
599606
errors = append(errors, errs...)

‎pkg/conversion/mapi2capi/openstack.go‎

Lines changed: 47 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func convertMAPOImageToCAPO(mapoImage string) openstackv1.ImageParam {
371371
}
372372

373373
//nolint:funlen
374-
func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.NetworkParam) ([]openstackv1.PortOpts, []string, field.ErrorList) { //nolint:gocognit,cyclop
374+
func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.NetworkParam) ([]openstackv1.PortOpts, []string, field.ErrorList) { //nolint:gocognit,cyclop,gocyclo
375375
errors := field.ErrorList{}
376376
warnings := []string{}
377377

@@ -397,31 +397,34 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.
397397
}
398398

399399
// convert .Filter
400-
projectID := mapoNetwork.Filter.ProjectID
401-
if projectID == "" {
402-
projectID = mapoNetwork.Filter.TenantID
403-
}
400+
if (mapoNetwork.Filter != mapiv1alpha1.Filter{}) {
401+
projectID := mapoNetwork.Filter.ProjectID
402+
if projectID == "" {
403+
projectID = mapoNetwork.Filter.TenantID
404+
}
404405

405-
capoNetwork.Filter = &openstackv1.NetworkFilter{
406-
Name: mapoNetwork.Filter.Name,
407-
Description: mapoNetwork.Filter.Description,
408-
ProjectID: projectID,
409-
FilterByNeutronTags: openstackv1.FilterByNeutronTags{
410-
NotTags: splitTags(mapoNetwork.Filter.NotTags),
411-
NotTagsAny: splitTags(mapoNetwork.Filter.NotTagsAny),
412-
Tags: splitTags(mapoNetwork.Filter.Tags),
413-
TagsAny: splitTags(mapoNetwork.Filter.TagsAny),
414-
},
406+
capoNetwork.Filter = &openstackv1.NetworkFilter{
407+
Name: mapoNetwork.Filter.Name,
408+
Description: mapoNetwork.Filter.Description,
409+
ProjectID: projectID,
410+
FilterByNeutronTags: openstackv1.FilterByNeutronTags{
411+
NotTags: splitTags(mapoNetwork.Filter.NotTags),
412+
NotTagsAny: splitTags(mapoNetwork.Filter.NotTagsAny),
413+
Tags: splitTags(mapoNetwork.Filter.Tags),
414+
TagsAny: splitTags(mapoNetwork.Filter.TagsAny),
415+
},
416+
}
415417
}
416418

417419
tags := mapoNetwork.PortTags
418420

419421
// convert .Subnets
420-
if networkID == "" && (mapoNetwork.Filter == mapiv1alpha1.Filter{}) { //nolint:nestif
422+
if (capoNetwork == openstackv1.NetworkParam{}) { //nolint:nestif
421423
// Case: network is undefined and only has subnets
422424
// Create a port for each subnet
423425
for j, mapoSubnet := range mapoNetwork.Subnets {
424426
portTags := append(tags, mapoSubnet.PortTags...) //nolint:gocritic
427+
capoPort := openstackv1.PortOpts{Tags: portTags}
425428

426429
subnetID := mapoSubnet.UUID
427430
if subnetID == "" {
@@ -438,33 +441,35 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.
438441
warnings = append(warnings, field.Invalid(fldPath.Index(i).Child("subnets").Index(j).Child("filter", "networkId"), mapoSubnet.Filter.NetworkID, "networkId is ignored by MAPO, ignoring").Error())
439442
}
440443

441-
capoPort := openstackv1.PortOpts{
442-
Network: &capoNetwork,
443-
FixedIPs: []openstackv1.FixedIP{
444-
{
445-
Subnet: &openstackv1.SubnetParam{
446-
ID: &subnetID,
447-
Filter: &openstackv1.SubnetFilter{
448-
CIDR: mapoSubnet.Filter.CIDR,
449-
Description: mapoSubnet.Filter.Description,
450-
GatewayIP: mapoSubnet.Filter.GatewayIP,
451-
IPVersion: mapoSubnet.Filter.IPVersion,
452-
IPv6AddressMode: mapoSubnet.Filter.IPv6AddressMode,
453-
IPv6RAMode: mapoSubnet.Filter.IPv6RAMode,
454-
Name: mapoSubnet.Filter.Name,
455-
// We ignore NetworkID since it's silently ignored by MAPO itself
456-
ProjectID: projectID,
457-
FilterByNeutronTags: openstackv1.FilterByNeutronTags{
458-
NotTags: splitTags(mapoSubnet.Filter.NotTags),
459-
NotTagsAny: splitTags(mapoSubnet.Filter.NotTagsAny),
460-
Tags: splitTags(mapoSubnet.Filter.Tags),
461-
TagsAny: splitTags(mapoSubnet.Filter.TagsAny),
462-
},
463-
},
464-
},
444+
capoSubnet := openstackv1.SubnetParam{}
445+
if subnetID != "" {
446+
capoSubnet.ID = &subnetID
447+
}
448+
449+
if (mapoSubnet.Filter != mapiv1alpha1.SubnetFilter{}) {
450+
capoSubnet.Filter = &openstackv1.SubnetFilter{
451+
CIDR: mapoSubnet.Filter.CIDR,
452+
Description: mapoSubnet.Filter.Description,
453+
GatewayIP: mapoSubnet.Filter.GatewayIP,
454+
IPVersion: mapoSubnet.Filter.IPVersion,
455+
IPv6AddressMode: mapoSubnet.Filter.IPv6AddressMode,
456+
IPv6RAMode: mapoSubnet.Filter.IPv6RAMode,
457+
Name: mapoSubnet.Filter.Name,
458+
// We ignore NetworkID since it's silently ignored by MAPO itself
459+
ProjectID: projectID,
460+
FilterByNeutronTags: openstackv1.FilterByNeutronTags{
461+
NotTags: splitTags(mapoSubnet.Filter.NotTags),
462+
NotTagsAny: splitTags(mapoSubnet.Filter.NotTagsAny),
463+
Tags: splitTags(mapoSubnet.Filter.Tags),
464+
TagsAny: splitTags(mapoSubnet.Filter.TagsAny),
465465
},
466-
},
467-
Tags: portTags,
466+
}
467+
}
468+
469+
capoFixedIP := openstackv1.FixedIP{}
470+
if (capoSubnet != openstackv1.SubnetParam{}) {
471+
capoFixedIP.Subnet = &capoSubnet
472+
capoPort.FixedIPs = []openstackv1.FixedIP{capoFixedIP}
468473
}
469474

470475
if mapoSubnet.PortSecurity != nil && !*mapoSubnet.PortSecurity {

‎pkg/conversion/mapi2capi/openstack_fuzz_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,13 @@ func openstackProviderSpecFuzzerFuncs(codecs runtimeserializer.CodecFactory) []a
126126
func(network *mapiv1alpha1.NetworkParam, c randfill.Continue) {
127127
switch c.Int31n(2) {
128128
case 0:
129-
network.UUID = uuid.NewString()
129+
network.UUID = ""
130130
network.Filter = mapiv1alpha1.Filter{}
131+
network.Subnets = []mapiv1alpha1.SubnetParam{
132+
{
133+
UUID: uuid.NewString(),
134+
},
135+
}
131136
case 1:
132137
network.UUID = ""
133138
c.Fill(&network.Filter)

0 commit comments

Comments
 (0)