Skip to content

Commit 48929b8

Browse files
author
Eric Stroczynski
authored
[v0.19.x] generate: consider service accounts when generating a CSV (#3610) (#3714)
1 parent 91c499d commit 48929b8

File tree

6 files changed

+735
-28
lines changed

6 files changed

+735
-28
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
entries:
2+
- description: >
3+
Fixed incorrect (cluster) role name assignments in generated CSVs
4+
[#3600](https://github.com/operator-framework/operator-sdk/issues/3600).
5+
kind: bugfix

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

Lines changed: 85 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
admissionregv1 "k8s.io/api/admissionregistration/v1"
2929
appsv1 "k8s.io/api/apps/v1"
3030
corev1 "k8s.io/api/core/v1"
31+
rbacv1 "k8s.io/api/rbac/v1"
3132
"k8s.io/apimachinery/pkg/version"
3233

3334
"github.com/operator-framework/operator-sdk/internal/generate/collector"
@@ -39,11 +40,10 @@ import (
3940
func ApplyTo(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) error {
4041
// Apply manifests to the CSV object.
4142
if err := apply(c, csv); err != nil {
42-
return fmt.Errorf("error updating ClusterServiceVersion: %v", err)
43+
return err
4344
}
4445

45-
// Set fields required by namespaced operators. This is a no-op for cluster-
46-
// scoped operators.
46+
// Set fields required by namespaced operators. This is a no-op for cluster-scoped operators.
4747
setNamespacedFields(csv)
4848

4949
// Sort all updated fields.
@@ -65,7 +65,7 @@ func apply(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion)
6565

6666
applyCustomResourceDefinitions(c, csv)
6767
if err := applyCustomResources(c, csv); err != nil {
68-
return fmt.Errorf("error applying Custom Resource: %v", err)
68+
return fmt.Errorf("error applying Custom Resource examples to CSV %s: %v", csv.GetName(), err)
6969
}
7070
applyWebhooks(c, csv)
7171
return nil
@@ -80,27 +80,93 @@ func getCSVInstallStrategy(csv *operatorsv1alpha1.ClusterServiceVersion) operato
8080
return csv.Spec.InstallStrategy
8181
}
8282

83-
// applyRoles updates strategy's permissions with the Roles in the collector.
84-
func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) {
83+
// This service account exists in every namespace as the default.
84+
const defaultServiceAccountName = "default"
85+
86+
// applyRoles applies Roles to strategy's permissions field by combining Roles bound to ServiceAccounts
87+
// into one set of permissions.
88+
func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl
89+
objs, _ := c.SplitCSVPermissionsObjects()
90+
roleSet := make(map[string]*rbacv1.Role)
91+
for i := range objs {
92+
switch t := objs[i].(type) {
93+
case *rbacv1.Role:
94+
roleSet[t.GetName()] = t
95+
}
96+
}
97+
98+
saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions)
99+
for _, dep := range c.Deployments {
100+
saName := dep.Spec.Template.Spec.ServiceAccountName
101+
if saName == "" {
102+
saName = defaultServiceAccountName
103+
}
104+
saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
105+
}
106+
107+
// Collect all role names by their corresponding service accounts via bindings. This lets us
108+
// look up all service accounts a role is bound to and create one set of permissions per service account.
109+
for _, binding := range c.RoleBindings {
110+
if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole {
111+
for _, subject := range binding.Subjects {
112+
if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" {
113+
perm.Rules = append(perm.Rules, role.Rules...)
114+
saToPermissions[subject.Name] = perm
115+
}
116+
}
117+
}
118+
}
119+
120+
// Apply relevant roles to each service account.
85121
perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
86-
for _, role := range c.Roles {
87-
perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{
88-
ServiceAccountName: role.GetName(),
89-
Rules: role.Rules,
90-
})
122+
for _, perm := range saToPermissions {
123+
if len(perm.Rules) != 0 {
124+
perms = append(perms, perm)
125+
}
91126
}
92127
strategy.Permissions = perms
93128
}
94129

