Skip to content

Commit 3951890

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3189 from nikParasyr/v1beta2_positive
⚠️ V1beta2 Switch to positive polarity for bool fields
2 parents 1cd81a1 + b5670f9 commit 3951890

42 files changed

Lines changed: 632 additions & 349 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎api/v1beta1/conversion.go‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,15 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1beta2_OpenStackClusterSpec(
378378

379379
if in.NetworkMTU != nil || in.DisablePortSecurity != nil {
380380
out.ManagedNetwork = &infrav1.ManagedNetwork{
381-
MTU: in.NetworkMTU,
382-
DisablePortSecurity: in.DisablePortSecurity,
381+
MTU: in.NetworkMTU,
383382
}
383+
if in.DisablePortSecurity != nil {
384+
out.ManagedNetwork.EnablePortSecurity = ptr.To(!*in.DisablePortSecurity)
385+
}
386+
}
387+
388+
if in.DisableExternalNetwork != nil {
389+
out.EnableExternalNetwork = ptr.To(!*in.DisableExternalNetwork)
384390
}
385391

386392
// ExternalRouterIPs and ExternalRouterIPParam are structurally identical between versions,
@@ -406,10 +412,12 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1beta2_OpenStackClusterSpec(
406412
in.APIServerFixedIP != nil ||
407413
in.APIServerPort != nil {
408414
out.APIServer = &infrav1.APIServer{
409-
DisableFloatingIP: in.DisableAPIServerFloatingIP,
410-
FloatingIP: in.APIServerFloatingIP,
411-
FixedIP: in.APIServerFixedIP,
412-
Port: in.APIServerPort,
415+
FloatingIP: in.APIServerFloatingIP,
416+
FixedIP: in.APIServerFixedIP,
417+
Port: in.APIServerPort,
418+
}
419+
if in.DisableAPIServerFloatingIP != nil {
420+
out.APIServer.EnableFloatingIP = ptr.To(!*in.DisableAPIServerFloatingIP)
413421
}
414422
// APIServerLoadBalancer is structurally identical between versions,
415423
// so an unsafe cast is safe here (same field layout).
@@ -432,7 +440,13 @@ func Convert_v1beta2_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(
432440

433441
if in.ManagedNetwork != nil {
434442
out.NetworkMTU = in.ManagedNetwork.MTU
435-
out.DisablePortSecurity = in.ManagedNetwork.DisablePortSecurity
443+
if in.ManagedNetwork.EnablePortSecurity != nil {
444+
out.DisablePortSecurity = ptr.To(!*in.ManagedNetwork.EnablePortSecurity)
445+
}
446+
}
447+
448+
if in.EnableExternalNetwork != nil {
449+
out.DisableExternalNetwork = ptr.To(!*in.EnableExternalNetwork)
436450
}
437451

438452
// ExternalRouterIPs and ExternalRouterIPParam are structurally identical between versions,
@@ -450,10 +464,12 @@ func Convert_v1beta2_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(
450464

451465
// Expand the v1beta2 APIServer struct back into the flat v1beta1 fields.
452466
if in.APIServer != nil {
453-
out.DisableAPIServerFloatingIP = in.APIServer.DisableFloatingIP
454467
out.APIServerFloatingIP = in.APIServer.FloatingIP
455468
out.APIServerFixedIP = in.APIServer.FixedIP
456469
out.APIServerPort = in.APIServer.Port
470+
if in.APIServer.EnableFloatingIP != nil {
471+
out.DisableAPIServerFloatingIP = ptr.To(!*in.APIServer.EnableFloatingIP)
472+
}
457473

458474
if in.APIServer.ManagedLoadBalancer != nil {
459475
out.APIServerLoadBalancer = (*APIServerLoadBalancer)(unsafe.Pointer(in.APIServer.ManagedLoadBalancer))
@@ -572,3 +588,27 @@ func ConvertAllTagsFrom(neutronTags *FilterByNeutronTags, tags, tagsAny, notTags
572588
*notTags = JoinTags(neutronTags.NotTags)
573589
*notTagsAny = JoinTags(neutronTags.NotTagsAny)
574590
}
591+
592+
func Convert_v1beta1_ResolvedPortSpecFields_To_v1beta2_ResolvedPortSpecFields(in *ResolvedPortSpecFields, out *infrav1.ResolvedPortSpecFields, s apiconversion.Scope) error {
593+
if err := autoConvert_v1beta1_ResolvedPortSpecFields_To_v1beta2_ResolvedPortSpecFields(in, out, s); err != nil {
594+
return err
595+
}
596+
597+
if in.DisablePortSecurity != nil {
598+
out.EnablePortSecurity = ptr.To(!*in.DisablePortSecurity)
599+
}
600+
601+
return nil
602+
}
603+
604+
func Convert_v1beta2_ResolvedPortSpecFields_To_v1beta1_ResolvedPortSpecFields(in *infrav1.ResolvedPortSpecFields, out *ResolvedPortSpecFields, s apiconversion.Scope) error {
605+
if err := autoConvert_v1beta2_ResolvedPortSpecFields_To_v1beta1_ResolvedPortSpecFields(in, out, s); err != nil {
606+
return err
607+
}
608+
609+
if in.EnablePortSecurity != nil {
610+
out.DisablePortSecurity = ptr.To(!*in.EnablePortSecurity)
611+
}
612+
613+
return nil
614+
}

‎api/v1beta1/conversion_test.go‎

Lines changed: 142 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -977,8 +977,9 @@ func TestOpenStackCluster_RoundTrip_ManagedNetwork(t *testing.T) {
977977
disablePS := optional.Bool(ptr.To(true))
978978

979979
tests := []struct {
980-
name string
981-
in OpenStackCluster
980+
name string
981+
in OpenStackCluster
982+
expectedEnablePS optional.Bool
982983
}{
983984
{
984985
name: "both fields set",
@@ -988,22 +989,33 @@ func TestOpenStackCluster_RoundTrip_ManagedNetwork(t *testing.T) {
988989
DisablePortSecurity: disablePS,
989990
},
990991
},
992+
expectedEnablePS: optional.Bool(ptr.To(false)), // disablePS=true → enablePS=false
991993
},
992994
{
993995
name: "only MTU set",
994996
in: OpenStackCluster{
995997
Spec: OpenStackClusterSpec{NetworkMTU: mtu},
996998
},
999+
expectedEnablePS: nil, // unset stays unset
9971000
},
9981001
{
999-
name: "only DisablePortSecurity set",
1002+
name: "only DisablePortSecurity set to true",
10001003
in: OpenStackCluster{
10011004
Spec: OpenStackClusterSpec{DisablePortSecurity: disablePS},
10021005
},
1006+
expectedEnablePS: optional.Bool(ptr.To(false)), // disablePS=true → enablePS=false
10031007
},
10041008
{
1005-
name: "neither set — ManagedNetwork stays nil",
1006-
in: OpenStackCluster{},
1009+
name: "DisablePortSecurity explicitly false",
1010+
in: OpenStackCluster{
1011+
Spec: OpenStackClusterSpec{DisablePortSecurity: optional.Bool(ptr.To(false))},
1012+
},
1013+
expectedEnablePS: optional.Bool(ptr.To(true)), // disablePS=false → enablePS=true
1014+
},
1015+
{
1016+
name: "neither set — ManagedNetwork stays nil",
1017+
in: OpenStackCluster{},
1018+
expectedEnablePS: nil,
10071019
},
10081020
}
10091021

@@ -1020,13 +1032,13 @@ func TestOpenStackCluster_RoundTrip_ManagedNetwork(t *testing.T) {
10201032
} else {
10211033
g.Expect(hub.Spec.ManagedNetwork).NotTo(BeNil())
10221034
g.Expect(hub.Spec.ManagedNetwork.MTU).To(Equal(tt.in.Spec.NetworkMTU))
1023-
g.Expect(hub.Spec.ManagedNetwork.DisablePortSecurity).To(Equal(tt.in.Spec.DisablePortSecurity))
1035+
g.Expect(hub.Spec.ManagedNetwork.EnablePortSecurity).To(Equal(tt.expectedEnablePS))
10241036
}
10251037

10261038
restored := &OpenStackCluster{}
10271039
g.Expect(restored.ConvertFrom(hub)).To(Succeed())
10281040

1029-
// Verify final v1beta1 state
1041+
// Verify final v1beta1 state round-trips correctly
10301042
g.Expect(restored.Spec.NetworkMTU).To(Equal(tt.in.Spec.NetworkMTU))
10311043
g.Expect(restored.Spec.DisablePortSecurity).To(Equal(tt.in.Spec.DisablePortSecurity))
10321044
})
@@ -1224,8 +1236,9 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12241236
disable := optional.Bool(ptr.To(true))
12251237

12261238
tests := []struct {
1227-
name string
1228-
in OpenStackCluster
1239+
name string
1240+
in OpenStackCluster
1241+
expectedEnableFloatingIP optional.Bool
12291242
}{
12301243
{
12311244
name: "all fields set",
@@ -1240,6 +1253,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12401253
},
12411254
},
12421255
},
1256+
expectedEnableFloatingIP: optional.Bool(ptr.To(false)), // disable=true → enable=false
12431257
},
12441258
{
12451259
name: "only floatingIP set",
@@ -1248,6 +1262,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12481262
APIServerFloatingIP: floatingIP,
12491263
},
12501264
},
1265+
expectedEnableFloatingIP: nil,
12511266
},
12521267
{
12531268
name: "only fixedIP set",
@@ -1256,6 +1271,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12561271
APIServerFixedIP: fixedIP,
12571272
},
12581273
},
1274+
expectedEnableFloatingIP: nil,
12591275
},
12601276
{
12611277
name: "only port set",
@@ -1264,14 +1280,25 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12641280
APIServerPort: port,
12651281
},
12661282
},
1283+
expectedEnableFloatingIP: nil,
12671284
},
12681285
{
1269-
name: "only disableFloatingIP set",
1286+
name: "DisableAPIServerFloatingIP explicitly true",
12701287
in: OpenStackCluster{
12711288
Spec: OpenStackClusterSpec{
12721289
DisableAPIServerFloatingIP: disable,
12731290
},
12741291
},
1292+
expectedEnableFloatingIP: optional.Bool(ptr.To(false)), // disable=true → enable=false
1293+
},
1294+
{
1295+
name: "DisableAPIServerFloatingIP explicitly false",
1296+
in: OpenStackCluster{
1297+
Spec: OpenStackClusterSpec{
1298+
DisableAPIServerFloatingIP: optional.Bool(ptr.To(false)),
1299+
},
1300+
},
1301+
expectedEnableFloatingIP: optional.Bool(ptr.To(true)), // disable=false → enable=true
12751302
},
12761303
{
12771304
name: "only loadBalancer set",
@@ -1282,6 +1309,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12821309
},
12831310
},
12841311
},
1312+
expectedEnableFloatingIP: nil,
12851313
},
12861314
{
12871315
name: "loadBalancer disabled explicitly",
@@ -1292,6 +1320,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
12921320
},
12931321
},
12941322
},
1323+
expectedEnableFloatingIP: nil,
12951324
},
12961325
{
12971326
name: "disableFloatingIP with fixedIP (no-LB VIP case)",
@@ -1301,10 +1330,12 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
13011330
APIServerFixedIP: fixedIP,
13021331
},
13031332
},
1333+
expectedEnableFloatingIP: optional.Bool(ptr.To(false)), // disable=true → enable=false
13041334
},
13051335
{
1306-
name: "no APIServer fields set — APIServer stays nil",
1307-
in: OpenStackCluster{},
1336+
name: "no APIServer fields set — APIServer stays nil",
1337+
in: OpenStackCluster{},
1338+
expectedEnableFloatingIP: nil,
13081339
},
13091340
}
13101341

