From 3a780a1c1b535cf155da8c2577cd196b62e2b3ed Mon Sep 17 00:00:00 2001 From: Vinayak Goyal Date: Fri, 24 Jan 2025 02:20:27 +0000 Subject: [PATCH] KEP-2862: Graduate to BETA. --- pkg/features/versioned_kube_features.go | 1 + pkg/kubelet/server/server_test.go | 129 ++++++++++-------- .../testdata/cluster-roles.yaml | 8 ++ test/e2e/feature/feature.go | 2 + test/e2e/framework/auth/helpers.go | 23 ++++ test/e2e_node/kubelet_authz_test.go | 126 +++++++++++++++++ .../test_data/versioned_feature_list.yaml | 4 + 7 files changed, 233 insertions(+), 60 deletions(-) create mode 100644 test/e2e_node/kubelet_authz_test.go diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index 5f49f6b3f47..cfa8b471c22 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -458,6 +458,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate KubeletFineGrainedAuthz: { {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, + {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Beta}, }, KubeletInUserNamespace: { diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index 7540a14fbde..e3a9340962c 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -534,39 +534,44 @@ func TestAuthzCoverage(t *testing.T) { fw := newServerTest() defer fw.testHTTPServer.Close() - // method:path -> has coverage - expectedCases := map[string]bool{} + for _, fineGrained := range []bool{false, true} { + t.Run(fmt.Sprintf("fineGrained=%v", fineGrained), func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletFineGrainedAuthz, fineGrained) + // method:path -> has coverage + expectedCases := map[string]bool{} - // Test all the non-web-service handlers - for _, path := range fw.serverUnderTest.restfulCont.RegisteredHandlePaths() { - expectedCases["GET:"+path] = false - expectedCases["POST:"+path] = false - } + // Test all the non-web-service handlers + for _, path := range fw.serverUnderTest.restfulCont.RegisteredHandlePaths() { + expectedCases["GET:"+path] = false + expectedCases["POST:"+path] = false + } - // Test all the generated web-service paths - for _, ws := range fw.serverUnderTest.restfulCont.RegisteredWebServices() { - for _, r := range ws.Routes() { - expectedCases[r.Method+":"+r.Path] = false - } - } + // Test all the generated web-service paths + for _, ws := range fw.serverUnderTest.restfulCont.RegisteredWebServices() { + for _, r := range ws.Routes() { + expectedCases[r.Method+":"+r.Path] = false + } + } - // This is a sanity check that the Handle->HandleWithFilter() delegation is working - // Ideally, these would move to registered web services and this list would get shorter - expectedPaths := []string{"/healthz", "/metrics", "/metrics/cadvisor"} - for _, expectedPath := range expectedPaths { - if _, expected := expectedCases["GET:"+expectedPath]; !expected { - t.Errorf("Expected registered handle path %s was missing", expectedPath) - } - } + // This is a sanity check that the Handle->HandleWithFilter() delegation is working + // Ideally, these would move to registered web services and this list would get shorter + expectedPaths := []string{"/healthz", "/metrics", "/metrics/cadvisor"} + for _, expectedPath := range expectedPaths { + if _, expected := expectedCases["GET:"+expectedPath]; !expected { + t.Errorf("Expected registered handle path %s was missing", expectedPath) + } + } - for _, tc := range AuthzTestCases(false) { - expectedCases[tc.Method+":"+tc.Path] = true - } + for _, tc := range AuthzTestCases(fineGrained) { + expectedCases[tc.Method+":"+tc.Path] = true + } - for tc, found := range expectedCases { - if !found { - t.Errorf("Missing authz test case for %s", tc) - } + for tc, found := range expectedCases { + if !found { + t.Errorf("Missing authz test case for %s", tc) + } + } + }) } } @@ -580,43 +585,47 @@ func TestAuthFilters(t *testing.T) { attributesGetter := NewNodeAuthorizerAttributesGetter(authzTestNodeName) - for _, tc := range AuthzTestCases(false) { - t.Run(tc.Method+":"+tc.Path, func(t *testing.T) { - var ( - expectedUser = AuthzTestUser() + for _, fineGraned := range []bool{false, true} { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletFineGrainedAuthz, fineGraned) + for _, tc := range AuthzTestCases(fineGraned) { + t.Run(fmt.Sprintf("method=%v:path=%v:fineGrained=%v", tc.Method, tc.Method, fineGraned), func(t *testing.T) { + var ( + expectedUser = AuthzTestUser() - calledAuthenticate = false - calledAuthorize = false - calledAttributes = false - ) + calledAuthenticate = false + calledAuthorize = false + calledAttributes = false + ) - fw.fakeAuth.authenticateFunc = func(req *http.Request) (*authenticator.Response, bool, error) { - calledAuthenticate = true - return &authenticator.Response{User: expectedUser}, true, nil - } - fw.fakeAuth.attributesFunc = func(u user.Info, req *http.Request) []authorizer.Attributes { - calledAttributes = true - require.Equal(t, expectedUser, u) - return attributesGetter.GetRequestAttributes(u, req) - } - fw.fakeAuth.authorizeFunc = func(a authorizer.Attributes) (decision authorizer.Decision, reason string, err error) { - calledAuthorize = true - tc.AssertAttributes(t, []authorizer.Attributes{a}) - return authorizer.DecisionNoOpinion, "", nil - } + fw.fakeAuth.authenticateFunc = func(req *http.Request) (*authenticator.Response, bool, error) { + calledAuthenticate = true + return &authenticator.Response{User: expectedUser}, true, nil + } + fw.fakeAuth.attributesFunc = func(u user.Info, req *http.Request) []authorizer.Attributes { + calledAttributes = true + require.Equal(t, expectedUser, u) + attrs := attributesGetter.GetRequestAttributes(u, req) + tc.AssertAttributes(t, attrs) + return attrs + } + fw.fakeAuth.authorizeFunc = func(a authorizer.Attributes) (decision authorizer.Decision, reason string, err error) { + calledAuthorize = true + return authorizer.DecisionNoOpinion, "", nil + } - req, err := http.NewRequest(tc.Method, fw.testHTTPServer.URL+tc.Path, nil) - require.NoError(t, err) + req, err := http.NewRequest(tc.Method, fw.testHTTPServer.URL+tc.Path, nil) + require.NoError(t, err) - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - defer resp.Body.Close() + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() //nolint:errcheck - assert.Equal(t, http.StatusForbidden, resp.StatusCode) - assert.True(t, calledAuthenticate, "Authenticate was not called") - assert.True(t, calledAttributes, "Attributes were not called") - assert.True(t, calledAuthorize, "Authorize was not called") - }) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.True(t, calledAuthenticate, "Authenticate was not called") + assert.True(t, calledAttributes, "Attributes were not called") + assert.True(t, calledAuthorize, "Authorize was not called") + }) + } } } diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml index cfb27005f85..367b630254e 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml @@ -916,6 +916,14 @@ items: - nodes/stats verbs: - '*' + - apiGroups: + - "" + resources: + - nodes/configz + - nodes/healthz + - nodes/pods + verbs: + - '*' - apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/test/e2e/feature/feature.go b/test/e2e/feature/feature.go index e9439d69d79..9382ffeb503 100644 --- a/test/e2e/feature/feature.go +++ b/test/e2e/feature/feature.go @@ -244,6 +244,8 @@ var ( // TODO: document the feature (owning SIG, when to use this feature for a test) KubeletCredentialProviders = framework.WithFeature(framework.ValidFeatures.Add("KubeletCredentialProviders")) + KubeletFineGrainedAuthz = framework.WithFeature(framework.ValidFeatures.Add("KubeletFineGrainedAuthz")) + // TODO: document the feature (owning SIG, when to use this feature for a test) KubeletSecurity = framework.WithFeature(framework.ValidFeatures.Add("KubeletSecurity")) diff --git a/test/e2e/framework/auth/helpers.go b/test/e2e/framework/auth/helpers.go index 6200d406b3c..28a12c7ba6c 100644 --- a/test/e2e/framework/auth/helpers.go +++ b/test/e2e/framework/auth/helpers.go @@ -43,6 +43,29 @@ type bindingsGetter interface { v1rbac.ClusterRolesGetter } +// WaitForAuthzUpdate checks if the give user can perform named verb and action +// on a resource or subresource. +func WaitForAuthzUpdate(ctx context.Context, c v1authorization.SubjectAccessReviewsGetter, user string, ra *authorizationv1.ResourceAttributes, allowed bool) error { + review := &authorizationv1.SubjectAccessReview{ + Spec: authorizationv1.SubjectAccessReviewSpec{ + ResourceAttributes: ra, + User: user, + }, + } + + err := wait.PollUntilContextTimeout(ctx, policyCachePollInterval, policyCachePollTimeout, false, func(ctx context.Context) (bool, error) { + response, err := c.SubjectAccessReviews().Create(ctx, review, metav1.CreateOptions{}) + if err != nil { + return false, err + } + if response.Status.Allowed != allowed { + return false, nil + } + return true, nil + }) + return err +} + // WaitForAuthorizationUpdate checks if the given user can perform the named verb and action. // If policyCachePollTimeout is reached without the expected condition matching, an error is returned func WaitForAuthorizationUpdate(ctx context.Context, c v1authorization.SubjectAccessReviewsGetter, user, namespace, verb string, resource schema.GroupResource, allowed bool) error { diff --git a/test/e2e_node/kubelet_authz_test.go b/test/e2e_node/kubelet_authz_test.go new file mode 100644 index 00000000000..ff4df6f0661 --- /dev/null +++ b/test/e2e_node/kubelet_authz_test.go @@ -0,0 +1,126 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2enode + +import ( + "context" + "crypto/tls" + "fmt" + "net/http" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + authenticationv1 "k8s.io/api/authentication/v1" + authorizationv1 "k8s.io/api/authorization/v1" + v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/authentication/serviceaccount" + "k8s.io/kubernetes/pkg/cluster/ports" + "k8s.io/kubernetes/test/e2e/feature" + "k8s.io/kubernetes/test/e2e/framework" + e2eauth "k8s.io/kubernetes/test/e2e/framework/auth" +) + +var _ = SIGDescribe("Kubelet Authz", feature.KubeletFineGrainedAuthz, func() { + f := framework.NewDefaultFramework("kubelet-authz-test") + ginkgo.Context("when calling kubelet API", func() { + ginkgo.It("check /healthz enpoint is accessible via nodes/healthz RBAC", func(ctx context.Context) { + sc := runKubeletAuthzTest(ctx, f, "healthz", "healthz") + gomega.Expect(sc).To(gomega.Equal(http.StatusOK)) + }) + ginkgo.It("check /healthz enpoint is accessible via nodes/proxy RBAC", func(ctx context.Context) { + sc := runKubeletAuthzTest(ctx, f, "healthz", "proxy") + gomega.Expect(sc).To(gomega.Equal(http.StatusOK)) + }) + ginkgo.It("check /healthz enpoint is not accessible via nodes/configz RBAC", func(ctx context.Context) { + sc := runKubeletAuthzTest(ctx, f, "healthz", "configz") + gomega.Expect(sc).To(gomega.Equal(http.StatusUnauthorized)) + }) + }) +}) + +func runKubeletAuthzTest(ctx context.Context, f *framework.Framework, endpoint, authzSubresource string) int { + ns := f.Namespace.Name + saName := authzSubresource + crName := authzSubresource + verb := "get" + resource := "nodes" + _, err := f.ClientSet.CoreV1().ServiceAccounts(ns).Create(ctx, &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Namespace: ns, + }, + }, metav1.CreateOptions{}) + framework.ExpectNoError(err) + + _, err = f.ClientSet.RbacV1().ClusterRoles().Create(ctx, &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: crName, + }, + Rules: []rbacv1.PolicyRule{ + { + Verbs: []string{verb}, + Resources: []string{resource + "/" + authzSubresource}, + }, + }, + }, metav1.CreateOptions{}) + framework.ExpectNoError(err) + + subject := rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Namespace: ns, + Name: saName, + } + + err = e2eauth.BindClusterRole(ctx, f.ClientSet.RbacV1(), crName, ns, subject) + framework.ExpectNoError(err) + + err = e2eauth.WaitForAuthzUpdate(ctx, f.ClientSet.AuthorizationV1(), + serviceaccount.MakeUsername(ns, saName), + &authorizationv1.ResourceAttributes{ + Namespace: ns, + Verb: verb, + Resource: resource, + Subresource: authzSubresource, + }, + true, + ) + framework.ExpectNoError(err) + + tr, err := f.ClientSet.CoreV1().ServiceAccounts(ns).CreateToken(ctx, saName, &authenticationv1.TokenRequest{}, metav1.CreateOptions{}) + framework.ExpectNoError(err) + + resp, err := healthCheck(fmt.Sprintf("https://127.0.0.1:%d/%s", ports.KubeletPort, endpoint), tr.Status.Token) + framework.ExpectNoError(err) + return resp.StatusCode +} + +func healthCheck(url, token string) (*http.Response, error) { + insecureTransport := http.DefaultTransport.(*http.Transport).Clone() + insecureTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + insecureHTTPClient := &http.Client{ + Transport: insecureTransport, + } + + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + return insecureHTTPClient.Do(req) +} diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index 33f350f70d6..90c8a6d7895 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -670,6 +670,10 @@ lockToDefault: false preRelease: Alpha version: "1.32" + - default: true + lockToDefault: false + preRelease: Beta + version: "1.33" - name: KubeletInUserNamespace versionedSpecs: - default: false