diff --git a/charts/hub-agent/templates/deployment.yaml b/charts/hub-agent/templates/deployment.yaml index e7b238ba1..fd2102000 100644 --- a/charts/hub-agent/templates/deployment.yaml +++ b/charts/hub-agent/templates/deployment.yaml @@ -22,6 +22,7 @@ spec: args: - --leader-elect=true - --enable-webhook={{ .Values.enableWebhook }} + - --whitelisted-users=system:serviceaccount:fleet-system:hub-agent-sa - --webhook-client-connection-type={{.Values.webhookClientConnectionType}} - --v={{ .Values.logVerbosity }} - -add_dir_header diff --git a/cmd/hubagent/main.go b/cmd/hubagent/main.go index 6df887790..9254993c2 100644 --- a/cmd/hubagent/main.go +++ b/cmd/hubagent/main.go @@ -8,6 +8,7 @@ package main import ( "flag" "os" + "strings" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -109,7 +110,8 @@ func main() { } if opts.EnableWebhook { - if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType)); err != nil { + whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",") + if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), whiteListedUsers); err != nil { klog.ErrorS(err, "unable to set up webhook") exitWithErrorFunc() } @@ -130,7 +132,7 @@ func main() { } // SetupWebhook generates the webhook cert and then set up the webhook configurator. -func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType) error { +func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, whiteListedUsers []string) error { // Generate self-signed key and crt files in FleetWebhookCertDir for the webhook server to start. w, err := webhook.NewWebhookConfig(mgr, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir) if err != nil { @@ -141,7 +143,7 @@ func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.Webho klog.ErrorS(err, "unable to add WebhookConfig") return err } - if err = webhook.AddToManager(mgr); err != nil { + if err = webhook.AddToManager(mgr, whiteListedUsers); err != nil { klog.ErrorS(err, "unable to register webhooks to the manager") return err } diff --git a/cmd/hubagent/options/options.go b/cmd/hubagent/options/options.go index 7a5039577..c91852de5 100644 --- a/cmd/hubagent/options/options.go +++ b/cmd/hubagent/options/options.go @@ -35,6 +35,8 @@ type Options struct { MetricsBindAddress string // EnableWebhook indicates if we will run a webhook EnableWebhook bool + // WhiteListedUsers indicates the list of user who are allowed to modify fleet resources + WhiteListedUsers string // Sets the connection type for the webhook. WebhookClientConnectionType string // NetworkingAgentsEnabled indicates if we enable network agents @@ -91,6 +93,7 @@ func (o *Options) AddFlags(flags *flag.FlagSet) { flags.DurationVar(&o.LeaderElection.LeaseDuration.Duration, "leader-lease-duration", 15*time.Second, "This is effectively the maximum duration that a leader can be stopped before someone else will replace it.") flag.StringVar(&o.LeaderElection.ResourceNamespace, "leader-election-namespace", utils.FleetSystemNamespace, "The namespace in which the leader election resource will be created.") flag.BoolVar(&o.EnableWebhook, "enable-webhook", true, "If set, the fleet webhook is enabled.") + flag.StringVar(&o.WhiteListedUsers, "whitelisted-users", "", "If set, white listed users can modify fleet related resources.") flag.StringVar(&o.WebhookClientConnectionType, "webhook-client-connection-type", "url", "Sets the connection type used by the webhook client. Only URL or Service is valid.") flag.BoolVar(&o.NetworkingAgentsEnabled, "networking-agents-enabled", false, "Whether the networking agents are enabled or not.") flags.DurationVar(&o.ClusterUnhealthyThreshold.Duration, "cluster-unhealthy-threshold", 60*time.Second, "The duration for a member cluster to be in a degraded state before considered unhealthy.") diff --git a/pkg/webhook/add_clusterresourceplacement.go b/pkg/webhook/add_clusterresourceplacement.go deleted file mode 100644 index b9cc50737..000000000 --- a/pkg/webhook/add_clusterresourceplacement.go +++ /dev/null @@ -1,15 +0,0 @@ -/* -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -*/ - -package webhook - -import ( - "go.goms.io/fleet/pkg/webhook/clusterresourceplacement" -) - -func init() { - // AddToManagerFuncs is a list of functions to create webhook and add them to a manager. - AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add) -} diff --git a/pkg/webhook/add_fleetresourcehandler.go b/pkg/webhook/add_fleetresourcehandler.go deleted file mode 100644 index 3172bcc19..000000000 --- a/pkg/webhook/add_fleetresourcehandler.go +++ /dev/null @@ -1,8 +0,0 @@ -package webhook - -import "go.goms.io/fleet/pkg/webhook/fleetresourcehandler" - -func init() { - // AddToManagerFuncs is a list of functions to create webhook and add them to a manager. - AddToManagerFuncs = append(AddToManagerFuncs, fleetresourcehandler.Add) -} diff --git a/pkg/webhook/add_handler.go b/pkg/webhook/add_handler.go new file mode 100644 index 000000000..b135606b5 --- /dev/null +++ b/pkg/webhook/add_handler.go @@ -0,0 +1,16 @@ +package webhook + +import ( + "go.goms.io/fleet/pkg/webhook/clusterresourceplacement" + "go.goms.io/fleet/pkg/webhook/fleetresourcehandler" + "go.goms.io/fleet/pkg/webhook/pod" + "go.goms.io/fleet/pkg/webhook/replicaset" +) + +func init() { + // AddToManagerFuncs is a list of functions to create webhook and add them to a manager. + AddToManagerFuncs = append(AddToManagerFuncs, fleetresourcehandler.Add) + AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add) + AddToManagerFuncs = append(AddToManagerFuncs, pod.Add) + AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add) +} diff --git a/pkg/webhook/add_pod.go b/pkg/webhook/add_pod.go deleted file mode 100644 index c5a82fa86..000000000 --- a/pkg/webhook/add_pod.go +++ /dev/null @@ -1,14 +0,0 @@ -/* -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -*/ - -package webhook - -import ( - "go.goms.io/fleet/pkg/webhook/pod" -) - -func init() { - AddToManagerFuncs = append(AddToManagerFuncs, pod.Add) -} diff --git a/pkg/webhook/add_replicaset.go b/pkg/webhook/add_replicaset.go deleted file mode 100644 index 9d1e37e90..000000000 --- a/pkg/webhook/add_replicaset.go +++ /dev/null @@ -1,14 +0,0 @@ -/* -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -*/ - -package webhook - -import ( - "go.goms.io/fleet/pkg/webhook/replicaset" -) - -func init() { - AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add) -} diff --git a/pkg/webhook/clusterresourceplacement/clusterresourceplacement_webhook.go b/pkg/webhook/clusterresourceplacement/clusterresourceplacement_webhook.go index 5e1a2a4b6..15fbe500f 100644 --- a/pkg/webhook/clusterresourceplacement/clusterresourceplacement_webhook.go +++ b/pkg/webhook/clusterresourceplacement/clusterresourceplacement_webhook.go @@ -16,6 +16,6 @@ const ( ValidationPath = "/validate-fleet-azure-com-v1alpha1-clusterresourceplacement" ) -func Add(mgr manager.Manager) error { +func Add(mgr manager.Manager, _ []string) error { return (&fleetv1alpha1.ClusterResourcePlacement{}).SetupWebhookWithManager(mgr) } diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index 99640070c..bc9650b03 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -9,41 +9,48 @@ import ( admissionv1 "k8s.io/api/admission/v1" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/webhook/validation" ) const ( // ValidationPath is the webhook service path which admission requests are routed to for validating custom resource definition resources. - ValidationPath = "/validate-v1-fleetresourcehandler" - groupMatch = `^[^.]*\.(.*)` - crdKind = "CustomResourceDefinition" + ValidationPath = "/validate-v1-fleetresourcehandler" + groupMatch = `^[^.]*\.(.*)` + crdKind = "CustomResourceDefinition" + memberClusterKind = "MemberCluster" ) // Add registers the webhook for K8s bulit-in object types. -func Add(mgr manager.Manager) error { +func Add(mgr manager.Manager, whiteListedUsers []string) error { hookServer := mgr.GetWebhookServer() - hookServer.Register(ValidationPath, &webhook.Admission{Handler: &fleetResourceValidator{Client: mgr.GetClient()}}) + hookServer.Register(ValidationPath, &webhook.Admission{Handler: &fleetResourceValidator{client: mgr.GetClient(), whiteListedUsers: whiteListedUsers}}) return nil } type fleetResourceValidator struct { - Client client.Client - decoder *admission.Decoder + client client.Client + whiteListedUsers []string + decoder *admission.Decoder } -func (v *fleetResourceValidator) Handle(_ context.Context, req admission.Request) admission.Response { +func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Request) admission.Response { var response admission.Response if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update || req.Operation == admissionv1.Delete { switch req.Kind { case createCRDGVK(): klog.V(2).InfoS("handling CRD resource", "GVK", createCRDGVK()) response = v.handleCRD(req) + case createMemberClusterGVK(): + klog.V(2).InfoS("handling Member cluster resource", "GVK", createMemberClusterGVK()) + response = v.handleMemberCluster(ctx, req) default: klog.V(2).InfoS("resource is not monitored by fleet resource validator webhook", "GVK", req.Kind.String()) response = admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify resource with GVK: %s", req.UserInfo.Username, req.UserInfo.Groups, req.Kind.String())) @@ -54,25 +61,50 @@ func (v *fleetResourceValidator) Handle(_ context.Context, req admission.Request func (v *fleetResourceValidator) handleCRD(req admission.Request) admission.Response { var crd v1.CustomResourceDefinition + if err := v.decodeRequestObject(req, &crd); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + // This regex works because every CRD name in kubernetes follows this pattern .. + group := regexp.MustCompile(groupMatch).FindStringSubmatch(crd.Name)[1] + if validation.CheckCRDGroup(group) && !validation.ValidateUserForCRD(v.whiteListedUsers, req.UserInfo) { + return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify fleet CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) + } + return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) +} + +func (v *fleetResourceValidator) handleMemberCluster(ctx context.Context, req admission.Request) admission.Response { + var mc fleetv1alpha1.MemberCluster + if err := v.decodeRequestObject(req, &mc); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if !validation.ValidateUserForFleetCR(ctx, v.client, v.whiteListedUsers, req.UserInfo) { + return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify member cluster CR: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) + } + klog.V(2).InfoS("user in groups is allowed to modify member cluster CR", "user", req.UserInfo.Username, "groups", req.UserInfo.Groups) + return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify member cluster: %s", req.UserInfo.Username, req.UserInfo.Groups, mc.Name)) +} + +func (v *fleetResourceValidator) decodeRequestObject(req admission.Request, obj runtime.Object) error { if req.Operation == admissionv1.Delete { // req.Object is not populated for delete: https://github.com/kubernetes-sigs/controller-runtime/issues/1762. - if err := v.decoder.DecodeRaw(req.OldObject, &crd); err != nil { + if err := v.decoder.DecodeRaw(req.OldObject, obj); err != nil { klog.ErrorS(err, "failed to decode old request object for delete operation", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Errored(http.StatusBadRequest, err) + return err } } else { - if err := v.decoder.Decode(req, &crd); err != nil { + if err := v.decoder.Decode(req, obj); err != nil { klog.ErrorS(err, "failed to decode request object for create/update operation", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Errored(http.StatusBadRequest, err) + return err } } + return nil +} - // This regex works because every CRD name in kubernetes follows this pattern .. - group := regexp.MustCompile(groupMatch).FindStringSubmatch(crd.Name)[1] - if validation.CheckCRDGroup(group) && !validation.ValidateUserForCRD(req.UserInfo) { - return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify fleet CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) - } - return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) +func (v *fleetResourceValidator) InjectDecoder(d *admission.Decoder) error { + v.decoder = d + return nil } func createCRDGVK() metav1.GroupVersionKind { @@ -83,7 +115,10 @@ func createCRDGVK() metav1.GroupVersionKind { } } -func (v *fleetResourceValidator) InjectDecoder(d *admission.Decoder) error { - v.decoder = d - return nil +func createMemberClusterGVK() metav1.GroupVersionKind { + return metav1.GroupVersionKind{ + Group: fleetv1alpha1.GroupVersion.Group, + Version: fleetv1alpha1.GroupVersion.Version, + Kind: memberClusterKind, + } } diff --git a/pkg/webhook/pod/pod_validating_webhook.go b/pkg/webhook/pod/pod_validating_webhook.go index 347cf92aa..2cc937540 100644 --- a/pkg/webhook/pod/pod_validating_webhook.go +++ b/pkg/webhook/pod/pod_validating_webhook.go @@ -26,7 +26,7 @@ const ( ) // Add registers the webhook for K8s bulit-in object types. -func Add(mgr manager.Manager) error { +func Add(mgr manager.Manager, _ []string) error { hookServer := mgr.GetWebhookServer() hookServer.Register(ValidationPath, &webhook.Admission{Handler: &podValidator{Client: mgr.GetClient()}}) return nil diff --git a/pkg/webhook/replicaset/replicaset_validating_webhook.go b/pkg/webhook/replicaset/replicaset_validating_webhook.go index c5b92110f..1c747dc74 100644 --- a/pkg/webhook/replicaset/replicaset_validating_webhook.go +++ b/pkg/webhook/replicaset/replicaset_validating_webhook.go @@ -31,7 +31,7 @@ type replicaSetValidator struct { } // Add registers the webhook for K8s bulit-in object types. -func Add(mgr manager.Manager) error { +func Add(mgr manager.Manager, _ []string) error { hookServer := mgr.GetWebhookServer() hookServer.Register(ValidationPath, &webhook.Admission{Handler: &replicaSetValidator{Client: mgr.GetClient()}}) return nil diff --git a/pkg/webhook/validation/uservalidation.go b/pkg/webhook/validation/uservalidation.go index cc4e7ec01..41a88d99d 100644 --- a/pkg/webhook/validation/uservalidation.go +++ b/pkg/webhook/validation/uservalidation.go @@ -1,17 +1,43 @@ package validation import ( + "context" + authenticationv1 "k8s.io/api/authentication/v1" + "k8s.io/klog/v2" "k8s.io/utils/strings/slices" + "sigs.k8s.io/controller-runtime/pkg/client" + + fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" ) const ( mastersGroup = "system:masters" ) -// TODO:(Arvindthiru) Get valid usernames as flag and allow those usernames. - // ValidateUserForCRD checks to see if user is authenticated to make a request to modify fleet CRDs. -func ValidateUserForCRD(userInfo authenticationv1.UserInfo) bool { - return slices.Contains(userInfo.Groups, mastersGroup) +func ValidateUserForCRD(whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { + return isMasterGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) +} + +// ValidateUserForFleetCR checks to see if user is authenticated to make a request to modify Fleet CRs. +func ValidateUserForFleetCR(ctx context.Context, client client.Client, whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { + if isMasterGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) { + return true + } + var memberClusterList fleetv1alpha1.MemberClusterList + if err := client.List(ctx, &memberClusterList); err != nil { + klog.V(2).ErrorS(err, "failed to list member clusters") + return false + } + identities := make([]string, len(memberClusterList.Items)) + for i := range memberClusterList.Items { + identities = append(identities, memberClusterList.Items[i].Spec.Identity.Name) + } + // this ensures will allow all member agents are validated. + return slices.Contains(identities, userInfo.Username) +} + +func isMasterGroupUserOrWhiteListedUser(whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { + return slices.Contains(whiteListedUsers, userInfo.Username) || slices.Contains(userInfo.Groups, mastersGroup) } diff --git a/pkg/webhook/validation/uservalidation_test.go b/pkg/webhook/validation/uservalidation_test.go index d21d1a883..0ecaf05ab 100644 --- a/pkg/webhook/validation/uservalidation_test.go +++ b/pkg/webhook/validation/uservalidation_test.go @@ -7,26 +7,95 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/stretchr/testify/assert" v1 "k8s.io/api/authentication/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1" "go.goms.io/fleet/pkg/utils" ) func TestValidateUserForCRD(t *testing.T) { testCases := map[string]struct { - client client.Client - userInfo v1.UserInfo - wantResult bool + userInfo v1.UserInfo + whiteListedUsers []string + wantResult bool }{ "allow user in system:masters group": { + userInfo: v1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + wantResult: true, + }, + "allow white listed user not in system:masters group": { + userInfo: v1.UserInfo{ + Username: "test-user", + }, + whiteListedUsers: []string{"test-user"}, + wantResult: true, + }, + "fail to validate user with invalid username, groups": { + userInfo: v1.UserInfo{ + Username: "test-user", + Groups: []string{"test-group"}, + }, + wantResult: false, + }, + } + + for testName, testCase := range testCases { + t.Run(testName, func(t *testing.T) { + gotResult := ValidateUserForCRD(testCase.whiteListedUsers, testCase.userInfo) + assert.Equal(t, testCase.wantResult, gotResult, utils.TestCaseMsg, testName) + }) + } +} + +func TestValidateUserForFleetCR(t *testing.T) { + testCases := map[string]struct { + client client.Client + whiteListedUsers []string + userInfo v1.UserInfo + wantResult bool + }{ + "allow use in system:masters group": { + userInfo: v1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + wantResult: true, + }, + "allow white listed user not in system:masters group": { + userInfo: v1.UserInfo{ + Username: "test-user", + }, + whiteListedUsers: []string{"test-user"}, + wantResult: true, + }, + "allow member cluster identity": { client: &test.MockClient{ MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + o := list.(*fleetv1alpha1.MemberClusterList) + *o = fleetv1alpha1.MemberClusterList{ + Items: []fleetv1alpha1.MemberCluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + Spec: fleetv1alpha1.MemberClusterSpec{ + Identity: rbacv1.Subject{ + Name: "member-cluster-identity", + }, + }, + }, + }, + } return nil }, }, userInfo: v1.UserInfo{ - Username: "test-user", - Groups: []string{"system:masters"}, + Username: "member-cluster-identity", }, wantResult: true, }, @@ -46,7 +115,7 @@ func TestValidateUserForCRD(t *testing.T) { for testName, testCase := range testCases { t.Run(testName, func(t *testing.T) { - gotResult := ValidateUserForCRD(testCase.userInfo) + gotResult := ValidateUserForFleetCR(context.Background(), testCase.client, testCase.whiteListedUsers, testCase.userInfo) assert.Equal(t, testCase.wantResult, gotResult, utils.TestCaseMsg, testName) }) } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 97e151ca1..ee99349dc 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -46,21 +46,22 @@ const ( FleetWebhookCfgName = "fleet-validating-webhook-configuration" FleetWebhookSvcName = "fleetwebhook" - crdResourceName = "customresourcedefinitions" - replicaSetResourceName = "replicasets" - podResourceName = "pods" + crdResourceName = "customresourcedefinitions" + memberClusterResourceName = "memberclusters" + replicaSetResourceName = "replicasets" + podResourceName = "pods" ) var ( admissionReviewVersions = []string{admv1.SchemeGroupVersion.Version, admv1beta1.SchemeGroupVersion.Version} ) -var AddToManagerFuncs []func(manager.Manager) error +var AddToManagerFuncs []func(manager.Manager, []string) error // AddToManager adds all Controllers to the Manager -func AddToManager(m manager.Manager) error { +func AddToManager(m manager.Manager, whiteListedUsers []string) error { for _, f := range AddToManagerFuncs { - if err := f(m); err != nil { + if err := f(m, whiteListedUsers); err != nil { return err } } @@ -113,7 +114,7 @@ func (w *Config) Start(ctx context.Context) error { // createFleetWebhookConfiguration creates the ValidatingWebhookConfiguration object for the webhook. func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { - failPolicy := admv1.Fail + failPolicy := admv1.Ignore sideEffortsNone := admv1.SideEffectClassNone namespacedScope := admv1.NamespacedScope clusterScope := admv1.ClusterScope @@ -211,6 +212,28 @@ func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error { }, }, }, + { + Name: "fleet.membercluster.validating", + ClientConfig: w.createClientConfig(fleetv1alpha1.MemberCluster{}), + FailurePolicy: &failPolicy, + SideEffects: &sideEffortsNone, + AdmissionReviewVersions: admissionReviewVersions, + Rules: []admv1.RuleWithOperations{ + { + Operations: []admv1.OperationType{ + admv1.Create, + admv1.Update, + admv1.Delete, + }, + Rule: admv1.Rule{ + APIGroups: []string{fleetv1alpha1.GroupVersion.Group}, + APIVersions: []string{fleetv1alpha1.GroupVersion.Version}, + Resources: []string{memberClusterResourceName}, + Scope: &clusterScope, + }, + }, + }, + }, }, } @@ -261,6 +284,9 @@ func (w *Config) createClientConfig(webhookInterface interface{}) admv1.WebhookC case v1.CustomResourceDefinition: serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) + case fleetv1alpha1.MemberCluster: + serviceEndpoint = w.serviceURL + fleetresourcehandler.ValidationPath + serviceRef.Path = pointer.String(fleetresourcehandler.ValidationPath) } config := admv1.WebhookClientConfig{ diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 9f9046d7f..c12a1131e 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -45,6 +45,7 @@ const ( testUserClusterRoleBinding = "test-user-cluster-role-binding" crdStatusErrFormat = `failed to validate user: %s in groups: %v to modify fleet CRD: %s` + mcStatusErrFormat = `failed to validate user: %s in groups: %v to modify member cluster CR: %s` ) var _ = Describe("Fleet's Hub cluster webhook tests", func() { @@ -603,3 +604,134 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() { }) }) }) + +var _ = Describe("Fleet's CR Resource Handler webhook tests", func() { + BeforeEach(func() { + By("create cluster role to modify fleet CRs") + cr := rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUserClusterRole, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{fleetv1alpha1.GroupVersion.Group}, + Verbs: []string{"*"}, + Resources: []string{"*"}, + }, + }, + } + Expect(HubCluster.KubeClient.Create(ctx, &cr)).Should(Succeed()) + + By("create cluster role binding for test-user to modify fleet CRs") + crb := rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUserClusterRoleBinding, + }, + Subjects: []rbacv1.Subject{ + { + APIGroup: rbacv1.GroupName, + Kind: "User", + Name: testUser, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: testUserClusterRole, + }, + } + Expect(HubCluster.KubeClient.Create(ctx, &crb)).Should(Succeed()) + }) + + AfterEach(func() { + By("remove cluster role binding") + crb := rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUserClusterRoleBinding, + }, + } + Expect(HubCluster.KubeClient.Delete(ctx, &crb)).Should(Succeed()) + + By("remove cluster role") + cr := rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUserClusterRole, + }, + } + Expect(HubCluster.KubeClient.Delete(ctx, &cr)).Should(Succeed()) + }) + + Context("CR validation webhook", func() { + It("should deny CREATE operation on member cluster CR for user not in system:masters group", func() { + mc := fleetv1alpha1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-member-cluster", + }, + Spec: fleetv1alpha1.MemberClusterSpec{ + State: fleetv1alpha1.ClusterStateJoin, + Identity: rbacv1.Subject{ + Kind: "User", + APIGroup: "", + Name: "test-subject", + Namespace: "fleet-system", + }, + }, + } + + By("expecting denial of operation CREATE of member cluster") + err := HubCluster.ImpersonateKubeClient.Create(ctx, &mc) + fmt.Println(err) + var statusErr *k8sErrors.StatusError + Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) + Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) + }) + + It("should deny UPDATE operation on member cluster CR for user not in system:masters group", func() { + var mc fleetv1alpha1.MemberCluster + Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: MemberCluster.ClusterName}, &mc)).Should(Succeed()) + + By("update member cluster spec") + mc.Spec.State = fleetv1alpha1.ClusterStateLeave + + By("expecting denial of operation UPDATE of member cluster") + err := HubCluster.ImpersonateKubeClient.Update(ctx, &mc) + var statusErr *k8sErrors.StatusError + fmt.Println(err.Error()) + Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Update member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) + Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) + }) + + It("should deny DELETE operation on member cluster CR for user not in system:masters group", func() { + mc := fleetv1alpha1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: MemberCluster.ClusterName, + }, + } + + By("expecting denial of operation DELETE of member cluster") + err := HubCluster.ImpersonateKubeClient.Delete(ctx, &mc) + var statusErr *k8sErrors.StatusError + Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Delete member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) + Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(mcStatusErrFormat, testUser, testGroups, mc.Name))) + }) + + It("should allow update operation on member cluster CR for user in system:masters group", func() { + var mc fleetv1alpha1.MemberCluster + Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: MemberCluster.ClusterName}, &mc)).Should(Succeed()) + + By("update labels in CRD") + labels := make(map[string]string) + labels[testKey] = testValue + mc.SetLabels(labels) + + By("expecting denial of operation UPDATE of CRD") + // The user associated with KubeClient is kubernetes-admin in groups: [system:masters, system:authenticated] + Expect(HubCluster.KubeClient.Update(ctx, &mc)).To(Succeed()) + + By("remove new label added for test") + labels = mc.GetLabels() + delete(labels, testKey) + mc.SetLabels(labels) + }) + }) +})