Skip to content

Commit 686f3e2

Browse files
ryanzhang-ossryanzhang-oss
authored
Merge branch 'main' into weiweng/wait-for-webhook-informer-cache
2 parents 573400b + 58b6de2 commit 686f3e2

File tree

6 files changed

+168
-58
lines changed

6 files changed

+168
-58
lines changed

‎cmd/hubagent/main.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ func main() {
157157

158158
if opts.EnableWebhook {
159159
whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",")
160-
if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers, opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload); err != nil {
160+
if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers,
161+
opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled); err != nil {
161162
klog.ErrorS(err, "unable to set up webhook")
162163
exitWithErrorFunc()
163164
}
@@ -200,7 +201,8 @@ func main() {
200201
}
201202

202203
// SetupWebhook generates the webhook cert and then set up the webhook configurator.
203-
func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string, whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool) error {
204+
func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string,
205+
whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool) error {
204206
// Generate self-signed key and crt files in FleetWebhookCertDir for the webhook server to start.
205207
w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir, enableGuardRail, denyModifyMemberClusterLabels, enableWorkload)
206208
if err != nil {
@@ -211,7 +213,7 @@ func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.Webho
211213
klog.ErrorS(err, "unable to add WebhookConfig")
212214
return err
213215
}
214-
if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels); err != nil {
216+
if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels, networkingAgentsEnabled); err != nil {
215217
klog.ErrorS(err, "unable to register webhooks to the manager")
216218
return err
217219
}

‎pkg/webhook/add_handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ import (
1616
func init() {
1717
// AddToManagerFleetResourceValidator is a function to register fleet guard rail resource validator to the webhook server
1818
AddToManagerFleetResourceValidator = fleetresourcehandler.Add
19+
AddToManagerMemberclusterValidator = membercluster.Add
1920
// AddToManagerFuncs is a list of functions to register webhook validators and mutators to the webhook server
2021
AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.AddMutating)
2122
AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add)
2223
AddToManagerFuncs = append(AddToManagerFuncs, resourceplacement.Add)
2324
AddToManagerFuncs = append(AddToManagerFuncs, pod.Add)
2425
AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add)
25-
AddToManagerFuncs = append(AddToManagerFuncs, membercluster.Add)
2626
AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceoverride.Add)
2727
AddToManagerFuncs = append(AddToManagerFuncs, resourceoverride.Add)
2828
AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacementeviction.Add)

‎pkg/webhook/membercluster/membercluster_validating_webhook.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,19 @@ var (
2626
)
2727

2828
type memberClusterValidator struct {
29-
client client.Client
30-
decoder webhook.AdmissionDecoder
29+
client client.Client
30+
decoder webhook.AdmissionDecoder
31+
networkingAgentsEnabled bool
3132
}
3233

3334
// Add registers the webhook for K8s bulit-in object types.
34-
func Add(mgr manager.Manager) error {
35+
func Add(mgr manager.Manager, networkingAgentsEnabled bool) {
3536
hookServer := mgr.GetWebhookServer()
36-
hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{client: mgr.GetClient(), decoder: admission.NewDecoder(mgr.GetScheme())}})
37-
return nil
37+
hookServer.Register(ValidationPath, &webhook.Admission{Handler: &memberClusterValidator{
38+
client: mgr.GetClient(),
39+
decoder: admission.NewDecoder(mgr.GetScheme()),
40+
networkingAgentsEnabled: networkingAgentsEnabled,
41+
}})
3842
}
3943

4044
// Handle memberClusterValidator checks to see if member cluster has valid fields.
@@ -50,8 +54,12 @@ func (v *memberClusterValidator) Handle(ctx context.Context, req admission.Reque
5054
}
5155

5256
if mc.Spec.DeleteOptions != nil && mc.Spec.DeleteOptions.ValidationMode == clusterv1beta1.DeleteValidationModeSkip {
53-
klog.V(2).InfoS("Skipping validation for member cluster DELETE", "memberCluster", mcObjectName)
54-
return admission.Allowed("Skipping validation for member cluster DELETE")
57+
klog.V(2).InfoS("Skipping validation for member cluster DELETE when the validation mode is set to skip", "memberCluster", mcObjectName)
58+
return admission.Allowed("Skipping validation for member cluster DELETE when the validation mode is set to skip")
59+
}
60+
if !v.networkingAgentsEnabled {
61+
klog.V(2).InfoS("Networking agents disabled; skipping ServiceExport validation", "memberCluster", mcObjectName)
62+
return admission.Allowed("Networking agents disabled; skipping ServiceExport validation")
5563
}
5664

5765
klog.V(2).InfoS("Validating webhook member cluster DELETE", "memberCluster", mcObjectName)

