From b9e10a6b0d2d8393b3ccd896b57572c41dd453df Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Mon, 10 Aug 2020 12:10:56 -0700 Subject: [PATCH] cherry-pick #3610 --- changelog/fragments/fix-csv-role-names.yaml | 5 + .../clusterserviceversion_updaters.go | 104 +++++- .../collector/clusterserviceversion.go | 220 ++++++++++++ .../collector/clusterserviceversion_test.go | 329 ++++++++++++++++++ internal/generate/collector/collect.go | 78 ++++- .../collector/collector_suite_test.go | 27 ++ 6 files changed, 735 insertions(+), 28 deletions(-) create mode 100644 changelog/fragments/fix-csv-role-names.yaml create mode 100644 internal/generate/collector/clusterserviceversion.go create mode 100644 internal/generate/collector/clusterserviceversion_test.go create mode 100644 internal/generate/collector/collector_suite_test.go diff --git a/changelog/fragments/fix-csv-role-names.yaml b/changelog/fragments/fix-csv-role-names.yaml new file mode 100644 index 0000000000..ba6c3e0d98 --- /dev/null +++ b/changelog/fragments/fix-csv-role-names.yaml @@ -0,0 +1,5 @@ +entries: + - description: > + Fixed incorrect (cluster) role name assignments in generated CSVs + [#3600](https://github.com/operator-framework/operator-sdk/issues/3600). + kind: bugfix diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go index 73bd0a65be..3b15315178 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go @@ -28,6 +28,7 @@ import ( admissionregv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/version" "github.com/operator-framework/operator-sdk/internal/generate/collector" @@ -39,11 +40,10 @@ import ( func ApplyTo(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) error { // Apply manifests to the CSV object. if err := apply(c, csv); err != nil { - return fmt.Errorf("error updating ClusterServiceVersion: %v", err) + return err } - // Set fields required by namespaced operators. This is a no-op for cluster- - // scoped operators. + // Set fields required by namespaced operators. This is a no-op for cluster-scoped operators. setNamespacedFields(csv) // Sort all updated fields. @@ -65,7 +65,7 @@ func apply(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) applyCustomResourceDefinitions(c, csv) if err := applyCustomResources(c, csv); err != nil { - return fmt.Errorf("error applying Custom Resource: %v", err) + return fmt.Errorf("error applying Custom Resource examples to CSV %s: %v", csv.GetName(), err) } applyWebhooks(c, csv) return nil @@ -80,27 +80,93 @@ func getCSVInstallStrategy(csv *operatorsv1alpha1.ClusterServiceVersion) operato return csv.Spec.InstallStrategy } -// applyRoles updates strategy's permissions with the Roles in the collector. -func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { +// This service account exists in every namespace as the default. +const defaultServiceAccountName = "default" + +// applyRoles applies Roles to strategy's permissions field by combining Roles bound to ServiceAccounts +// into one set of permissions. +func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl + objs, _ := c.SplitCSVPermissionsObjects() + roleSet := make(map[string]*rbacv1.Role) + for i := range objs { + switch t := objs[i].(type) { + case *rbacv1.Role: + roleSet[t.GetName()] = t + } + } + + saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions) + for _, dep := range c.Deployments { + saName := dep.Spec.Template.Spec.ServiceAccountName + if saName == "" { + saName = defaultServiceAccountName + } + saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName} + } + + // Collect all role names by their corresponding service accounts via bindings. This lets us + // look up all service accounts a role is bound to and create one set of permissions per service account. + for _, binding := range c.RoleBindings { + if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole { + for _, subject := range binding.Subjects { + if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" { + perm.Rules = append(perm.Rules, role.Rules...) + saToPermissions[subject.Name] = perm + } + } + } + } + + // Apply relevant roles to each service account. perms := []operatorsv1alpha1.StrategyDeploymentPermissions{} - for _, role := range c.Roles { - perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: role.GetName(), - Rules: role.Rules, - }) + for _, perm := range saToPermissions { + if len(perm.Rules) != 0 { + perms = append(perms, perm) + } } strategy.Permissions = perms } -// applyClusterRoles updates strategy's cluserPermissions with the ClusterRoles -// in the collector. -func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { +// applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field by combining ClusterRoles +// bound to ServiceAccounts into one set of clusterPermissions. +func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl + objs, _ := c.SplitCSVClusterPermissionsObjects() + roleSet := make(map[string]*rbacv1.ClusterRole) + for i := range objs { + switch t := objs[i].(type) { + case *rbacv1.ClusterRole: + roleSet[t.GetName()] = t + } + } + + saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions) + for _, dep := range c.Deployments { + saName := dep.Spec.Template.Spec.ServiceAccountName + if saName == "" { + saName = defaultServiceAccountName + } + saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName} + } + + // Collect all role names by their corresponding service accounts via bindings. This lets us + // look up all service accounts a role is bound to and create one set of permissions per service account. + for _, binding := range c.ClusterRoleBindings { + if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole { + for _, subject := range binding.Subjects { + if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" { + perm.Rules = append(perm.Rules, role.Rules...) + saToPermissions[subject.Name] = perm + } + } + } + } + + // Apply relevant roles to each service account. perms := []operatorsv1alpha1.StrategyDeploymentPermissions{} - for _, role := range c.ClusterRoles { - perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: role.GetName(), - Rules: role.Rules, - }) + for _, perm := range saToPermissions { + if len(perm.Rules) != 0 { + perms = append(perms, perm) + } } strategy.ClusterPermissions = perms } diff --git a/internal/generate/collector/clusterserviceversion.go b/internal/generate/collector/clusterserviceversion.go new file mode 100644 index 0000000000..381cbc4eec --- /dev/null +++ b/internal/generate/collector/clusterserviceversion.go @@ -0,0 +1,220 @@ +// Copyright 2020 The Operator-SDK 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 collector + +import ( + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// TODO(estroz): there's a significant amount of code dupliation here, a byproduct of Go's type system. +// However at least a few bits can be refactored so each method is smaller. + +const ( + // This service account exists in every namespace as the default. + defaultServiceAccountName = "default" +) + +// SplitCSVPermissionsObjects splits roles that should be written to a CSV as permissions (in) +// from roles and role bindings that should be written directly to the bundle (out). +func (c *Manifests) SplitCSVPermissionsObjects() (in, out []runtime.Object) { //nolint:dupl + roleMap := make(map[string]*rbacv1.Role) + for i := range c.Roles { + roleMap[c.Roles[i].GetName()] = &c.Roles[i] + } + roleBindingMap := make(map[string]*rbacv1.RoleBinding) + for i := range c.RoleBindings { + roleBindingMap[c.RoleBindings[i].GetName()] = &c.RoleBindings[i] + } + + // Check for unbound roles. + for roleName, role := range roleMap { + hasRef := false + for _, roleBinding := range roleBindingMap { + roleRef := roleBinding.RoleRef + if roleRef.Kind == "Role" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) { + if roleRef.Name == roleName { + hasRef = true + break + } + } + } + if !hasRef { + out = append(out, role) + delete(roleMap, roleName) + } + } + + // If a role is bound and: + // 1. the binding only has one subject and it is a service account that maps to a deployment service account, + // add the role to in. + // 2. the binding only has one subject and it does not map to a deployment service account or is not a service account, + // add both role and binding to out. + // 3. the binding has more than one subject and: + // a. one of those subjects is a deployment's service account, add both role and binding to out and role to in. + // b. none of those subjects is a service account or maps to a deployment's service account, add both role and binding to out. + deploymentSANames := make(map[string]struct{}) + for _, dep := range c.Deployments { + saName := dep.Spec.Template.Spec.ServiceAccountName + if saName == "" { + saName = defaultServiceAccountName + } + deploymentSANames[saName] = struct{}{} + } + + inRoleNames := make(map[string]struct{}) + outRoleNames := make(map[string]struct{}) + outRoleBindingNames := make(map[string]struct{}) + for _, binding := range c.RoleBindings { + roleRef := binding.RoleRef + if roleRef.Kind == roleKind && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) { + numSubjects := len(binding.Subjects) + if numSubjects == 1 { + // cases (1) and (2). + if _, hasSA := deploymentSANames[binding.Subjects[0].Name]; hasSA && binding.Subjects[0].Kind == serviceAccountKind { + inRoleNames[roleRef.Name] = struct{}{} + } else { + outRoleNames[roleRef.Name] = struct{}{} + outRoleBindingNames[binding.GetName()] = struct{}{} + } + } else { + // case (3). + for _, subject := range binding.Subjects { + if _, hasSA := deploymentSANames[subject.Name]; hasSA && subject.Kind == serviceAccountKind { + // case (3a). + inRoleNames[roleRef.Name] = struct{}{} + } + } + // case (3b). + outRoleNames[roleRef.Name] = struct{}{} + outRoleBindingNames[binding.GetName()] = struct{}{} + } + } + } + + for roleName := range inRoleNames { + if role, hasRoleName := roleMap[roleName]; hasRoleName { + in = append(in, role) + } + } + for roleName := range outRoleNames { + if role, hasRoleName := roleMap[roleName]; hasRoleName { + out = append(out, role) + } + } + for roleBindingName := range outRoleBindingNames { + if roleBinding, hasRoleBindingName := roleBindingMap[roleBindingName]; hasRoleBindingName { + out = append(out, roleBinding) + } + } + + return in, out +} + +// SplitCSVClusterPermissionsObjects splits cluster roles that should be written to a CSV as clusterPermissions (in) +// from cluster roles and cluster role bindings that should be written directly to the bundle (out). +func (c *Manifests) SplitCSVClusterPermissionsObjects() (in, out []runtime.Object) { //nolint:dupl + roleMap := make(map[string]*rbacv1.ClusterRole) + for i := range c.ClusterRoles { + roleMap[c.ClusterRoles[i].GetName()] = &c.ClusterRoles[i] + } + roleBindingMap := make(map[string]*rbacv1.ClusterRoleBinding) + for i := range c.ClusterRoleBindings { + roleBindingMap[c.ClusterRoleBindings[i].GetName()] = &c.ClusterRoleBindings[i] + } + + // Check for unbound roles. + for roleName, role := range roleMap { + hasRef := false + for _, roleBinding := range roleBindingMap { + roleRef := roleBinding.RoleRef + if roleRef.Kind == clusterRoleKind && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) { + if roleRef.Name == roleName { + hasRef = true + break + } + } + } + if !hasRef { + out = append(out, role) + delete(roleMap, roleName) + } + } + + // If a role is bound and: + // 1. the binding only has one subject and it is a service account that maps to a deployment service account, + // add the role to in. + // 2. the binding only has one subject and it does not map to a deployment service account or is not a service account, + // add both role and binding to out. + // 3. the binding has more than one subject and: + // a. one of those subjects is a deployment's service account, add both role and binding to out and role to in. + // b. none of those subjects is a service account or maps to a deployment's service account, add both role and binding to out. + deploymentSANames := make(map[string]struct{}) + for _, dep := range c.Deployments { + saName := dep.Spec.Template.Spec.ServiceAccountName + if saName == "" { + saName = defaultServiceAccountName + } + deploymentSANames[saName] = struct{}{} + } + + inRoleNames := make(map[string]struct{}) + outRoleNames := make(map[string]struct{}) + outRoleBindingNames := make(map[string]struct{}) + for _, binding := range c.ClusterRoleBindings { + roleRef := binding.RoleRef + if roleRef.Kind == "ClusterRole" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) { + numSubjects := len(binding.Subjects) + if numSubjects == 1 { + // cases (1) and (2). + if _, hasSA := deploymentSANames[binding.Subjects[0].Name]; hasSA && binding.Subjects[0].Kind == serviceAccountKind { + inRoleNames[roleRef.Name] = struct{}{} + } else { + outRoleNames[roleRef.Name] = struct{}{} + outRoleBindingNames[binding.GetName()] = struct{}{} + } + } else { + // case (3). + for _, subject := range binding.Subjects { + if _, hasSA := deploymentSANames[subject.Name]; hasSA && subject.Kind == serviceAccountKind { + // case (3a). + inRoleNames[roleRef.Name] = struct{}{} + } + } + // case (3b). + outRoleNames[roleRef.Name] = struct{}{} + outRoleBindingNames[binding.GetName()] = struct{}{} + } + } + } + + for roleName := range inRoleNames { + if role, hasRoleName := roleMap[roleName]; hasRoleName { + in = append(in, role) + } + } + for roleName := range outRoleNames { + if role, hasRoleName := roleMap[roleName]; hasRoleName { + out = append(out, role) + } + } + for roleBindingName := range outRoleBindingNames { + if roleBinding, hasRoleBindingName := roleBindingMap[roleBindingName]; hasRoleBindingName { + out = append(out, roleBinding) + } + } + + return in, out +} diff --git a/internal/generate/collector/clusterserviceversion_test.go b/internal/generate/collector/clusterserviceversion_test.go new file mode 100644 index 0000000000..3ced91e35c --- /dev/null +++ b/internal/generate/collector/clusterserviceversion_test.go @@ -0,0 +1,329 @@ +// Copyright 2020 The Operator-SDK 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. + +//nolint:dupl +package collector + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe("ClusterServiceVersion", func() { + var ( + c *Manifests + in, out []runtime.Object + ) + + BeforeEach(func() { + c = &Manifests{} + }) + + Describe("SplitCSVPermissionsObjects", func() { + + It("should return empty lists for an empty Manifests", func() { + c.Roles = []rbacv1.Role{} + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(0)) + }) + It("should return non-empty lists", func() { + By("splitting 1 Role no RoleBinding") + c.Roles = []rbacv1.Role{newRole("my-role")} + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(1)) + Expect(getRoleNames(out)).To(ContainElement("my-role")) + + By("splitting 1 Role 1 RoleBinding with 1 Subject not containing Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.Roles = []rbacv1.Role{newRole("my-role")} + c.RoleBindings = []rbacv1.RoleBinding{ + newRoleBinding("my-role-binding", newRoleRef("my-role"), newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(2)) + Expect(getRoleNames(out)).To(ContainElement("my-role")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding")) + + By("splitting 1 Role 1 RoleBinding with 1 Subject containing Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.Roles = []rbacv1.Role{newRole("my-role")} + c.RoleBindings = []rbacv1.RoleBinding{ + newRoleBinding("my-role-binding", newRoleRef("my-role"), newServiceAccountSubject("my-dep-account")), + } + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getRoleNames(in)).To(ContainElement("my-role")) + Expect(out).To(HaveLen(0)) + + By("splitting 1 Role 1 RoleBinding with 2 Subjects containing a Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.Roles = []rbacv1.Role{newRole("my-role")} + c.RoleBindings = []rbacv1.RoleBinding{ + newRoleBinding("my-role-binding", + newRoleRef("my-role"), + newServiceAccountSubject("my-dep-account"), newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getRoleNames(in)).To(ContainElement("my-role")) + Expect(out).To(HaveLen(2)) + Expect(getRoleNames(out)).To(ContainElement("my-role")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding")) + + By("splitting 2 Roles 2 RoleBinding, one with 1 Subject not containing and the other with 2 Subjects containing a Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.Roles = []rbacv1.Role{newRole("my-role-1"), newRole("my-role-2")} + c.RoleBindings = []rbacv1.RoleBinding{ + newRoleBinding("my-role-binding-1", + newRoleRef("my-role-1"), + newServiceAccountSubject("my-dep-account"), newServiceAccountSubject("my-other-account")), + newRoleBinding("my-role-binding-2", + newRoleRef("my-role-2"), + newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getRoleNames(in)).To(ContainElement("my-role-1")) + Expect(out).To(HaveLen(4)) + Expect(getRoleNames(out)).To(ContainElement("my-role-1")) + Expect(getRoleNames(out)).To(ContainElement("my-role-2")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding-1")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding-2")) + + By("splitting on 2 different Deployments") + c.Deployments = []appsv1.Deployment{ + newDeploymentWithServiceAccount("my-dep-account-1"), + newDeploymentWithServiceAccount("my-dep-account-2"), + } + c.Roles = []rbacv1.Role{newRole("my-role-1"), newRole("my-role-2"), newRole("my-role-3")} + c.RoleBindings = []rbacv1.RoleBinding{ + newRoleBinding("my-role-binding-1", + newRoleRef("my-role-1"), + newServiceAccountSubject("my-dep-account-1"), newServiceAccountSubject("my-other-account")), + newRoleBinding("my-role-binding-2", + newRoleRef("my-role-2"), + newServiceAccountSubject("my-other-account")), + newRoleBinding("my-role-binding-3", + newRoleRef("my-role-3"), + newServiceAccountSubject("my-dep-account-2")), + } + in, out = c.SplitCSVPermissionsObjects() + Expect(in).To(HaveLen(2)) + Expect(getRoleNames(in)).To(ContainElement("my-role-1")) + Expect(getRoleNames(in)).To(ContainElement("my-role-3")) + Expect(out).To(HaveLen(4)) + Expect(getRoleNames(out)).To(ContainElement("my-role-1")) + Expect(getRoleNames(out)).To(ContainElement("my-role-2")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding-1")) + Expect(getRoleBindingNames(out)).To(ContainElement("my-role-binding-2")) + }) + }) + + Describe("SplitCSVClusterPermissionsObjects", func() { + It("should return empty lists for an empty Manifests", func() { + c.ClusterRoles = []rbacv1.ClusterRole{} + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(0)) + }) + It("should return non-empty lists", func() { + By("splitting 1 ClusterRole no ClusterRoleBinding") + c.ClusterRoles = []rbacv1.ClusterRole{newClusterRole("my-role")} + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(1)) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role")) + + By("splitting 1 ClusterRole 1 ClusterRoleBinding with 1 Subject not containing Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.ClusterRoles = []rbacv1.ClusterRole{newClusterRole("my-role")} + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{ + newClusterRoleBinding("my-role-binding", newClusterRoleRef("my-role"), newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(0)) + Expect(out).To(HaveLen(2)) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding")) + + By("splitting 1 ClusterRole 1 ClusterRoleBinding with 1 Subject containing Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.ClusterRoles = []rbacv1.ClusterRole{newClusterRole("my-role")} + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{ + newClusterRoleBinding("my-role-binding", newClusterRoleRef("my-role"), newServiceAccountSubject("my-dep-account")), + } + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getClusterRoleNames(in)).To(ContainElement("my-role")) + Expect(out).To(HaveLen(0)) + + By("splitting 1 ClusterRole 1 ClusterRoleBinding with 2 Subjects containing a Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.ClusterRoles = []rbacv1.ClusterRole{newClusterRole("my-role")} + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{ + newClusterRoleBinding("my-role-binding", + newClusterRoleRef("my-role"), + newServiceAccountSubject("my-dep-account"), newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getClusterRoleNames(in)).To(ContainElement("my-role")) + Expect(out).To(HaveLen(2)) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding")) + + By("splitting 2 ClusterRoles 2 ClusterRoleBindings, one with 1 Subject not containing and the other with 2 Subjects containing a Deployment serviceAccountName") + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount("my-dep-account")} + c.ClusterRoles = []rbacv1.ClusterRole{newClusterRole("my-role-1"), newClusterRole("my-role-2")} + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{ + newClusterRoleBinding("my-role-binding-1", + newClusterRoleRef("my-role-1"), + newServiceAccountSubject("my-dep-account"), newServiceAccountSubject("my-other-account")), + newClusterRoleBinding("my-role-binding-2", + newClusterRoleRef("my-role-2"), + newServiceAccountSubject("my-other-account")), + } + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(1)) + Expect(getClusterRoleNames(in)).To(ContainElement("my-role-1")) + Expect(out).To(HaveLen(4)) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role-1")) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role-2")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding-1")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding-2")) + + By("splitting on 2 different Deployments") + c.Deployments = []appsv1.Deployment{ + newDeploymentWithServiceAccount("my-dep-account-1"), + newDeploymentWithServiceAccount("my-dep-account-2"), + } + c.ClusterRoles = []rbacv1.ClusterRole{ + newClusterRole("my-role-1"), + newClusterRole("my-role-2"), + newClusterRole("my-role-3"), + } + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{ + newClusterRoleBinding("my-role-binding-1", + newClusterRoleRef("my-role-1"), + newServiceAccountSubject("my-dep-account-1"), newServiceAccountSubject("my-other-account")), + newClusterRoleBinding("my-role-binding-2", + newClusterRoleRef("my-role-2"), + newServiceAccountSubject("my-other-account")), + newClusterRoleBinding("my-role-binding-3", + newClusterRoleRef("my-role-3"), + newServiceAccountSubject("my-dep-account-2")), + } + in, out = c.SplitCSVClusterPermissionsObjects() + Expect(in).To(HaveLen(2)) + Expect(getClusterRoleNames(in)).To(ContainElement("my-role-1")) + Expect(getClusterRoleNames(in)).To(ContainElement("my-role-3")) + Expect(out).To(HaveLen(4)) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role-1")) + Expect(getClusterRoleNames(out)).To(ContainElement("my-role-2")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding-1")) + Expect(getClusterRoleBindingNames(out)).To(ContainElement("my-role-binding-2")) + }) + }) + +}) + +func getRoleNames(objs []runtime.Object) []string { + return getNamesForKind(roleKind, objs) +} + +func getRoleBindingNames(objs []runtime.Object) []string { + return getNamesForKind(roleBindingKind, objs) +} + +func getClusterRoleNames(objs []runtime.Object) []string { + return getNamesForKind(clusterRoleKind, objs) +} + +func getClusterRoleBindingNames(objs []runtime.Object) []string { + return getNamesForKind(clusterRoleBindingKind, objs) +} + +func getNamesForKind(kind string, objs []runtime.Object) (names []string) { + for _, obj := range objs { + if obj.GetObjectKind().GroupVersionKind().Kind == kind { + names = append(names, obj.(metav1.Object).GetName()) + } + } + return +} + +func newDeploymentWithServiceAccount(name string) (d appsv1.Deployment) { + d.Spec.Template.Spec.ServiceAccountName = name + return d +} + +func newRole(name string) (r rbacv1.Role) { + r.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind(roleKind)) + r.SetName(name) + return r +} + +func newClusterRole(name string) (r rbacv1.ClusterRole) { + r.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind(clusterRoleKind)) + r.SetName(name) + return r +} + +func newRoleBinding(name string, ref rbacv1.RoleRef, subjects ...rbacv1.Subject) (r rbacv1.RoleBinding) { + r.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind(roleBindingKind)) + r.SetName(name) + r.RoleRef = ref + r.Subjects = subjects + return r +} + +func newClusterRoleBinding(name string, ref rbacv1.RoleRef, subjects ...rbacv1.Subject) (r rbacv1.ClusterRoleBinding) { + r.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind(clusterRoleBindingKind)) + r.SetName(name) + r.RoleRef = ref + r.Subjects = subjects + return r +} + +func newRef(name, kind, apiGroup string) (s rbacv1.RoleRef) { + s.Name = name + s.Kind = kind + s.APIGroup = apiGroup + return s +} + +func newRoleRef(name string) (s rbacv1.RoleRef) { + return newRef(name, roleKind, rbacv1.SchemeGroupVersion.Group) +} + +func newClusterRoleRef(name string) (s rbacv1.RoleRef) { + return newRef(name, clusterRoleKind, rbacv1.SchemeGroupVersion.Group) +} + +func newSubject(name, kind string) (s rbacv1.Subject) { + s.Name = name + s.Kind = kind + return s +} + +func newServiceAccountSubject(name string) (s rbacv1.Subject) { + return newSubject(name, "ServiceAccount") +} diff --git a/internal/generate/collector/collect.go b/internal/generate/collector/collect.go index 0e672b7d24..30be39f704 100644 --- a/internal/generate/collector/collect.go +++ b/internal/generate/collector/collect.go @@ -25,6 +25,7 @@ import ( log "github.com/sirupsen/logrus" admissionregv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -34,11 +35,22 @@ import ( "github.com/operator-framework/operator-sdk/internal/util/k8sutil" ) +const ( + roleKind = "Role" + roleBindingKind = "RoleBinding" + clusterRoleKind = "ClusterRole" + clusterRoleBindingKind = "ClusterRoleBinding" + serviceAccountKind = "ServiceAccount" +) + // Manifests holds a collector of all manifests relevant to CSV updates. type Manifests struct { Roles []rbacv1.Role ClusterRoles []rbacv1.ClusterRole + RoleBindings []rbacv1.RoleBinding + ClusterRoleBindings []rbacv1.ClusterRoleBinding Deployments []appsv1.Deployment + ServiceAccounts []corev1.ServiceAccount V1CustomResourceDefinitions []apiextv1.CustomResourceDefinition V1beta1CustomResourceDefinitions []apiextv1beta1.CustomResourceDefinition ValidatingWebhooks []admissionregv1.ValidatingWebhook @@ -73,10 +85,16 @@ func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error { gvk := typeMeta.GroupVersionKind() switch gvk.Kind { - case "Role": + case roleKind: err = c.addRoles(manifest) - case "ClusterRole": + case clusterRoleKind: err = c.addClusterRoles(manifest) + case roleBindingKind: + err = c.addRoleBindings(manifest) + case clusterRoleBindingKind: + err = c.addClusterRoleBindings(manifest) + case "ServiceAccount": + err = c.addServiceAccounts(manifest) case "Deployment": err = c.addDeployments(manifest) case "CustomResourceDefinition": @@ -133,10 +151,16 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error { gvk := typeMeta.GroupVersionKind() switch gvk.Kind { - case "Role": + case roleKind: err = c.addRoles(manifest) - case "ClusterRole": + case clusterRoleKind: err = c.addClusterRoles(manifest) + case roleBindingKind: + err = c.addRoleBindings(manifest) + case clusterRoleBindingKind: + err = c.addClusterRoleBindings(manifest) + case "ServiceAccount": + err = c.addServiceAccounts(manifest) case "Deployment": err = c.addDeployments(manifest) case "CustomResourceDefinition": @@ -167,7 +191,7 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error { return nil } -// addRoles assumes add manifest data in rawManifests are Roles and adds them +// addRoles assumes all manifest data in rawManifests are Roles and adds them // to the collector. func (c *Manifests) addRoles(rawManifests ...[]byte) error { for _, rawManifest := range rawManifests { @@ -180,7 +204,7 @@ func (c *Manifests) addRoles(rawManifests ...[]byte) error { return nil } -// addClusterRoles assumes add manifest data in rawManifests are ClusterRoles +// addClusterRoles assumes all manifest data in rawManifests are ClusterRoles // and adds them to the collector. func (c *Manifests) addClusterRoles(rawManifests ...[]byte) error { for _, rawManifest := range rawManifests { @@ -193,7 +217,43 @@ func (c *Manifests) addClusterRoles(rawManifests ...[]byte) error { return nil } -// addDeployments assumes add manifest data in rawManifests are Deployments +// addRoleBindings assumes all manifest data in rawManifests are RoleBindings and adds them to the collector. +func (c *Manifests) addRoleBindings(rawManifests ...[]byte) error { + for _, rawManifest := range rawManifests { + binding := rbacv1.RoleBinding{} + if err := yaml.Unmarshal(rawManifest, &binding); err != nil { + return err + } + c.RoleBindings = append(c.RoleBindings, binding) + } + return nil +} + +// addClusterRoleBindings assumes all manifest data in rawManifests are ClusterRoleBindings and adds them to the collector. +func (c *Manifests) addClusterRoleBindings(rawManifests ...[]byte) error { + for _, rawManifest := range rawManifests { + binding := rbacv1.ClusterRoleBinding{} + if err := yaml.Unmarshal(rawManifest, &binding); err != nil { + return err + } + c.ClusterRoleBindings = append(c.ClusterRoleBindings, binding) + } + return nil +} + +// addServiceAccounts assumes all manifest data in rawManifests are ServiceAccounts and adds them to the collector. +func (c *Manifests) addServiceAccounts(rawManifests ...[]byte) error { + for _, rawManifest := range rawManifests { + sa := corev1.ServiceAccount{} + if err := yaml.Unmarshal(rawManifest, &sa); err != nil { + return err + } + c.ServiceAccounts = append(c.ServiceAccounts, sa) + } + return nil +} + +// addDeployments assumes all manifest data in rawManifests are Deployments // and adds them to the collector. func (c *Manifests) addDeployments(rawManifests ...[]byte) error { for _, rawManifest := range rawManifests { @@ -206,7 +266,7 @@ func (c *Manifests) addDeployments(rawManifests ...[]byte) error { return nil } -// addCustomResourceDefinitions assumes add manifest data in rawManifests are +// addCustomResourceDefinitions assumes all manifest data in rawManifests are // CustomResourceDefinitions and adds them to the collector. version determines // which CustomResourceDefinition type is used for all manifests in rawManifests. func (c *Manifests) addCustomResourceDefinitions(version string, rawManifests ...[]byte) (err error) { @@ -257,7 +317,7 @@ func (c *Manifests) addMutatingWebhookConfigurations(rawManifests ...[]byte) err return nil } -// addOthers assumes add manifest data in rawManifests are able to be +// addOthers assumes all manifest data in rawManifests are able to be // unmarshalled into an Unstructured object and adds them to the collector. func (c *Manifests) addOthers(rawManifests ...[]byte) error { for _, rawManifest := range rawManifests { diff --git a/internal/generate/collector/collector_suite_test.go b/internal/generate/collector/collector_suite_test.go new file mode 100644 index 0000000000..6066452f6d --- /dev/null +++ b/internal/generate/collector/collector_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Operator-SDK 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 collector + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestCollector(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Collector Suite") +}