diff --git a/pkg/conversion/capi2mapi/openstack.go b/pkg/conversion/capi2mapi/openstack.go index 92052922c..4de2b7259 100644 --- a/pkg/conversion/capi2mapi/openstack.go +++ b/pkg/conversion/capi2mapi/openstack.go @@ -354,33 +354,36 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1. mapoNetwork := mapiv1alpha1.NetworkParam{} - // We have already asserted that .Network is non-nil in the caller - - switch { - case capoPort.Network.ID != nil: - mapoNetwork.UUID = *capoPort.Network.ID - case capoPort.Network.Filter != nil: - mapoNetwork.Filter = mapiv1alpha1.Filter{ - // DeprecatedAdminStateUp is deprecated and ignored by MAPO so we don't set it - // DeprecatedLimit is deprecated and ignored by MAPO so we don't set it - // DeprecatedMarker is deprecated and ignored by MAPO so we don't set it - // DeprecatedShared is deprecated and ignored by MAPO so we don't set it - // DeprecatedSortKey is deprecated and ignored by MAPO so we don't set it - // DeprecatedSortDir is deprecated and ignored by MAPO so we don't set it - // DeprecatedStatus is deprecated and ignored by MAPO so we don't set it - Description: capoPort.Network.Filter.Description, - // ID is deprecated and covered by UUID on the parent NetworkParam so we don't set it - Name: capoPort.Network.Filter.Name, - NotTags: joinTags(capoPort.Network.Filter.NotTags), - NotTagsAny: joinTags(capoPort.Network.Filter.NotTagsAny), - ProjectID: capoPort.Network.Filter.ProjectID, - Tags: joinTags(capoPort.Network.Filter.Tags), - TagsAny: joinTags(capoPort.Network.Filter.TagsAny), - // TenantID is deprecated and covered by ProjectID so we don't set it + if capoPort.Network != nil { + switch { + case capoPort.Network.ID != nil: + mapoNetwork.UUID = *capoPort.Network.ID + case capoPort.Network.Filter != nil: + mapoNetwork.Filter = mapiv1alpha1.Filter{ + // DeprecatedAdminStateUp is deprecated and ignored by MAPO so we don't set it + // DeprecatedLimit is deprecated and ignored by MAPO so we don't set it + // DeprecatedMarker is deprecated and ignored by MAPO so we don't set it + // DeprecatedShared is deprecated and ignored by MAPO so we don't set it + // DeprecatedSortKey is deprecated and ignored by MAPO so we don't set it + // DeprecatedSortDir is deprecated and ignored by MAPO so we don't set it + // DeprecatedStatus is deprecated and ignored by MAPO so we don't set it + Description: capoPort.Network.Filter.Description, + // ID is deprecated and covered by UUID on the parent NetworkParam so we don't set it + Name: capoPort.Network.Filter.Name, + NotTags: joinTags(capoPort.Network.Filter.NotTags), + NotTagsAny: joinTags(capoPort.Network.Filter.NotTagsAny), + ProjectID: capoPort.Network.Filter.ProjectID, + Tags: joinTags(capoPort.Network.Filter.Tags), + TagsAny: joinTags(capoPort.Network.Filter.TagsAny), + // TenantID is deprecated and covered by ProjectID so we don't set it + } + default: + // TODO(OSASINFRA-3779): Add VAP to prevent usage of the below field. + errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "A network must be referenced by a UUID or filter")) } - default: + } else if len(capoPort.FixedIPs) == 0 { // TODO(OSASINFRA-3779): Add VAP to prevent usage of the below field. - errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "A network must be referenced by a UUID or filter")) + errors = append(errors, field.Invalid(fldPath.Child("network"), capoPort.Network, "Either a network or fixedIPs must be specified on a port")) } if capoPort.DisablePortSecurity != nil { @@ -397,8 +400,16 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1. } mapoSubnet := mapiv1alpha1.SubnetParam{ - UUID: *capoFixedIP.Subnet.ID, - Filter: mapiv1alpha1.SubnetFilter{ + PortTags: capoPort.Tags, + // PortSecurity is deprecated and ignored by MAPO so we don't set it here + } + + if capoFixedIP.Subnet.ID != nil { + mapoSubnet.UUID = *capoFixedIP.Subnet.ID + } + + if capoFixedIP.Subnet.Filter != nil { + mapoSubnet.Filter = mapiv1alpha1.SubnetFilter{ CIDR: capoFixedIP.Subnet.Filter.CIDR, Description: capoFixedIP.Subnet.Filter.Description, GatewayIP: capoFixedIP.Subnet.Filter.GatewayIP, @@ -412,10 +423,9 @@ func convertCAPOPortOptsToMAPONetwork(fldPath *field.Path, capoPort openstackv1. ProjectID: capoFixedIP.Subnet.Filter.ProjectID, Tags: joinTags(capoFixedIP.Subnet.Filter.Tags), TagsAny: joinTags(capoFixedIP.Subnet.Filter.TagsAny), - }, - PortTags: capoPort.Tags, - // PortSecurity is deprecated and ignored by MAPO so we don't set it here + } } + mapoSubnets[i] = mapoSubnet } @@ -593,7 +603,7 @@ func convertCAPOPortOptsToMAPO(fldPath *field.Path, capoPorts []openstackv1.Port warnings := []string{} for i, capoPort := range capoPorts { - if capoPort.Network != nil && capoPort.Network.Filter != nil { + if (capoPort.Network != nil && capoPort.Network.Filter != nil) || (capoPort.Network == nil && len(capoPort.FixedIPs) > 0 && capoPort.FixedIPs[0].Subnet != nil) { mapoNetwork, warns, errs := convertCAPOPortOptsToMAPONetwork(fldPath.Index(i), capoPort) mapoNetworks = append(mapoNetworks, mapoNetwork) errors = append(errors, errs...) diff --git a/pkg/conversion/mapi2capi/openstack.go b/pkg/conversion/mapi2capi/openstack.go index b01ca2c27..30641df0b 100644 --- a/pkg/conversion/mapi2capi/openstack.go +++ b/pkg/conversion/mapi2capi/openstack.go @@ -371,7 +371,7 @@ func convertMAPOImageToCAPO(mapoImage string) openstackv1.ImageParam { } //nolint:funlen -func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.NetworkParam) ([]openstackv1.PortOpts, []string, field.ErrorList) { //nolint:gocognit,cyclop +func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1.NetworkParam) ([]openstackv1.PortOpts, []string, field.ErrorList) { //nolint:gocognit,cyclop,gocyclo errors := field.ErrorList{} warnings := []string{} @@ -385,7 +385,7 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. warnings = append(warnings, field.Invalid(fldPath.Index(i).Child("fixedIP"), mapoNetwork.FixedIp, "fixedIp is ignored by MAPO, ignoring").Error()) } - network := openstackv1.NetworkParam{} + capoNetwork := openstackv1.NetworkParam{} networkID := mapoNetwork.UUID if networkID == "" { @@ -393,35 +393,38 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. } if networkID != "" { - network.ID = &networkID + capoNetwork.ID = &networkID } // convert .Filter - projectID := mapoNetwork.Filter.ProjectID - if projectID == "" { - projectID = mapoNetwork.Filter.TenantID - } + if (capoNetwork.ID == nil && mapoNetwork.Filter != mapiv1alpha1.Filter{}) { + projectID := mapoNetwork.Filter.ProjectID + if projectID == "" { + projectID = mapoNetwork.Filter.TenantID + } - network.Filter = &openstackv1.NetworkFilter{ - Name: mapoNetwork.Filter.Name, - Description: mapoNetwork.Filter.Description, - ProjectID: projectID, - FilterByNeutronTags: openstackv1.FilterByNeutronTags{ - NotTags: splitTags(mapoNetwork.Filter.NotTags), - NotTagsAny: splitTags(mapoNetwork.Filter.NotTagsAny), - Tags: splitTags(mapoNetwork.Filter.Tags), - TagsAny: splitTags(mapoNetwork.Filter.TagsAny), - }, + capoNetwork.Filter = &openstackv1.NetworkFilter{ + Name: mapoNetwork.Filter.Name, + Description: mapoNetwork.Filter.Description, + ProjectID: projectID, + FilterByNeutronTags: openstackv1.FilterByNeutronTags{ + NotTags: splitTags(mapoNetwork.Filter.NotTags), + NotTagsAny: splitTags(mapoNetwork.Filter.NotTagsAny), + Tags: splitTags(mapoNetwork.Filter.Tags), + TagsAny: splitTags(mapoNetwork.Filter.TagsAny), + }, + } } tags := mapoNetwork.PortTags // convert .Subnets - if networkID == "" && (mapoNetwork.Filter == mapiv1alpha1.Filter{}) { //nolint:nestif + if (capoNetwork == openstackv1.NetworkParam{}) { //nolint:nestif // Case: network is undefined and only has subnets // Create a port for each subnet for j, mapoSubnet := range mapoNetwork.Subnets { portTags := append(tags, mapoSubnet.PortTags...) //nolint:gocritic + capoPort := openstackv1.PortOpts{Tags: portTags} subnetID := mapoSubnet.UUID if subnetID == "" { @@ -438,33 +441,35 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. 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()) } - capoPort := openstackv1.PortOpts{ - Network: &network, - FixedIPs: []openstackv1.FixedIP{ - { - Subnet: &openstackv1.SubnetParam{ - ID: &subnetID, - Filter: &openstackv1.SubnetFilter{ - CIDR: mapoSubnet.Filter.CIDR, - Description: mapoSubnet.Filter.Description, - GatewayIP: mapoSubnet.Filter.GatewayIP, - IPVersion: mapoSubnet.Filter.IPVersion, - IPv6AddressMode: mapoSubnet.Filter.IPv6AddressMode, - IPv6RAMode: mapoSubnet.Filter.IPv6RAMode, - Name: mapoSubnet.Filter.Name, - // We ignore NetworkID since it's silently ignored by MAPO itself - ProjectID: projectID, - FilterByNeutronTags: openstackv1.FilterByNeutronTags{ - NotTags: splitTags(mapoSubnet.Filter.NotTags), - NotTagsAny: splitTags(mapoSubnet.Filter.NotTagsAny), - Tags: splitTags(mapoSubnet.Filter.Tags), - TagsAny: splitTags(mapoSubnet.Filter.TagsAny), - }, - }, - }, + capoSubnet := openstackv1.SubnetParam{} + if subnetID != "" { + capoSubnet.ID = &subnetID + } + + if (mapoSubnet.Filter != mapiv1alpha1.SubnetFilter{}) { + capoSubnet.Filter = &openstackv1.SubnetFilter{ + CIDR: mapoSubnet.Filter.CIDR, + Description: mapoSubnet.Filter.Description, + GatewayIP: mapoSubnet.Filter.GatewayIP, + IPVersion: mapoSubnet.Filter.IPVersion, + IPv6AddressMode: mapoSubnet.Filter.IPv6AddressMode, + IPv6RAMode: mapoSubnet.Filter.IPv6RAMode, + Name: mapoSubnet.Filter.Name, + // We ignore NetworkID since it's silently ignored by MAPO itself + ProjectID: projectID, + FilterByNeutronTags: openstackv1.FilterByNeutronTags{ + NotTags: splitTags(mapoSubnet.Filter.NotTags), + NotTagsAny: splitTags(mapoSubnet.Filter.NotTagsAny), + Tags: splitTags(mapoSubnet.Filter.Tags), + TagsAny: splitTags(mapoSubnet.Filter.TagsAny), }, - }, - Tags: portTags, + } + } + + if (capoSubnet != openstackv1.SubnetParam{}) { + capoPort.FixedIPs = []openstackv1.FixedIP{ + {Subnet: &capoSubnet}, + } } if mapoSubnet.PortSecurity != nil && !*mapoSubnet.PortSecurity { @@ -478,7 +483,7 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. } else { // Case: network and subnet are defined // Create a single port with an interface for each subnet - fixedIPs := make([]openstackv1.FixedIP, len(mapoNetwork.Subnets)) + capoFixedIPs := make([]openstackv1.FixedIP, len(mapoNetwork.Subnets)) for j, mapoSubnet := range mapoNetwork.Subnets { subnetID := mapoSubnet.UUID @@ -496,7 +501,7 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. warnings = append(warnings, field.Invalid(fldPath.Index(j).Child("subnets").Index(j).Child("filter", "networkId"), mapoSubnet.Filter.NetworkID, "networkId is ignored by MAPO, ignoring").Error()) } - fixedIPs[j] = openstackv1.FixedIP{ + capoFixedIPs[j] = openstackv1.FixedIP{ Subnet: &openstackv1.SubnetParam{ ID: &subnetID, Filter: &openstackv1.SubnetFilter{ @@ -523,8 +528,8 @@ func convertMAPONetworksToCAPO(fldPath *field.Path, mapoNetworks []mapiv1alpha1. } capoPort := openstackv1.PortOpts{ - FixedIPs: fixedIPs, - Network: &network, + FixedIPs: capoFixedIPs, + Network: &capoNetwork, Tags: tags, } diff --git a/pkg/conversion/mapi2capi/openstack_fuzz_test.go b/pkg/conversion/mapi2capi/openstack_fuzz_test.go index 1adc84262..7ce52ee47 100644 --- a/pkg/conversion/mapi2capi/openstack_fuzz_test.go +++ b/pkg/conversion/mapi2capi/openstack_fuzz_test.go @@ -126,8 +126,13 @@ func openstackProviderSpecFuzzerFuncs(codecs runtimeserializer.CodecFactory) []a func(network *mapiv1alpha1.NetworkParam, c randfill.Continue) { switch c.Int31n(2) { case 0: - network.UUID = uuid.NewString() + network.UUID = "" network.Filter = mapiv1alpha1.Filter{} + network.Subnets = []mapiv1alpha1.SubnetParam{ + { + UUID: uuid.NewString(), + }, + } case 1: network.UUID = "" c.Fill(&network.Filter)