diff --git a/pkg/utils/common.go b/pkg/utils/common.go index 959aeeab4..669268303 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -47,15 +47,16 @@ import ( ) const ( - kubePrefix = "kube-" - fleetPrefix = "fleet-" - FleetSystemNamespace = fleetPrefix + "system" - NamespaceNameFormat = fleetPrefix + "member-%s" - RoleNameFormat = fleetPrefix + "role-%s" - RoleBindingNameFormat = fleetPrefix + "rolebinding-%s" - ValidationPathFmt = "/validate-%s-%s-%s" - lessGroupsStringFormat = "groups: %v" - moreGroupsStringFormat = "groups: [%s, %s, %s,......]" + kubePrefix = "kube-" + fleetPrefix = "fleet-" + fleetMemberNamespacePrefix = fleetPrefix + "member-" + FleetSystemNamespace = fleetPrefix + "system" + NamespaceNameFormat = fleetMemberNamespacePrefix + "%s" + RoleNameFormat = fleetPrefix + "role-%s" + RoleBindingNameFormat = fleetPrefix + "rolebinding-%s" + ValidationPathFmt = "/validate-%s-%s-%s" + lessGroupsStringFormat = "groups: %v" + moreGroupsStringFormat = "groups: [%s, %s, %s,......]" ) const ( @@ -592,6 +593,11 @@ func IsReservedNamespace(namespace string) bool { return strings.HasPrefix(namespace, fleetPrefix) || strings.HasPrefix(namespace, kubePrefix) } +// IsFleetMemberNamespace indicates if an argued namespace is a fleet member namespace. +func IsFleetMemberNamespace(namespace string) bool { + return strings.HasPrefix(namespace, fleetMemberNamespacePrefix) +} + // ShouldPropagateNamespace decides if we should propagate the resources in the namespace. func ShouldPropagateNamespace(namespace string, skippedNamespaces map[string]bool) bool { if IsReservedNamespace(namespace) { diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index 5b8c40ffd..8cfef4fce 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "regexp" - "strings" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/runtime" @@ -24,11 +23,15 @@ import ( const ( // ValidationPath is the webhook service path which admission requests are routed to for validating custom resource definition resources. - ValidationPath = "/validate-fleetresourcehandler" - groupMatch = `^[^.]*\.(.*)` - fleetMemberNamespacePrefix = "fleet-member" - fleetNamespacePrefix = "fleet" - kubeNamespacePrefix = "kube" + ValidationPath = "/validate-fleetresourcehandler" + groupMatch = `^[^.]*\.(.*)` +) + +const ( + // allowed messages. + allowedMessageMemberCluster = "upstream member cluster resource is allowed to be created/deleted by any user" + allowedMessageNonReservedNamespace = "namespace name doesn't begin with fleet-/kube- prefix so we allow all operations on this namespace" + allowedMessageFleetReservedNamespacedResource = "namespace name of resource object doesn't begin with fleet-/kube- prefix so we allow all operations on request objects in these namespace" ) // Add registers the webhook for K8s built-in object types. @@ -138,15 +141,15 @@ func (v *fleetResourceValidator) handleMemberCluster(req admission.Request) admi if isFleetMC { return validation.ValidateUserForResource(req, v.whiteListedUsers) } - klog.V(3).InfoS("upstream member cluster resource is allowed to be created/deleted by any user", + klog.V(3).InfoS(allowedMessageMemberCluster, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "kind", req.RequestKind.Kind, "subResource", req.SubResource, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) - return admission.Allowed("upstream member cluster resource is allowed to be created/deleted by any user") + return admission.Allowed(allowedMessageMemberCluster) } // handleFleetReservedNamespacedResource allows/denies the request to modify object after validation. func (v *fleetResourceValidator) handleFleetReservedNamespacedResource(ctx context.Context, req admission.Request) admission.Response { var response admission.Response - if strings.HasPrefix(req.Namespace, fleetMemberNamespacePrefix) { + if utils.IsFleetMemberNamespace(req.Namespace) { // check to see if valid users other than member agent is making the request. response = validation.ValidateUserForResource(req, v.whiteListedUsers) // check to see if member agent is making the request only on Update. @@ -156,12 +159,12 @@ func (v *fleetResourceValidator) handleFleetReservedNamespacedResource(ctx conte return validation.ValidateMCIdentity(ctx, v.client, req, mcName, v.isFleetV1Beta1API) } return response - } else if strings.HasPrefix(req.Namespace, fleetNamespacePrefix) || strings.HasPrefix(req.Namespace, kubeNamespacePrefix) { + } else if utils.IsReservedNamespace(req.Namespace) { return validation.ValidateUserForResource(req, v.whiteListedUsers) } - klog.V(3).InfoS("namespace name doesn't begin with fleet/kube prefix so we allow all operations on these namespaces", + klog.V(3).InfoS(allowedMessageFleetReservedNamespacedResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "kind", req.RequestKind.Kind, "subResource", req.SubResource, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) - return admission.Allowed("namespace name doesn't begin with fleet/kube prefix so we allow all operations on these namespaces for the request object") + return admission.Allowed(allowedMessageFleetReservedNamespacedResource) } // handleEvent allows/denies request to modify event after validation. @@ -172,13 +175,12 @@ func (v *fleetResourceValidator) handleEvent(_ context.Context, _ admission.Requ // handlerNamespace allows/denies request to modify namespace after validation. func (v *fleetResourceValidator) handleNamespace(req admission.Request) admission.Response { - fleetMatchResult := strings.HasPrefix(req.Name, fleetNamespacePrefix) - kubeMatchResult := strings.HasPrefix(req.Name, kubeNamespacePrefix) - if fleetMatchResult || kubeMatchResult { + if utils.IsReservedNamespace(req.Name) { return validation.ValidateUserForResource(req, v.whiteListedUsers) } - // only handling reserved namespaces with prefix fleet/kube. - return admission.Allowed("namespace name doesn't begin with fleet/kube prefix so we allow all operations on these namespaces") + klog.V(3).InfoS(allowedMessageNonReservedNamespace, + "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "kind", req.RequestKind.Kind, "subResource", req.SubResource, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) + return admission.Allowed(allowedMessageNonReservedNamespace) } // decodeRequestObject decodes the request object into the passed runtime object. diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go index 9697d7d4a..04d036852 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go @@ -518,6 +518,26 @@ func TestHandleMemberCluster(t *testing.T) { resourceValidator fleetResourceValidator wantResponse admission.Response }{ + "allow create upstream fleet MC by any user": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-mc", + Object: runtime.RawExtension{ + Raw: mcObjectBytes, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"test-group"}, + }, + RequestKind: &utils.MCMetaGVK, + Operation: admissionv1.Create, + }, + }, + resourceValidator: fleetResourceValidator{ + decoder: decoder, + }, + wantResponse: admission.Allowed(allowedMessageMemberCluster), + }, "deny update of fleet MC spec by non whitelisted user": { req: admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ @@ -681,7 +701,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { Operation: admissionv1.Create, }, }, - wantResponse: admission.Allowed("namespace name doesn't begin with fleet/kube prefix so we allow all operations on these namespaces for the request object"), + wantResponse: admission.Allowed(allowedMessageFleetReservedNamespacedResource), }, "allow hub-agent-sa in MC identity with create with v1alpha1 IMC": { req: admission.Request{ @@ -953,14 +973,47 @@ func TestHandleNamespace(t *testing.T) { resourceValidator fleetResourceValidator wantResponse admission.Response }{ - "allow user to modify non-reserved namespace": { + "allow any user to modify non-reserved namespace": { req: admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-namespace", - Operation: admissionv1.Create, + Name: "test-namespace", + UserInfo: authenticationv1.UserInfo{ + Username: "testUser", + Groups: []string{"testGroup"}, + }, + RequestKind: &utils.NamespaceMetaGVK, + Operation: admissionv1.Create, + }, + }, + wantResponse: admission.Allowed(allowedMessageNonReservedNamespace), + }, + "allow any user to modify non-reserved namespace without fleet- prefix": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "fleettest", + UserInfo: authenticationv1.UserInfo{ + Username: "testUser", + Groups: []string{"testGroup"}, + }, + RequestKind: &utils.NamespaceMetaGVK, + Operation: admissionv1.Update, + }, + }, + wantResponse: admission.Allowed(allowedMessageNonReservedNamespace), + }, + "allow any user to modify non-reserved namespace without kube- prefix": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "kubetest", + UserInfo: authenticationv1.UserInfo{ + Username: "testUser", + Groups: []string{"testGroup"}, + }, + RequestKind: &utils.NamespaceMetaGVK, + Operation: admissionv1.Delete, }, }, - wantResponse: admission.Allowed("namespace name doesn't begin with fleet/kube prefix so we allow all operations on these namespaces"), + wantResponse: admission.Allowed(allowedMessageNonReservedNamespace), }, "deny user not in system:masters group to modify fleet namespace": { req: admission.Request{