From 7cd16b179c5e29b395c6b52bfb6ff5efa3ec78f5 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 3 Apr 2023 14:16:02 -0700 Subject: [PATCH] Fix integration tests to pass with Kube 1.27/1.28 pre-release builds Fix test failures that occurred in the k8s-main integration test CI job when using Kube 1.27 and 1.28 pre-release builds. --- test/integration/kube_api_discovery_test.go | 15 +++++++++++--- test/integration/rbac_test.go | 22 +++++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/test/integration/kube_api_discovery_test.go b/test/integration/kube_api_discovery_test.go index 538e0a50..71b8219d 100644 --- a/test/integration/kube_api_discovery_test.go +++ b/test/integration/kube_api_discovery_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -458,10 +458,19 @@ func TestGetAPIResourceList(t *testing.T) { //nolint:gocyclo // each t.Run is pr } require.NotNilf(t, actualResourceList, "could not find groupVersion %s", groupVersion) - // Because its hard to predict the storage version hash (e.g. "t/+v41y+3e4="), we just don't - // worry about comparing that field. for i := range actualResourceList.APIResources { + // Because its hard to predict the storage version hash (e.g. "t/+v41y+3e4="), we just don't + // worry about comparing that field. actualResourceList.APIResources[i].StorageVersionHash = "" + + // These fields were empty for a long time but started to be non-empty at some Kubernetes version. + // The filled-in fields were first noticed when CI tested against a 1.27 pre-release. + // To make this test pass on all versions of Kube, just ignore these fields for now. + actualResourceList.APIResources[i].Group = "" + actualResourceList.APIResources[i].Version = "" + if strings.HasSuffix(actualResourceList.APIResources[i].Name, "/status") { + actualResourceList.APIResources[i].SingularName = "" + } } require.ElementsMatch(t, expectedResources, actualResourceList.APIResources, "unexpected API resources") } diff --git a/test/integration/rbac_test.go b/test/integration/rbac_test.go index 598efbc7..e8e01cfb 100644 --- a/test/integration/rbac_test.go +++ b/test/integration/rbac_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -47,9 +47,6 @@ func TestServiceAccountPermissions(t *testing.T) { // the impersonation proxy SA has the same permissions for all checks because it should only be authorized via cluster role bindings expectedResourceRules := []authorizationv1.ResourceRule{ - // system:basic-user is bound to system:authenticated by default - {Verbs: []string{"create"}, APIGroups: []string{"authorization.k8s.io"}, Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}}, - // the expected impersonation permissions {Verbs: []string{"impersonate"}, APIGroups: []string{""}, Resources: []string{"users", "groups", "serviceaccounts"}}, {Verbs: []string{"impersonate"}, APIGroups: []string{"authentication.k8s.io"}, Resources: []string{"*"}}, @@ -59,6 +56,23 @@ func TestServiceAccountPermissions(t *testing.T) { {Verbs: []string{"create", "list"}, APIGroups: []string{"identity.concierge." + env.APIGroupSuffix}, Resources: []string{"whoamirequests"}}, } + // system:basic-user is bound to system:authenticated by default, so the SA gets these permissions too. + // See https://kubernetes.io/docs/reference/access-authn-authz/rbac/#discovery-roles. + // Note that this list previously only included "selfsubjectaccessreviews" and "selfsubjectrulesreviews", + // but later was updated in Kubernetes to also include "selfsubjectreviews". + // Rather than explicitly listing them all as expectations, dynamically append them here, so this test + // can pass against all versions of Kubernetes. + basicUserClusterRole, err := testlib.NewKubernetesClientset(t).RbacV1().ClusterRoles().Get(ctx, "system:basic-user", metav1.GetOptions{}) + require.NoError(t, err) + for _, policyRule := range basicUserClusterRole.Rules { + expectedResourceRules = append(expectedResourceRules, authorizationv1.ResourceRule{ + Verbs: policyRule.Verbs, + APIGroups: policyRule.APIGroups, + Resources: policyRule.Resources, + ResourceNames: policyRule.ResourceNames, + }) + } + if otherPinnipedGroupSuffix := getOtherPinnipedGroupSuffix(t); len(otherPinnipedGroupSuffix) > 0 { expectedResourceRules = append(expectedResourceRules, // we bind these to system:authenticated in the other instance of pinniped