Skip to content

Commit 894f2d3

Browse files
jstueverjstuever
committed
privatelink: ShouldSync returns false when privatelink is disabled
1 parent 9af1338 commit 894f2d3

File tree

4 files changed

+117
-37
lines changed

4 files changed

+117
-37
lines changed

‎pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go

+5
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,11 @@ func (a *AWSHubActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hivev
184184

185185
// ShouldSync is the actuator interface to determine if there are changes that need to be made.
186186
func (a *AWSHubActuator) ShouldSync(cd *hivev1.ClusterDeployment) bool {
187+
if cd.Spec.Platform.AWS == nil ||
188+
cd.Spec.Platform.AWS.PrivateLink == nil ||
189+
!cd.Spec.Platform.AWS.PrivateLink.Enabled {
190+
return false
191+
}
187192
return cd.Status.Platform == nil ||
188193
cd.Status.Platform.AWS == nil ||
189194
cd.Status.Platform.AWS.PrivateLink == nil ||

‎pkg/controller/privatelink/actuator/awsactuator/awshubactuator_test.go

+46-17
Original file line numberDiff line numberDiff line change
@@ -419,41 +419,70 @@ func Test_ShouldSync(t *testing.T) {
419419
name string
420420
cd *hivev1.ClusterDeployment
421421

422-
expectResult bool
423-
}{{ // Sync is required when status.platform is nil
424-
name: "status.platform is nil",
425-
cd: cdBuilder.Build(),
426-
expectResult: true,
422+
expect bool
423+
}{{ // Sync is not required when spec.platform.aws is nil
424+
name: "spec.plaform is nil",
425+
cd: cdBuilder.Build(),
426+
}, { // Sync is not required when spec.platform.aws.privatelink is nil
427+
name: "spec.plaform.aws.privatelink is nil",
428+
cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})),
429+
}, { // Sync is not required when spec.platform.aws.privatelink.enabled is false
430+
name: "privatelink is not enabled",
431+
cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{
432+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: false},
433+
})),
434+
}, { // Sync is required when status.platform is nil
435+
name: "status.platform is nil",
436+
cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{
437+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: true},
438+
})),
439+
expect: true,
427440
}, { // Sync is required when status.platform.aws is nil
428-
name: "status.platform.aws is nil",
429-
cd: cdBuilder.Options(testcd.WithEmptyPlatformStatus()).Build(),
430-
expectResult: true,
441+
name: "status.platform.aws is nil",
442+
cd: cdBuilder.Build(
443+
testcd.WithAWSPlatform(&hivev1aws.Platform{
444+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: true},
445+
}),
446+
testcd.WithEmptyPlatformStatus(),
447+
),
448+
expect: true,
431449
}, { // Sync is required when status.platform.aws.privatelink is nil
432-
name: "status.platform.aws.privatelink is nil",
433-
cd: cdBuilder.Options(testcd.WithAWSPlatformStatus(&hivev1aws.PlatformStatus{})).Build(),
434-
expectResult: true,
450+
name: "status.platform.aws.privatelink is nil",
451+
cd: cdBuilder.Build(
452+
testcd.WithAWSPlatform(&hivev1aws.Platform{
453+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: true},
454+
}),
455+
testcd.WithAWSPlatformStatus(&hivev1aws.PlatformStatus{}),
456+
),
457+
expect: true,
435458
}, { // Sync is required when hostedZoneID is empty
436459
name: "hostedZoneID is empty",
437-
cd: cdBuilder.Options(
460+
cd: cdBuilder.Build(
461+
testcd.WithAWSPlatform(&hivev1aws.Platform{
462+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: true},
463+
}),
438464
testcd.WithAWSPlatformStatus(&hivev1aws.PlatformStatus{
439465
PrivateLink: &hivev1aws.PrivateLinkAccessStatus{},
440466
}),
441-
).Build(),
442-
expectResult: true,
467+
),
468+
expect: true,
443469
}, { // Sync is not required when hostedZoneID is set
444470
name: "hostedZoneID is not empty",
445-
cd: cdBuilder.Options(
471+
cd: cdBuilder.Build(
472+
testcd.WithAWSPlatform(&hivev1aws.Platform{
473+
PrivateLink: &hivev1aws.PrivateLinkAccess{Enabled: true},
474+
}),
446475
testcd.WithAWSPlatformStatus(&hivev1aws.PlatformStatus{
447476
PrivateLink: &hivev1aws.PrivateLinkAccessStatus{
448477
HostedZoneID: testHostedZone,
449478
},
450479
}),
451-
).Build(),
480+
),
452481
}}
453482
for _, test := range cases {
454483
t.Run(test.name, func(t *testing.T) {
455484
result := awsHubActuator.ShouldSync(test.cd)
456-
assert.Equal(t, test.expectResult, result)
485+
assert.Equal(t, test.expect, result)
457486
})
458487
}
459488
}