@@ -1331,7 +1362,7 @@ func TestOpenStackCluster_RoundTrip_APIServer(t *testing.T) {
13311362
g.Expect(hub.Spec.APIServer.FloatingIP).To(Equal(src.APIServerFloatingIP))
13321363
g.Expect(hub.Spec.APIServer.FixedIP).To(Equal(src.APIServerFixedIP))
13331364
g.Expect(hub.Spec.APIServer.Port).To(Equal(src.APIServerPort))
1334-
g.Expect(hub.Spec.APIServer.DisableFloatingIP).To(Equal(src.DisableAPIServerFloatingIP))
1365+
g.Expect(hub.Spec.APIServer.EnableFloatingIP).To(Equal(tt.expectedEnableFloatingIP))
13351366

13361367
if src.APIServerLoadBalancer == nil {
13371368
g.Expect(hub.Spec.APIServer.ManagedLoadBalancer).To(BeNil())
@@ -1431,3 +1462,101 @@ func TestOpenStackClusterStatusAPIServerLoadBalancerNilConversion(t *testing.T)
14311462
// Verify round-trip preserves nil
14321463
g.Expect(restored.Status.APIServerLoadBalancer).To(BeNil())
14331464
}
1465+
1466+
func TestOpenStackCluster_RoundTrip_ExternalNetwork(t *testing.T) {
1467+
tests := []struct {
1468+
name string
1469+
in OpenStackCluster
1470+
expectedEnableEN optional.Bool
1471+
}{
1472+
{
1473+
name: "DisableExternalNetwork explicitly true",
1474+
in: OpenStackCluster{
1475+
Spec: OpenStackClusterSpec{
1476+
DisableExternalNetwork: optional.Bool(ptr.To(true)),
1477+
},
1478+
},
1479+
expectedEnableEN: optional.Bool(ptr.To(false)), // disable=true → enable=false
1480+
},
1481+
{
1482+
name: "DisableExternalNetwork explicitly false",
1483+
in: OpenStackCluster{
1484+
Spec: OpenStackClusterSpec{
1485+
DisableExternalNetwork: optional.Bool(ptr.To(false)),
1486+
},
1487+
},
1488+
expectedEnableEN: optional.Bool(ptr.To(true)), // disable=false → enable=true
1489+
},
1490+
{
1491+
name: "DisableExternalNetwork unset — EnableExternalNetwork stays nil",
1492+
in: OpenStackCluster{},
1493+
expectedEnableEN: nil, // unset stays unset, controller applies default
1494+
},
1495+
}
1496+
1497+
for _, tt := range tests {
1498+
t.Run(tt.name, func(t *testing.T) {
1499+
g := NewWithT(t)
1500+
1501+
hub := &infrav1.OpenStackCluster{}
1502+
g.Expect(tt.in.ConvertTo(hub)).To(Succeed())
1503+
1504+
// --- Verify intermediate v1beta2 state ---
1505+
g.Expect(hub.Spec.EnableExternalNetwork).To(Equal(tt.expectedEnableEN))
1506+
1507+
// --- Verify full round-trip back to v1beta1 ---
1508+
restored := &OpenStackCluster{}
1509+
g.Expect(restored.ConvertFrom(hub)).To(Succeed())
1510+
1511+
g.Expect(restored.Spec.DisableExternalNetwork).To(Equal(tt.in.Spec.DisableExternalNetwork))
1512+
})
1513+
}
1514+
}
1515+
1516+
func TestResolvedPortSpecFields_RoundTrip_PortSecurity(t *testing.T) {
1517+
tests := []struct {
1518+
name string
1519+
in ResolvedPortSpecFields
1520+
expectedEnablePS *bool
1521+
}{
1522+
{
1523+
name: "DisablePortSecurity explicitly true",
1524+
in: ResolvedPortSpecFields{
1525+
DisablePortSecurity: ptr.To(true),
1526+
},
1527+
expectedEnablePS: ptr.To(false), // disable=true → enable=false
1528+
},
1529+
{
1530+
name: "DisablePortSecurity explicitly false",
1531+
in: ResolvedPortSpecFields{
1532+
DisablePortSecurity: ptr.To(false),
1533+
},
1534+
expectedEnablePS: ptr.To(true), // disable=false → enable=true
1535+
},
1536+
{
1537+
name: "DisablePortSecurity unset — EnablePortSecurity stays nil",
1538+
in: ResolvedPortSpecFields{},
1539+
expectedEnablePS: nil, // unset stays unset, inherits from network level
1540+
},
1541+
}
1542+
1543+
for _, tt := range tests {
1544+
t.Run(tt.name, func(t *testing.T) {
1545+
g := NewWithT(t)
1546+
1547+
// --- Convert to v1beta2 ---
1548+
out := &infrav1.ResolvedPortSpecFields{}
1549+
g.Expect(Convert_v1beta1_ResolvedPortSpecFields_To_v1beta2_ResolvedPortSpecFields(&tt.in, out, nil)).To(Succeed())
1550+
1551+
// --- Verify intermediate v1beta2 state ---
1552+
g.Expect(out.EnablePortSecurity).To(Equal(tt.expectedEnablePS))
1553+
1554+
// --- Convert back to v1beta1 ---
1555+
restored := &ResolvedPortSpecFields{}
1556+
g.Expect(Convert_v1beta2_ResolvedPortSpecFields_To_v1beta1_ResolvedPortSpecFields(out, restored, nil)).To(Succeed())
1557+
1558+
// --- Verify full round-trip ---
1559+
g.Expect(restored.DisablePortSecurity).To(Equal(tt.in.DisablePortSecurity))
1560+
})
1561+
}
1562+
}

0 commit comments

Comments
 (0)