From f0932b97a3ffec0c95e61261ce907a9a27ad192c Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 May 2024 11:20:53 +0200 Subject: [PATCH] apiexports: make SubjectAccessReview and LocalSubjectAccessReview claimable Signed-off-by: Dr. Stefan Schimanski --- .../permissionclaim_labeler.go | 17 ++++ .../apiexport/schemas/builtin/builtin.go | 21 +++++ .../apiexport/virtualworkspace_test.go | 82 ++++++++++++++++--- 3 files changed, 107 insertions(+), 13 deletions(-) diff --git a/pkg/permissionclaim/permissionclaim_labeler.go b/pkg/permissionclaim/permissionclaim_labeler.go index 6ae8b927832..e2b60d28d82 100644 --- a/pkg/permissionclaim/permissionclaim_labeler.go +++ b/pkg/permissionclaim/permissionclaim_labeler.go @@ -22,6 +22,8 @@ import ( "github.com/kcp-dev/logicalcluster/v3" + authenticationv1 "k8s.io/api/authentication/v1" + authorizationv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" @@ -34,6 +36,18 @@ import ( apisv1alpha1informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/apis/v1alpha1" ) +// NonPersistedResourcesClaimable is a list of resources that are not persisted +// to etcd, and therefore should not be labeled with permission claims. The value +// means whether they are claimable or not. +var NonPersistedResourcesClaimable = map[schema.GroupResource]bool{ + authorizationv1.SchemeGroupVersion.WithResource("localsubjectaccessreviews").GroupResource(): true, + authorizationv1.SchemeGroupVersion.WithResource("selfsubjectaccessreviews").GroupResource(): false, + authorizationv1.SchemeGroupVersion.WithResource("selfsubjectrulesreviews").GroupResource(): false, + authorizationv1.SchemeGroupVersion.WithResource("subjectaccessreviews").GroupResource(): true, + authenticationv1.SchemeGroupVersion.WithResource("selfsubjectreviews").GroupResource(): false, + authenticationv1.SchemeGroupVersion.WithResource("tokenreviews").GroupResource(): false, +} + // Labeler calculates labels to apply to all instances of a cluster-group-resource based on permission claims. type Labeler struct { listAPIBindingsAcceptingClaimedGroupResource func(clusterName logicalcluster.Name, groupResource schema.GroupResource) ([]*apisv1alpha1.APIBinding, error) @@ -70,6 +84,9 @@ func NewLabeler( // associated APIExports that are claiming group-resource. func (l *Labeler) LabelsFor(ctx context.Context, cluster logicalcluster.Name, groupResource schema.GroupResource, resourceName string) (map[string]string, error) { labels := map[string]string{} + if _, nonPersisted := NonPersistedResourcesClaimable[groupResource]; nonPersisted { + return labels, nil + } bindings, err := l.listAPIBindingsAcceptingClaimedGroupResource(cluster, groupResource) if err != nil { diff --git a/pkg/virtual/apiexport/schemas/builtin/builtin.go b/pkg/virtual/apiexport/schemas/builtin/builtin.go index b7cfbbc4812..8c7216fcc07 100644 --- a/pkg/virtual/apiexport/schemas/builtin/builtin.go +++ b/pkg/virtual/apiexport/schemas/builtin/builtin.go @@ -21,6 +21,7 @@ import ( admissionregistrationv1 "k8s.io/api/admissionregistration/v1" admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" + authorizationv1 "k8s.io/api/authorization/v1" certificatesv1 "k8s.io/api/certificates/v1" coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" @@ -282,4 +283,24 @@ var BuiltInAPIs = []internalapis.InternalAPI{ ResourceScope: apiextensionsv1.ClusterScoped, HasStatus: true, }, + { + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "subjectaccessreviews", + Singular: "subjectaccessreview", + Kind: "SubjectAccessReview", + }, + GroupVersion: schema.GroupVersion{Group: "authorization.k8s.io", Version: "v1"}, + Instance: &authorizationv1.SubjectAccessReview{}, + ResourceScope: apiextensionsv1.ClusterScoped, + }, + { + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "localsubjectaccessreviews", + Singular: "localsubjectaccessreview", + Kind: "LocalSubjectAccessReview", + }, + GroupVersion: schema.GroupVersion{Group: "authorization.k8s.io", Version: "v1"}, + Instance: &authorizationv1.LocalSubjectAccessReview{}, + ResourceScope: apiextensionsv1.NamespaceScoped, + }, } diff --git a/test/e2e/virtual/apiexport/virtualworkspace_test.go b/test/e2e/virtual/apiexport/virtualworkspace_test.go index d35f5b54373..bc8a414a081 100644 --- a/test/e2e/virtual/apiexport/virtualworkspace_test.go +++ b/test/e2e/virtual/apiexport/virtualworkspace_test.go @@ -33,6 +33,7 @@ import ( "github.com/kcp-dev/logicalcluster/v3" "github.com/stretchr/testify/require" + authorizationv1 "k8s.io/api/authorization/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" extensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" @@ -53,6 +54,7 @@ import ( "sigs.k8s.io/yaml" "github.com/kcp-dev/kcp/config/helpers" + "github.com/kcp-dev/kcp/pkg/permissionclaim" kcpscheme "github.com/kcp-dev/kcp/pkg/server/scheme" apiexportbuiltin "github.com/kcp-dev/kcp/pkg/virtual/apiexport/schemas/builtin" "github.com/kcp-dev/kcp/pkg/virtual/framework/internalapis" @@ -624,6 +626,8 @@ func TestAPIExportPermissionClaims(t *testing.T) { t.Logf("Verify that we get empty lists for all claimed resources (other than apibindings) because the claims have not been accepted yet") dynamicVWClusterClient, err := kcpdynamic.NewForConfig(consumer1VWCfg) require.NoError(t, err) + kubeVWClusterClient, err := kcpkubernetesclientset.NewForConfig(consumer1VWCfg) + require.NoError(t, err) sheriffsGVR := schema.GroupVersionResource{Version: "v1", Resource: "sheriffs", Group: "wild.wild.west"} claimedGVRs := []schema.GroupVersionResource{ {Version: "v1", Resource: "configmaps"}, @@ -631,10 +635,17 @@ func TestAPIExportPermissionClaims(t *testing.T) { {Version: "v1", Resource: "serviceaccounts"}, {Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterroles"}, {Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterrolebindings"}, + {Group: "authorization.k8s.io", Version: "v1", Resource: "subjectaccessreviews"}, + {Group: "authorization.k8s.io", Version: "v1", Resource: "localsubjectaccessreviews"}, sheriffsGVR, apisv1alpha1.SchemeGroupVersion.WithResource("apibindings"), } for _, gvr := range claimedGVRs { + if _, notPersisted := permissionclaim.NonPersistedResourcesClaimable[gvr.GroupResource()]; notPersisted { + // these are create-only + continue + } + t.Logf("Trying to wildcard list %q", gvr) list, err := dynamicVWClusterClient.Resource(gvr).List(ctx, metav1.ListOptions{}) require.NoError(t, err, "error listing %q", gvr) @@ -774,9 +785,45 @@ func TestAPIExportPermissionClaims(t *testing.T) { require.NoError(t, err) return len(list.Items) == 0, fmt.Sprintf("waiting for empty list, got: %#v", list.Items) }, wait.ForeverTestTimeout, 100*time.Millisecond, "expected to eventually get 0 sheriffs") + + t.Logf("Verify that we can do SubjectAccessReview") + sar := &authorizationv1.SubjectAccessReview{ + Spec: authorizationv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Group: "", + Version: "v1", + Resource: "configmaps", + Verb: "list", + Namespace: "default", + }, + User: "elephant", // does not matter here, we only check that the API is available + Groups: []string{"mammals", "system:masters"}, + UID: "123", + }, + } + _, err = kubeVWClusterClient.Cluster(logicalcluster.NewPath(consumer1.Spec.Cluster)).AuthorizationV1().SubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Logf("Verify that we can do LocalSubjectAccessReview") + lsar := &authorizationv1.LocalSubjectAccessReview{ + Spec: authorizationv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Group: "", + Version: "v1", + Resource: "configmaps", + Verb: "list", + Namespace: "default", + }, + User: "elephant", // does not matter here, we only check that the API is available + Groups: []string{"mammals", "system:masters"}, + UID: "123", + }, + } + _, err = kubeVWClusterClient.Cluster(logicalcluster.NewPath(consumer1.Spec.Cluster)).AuthorizationV1().LocalSubjectAccessReviews("default").Create(ctx, lsar, metav1.CreateOptions{}) + require.NoError(t, err) } -func TestAPIExportInternalAPIsDrift(t *testing.T) { +func TestAPIExportClaimableBuiltInAPIsDrift(t *testing.T) { t.Parallel() framework.Suite(t, "control-plane") @@ -786,12 +833,13 @@ func TestAPIExportInternalAPIsDrift(t *testing.T) { discoveryClient, err := kcpdiscovery.NewForConfig(cfg) require.NoError(t, err, "failed to construct discovery client for server") + t.Logf("Creating a normal workspace") orgPath, _ := framework.NewOrganizationFixture(t, server) anyPath, _ := framework.NewWorkspaceFixture(t, server, orgPath) - apis, err := gatherInternalAPIs(t, discoveryClient.Cluster(anyPath)) + t.Logf("Gathering APIs for %s", anyPath) + apis, err := gatherClaimableBuiltInAPIs(t, discoveryClient.Cluster(anyPath)) require.NoError(t, err, "failed to gather built-in apis for server") - sort.Slice(apis, func(i, j int) bool { if apis[i].GroupVersion.String() == apis[j].GroupVersion.String() { return apis[i].Names.Plural < apis[j].Names.Plural @@ -800,7 +848,6 @@ func TestAPIExportInternalAPIsDrift(t *testing.T) { }) expected := apiexportbuiltin.BuiltInAPIs - sort.Slice(expected, func(i, j int) bool { if expected[i].GroupVersion.String() == expected[j].GroupVersion.String() { return expected[i].Names.Plural < expected[j].Names.Plural @@ -808,32 +855,33 @@ func TestAPIExportInternalAPIsDrift(t *testing.T) { return expected[i].GroupVersion.String() < expected[j].GroupVersion.String() }) - require.Empty(t, cmp.Diff(apis, expected)) + require.Empty(t, cmp.Diff(apis, expected), "unexpected internal APIs found") } -func gatherInternalAPIs(t *testing.T, discoveryClient discovery.DiscoveryInterface) ([]internalapis.InternalAPI, error) { +func gatherClaimableBuiltInAPIs(t *testing.T, discoveryClient discovery.DiscoveryInterface) ([]internalapis.InternalAPI, error) { t.Helper() _, apiResourcesLists, err := discoveryClient.ServerGroupsAndResources() if err != nil { return nil, err } - t.Logf("gathering internal apis, found %d", len(apiResourcesLists)) apisByGVK := map[schema.GroupVersionKind]internalapis.InternalAPI{} - for _, apiResourcesList := range apiResourcesLists { gv, err := schema.ParseGroupVersion(apiResourcesList.GroupVersion) require.NoError(t, err) - // ignore kcp resources except for core.kcp.io + + // skip kcp resources except for core.kcp.io if strings.HasSuffix(gv.Group, ".kcp.io") && gv.Group != "core.kcp.io" { continue } - // ignore authn/authz non-crud apis - if gv.Group == "authentication.k8s.io" || gv.Group == "authorization.k8s.io" { - continue - } + for _, apiResource := range apiResourcesList.APIResources { + claimable, notPersisted := permissionclaim.NonPersistedResourcesClaimable[schema.GroupResource{Group: gv.Group, Resource: apiResource.Name}] + if notPersisted && !claimable { + continue + } + gvk := schema.GroupVersionKind{Kind: apiResource.Kind, Version: gv.Version, Group: gv.Group} hasStatus := false @@ -915,6 +963,14 @@ func setUpServiceProviderWithPermissionClaims(ctx context.Context, t *testing.T, GroupResource: apisv1alpha1.GroupResource{Group: "rbac.authorization.k8s.io", Resource: "clusterrolebindings"}, All: true, }, + { + GroupResource: apisv1alpha1.GroupResource{Group: "authorization.k8s.io", Resource: "subjectaccessreviews"}, + All: true, + }, + { + GroupResource: apisv1alpha1.GroupResource{Group: "authorization.k8s.io", Resource: "localsubjectaccessreviews"}, + All: true, + }, { GroupResource: apisv1alpha1.GroupResource{Group: "apis.kcp.io", Resource: "apibindings"}, All: true,