Skip to content

Commit 934eaaa

Browse files
zhiying-linzhiying-lin
authored
feat: deny the member leave when member still has serviceExport (#1092)
1 parent 2dffaab commit 934eaaa

4 files changed

Lines changed: 134 additions & 13 deletions

File tree

‎pkg/webhook/membercluster/membercluster_validating_webhook.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@ import (
55
"fmt"
66
"net/http"
77

8+
admissionv1 "k8s.io/api/admission/v1"
89
"k8s.io/apimachinery/pkg/types"
910
"k8s.io/klog/v2"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
1012
"sigs.k8s.io/controller-runtime/pkg/manager"
1113
"sigs.k8s.io/controller-runtime/pkg/webhook"
1214
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1315

16+
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
1417
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
1518
"go.goms.io/fleet/pkg/utils"
1619
"go.goms.io/fleet/pkg/utils/validator"
@@ -22,26 +25,47 @@ var (
2225
)
2326

2427
type memberClusterValidator struct {
28+
client client.Client
2529
decoder webhook.AdmissionDecoder
2630
}
2731

2832
// Add registers the webhook for K8s bulit-in object types.
2933
func Add(mgr manager.Manager) error {
3034
hookServer := mgr.GetWebhookServer()
31-
hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{admission.NewDecoder(mgr.GetScheme())}})
35+
hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{client: mgr.GetClient(), decoder: admission.NewDecoder(mgr.GetScheme())}})
3236
return nil
3337
}
3438

3539
// Handle memberClusterValidator checks to see if member cluster has valid fields.
36-
func (v *memberClusterValidator) Handle(_ context.Context, req admission.Request) admission.Response {
40+
func (v *memberClusterValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
41+
mcObjectName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace}
42+
klog.V(2).InfoS("Validating webhook handling member cluster", "operation", req.Operation, "memberCluster", mcObjectName)
43+
44+
if req.Operation == admissionv1.Delete { // Will reject the requests whenever the serviceExport is not deleted
45+
klog.V(2).InfoS("Validating webhook member cluster DELETE", "memberCluster", mcObjectName)
46+
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, mcObjectName.Name)
47+
internalServiceExportList := &fleetnetworkingv1alpha1.InternalServiceExportList{}
48+
if err := v.client.List(ctx, internalServiceExportList, client.InNamespace(namespaceName)); err != nil {
49+
klog.ErrorS(err, "Failed to list internalServiceExportList when validating")
50+
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to list internalServiceExportList, please retry the request: %w", err))
51+
}
52+
for _, internalServiceExport := range internalServiceExportList.Items {
53+
if internalServiceExport.DeletionTimestamp.IsZero() {
54+
klog.Warning("ServiceExport exists in the member cluster, request is denied", "operation", req.Operation, "memberCluster", mcObjectName)
55+
return admission.Denied(fmt.Sprintf("Please delete serviceExport %s in the member cluster before leaving, request is denied", internalServiceExport.Spec.ServiceReference.NamespacedName))
56+
}
57+
}
58+
return admission.Allowed("Member cluster is ready to leave")
59+
}
60+
3761
var mc clusterv1beta1.MemberCluster
38-
klog.V(2).InfoS("Validating webhook handling member cluster", "operation", req.Operation, "namespacedName", types.NamespacedName{Name: req.Name})
3962
if err := v.decoder.Decode(req, &mc); err != nil {
4063
klog.ErrorS(err, "Failed to decode member cluster object for validating fields", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups)
4164
return admission.Errored(http.StatusBadRequest, err)
4265
}
66+
4367
if err := validator.ValidateMemberCluster(mc); err != nil {
44-
klog.V(2).ErrorS(err, "Member cluster has invalid fields, request is denied", "operation", req.Operation, "namespacedName", types.NamespacedName{Name: mc.Name})
68+
klog.V(2).ErrorS(err, "Member cluster has invalid fields, request is denied", "operation", req.Operation, "memberCluster", mcObjectName)
4569
return admission.Denied(err.Error())
4670
}
4771
return admission.Allowed("Member cluster has valid fields")

‎pkg/webhook/webhook.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ func (w *Config) buildFleetValidatingWebhooks() []admv1.ValidatingWebhook {
306306
Operations: []admv1.OperationType{
307307
admv1.Create,
308308
admv1.Update,
309+
admv1.Delete,
309310
},
310311
Rule: createRule([]string{clusterv1beta1.GroupVersion.Group}, []string{clusterv1beta1.GroupVersion.Version}, []string{memberClusterResourceName}, &clusterScope),
311312
},