‎pkg/webhook/membercluster/membercluster_validating_webhook_test.go‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package membercluster
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"strings"
8+
"testing"
9+
10+
admissionv1 "k8s.io/api/admission/v1"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/types"
14+
15+
clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
16+
"github.com/kubefleet-dev/kubefleet/pkg/utils"
17+
18+
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
19+
20+
"sigs.k8s.io/controller-runtime/pkg/client"
21+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
22+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
23+
)
24+
25+
func TestHandleDelete(t *testing.T) {
26+
t.Parallel()
27+
28+
testCases := map[string]struct {
29+
networkingEnabled bool
30+
validationMode clusterv1beta1.DeleteValidationMode
31+
wantAllowed bool
32+
wantMessageSubstr string
33+
}{
34+
"networking-disabled-allows-delete": {
35+
networkingEnabled: false,
36+
wantAllowed: true,
37+
validationMode: clusterv1beta1.DeleteValidationModeStrict,
38+
},
39+
"networking-enabled-denies-delete": {
40+
networkingEnabled: true,
41+
wantAllowed: false,
42+
validationMode: clusterv1beta1.DeleteValidationModeStrict,
43+
wantMessageSubstr: "Please delete serviceExport",
44+
},
45+
"delete-options-skip-bypasses-validation": {
46+
networkingEnabled: true,
47+
wantAllowed: true,
48+
validationMode: clusterv1beta1.DeleteValidationModeSkip,
49+
},
50+
}
51+
52+
for name, tc := range testCases {
53+
tc := tc
54+
t.Run(name, func(t *testing.T) {
55+
t.Parallel()
56+
57+
mcName := fmt.Sprintf("member-%s", name)
58+
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, mcName)
59+
svcExport := newInternalServiceExport(mcName, namespaceName)
60+
61+
validator := newMemberClusterValidatorForTest(t, tc.networkingEnabled, svcExport)
62+
mc := &clusterv1beta1.MemberCluster{ObjectMeta: metav1.ObjectMeta{Name: mcName}}
63+
mc.Spec.DeleteOptions = &clusterv1beta1.DeleteOptions{ValidationMode: tc.validationMode}
64+
req := buildDeleteRequestFromObject(t, mc)
65+
66+
resp := validator.Handle(context.Background(), req)
67+
if resp.Allowed != tc.wantAllowed {
68+
t.Fatalf("Handle() got response: %+v, want allowed %t", resp, tc.wantAllowed)
69+
}
70+
if tc.wantMessageSubstr != "" {
71+
if resp.Result == nil || !strings.Contains(resp.Result.Message, tc.wantMessageSubstr) {
72+
t.Fatalf("Handle() got response result: %v, want contain: %q", resp.Result, tc.wantMessageSubstr)
73+
}
74+
}
75+
})
76+
}
77+
}
78+
79+
func newMemberClusterValidatorForTest(t *testing.T, networkingEnabled bool, objs ...client.Object) *memberClusterValidator {
80+
t.Helper()
81+
82+
scheme := runtime.NewScheme()
83+
if err := clusterv1beta1.AddToScheme(scheme); err != nil {
84+
t.Fatalf("failed to add member cluster scheme: %v", err)
85+
}
86+
if err := fleetnetworkingv1alpha1.AddToScheme(scheme); err != nil {
87+
t.Fatalf("failed to add fleet networking scheme: %v", err)
88+
}
89+
scheme.AddKnownTypes(fleetnetworkingv1alpha1.GroupVersion,
90+
&fleetnetworkingv1alpha1.InternalServiceExport{},
91+
&fleetnetworkingv1alpha1.InternalServiceExportList{},
92+
)
93+
metav1.AddToGroupVersion(scheme, fleetnetworkingv1alpha1.GroupVersion)
94+
95+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build()
96+
decoder := admission.NewDecoder(scheme)
97+
98+
return &memberClusterValidator{
99+
client: fakeClient,
100+
decoder: decoder,
101+
networkingAgentsEnabled: networkingEnabled,
102+
}
103+
}
104+
105+
func buildDeleteRequestFromObject(t *testing.T, mc *clusterv1beta1.MemberCluster) admission.Request {
106+
t.Helper()
107+
108+
raw, err := json.Marshal(mc)
109+
if err != nil {
110+
t.Fatalf("failed to marshal member cluster: %v", err)
111+
}
112+
113+
return admission.Request{
114+
AdmissionRequest: admissionv1.AdmissionRequest{
115+
Operation: admissionv1.Delete,
116+
Name: mc.Name,
117+
OldObject: runtime.RawExtension{Raw: raw},
118+
},
119+
}
120+
}
121+
122+
func newInternalServiceExport(clusterID, namespace string) *fleetnetworkingv1alpha1.InternalServiceExport {
123+
return &fleetnetworkingv1alpha1.InternalServiceExport{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Name: "sample-service",
126+
Namespace: namespace,
127+
},
128+
Spec: fleetnetworkingv1alpha1.InternalServiceExportSpec{
129+
ServiceReference: fleetnetworkingv1alpha1.ExportedObjectReference{
130+
ClusterID: clusterID,
131+
Kind: "Service",
132+
Namespace: "work",
133+
Name: "sample-service",
134+
ResourceVersion: "1",
135+
Generation: 1,
136+
UID: types.UID("svc-uid"),
137+
NamespacedName: "work/sample-service",
138+
},
139+
},
140+
}
141+
}