95-
// applyClusterRoles updates strategy's cluserPermissions with the ClusterRoles
96-
// in the collector.
97-
func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) {
130+
// applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field by combining ClusterRoles
131+
// bound to ServiceAccounts into one set of clusterPermissions.
132+
func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl
133+
objs, _ := c.SplitCSVClusterPermissionsObjects()
134+
roleSet := make(map[string]*rbacv1.ClusterRole)
135+
for i := range objs {
136+
switch t := objs[i].(type) {
137+
case *rbacv1.ClusterRole:
138+
roleSet[t.GetName()] = t
139+
}
140+
}
141+
142+
saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions)
143+
for _, dep := range c.Deployments {
144+
saName := dep.Spec.Template.Spec.ServiceAccountName
145+
if saName == "" {
146+
saName = defaultServiceAccountName
147+
}
148+
saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
149+
}
150+
151+
// Collect all role names by their corresponding service accounts via bindings. This lets us
152+
// look up all service accounts a role is bound to and create one set of permissions per service account.
153+
for _, binding := range c.ClusterRoleBindings {
154+
if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole {
155+
for _, subject := range binding.Subjects {
156+
if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" {
157+
perm.Rules = append(perm.Rules, role.Rules...)
158+
saToPermissions[subject.Name] = perm
159+
}
160+
}
161+
}
162+
}
163+
164+
// Apply relevant roles to each service account.
98165
perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
99-
for _, role := range c.ClusterRoles {
100-
perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{
101-
ServiceAccountName: role.GetName(),
102-
Rules: role.Rules,
103-
})
166+
for _, perm := range saToPermissions {
167+
if len(perm.Rules) != 0 {
168+
perms = append(perms, perm)
169+
}
104170
}
105171
strategy.ClusterPermissions = perms
106172
}
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright 2020 The Operator-SDK Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package collector
16+
17+
import (
18+
rbacv1 "k8s.io/api/rbac/v1"
19+
"k8s.io/apimachinery/pkg/runtime"
20+
)
21+
22+
// TODO(estroz): there's a significant amount of code dupliation here, a byproduct of Go's type system.
23+
// However at least a few bits can be refactored so each method is smaller.
24+
25+
const (
26+
// This service account exists in every namespace as the default.
27+
defaultServiceAccountName = "default"
28+
)
29+
30+
// SplitCSVPermissionsObjects splits roles that should be written to a CSV as permissions (in)
31+
// from roles and role bindings that should be written directly to the bundle (out).
32+
func (c *Manifests) SplitCSVPermissionsObjects() (in, out []runtime.Object) { //nolint:dupl
33+
roleMap := make(map[string]*rbacv1.Role)
34+
for i := range c.Roles {
35+
roleMap[c.Roles[i].GetName()] = &c.Roles[i]
36+
}
37+
roleBindingMap := make(map[string]*rbacv1.RoleBinding)
38+
for i := range c.RoleBindings {
39+
roleBindingMap[c.RoleBindings[i].GetName()] = &c.RoleBindings[i]
40+
}
41+
42+
// Check for unbound roles.
43+
for roleName, role := range roleMap {
44+
hasRef := false
45+
for _, roleBinding := range roleBindingMap {
46+
roleRef := roleBinding.RoleRef
47+
if roleRef.Kind == "Role" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
48+
if roleRef.Name == roleName {
49+
hasRef = true
50+
break
51+
}
52+
}
53+
}
54+
if !hasRef {
55+
out = append(out, role)
56+
delete(roleMap, roleName)
57+
}
58+
}
59+
60+
// If a role is bound and:
61+
// 1. the binding only has one subject and it is a service account that maps to a deployment service account,
62+
// add the role to in.
63+
// 2. the binding only has one subject and it does not map to a deployment service account or is not a service account,
64+
// add both role and binding to out.
65+
// 3. the binding has more than one subject and:
66+
// a. one of those subjects is a deployment's service account, add both role and binding to out and role to in.
67+
// b. none of those subjects is a service account or maps to a deployment's service account, add both role and binding to out.
68+
deploymentSANames := make(map[string]struct{})
69+
for _, dep := range c.Deployments {
70+
saName := dep.Spec.Template.Spec.ServiceAccountName
71+
if saName == "" {
72+
saName = defaultServiceAccountName
73+
}
74+
deploymentSANames[saName] = struct{}{}
75+
}
76+
77+
inRoleNames := make(map[string]struct{})
78+
outRoleNames := make(map[string]struct{})
79+
outRoleBindingNames := make(map[string]struct{})
80+
for _, binding := range c.RoleBindings {
81+
roleRef := binding.RoleRef
82+
if roleRef.Kind == roleKind && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
83+
numSubjects := len(binding.Subjects)
84+
if numSubjects == 1 {
85+
// cases (1) and (2).
86+
if _, hasSA := deploymentSANames[binding.Subjects[0].Name]; hasSA && binding.Subjects[0].Kind == serviceAccountKind {
87+
inRoleNames[roleRef.Name] = struct{}{}
88+
} else {
89+
outRoleNames[roleRef.Name] = struct{}{}
90+
outRoleBindingNames[binding.GetName()] = struct{}{}
91+
}
92+
} else {
93+
// case (3).
94+
for _, subject := range binding.Subjects {
95+
if _, hasSA := deploymentSANames[subject.Name]; hasSA && subject.Kind == serviceAccountKind {
96+
// case (3a).
97+
inRoleNames[roleRef.Name] = struct{}{}
98+
}
99+
}
100+
// case (3b).
101+
outRoleNames[roleRef.Name] = struct{}{}
102+
outRoleBindingNames[binding.GetName()] = struct{}{}
103+
}
104+
}
105+
}
106+
107+
for roleName := range inRoleNames {
108+
if role, hasRoleName := roleMap[roleName]; hasRoleName {
109+
in = append(in, role)
110+
}
111+
}
112+
for roleName := range outRoleNames {
113+
if role, hasRoleName := roleMap[roleName]; hasRoleName {
114+
out = append(out, role)
115+
}
116+
}
117+
for roleBindingName := range outRoleBindingNames {
118+
if roleBinding, hasRoleBindingName := roleBindingMap[roleBindingName]; hasRoleBindingName {
119+
out = append(out, roleBinding)
120+
}
121+
}
122+
123+
return in, out
124+
}
125+
126+
// SplitCSVClusterPermissionsObjects splits cluster roles that should be written to a CSV as clusterPermissions (in)
127+
// from cluster roles and cluster role bindings that should be written directly to the bundle (out).
128+
func (c *Manifests) SplitCSVClusterPermissionsObjects() (in, out []runtime.Object) { //nolint:dupl
129+
roleMap := make(map[string]*rbacv1.ClusterRole)
130+
for i := range c.ClusterRoles {
131+
roleMap[c.ClusterRoles[i].GetName()] = &c.ClusterRoles[i]
132+
}
133+
roleBindingMap := make(map[string]*rbacv1.ClusterRoleBinding)
134+
for i := range c.ClusterRoleBindings {
135+
roleBindingMap[c.ClusterRoleBindings[i].GetName()] = &c.ClusterRoleBindings[i]
136+
}
137+
138+
// Check for unbound roles.
139+
for roleName, role := range roleMap {
140+
hasRef := false
141+
for _, roleBinding := range roleBindingMap {
142+
roleRef := roleBinding.RoleRef
143+
if roleRef.Kind == clusterRoleKind && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
144+
if roleRef.Name == roleName {
145+
hasRef = true
146+
break
147+
}
148+
}
149+
}
150+
if !hasRef {
151+
out = append(out, role)
152+
delete(roleMap, roleName)
153+
}
154+
}
155+
156+
// If a role is bound and:
157+
// 1. the binding only has one subject and it is a service account that maps to a deployment service account,
158+
// add the role to in.
159+
// 2. the binding only has one subject and it does not map to a deployment service account or is not a service account,
160+
// add both role and binding to out.
161+
// 3. the binding has more than one subject and:
162+
// a. one of those subjects is a deployment's service account, add both role and binding to out and role to in.
163+
// b. none of those subjects is a service account or maps to a deployment's service account, add both role and binding to out.
164+
deploymentSANames := make(map[string]struct{})
165+
for _, dep := range c.Deployments {
166+
saName := dep.Spec.Template.Spec.ServiceAccountName
167+
if saName == "" {
168+
saName = defaultServiceAccountName
169+
}
170+
deploymentSANames[saName] = struct{}{}
171+
}
172+
173+
inRoleNames := make(map[string]struct{})
174+
outRoleNames := make(map[string]struct{})
175+
outRoleBindingNames := make(map[string]struct{})
176+
for _, binding := range c.ClusterRoleBindings {
177+
roleRef := binding.RoleRef
178+
if roleRef.Kind == "ClusterRole" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
179+
numSubjects := len(binding.Subjects)
180+
if numSubjects == 1 {
181+
// cases (1) and (2).
182+
if _, hasSA := deploymentSANames[binding.Subjects[0].Name]; hasSA && binding.Subjects[0].Kind == serviceAccountKind {
183+
inRoleNames[roleRef.Name] = struct{}{}
184+
} else {
185+
outRoleNames[roleRef.Name] = struct{}{}
186+
outRoleBindingNames[binding.GetName()] = struct{}{}
187+
}
188+
} else {
189+
// case (3).
190+
for _, subject := range binding.Subjects {
191+
if _, hasSA := deploymentSANames[subject.Name]; hasSA && subject.Kind == serviceAccountKind {
192+
// case (3a).
193+
inRoleNames[roleRef.Name] = struct{}{}
194+
}
195+
}
196+
// case (3b).
197+
outRoleNames[roleRef.Name] = struct{}{}
198+
outRoleBindingNames[binding.GetName()] = struct{}{}
199+
}
200+
}
201+
}
202+
203+
for roleName := range inRoleNames {
204+
if role, hasRoleName := roleMap[roleName]; hasRoleName {
205+
in = append(in, role)
206+
}
207+
}
208+
for roleName := range outRoleNames {
209+
if role, hasRoleName := roleMap[roleName]; hasRoleName {
210+
out = append(out, role)
211+
}
212+
}
213+
for roleBindingName := range outRoleBindingNames {
214+
if roleBinding, hasRoleBindingName := roleBindingMap[roleBindingName]; hasRoleBindingName {
215+
out = append(out, roleBinding)
216+
}
217+
}
218+
219+
return in, out
220+
}

0 commit comments

Comments
 (0)