Skip to content

Commit 605af02

Browse files
synth-beatsynth-beat
committed
Update GNP validations for NetworkAttachments
1 parent 87a9842 commit 605af02

File tree

3 files changed

+320
-13
lines changed

3 files changed

+320
-13
lines changed

‎pkg/controller/gkenetworkparamset/gkenetworkparamset_controller.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,27 @@ func (c *Controller) syncGNP(ctx context.Context, params *networkv1.GKENetworkPa
406406
}
407407

408408
addFinalizerInPlace(params)
409+
410+
// Validate that the fields set are a valid combination.
411+
presenceValidation := c.validateFieldCombinations(ctx, params)
412+
if !presenceValidation.IsValid {
413+
meta.SetStatusCondition(&params.Status.Conditions, presenceValidation.toCondition())
414+
return nil
415+
}
416+
417+
// Validate specific fields once field combination is valid.
418+
netAttachment := params.Spec.NetworkAttachment
419+
if netAttachment != "" {
420+
networkAttachmentValidation := c.validateNetworkAttachment(ctx, netAttachment)
421+
meta.SetStatusCondition(&params.Status.Conditions, networkAttachmentValidation.toCondition())
422+
if !networkAttachmentValidation.IsValid {
423+
return nil
424+
}
425+
426+
return c.getAndSyncNetworkForGNP(ctx, params)
427+
}
428+
429+
// Validate params with (VPC + VPCSubnet).
409430
subnet, subnetValidation := c.getAndValidateSubnet(ctx, params)
410431
meta.SetStatusCondition(&params.Status.Conditions, subnetValidation.toCondition())
411432
if !subnetValidation.IsValid {
@@ -436,6 +457,13 @@ func (c *Controller) syncGNP(ctx context.Context, params *networkv1.GKENetworkPa
436457
CIDRBlocks: cidrs,
437458
}
438459

460+
return c.getAndSyncNetworkForGNP(ctx, params)
461+
}
462+
463+
// getAndSyncNetworkForGNP gets the network that refers to this GNP object, and
464+
// then does the cross sync of Network with GNP. GNP is guaranteed to have
465+
// minimum fields set (either NetworkAttachment or [VPC + VPCSubnet]).
466+
func (c *Controller) getAndSyncNetworkForGNP(ctx context.Context, params *networkv1.GKENetworkParamSet) error {
439467
network, err := c.getNetworkReferringToGNP(params.Name)
440468
if err != nil {
441469
return err

‎pkg/controller/gkenetworkparamset/gkenetworkparamset_controller_test.go

Lines changed: 207 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
498498
gkeNetworkParamSetName := "test-paramset"
499499
duplicateVPCName := "already-used-vpc"
500500
duplicateSubnetName := "already-used-subnet"
501+
netAttachmentName := "projects/test-project/regions/test-region/networkAttachments/testAttachment"
501502

502503
tests := []struct {
503504
name string
@@ -506,7 +507,7 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
506507
expectedCondition metav1.Condition
507508
}{
508509
{
509-
name: "Unspecified Subnet",
510+
name: "VPC with unspecified VPCSubnet",
510511
paramSet: &networkv1.GKENetworkParamSet{
511512
ObjectMeta: metav1.ObjectMeta{
512513
Name: gkeNetworkParamSetName,
@@ -518,7 +519,21 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
518519
expectedCondition: metav1.Condition{
519520
Type: "Ready",
520521
Status: metav1.ConditionFalse,
521-
Reason: "SubnetNotFound",
522+
Reason: "GNPConfigInvalid",
523+
},
524+
},
525+
{
526+
name: "Unspecified - no NetworkAttachment, no VPC or VPCSubnet",
527+
paramSet: &networkv1.GKENetworkParamSet{
528+
ObjectMeta: metav1.ObjectMeta{
529+
Name: gkeNetworkParamSetName,
530+
},
531+
Spec: networkv1.GKENetworkParamSetSpec{},
532+
},
533+
expectedCondition: metav1.Condition{
534+
Type: "Ready",
535+
Status: metav1.ConditionFalse,
536+
Reason: "GNPConfigInvalid",
522537
},
523538
},
524539
{
@@ -528,8 +543,9 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
528543
Name: gkeNetworkParamSetName,
529544
},
530545
Spec: networkv1.GKENetworkParamSetSpec{
531-
VPC: "test-vpc",
532-
VPCSubnet: "non-existant-test-subnet",
546+
VPC: "test-vpc",
547+
VPCSubnet: "non-existant-test-subnet",
548+
DeviceMode: "test-device-mode",
533549
},
534550
},
535551
expectedCondition: metav1.Condition{
@@ -584,7 +600,7 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
584600
},
585601
},
586602
{
587-
name: "Valid GKENetworkParamSet",
603+
name: "Valid GKENetworkParamSet with VPC, VPCSubnet, and PodIPv4Ranges",
588604
paramSet: &networkv1.GKENetworkParamSet{
589605
ObjectMeta: metav1.ObjectMeta{
590606
Name: gkeNetworkParamSetName,
@@ -605,6 +621,38 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
605621
Reason: "GNPReady",
606622
},
607623
},
624+
{
625+
name: "Valid GKENetworkParamSet with NetworkAttachment",
626+
paramSet: &networkv1.GKENetworkParamSet{
627+
ObjectMeta: metav1.ObjectMeta{
628+
Name: gkeNetworkParamSetName,
629+
},
630+
Spec: networkv1.GKENetworkParamSetSpec{
631+
NetworkAttachment: netAttachmentName,
632+
},
633+
},
634+
expectedCondition: metav1.Condition{
635+
Type: "Ready",
636+
Status: metav1.ConditionTrue,
637+
Reason: "GNPReady",
638+
},
639+
},
640+
{
641+
name: "GNP with NetworkAttachment - bad format",
642+
paramSet: &networkv1.GKENetworkParamSet{
643+
ObjectMeta: metav1.ObjectMeta{
644+
Name: gkeNetworkParamSetName,
645+
},
646+
Spec: networkv1.GKENetworkParamSetSpec{
647+
NetworkAttachment: "invalid",
648+
},
649+
},
650+
expectedCondition: metav1.Condition{
651+
Type: "Ready",
652+
Status: metav1.ConditionFalse,
653+
Reason: "NetworkAttachmentInvalid",
654+
},
655+
},
608656
{
609657
name: "GNP with deviceMode and referencing VPC is referenced in any other existing GNP",
610658
paramSet: &networkv1.GKENetworkParamSet{
@@ -642,7 +690,7 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
642690
},
643691
},
644692
{
645-
name: "GNP with VPC unspecified",
693+
name: "GNP with VPCSubnet - VPC unspecified",
646694
paramSet: &networkv1.GKENetworkParamSet{
647695
ObjectMeta: metav1.ObjectMeta{
648696
Name: gkeNetworkParamSetName,
@@ -655,11 +703,11 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
655703
expectedCondition: metav1.Condition{
656704
Type: "Ready",
657705
Status: metav1.ConditionFalse,
658-
Reason: "VPCNotFound",
706+
Reason: "GNPConfigInvalid",
659707
},
660708
},
661709
{
662-
name: "GNP with specified, but nonexistant VPC",
710+
name: "GNP with VPC and VPCSubnet - specified, but nonexistant VPC",
663711
paramSet: &networkv1.GKENetworkParamSet{
664712
ObjectMeta: metav1.ObjectMeta{
665713
Name: gkeNetworkParamSetName,
@@ -677,7 +725,7 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
677725
},
678726
},
679727
{
680-
name: "GNP without devicemode or secondary range specified",
728+
name: "GNP with VPC and VPCSubnet - without DeviceMode or secondary range specified",
681729
paramSet: &networkv1.GKENetworkParamSet{
682730
ObjectMeta: metav1.ObjectMeta{
683731
Name: gkeNetworkParamSetName,
@@ -711,6 +759,78 @@ func TestGKENetworkParamSetValidations(t *testing.T) {
711759
Reason: "DeviceModeCantUseDefaultVPC",
712760
},
713761
},
762+
{
763+
name: "GNP with NetworkAttachment - but VPC specified",
764+
paramSet: &networkv1.GKENetworkParamSet{
765+
ObjectMeta: metav1.ObjectMeta{
766+
Name: gkeNetworkParamSetName,
767+
},
768+
Spec: networkv1.GKENetworkParamSetSpec{
769+
NetworkAttachment: netAttachmentName,
770+
VPC: defaultTestNetworkName,
771+
},
772+
},
773+
expectedCondition: metav1.Condition{
774+
Type: "Ready",
775+
Status: metav1.ConditionFalse,
776+
Reason: "GNPConfigInvalid",
777+
},
778+
},
779+
{
780+
name: "GNP with NetworkAttachment - but VPCSubnet specified",
781+
paramSet: &networkv1.GKENetworkParamSet{
782+
ObjectMeta: metav1.ObjectMeta{
783+
Name: gkeNetworkParamSetName,
784+
},
785+
Spec: networkv1.GKENetworkParamSetSpec{
786+
NetworkAttachment: netAttachmentName,
787+
VPCSubnet: "test-subnet",
788+
},
789+
},
790+
expectedCondition: metav1.Condition{
791+
Type: "Ready",
792+
Status: metav1.ConditionFalse,
793+
Reason: "GNPConfigInvalid",
794+
},
795+
},
796+
{
797+
name: "GNP with NetworkAttachment - but PodIPv4Ranges specified",
798+
paramSet: &networkv1.GKENetworkParamSet{
799+
ObjectMeta: metav1.ObjectMeta{
800+
Name: gkeNetworkParamSetName,
801+
},
802+
Spec: networkv1.GKENetworkParamSetSpec{
803+
NetworkAttachment: netAttachmentName,
804+
PodIPv4Ranges: &networkv1.SecondaryRanges{
805+
RangeNames: []string{
806+
"nonexistent-secondary-range",
807+
},
808+
},
809+
},
810+
},
811+
expectedCondition: metav1.Condition{
812+
Type: "Ready",
813+
Status: metav1.ConditionFalse,
814+
Reason: "GNPConfigInvalid",
815+
},
816+
},
817+
{
818+
name: "GNP with NetworkAttachment - but DeviceMode specified",
819+
paramSet: &networkv1.GKENetworkParamSet{
820+
ObjectMeta: metav1.ObjectMeta{
821+
Name: gkeNetworkParamSetName,
822+
},
823+
Spec: networkv1.GKENetworkParamSetSpec{
824+
NetworkAttachment: netAttachmentName,
825+
DeviceMode: "test-device-mode",
826+
},
827+
},
828+
expectedCondition: metav1.Condition{
829+
Type: "Ready",
830+
Status: metav1.ConditionFalse,
831+
Reason: "GNPConfigInvalid",
832+
},
833+
},
714834
}
715835

716836
for _, test := range tests {
@@ -810,6 +930,7 @@ func TestCrossValidateNetworkAndGnp(t *testing.T) {
810930
subnetName := "test-subnet"
811931
subnetSecondaryRangeName := "test-secondary-range"
812932
networkName := "network-name"
933+
netAttachmentName := "projects/test-project/regions/test-region/networkAttachments/testAttachment"
813934

814935
tests := []struct {
815936
name string
@@ -818,7 +939,7 @@ func TestCrossValidateNetworkAndGnp(t *testing.T) {
818939
expectedCondition metav1.Condition
819940
}{
820941
{
821-
name: "L3NetworkType with missing PodIPv4Ranges",
942+
name: "L3NetworkType has VPC + VPCSubnet GNP missing PodIPv4Ranges",
822943
network: &networkv1.Network{
823944
ObjectMeta: metav1.ObjectMeta{
824945
Name: networkName,
@@ -844,6 +965,56 @@ func TestCrossValidateNetworkAndGnp(t *testing.T) {
844965
Reason: "L3SecondaryMissing",
845966
},
846967
},
968+
{
969+
name: "DeviceNetworkType with NetworkAttachment",
970+
network: &networkv1.Network{
971+
ObjectMeta: metav1.ObjectMeta{
972+
Name: networkName,
973+
},
974+
Spec: networkv1.NetworkSpec{
975+
Type: networkv1.DeviceNetworkType,
976+
ParametersRef: &networkv1.NetworkParametersReference{Name: gkeNetworkParamSetName, Kind: gnpKind},
977+
},
978+
},
979+
paramSet: &networkv1.GKENetworkParamSet{
980+
ObjectMeta: metav1.ObjectMeta{
981+
Name: gkeNetworkParamSetName,
982+
},
983+
Spec: networkv1.GKENetworkParamSetSpec{
984+
NetworkAttachment: netAttachmentName,
985+
},
986+
},
987+
expectedCondition: metav1.Condition{
988+
Type: "ParamsReady",
989+
Status: metav1.ConditionFalse,
990+
Reason: "NetworkAttachmentUnsupported",
991+
},
992+
},
993+
{
994+
name: "L2NetworkType with NetworkAttachment",
995+
network: &networkv1.Network{
996+
ObjectMeta: metav1.ObjectMeta{
997+
Name: networkName,
998+
},
999+
Spec: networkv1.NetworkSpec{
1000+
Type: networkv1.L2NetworkType,
1001+
ParametersRef: &networkv1.NetworkParametersReference{Name: gkeNetworkParamSetName, Kind: gnpKind},
1002+
},
1003+
},
1004+
paramSet: &networkv1.GKENetworkParamSet{
1005+
ObjectMeta: metav1.ObjectMeta{
1006+
Name: gkeNetworkParamSetName,
1007+
},
1008+
Spec: networkv1.GKENetworkParamSetSpec{
1009+
NetworkAttachment: netAttachmentName,
1010+
},
1011+
},
1012+
expectedCondition: metav1.Condition{
1013+
Type: "ParamsReady",
1014+
Status: metav1.ConditionFalse,
1015+
Reason: "NetworkAttachmentUnsupported",
1016+
},
1017+
},
8471018
{
8481019
name: "DeviceNetworkType with missing DeviceMode",
8491020
network: &networkv1.Network{
@@ -872,7 +1043,7 @@ func TestCrossValidateNetworkAndGnp(t *testing.T) {
8721043
},
8731044
},
8741045
{
875-
name: "Valid L3NetworkType",
1046+
name: "Valid L3NetworkType with PodIPv4Ranges",
8761047
network: &networkv1.Network{
8771048
ObjectMeta: metav1.ObjectMeta{
8781049
Name: networkName,
@@ -898,6 +1069,31 @@ func TestCrossValidateNetworkAndGnp(t *testing.T) {
8981069
Reason: "GNPParamsReady",
8991070
},
9001071
},
1072+
{
1073+
name: "Valid L3NetworkType with NetworkAttachment",
1074+
network: &networkv1.Network{
1075+
ObjectMeta: metav1.ObjectMeta{
1076+
Name: networkName,
1077+
},
1078+
Spec: networkv1.NetworkSpec{
1079+
Type: networkv1.L3NetworkType,
1080+
ParametersRef: &networkv1.NetworkParametersReference{Name: gkeNetworkParamSetName, Kind: gnpKind},
1081+
},
1082+
},
1083+
paramSet: &networkv1.GKENetworkParamSet{
1084+
ObjectMeta: metav1.ObjectMeta{
1085+
Name: gkeNetworkParamSetName,
1086+
},
1087+
Spec: networkv1.GKENetworkParamSetSpec{
1088+
NetworkAttachment: netAttachmentName,
1089+
},
1090+
},
1091+
expectedCondition: metav1.Condition{
1092+
Type: "ParamsReady",
1093+
Status: metav1.ConditionTrue,
1094+
Reason: "GNPParamsReady",
1095+
},
1096+
},
9011097
{
9021098
name: "Valid DeviceNetworkType",
9031099
network: &networkv1.Network{

0 commit comments

Comments
 (0)