From f655a700a97c1c22fcec02489d522f93fb48ad3e Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 23 Jan 2023 15:13:37 -0800 Subject: [PATCH] test/extended/authorization/rbac: Condition console RBAC on 'Console' capability Clusters that disable the 'Console' capability are currently failing this test-case [1]: : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less Run #0: Failed expand_less 3s { fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan 3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions: {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]} {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]} {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]} Ginkgo exit error 1: exit with code 1} This commit uses the pattern that 6d1143a0f2 (cli: remove metal3 CRDs when capabilities are none, 2022-04-08, #26998) began using for CRDs to only add the console-linked rules when Console is enabled, and to only add the snapshot-linked rules when `CSISnapshot` is enabled. MicroShift won't have a ClusterVersion custom resource definition, but the test is already failing there [2], so this pivot doesn't break them any worse. Once they have a plan for how they would like to handle it, they can come back and make those changes in follow-up work. It has also been around three years since bb09b26b8c (Add system:authenticated exception for CRDs used by console for extensions, 2019-06-21, #23231)'s "eliminating this exception in the near future", so I'm softening that to "may eventually". Extending system:authenticated is still not a great pattern to follow, but it may never be worth the time it would take the console team to build an alternative mechanism. [1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080 [2]: https://github.com/openshift/origin/pull/27681#discussion_r1088955339 --- .../rbac/groups_default_rules.go | 61 ++++++++++++++----- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/test/extended/authorization/rbac/groups_default_rules.go b/test/extended/authorization/rbac/groups_default_rules.go index f4ee67753993..6021755368f0 100644 --- a/test/extended/authorization/rbac/groups_default_rules.go +++ b/test/extended/authorization/rbac/groups_default_rules.go @@ -26,6 +26,7 @@ import ( "github.com/openshift/api/authorization" "github.com/openshift/api/build" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/api/console" "github.com/openshift/api/image" "github.com/openshift/api/oauth" @@ -118,19 +119,10 @@ var ( rbacv1helpers.NewRule("get", "list").Groups(authzGroup, legacyAuthzGroup).Resources("clusterroles").RuleOrDie(), rbacv1helpers.NewRule(read...).Groups(rbacGroup).Resources("clusterroles").RuleOrDie(), rbacv1helpers.NewRule("get", "list").Groups(storageGroup).Resources("storageclasses").RuleOrDie(), - rbacv1helpers.NewRule("get", "list", "watch").Groups(snapshotGroup).Resources("volumesnapshotclasses").RuleOrDie(), rbacv1helpers.NewRule("list", "watch").Groups(projectGroup, legacyProjectGroup).Resources("projects").RuleOrDie(), rbacv1helpers.NewRule("use").Groups(security.GroupName).Resources("securitycontextconstraints").Names("restricted-v2").RuleOrDie(), - // These custom resources are used to extend console functionality - // The console team is working on eliminating this exception in the near future - rbacv1helpers.NewRule(read...).Groups(consoleGroup).Resources("consoleclidownloads", "consolelinks", "consoleexternalloglinks", "consolenotifications", "consoleyamlsamples", "consolequickstarts", "consoleplugins").RuleOrDie(), - - // HelmChartRepository instances keep Helm chart repository configuration - // By default users are able to browse charts from all configured repositories through console UI - rbacv1helpers.NewRule("get", "list").Groups("helm.openshift.io").Resources("helmchartrepositories").RuleOrDie(), - // TODO: remove when openshift-apiserver has removed these rbacv1helpers.NewRule("get").URLs( "/healthz/", @@ -183,15 +175,52 @@ var _ = g.Describe("[sig-auth][Feature:OpenShiftAuthorization] The default clust oc := exutil.NewCLI("default-rbac-policy") g.It("should have correct RBAC rules", func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + enabledCapabilities := []configv1.ClusterVersionCapability{} + + exist, err := exutil.DoesApiResourceExist(oc.AdminConfig(), "clusterversions", "config.openshift.io") + o.Expect(err).NotTo(o.HaveOccurred()) + if exist { + clusterVersion, err := oc.AdminConfigClient().ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{}) + if err != nil { + e2e.Failf("Failed to get cluster version: %v", err) + } + enabledCapabilities = append(enabledCapabilities, clusterVersion.Status.Capabilities.EnabledCapabilities...) + } else { + e2e.Fail("Cluster version API resource does not exist") + } + + // Conditional, capability-specific rules + for _, capability := range enabledCapabilities { + switch capability { + case configv1.ClusterVersionCapabilityConsole: + allAuthenticatedRules = append( + allAuthenticatedRules, + []rbacv1.PolicyRule{ + // These custom resources are used to extend console functionality + // The console team may eventually eliminate this exception + rbacv1helpers.NewRule(read...).Groups(consoleGroup).Resources("consoleclidownloads", "consolelinks", "consoleexternalloglinks", "consolenotifications", "consoleyamlsamples", "consolequickstarts", "consoleplugins").RuleOrDie(), + + // HelmChartRepository instances keep Helm chart repository configuration + // By default users are able to browse charts from all configured repositories through console UI + rbacv1helpers.NewRule("get", "list").Groups("helm.openshift.io").Resources("helmchartrepositories").RuleOrDie(), + }..., + ) + + case configv1.ClusterVersionCapabilityCSISnapshot: + allAuthenticatedRules = append( + allAuthenticatedRules, + rbacv1helpers.NewRule("get", "list", "watch").Groups(snapshotGroup).Resources("volumesnapshotclasses").RuleOrDie(), + ) + } + } + kubeInformers := informers.NewSharedInformerFactory(oc.AdminKubeClient(), 20*time.Minute) ruleResolver := exutil.NewRuleResolver(kubeInformers.Rbac().V1()) // signal what informers we want to use early - stopCh := make(chan struct{}) - defer func() { close(stopCh) }() - kubeInformers.Start(stopCh) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() + kubeInformers.Start(ctx.Done()) if ok := cache.WaitForCacheSync(ctx.Done(), kubeInformers.Rbac().V1().ClusterRoles().Informer().HasSynced, @@ -202,7 +231,7 @@ var _ = g.Describe("[sig-auth][Feature:OpenShiftAuthorization] The default clust exutil.FatalErr("failed to sync RBAC cache") } - namespaces, err := oc.AdminKubeClient().CoreV1().Namespaces().List(context.Background(), metav1.ListOptions{}) + namespaces, err := oc.AdminKubeClient().CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) if err != nil { exutil.FatalErr(err) }