‎pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,13 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive
263263

264264
// ShouldSync is the actuator interface to determine if there are changes that need to be made.
265265
func (a *GCPLinkActuator) ShouldSync(cd *hivev1.ClusterDeployment) bool {
266-
shouldCreateSubnet := getServiceAttachmentSubnetExistingName(cd) == ""
266+
if cd.Spec.Platform.GCP == nil ||
267+
cd.Spec.Platform.GCP.PrivateServiceConnect == nil ||
268+
!cd.Spec.Platform.GCP.PrivateServiceConnect.Enabled {
269+
return false
270+
}
267271

272+
shouldCreateSubnet := getServiceAttachmentSubnetExistingName(cd) == ""
268273
return cd.Status.Platform == nil ||
269274
cd.Status.Platform.GCP == nil ||
270275
cd.Status.Platform.GCP.PrivateServiceConnect == nil ||

‎pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator_test.go

+60-19
Original file line numberDiff line numberDiff line change
@@ -838,21 +838,47 @@ func Test_ShouldSync(t *testing.T) {
838838
cd *hivev1.ClusterDeployment
839839

840840
expect bool
841-
}{{ // Sync is required when status.platform is nil
842-
name: "status.platform is nil",
843-
cd: cdBuilder.Build(),
841+
}{{ // Sync is not required when spec.platform.gcp is nil
842+
name: "spec.platform.gcp is nil",
843+
cd: cdBuilder.Build(),
844+
}, { // Sync is not required when spec.platform.gcp.privateServiceConnect is nil
845+
name: "spec.platform.gcp.privateServiceConnect",
846+
cd: cdBuilder.Build(testcd.WithGCPPlatform(&hivev1gcp.Platform{})),
847+
}, { // Sync is not required when spec.platform.gcp.privateServiceConnect.enabled is false
848+
name: "privateServiceConnect is not enabled",
849+
cd: cdBuilder.Build(testcd.WithGCPPlatform(&hivev1gcp.Platform{
850+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: false},
851+
})),
852+
}, { // Sync is required when status.platform is nil
853+
name: "status.platform is nil",
854+
cd: cdBuilder.Build(testcd.WithGCPPlatform(&hivev1gcp.Platform{
855+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
856+
})),
844857
expect: true,
845858
}, { // Sync is required when status.platform.gcp is nil
846-
name: "status.platform.gcp is nil",
847-
cd: cdBuilder.Options(testcd.WithEmptyPlatformStatus()).Build(),
859+
name: "status.platform.gcp is nil",
860+
cd: cdBuilder.Build(
861+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
862+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
863+
}),
864+
testcd.WithEmptyPlatformStatus(),
865+
),
848866
expect: true,
849867
}, { // Sync is required when status.platform.gcp.privateServiceConnect is nil
850-
name: "status.platform.aws.privateServiceConnect is nil",
851-
cd: cdBuilder.Options(testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{})).Build(),
868+
name: "status.platform.aws.privateServiceConnect is nil",
869+
cd: cdBuilder.Build(
870+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
871+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
872+
}),
873+
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{}),
874+
),
852875
expect: true,
853876
}, { // Sync is required when endpoint is empty
854877
name: "Endpoint is empty",
855-
cd: cdBuilder.Options(
878+
cd: cdBuilder.Build(
879+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
880+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
881+
}),
856882
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
857883
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
858884
EndpointAddress: mockAddress.SelfLink,
@@ -861,11 +887,14 @@ func Test_ShouldSync(t *testing.T) {
861887
ServiceAttachmentSubnet: mockSubnet.SelfLink,
862888
},
863889
}),
864-
).Build(),
890+
),
865891
expect: true,
866892
}, { // Sync is required when EndpointAddress is empty
867893
name: "EndpointAddress is empty",
868-
cd: cdBuilder.Options(
894+
cd: cdBuilder.Build(
895+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
896+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
897+
}),
869898
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
870899
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
871900
Endpoint: mockForwardingRule.SelfLink,
@@ -874,11 +903,14 @@ func Test_ShouldSync(t *testing.T) {
874903
ServiceAttachmentSubnet: mockSubnet.SelfLink,
875904
},
876905
}),
877-
).Build(),
906+
),
878907
expect: true,
879908
}, { // Sync is required when ServiceAttachment is empty
880909
name: "ServiceAttachment is empty",
881-
cd: cdBuilder.Options(
910+
cd: cdBuilder.Build(
911+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
912+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
913+
}),
882914
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
883915
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
884916
Endpoint: mockForwardingRule.SelfLink,
@@ -887,11 +919,14 @@ func Test_ShouldSync(t *testing.T) {
887919
ServiceAttachmentSubnet: mockSubnet.SelfLink,
888920
},
889921
}),
890-
).Build(),
922+
),
891923
expect: true,
892924
}, { // Sync is required when ServiceAttachmentFirewall is empty
893925
name: "ServiceAttachmentFirewall is empty",
894-
cd: cdBuilder.Options(
926+
cd: cdBuilder.Build(
927+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
928+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
929+
}),
895930
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
896931
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
897932
Endpoint: mockForwardingRule.SelfLink,
@@ -900,11 +935,14 @@ func Test_ShouldSync(t *testing.T) {
900935
ServiceAttachmentSubnet: mockSubnet.SelfLink,
901936
},
902937
}),
903-
).Build(),
938+
),
904939
expect: true,
905940
}, { // Sync is required when ServiceAttachmentSubnet is empty
906941
name: "ServiceAttachmentSubnet is empty",
907-
cd: cdBuilder.Options(
942+
cd: cdBuilder.Build(
943+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
944+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
945+
}),
908946
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
909947
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
910948
Endpoint: mockForwardingRule.SelfLink,
@@ -913,11 +951,14 @@ func Test_ShouldSync(t *testing.T) {
913951
ServiceAttachmentFirewall: mockFirewall.SelfLink,
914952
},
915953
}),
916-
).Build(),
954+
),
917955
expect: true,
918956
}, { // Sync is not required when hostedZoneID is set
919957
name: "no changes required",
920-
cd: cdBuilder.Options(
958+
cd: cdBuilder.Build(
959+
testcd.WithGCPPlatform(&hivev1gcp.Platform{
960+
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{Enabled: true},
961+
}),
921962
testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{
922963
PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{
923964
Endpoint: mockForwardingRule.SelfLink,
@@ -927,7 +968,7 @@ func Test_ShouldSync(t *testing.T) {
927968
ServiceAttachmentSubnet: mockSubnet.SelfLink,
928969
},
929970
}),
930-
).Build(),
971+
),
931972
}}
932973
for _, test := range cases {
933974
t.Run(test.name, func(t *testing.T) {

0 commit comments

Comments
 (0)