‎pkg/webhook/webhook.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,16 @@ var (
131131

132132
var AddToManagerFuncs []func(manager.Manager) error
133133
var AddToManagerFleetResourceValidator func(manager.Manager, []string, bool) error
134+
var AddToManagerMemberclusterValidator func(manager.Manager, bool)
134135

135136
// AddToManager adds all Controllers to the Manager
136-
func AddToManager(m manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool) error {
137+
func AddToManager(m manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool, networkingAgentsEnabled bool) error {
137138
for _, f := range AddToManagerFuncs {
138139
if err := f(m); err != nil {
139140
return err
140141
}
141142
}
143+
AddToManagerMemberclusterValidator(m, networkingAgentsEnabled)
142144
return AddToManagerFleetResourceValidator(m, whiteListedUsers, denyModifyMemberClusterLabels)
143145
}
144146

‎test/e2e/join_and_leave_test.go‎

Lines changed: 3 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ limitations under the License.
1717
package e2e
1818

1919
import (
20-
"errors"
2120
"fmt"
22-
"reflect"
2321

2422
. "github.com/onsi/ginkgo/v2"
2523
. "github.com/onsi/gomega"
@@ -163,34 +161,8 @@ var _ = Describe("Test member cluster join and leave flow", Label("joinleave"),
163161
}
164162
})
165163

166-
It("Should fail the unjoin requests", func() {
167-
for idx := range allMemberClusters {
168-
memberCluster := allMemberClusters[idx]
169-
mcObj := &clusterv1beta1.MemberCluster{
170-
ObjectMeta: metav1.ObjectMeta{
171-
Name: memberCluster.ClusterName,
172-
},
173-
}
174-
err := hubClient.Delete(ctx, mcObj)
175-
Expect(err).ShouldNot(Succeed(), "Want the deletion to be denied")
176-
var statusErr *apierrors.StatusError
177-
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{})))
178-
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("Please delete serviceExport test-namespace/test-svc in the member cluster before leaving, request is denied"))
179-
}
180-
})
181-
182-
It("Deleting the internalServiceExports", func() {
183-
for idx := range allMemberClusterNames {
184-
memberCluster := allMemberClusters[idx]
185-
namespaceName := fmt.Sprintf(utils.NamespaceNameFormat, memberCluster.ClusterName)
186-
187-
internalSvcExportKey := types.NamespacedName{Namespace: namespaceName, Name: internalServiceExportName}
188-
var export fleetnetworkingv1alpha1.InternalServiceExport
189-
Expect(hubClient.Get(ctx, internalSvcExportKey, &export)).Should(Succeed(), "Failed to get internalServiceExport")
190-
Expect(hubClient.Delete(ctx, &export)).To(Succeed(), "Failed to delete internalServiceExport")
191-
}
192-
})
193-
164+
// The network agent is not turned on by default in the e2e so we are still able to leave when we have internalServiceExport
165+
// TODO: add a test case for the network agent is turned on in the fleet network repository.
194166
It("Should be able to trigger the member cluster DELETE", func() {
195167
setAllMemberClustersToLeave()
196168
})
@@ -362,22 +334,7 @@ var _ = Describe("Test member cluster join and leave flow", Label("joinleave"),
362334
}
363335
})
364336

365-
It("Should fail the unjoin requests", func() {
366-
for idx := range allMemberClusters {
367-
memberCluster := allMemberClusters[idx]
368-
mcObj := &clusterv1beta1.MemberCluster{
369-
ObjectMeta: metav1.ObjectMeta{
370-
Name: memberCluster.ClusterName,
371-
},
372-
}
373-
err := hubClient.Delete(ctx, mcObj)
374-
Expect(err).ShouldNot(Succeed(), "Want the deletion to be denied")
375-
var statusErr *apierrors.StatusError
376-
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{})))
377-
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("Please delete serviceExport test-namespace/test-svc in the member cluster before leaving, request is denied"))
378-
}
379-
})
380-
337+
// It does not really matter here as the network agent is not turned on by default in the e2e so we are still able to leave when we have internalServiceExport
381338
It("Updating the member cluster to skip validation", func() {
382339
for idx := range allMemberClusterNames {
383340
memberCluster := allMemberClusters[idx]

0 commit comments

Comments
 (0)