‎test/e2e/fleet_guard_rail_test.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -789,24 +789,26 @@ var _ = Describe("fleet guard rail networking E2Es", Serial, Ordered, func() {
789789
mcName := fmt.Sprintf(mcNameTemplate, GinkgoParallelProcess())
790790
iseName := fmt.Sprintf(internalServiceExportNameTemplate, GinkgoParallelProcess())
791791
imcNamespace := fmt.Sprintf(utils.NamespaceNameFormat, mcName)
792+
var ise fleetnetworkingv1alpha1.InternalServiceExport
792793

793794
BeforeEach(func() {
794795
createMemberCluster(mcName, "random-user", nil, map[string]string{fleetClusterResourceIDAnnotationKey: clusterID1})
795796
checkInternalMemberClusterExists(mcName, imcNamespace)
796-
ise := internalServiceExport(iseName, imcNamespace)
797+
ise = internalServiceExport(iseName, imcNamespace)
797798
// can return no kind match error.
798799
Eventually(func(g Gomega) error {
799800
return hubClient.Create(ctx, &ise)
800801
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
801802
})
802803

803804
AfterEach(func() {
805+
Expect(hubClient.Delete(ctx, &ise)).Should(Succeed(), "failed to delete Internal Service Export")
806+
804807
ensureMemberClusterAndRelatedResourcesDeletion(mcName)
805808
})
806809

807810
It("should deny update operation on Internal service export resource in fleet-member namespace for user not in member cluster identity", func() {
808811
Eventually(func(g Gomega) error {
809-
var ise fleetnetworkingv1alpha1.InternalServiceExport
810812
err := hubClient.Get(ctx, types.NamespacedName{Name: iseName, Namespace: imcNamespace}, &ise)
811813
if err != nil {
812814
return err
@@ -841,37 +843,49 @@ var _ = Describe("fleet guard rail networking E2Es", Serial, Ordered, func() {
841843
ise := internalServiceExport(iseName, imcNamespace)
842844
By("expecting successful CREATE of Internal Service Export")
843845
Expect(impersonateHubClient.Create(ctx, &ise)).Should(Succeed())
846+
847+
By("deleting Internal Service Export")
848+
// Cleanup the internalServiceExport so that it won't block the member deletion.
849+
Expect(impersonateHubClient.Delete(ctx, &ise)).Should(Succeed(), "failed to delete Internal Service Export")
844850
})
845851

846852
It("should allow CREATE operation on Internal service import resource in fleet-member namespace for user in member cluster identity", func() {
847-
ise := internalServiceImport(isiName, imcNamespace)
853+
isi := internalServiceImport(isiName, imcNamespace)
848854
By("expecting successful CREATE of Internal Service Import")
849-
Expect(impersonateHubClient.Create(ctx, &ise)).Should(Succeed())
855+
Expect(impersonateHubClient.Create(ctx, &isi)).Should(Succeed())
856+
857+
By("deleting Internal Service Import")
858+
Expect(impersonateHubClient.Delete(ctx, &isi)).Should(Succeed(), "failed to delete Internal Service Import")
850859
})
851860

852861
It("should allow CREATE operation on Endpoint slice export resource in fleet-member namespace for user in member cluster identity", func() {
853-
ise := endpointSliceExport(epName, imcNamespace)
862+
esx := endpointSliceExport(epName, imcNamespace)
854863
By("expecting successful CREATE of Endpoint slice export")
855-
Expect(impersonateHubClient.Create(ctx, &ise)).Should(Succeed())
864+
Expect(impersonateHubClient.Create(ctx, &esx)).Should(Succeed())
865+
866+
By("deleting Endpoint Slice Export")
867+
Expect(impersonateHubClient.Delete(ctx, &esx)).Should(Succeed(), "failed to delete EndpointSlice Export")
856868
})
857869
})
858870

859871
Context("allow request to modify network resources in fleet member namespaces, for user in member cluster identity", func() {
860872
mcName := fmt.Sprintf(mcNameTemplate, GinkgoParallelProcess())
861873
iseName := fmt.Sprintf(internalServiceExportNameTemplate, GinkgoParallelProcess())
862874
imcNamespace := fmt.Sprintf(utils.NamespaceNameFormat, mcName)
875+
var ise fleetnetworkingv1alpha1.InternalServiceExport
863876

864877
BeforeEach(func() {
865878
createMemberCluster(mcName, testUser, nil, map[string]string{fleetClusterResourceIDAnnotationKey: clusterID1})
866879
checkInternalMemberClusterExists(mcName, imcNamespace)
867-
ise := internalServiceExport(iseName, imcNamespace)
880+
ise = internalServiceExport(iseName, imcNamespace)
868881
// can return no kind match error.
869882
Eventually(func(g Gomega) error {
870883
return hubClient.Create(ctx, &ise)
871884
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
872885
})
873886

874887
AfterEach(func() {
888+
Expect(hubClient.Delete(ctx, &ise)).Should(Succeed(), "failed to delete Internal Service Export")
875889
ensureMemberClusterAndRelatedResourcesDeletion(mcName)
876890
})
877891

‎test/e2e/join_and_leave_test.go‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ Licensed under the MIT license.
66
package e2e
77

88
import (
9+
"errors"
910
"fmt"
11+
"reflect"
1012

1113
. "github.com/onsi/ginkgo/v2"
1214
. "github.com/onsi/gomega"
@@ -18,6 +20,8 @@ import (
1820
"k8s.io/utils/ptr"
1921
"sigs.k8s.io/controller-runtime/pkg/client"
2022

23+
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
24+
2125
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
2226
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
2327
"go.goms.io/fleet/pkg/utils"
@@ -31,6 +35,7 @@ const (
3135
var _ = Describe("Test member cluster join and leave flow", Ordered, Serial, func() {
3236
crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
3337
workNamespaceName := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess())
38+
internalServiceExportName := fmt.Sprintf("internal-service-export-%d", GinkgoParallelProcess())
3439
var wantSelectedResources []placementv1beta1.ResourceIdentifier
3540
BeforeAll(func() {
3641
// Create the test resources.
@@ -95,9 +100,86 @@ var _ = Describe("Test member cluster join and leave flow", Ordered, Serial, fun
95100
}
96101
})
97102

98-
It("Should be able to unjoin a cluster with crp still running", func() {
99-
By("remove all the clusters without deleting the CRP")
103+
It("create a dummy internalServiceExport in the reserved member namespace", func() {
104+
for idx := range allMemberClusterNames {
105+
memberCluster := allMemberClusters[idx]
106+
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster.ClusterName)
107+
internalServiceExport := &fleetnetworkingv1alpha1.InternalServiceExport{
108+
ObjectMeta: metav1.ObjectMeta{
109+
Namespace: namespaceName,
110+
Name: internalServiceExportName,
111+
// Add a custom finalizer; this would allow us to better observe
112+
// the behavior of the controllers.
113+
Finalizers: []string{customDeletionBlockerFinalizer},
114+
},
115+
Spec: fleetnetworkingv1alpha1.InternalServiceExportSpec{
116+
ServiceReference: fleetnetworkingv1alpha1.ExportedObjectReference{
117+
NamespacedName: "test-namespace/test-svc",
118+
ClusterID: memberCluster.ClusterName,
119+
Kind: "Service",
120+
Namespace: "test-namespace",
121+
Name: "test-svc",
122+
ResourceVersion: "0",
123+
UID: "00000000-0000-0000-0000-000000000000",
124+
},
125+
Ports: []fleetnetworkingv1alpha1.ServicePort{
126+
{
127+
Protocol: corev1.ProtocolTCP,
128+
Port: 4848,
129+
},
130+
},
131+
},
132+
}
133+
Expect(hubClient.Create(ctx, internalServiceExport)).To(Succeed(), "Failed to create internalServiceExport")
134+
}
135+
})
136+
137+
It("Should fail the unjoin requests", func() {
138+
for idx := range allMemberClusters {
139+
memberCluster := allMemberClusters[idx]
140+
mcObj := &clusterv1beta1.MemberCluster{
141+
ObjectMeta: metav1.ObjectMeta{
142+
Name: memberCluster.ClusterName,
143+
},
144+
}
145+
err := hubClient.Delete(ctx, mcObj)
146+
Expect(err).ShouldNot(Succeed(), "Want the deletion to be denied")
147+
var statusErr *apierrors.StatusError
148+
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Delete memberCluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&apierrors.StatusError{})))
149+
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("Please delete serviceExport test-namespace/test-svc in the member cluster before leaving, request is denied"))
150+
}
151+
})
152+
153+
It("Deleting the internalServiceExports", func() {
154+
for idx := range allMemberClusterNames {
155+
memberCluster := allMemberClusters[idx]
156+
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster.ClusterName)
157+
158+
internalSvcExportKey := types.NamespacedName{Namespace: namespaceName, Name: internalServiceExportName}
159+
var export fleetnetworkingv1alpha1.InternalServiceExport
160+
Expect(hubClient.Get(ctx, internalSvcExportKey, &export)).Should(Succeed(), "Failed to get internalServiceExport")
161+
Expect(hubClient.Delete(ctx, &export)).To(Succeed(), "Failed to delete internalServiceExport")
162+
}
163+
})
164+
165+
It("Should be able to trigger the member cluster DELETE", func() {
100166
setAllMemberClustersToLeave()
167+
})
168+
169+
It("Removing the finalizer from the internalServiceExport", func() {
170+
for idx := range allMemberClusterNames {
171+
memberCluster := allMemberClusters[idx]
172+
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster.ClusterName)
173+
174+
internalSvcExportKey := types.NamespacedName{Namespace: namespaceName, Name: internalServiceExportName}
175+
var export fleetnetworkingv1alpha1.InternalServiceExport
176+
Expect(hubClient.Get(ctx, internalSvcExportKey, &export)).Should(Succeed(), "Failed to get internalServiceExport")
177+
export.Finalizers = nil
178+
Expect(hubClient.Update(ctx, &export)).To(Succeed(), "Failed to update internalServiceExport")
179+
}
180+
})
181+
182+
It("Should be able to unjoin a cluster with crp still running", func() {
101183
checkIfAllMemberClustersHaveLeft()
102184
})
103185

0 commit comments

Comments
 (0)