Skip to content

Commit d5e2649

Browse files
author
Jeff Peeler
committed
fix(olm): fix RBAC cluster role creation
specifically in this case the resource names must use the plural form
1 parent b4c1f64 commit d5e2649

File tree

3 files changed

+60
-27
lines changed

3 files changed

+60
-27
lines changed

‎pkg/controller/operators/olm/operator_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,10 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
133133
// Create the new operator
134134
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake)
135135
op := &Operator{
136-
Operator: queueOperator,
137-
client: clientFake,
138-
lister: operatorlister.NewLister(),
139-
resolver: resolver,
136+
Operator: queueOperator,
137+
client: clientFake,
138+
lister: operatorlister.NewLister(),
139+
resolver: resolver,
140140
csvQueues: make(map[string]workqueue.RateLimitingInterface),
141141
recorder: eventRecorder,
142142
}
@@ -2175,13 +2175,12 @@ func TestSyncOperatorGroups(t *testing.T) {
21752175
},
21762176
},
21772177
},
2178-
21792178
clientObjs: []runtime.Object{
21802179
csv("csv1",
21812180
testNS,
21822181
"",
21832182
installStrategy("csv1-dep1", nil, nil),
2184-
[]*v1beta1.CustomResourceDefinition{crd("c1", "v1")},
2183+
[]*v1beta1.CustomResourceDefinition{crd("c1.fake.api.group", "v1")},
21852184
[]*v1beta1.CustomResourceDefinition{},
21862185
v1alpha1.CSVPhaseSucceeded,
21872186
),
@@ -2232,7 +2231,7 @@ func TestSyncOperatorGroups(t *testing.T) {
22322231
testNS,
22332232
"",
22342233
installStrategy("csv1-dep1", nil, nil),
2235-
[]*v1beta1.CustomResourceDefinition{crd("c1", "v1")},
2234+
[]*v1beta1.CustomResourceDefinition{crd("c1.fake.api.group", "v1")},
22362235
[]*v1beta1.CustomResourceDefinition{},
22372236
v1alpha1.CSVPhaseSucceeded,
22382237
),

‎pkg/controller/operators/olm/operatorgroup.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -180,22 +180,18 @@ func (a *Operator) ensureClusterRoles(op *v1alpha2.OperatorGroup) error {
180180
apiEditPolicyRules := []rbacv1.PolicyRule{}
181181
apiViewPolicyRules := []rbacv1.PolicyRule{}
182182
for _, owned := range csv.Spec.CustomResourceDefinitions.Owned {
183-
resourceNames := []string{}
184-
for _, resource := range owned.Resources {
185-
resourceNames = append(resourceNames, resource.Name)
183+
nameGroupPair := strings.SplitN(owned.Name, ".", 2) // -> etcdclusters etcd.database.coreos.com
184+
if len(nameGroupPair) != 2 {
185+
return fmt.Errorf("Invalid parsing of name '%v', got %v", owned.Name, nameGroupPair)
186186
}
187-
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Name}, Resources: resourceNames})
188-
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Name}, Resources: []string{owned.Kind}})
189-
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Name}, Resources: []string{owned.Kind}})
187+
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
188+
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
189+
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
190190
}
191191
for _, owned := range csv.Spec.APIServiceDefinitions.Owned {
192-
resourceNames := []string{}
193-
for _, resource := range owned.Resources {
194-
resourceNames = append(resourceNames, resource.Name)
195-
}
196-
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Group}, Resources: resourceNames})
197-
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Kind}})
198-
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Kind}})
192+
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
193+
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
194+
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
199195
}
200196

201197
clusterRole := &rbacv1.ClusterRole{

‎test/e2e/operator_groups_e2e_test.go

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import (
66
"strings"
77
"testing"
88

9-
appsv1 "k8s.io/api/apps/v1"
10-
"k8s.io/apimachinery/pkg/api/errors"
11-
"k8s.io/apimachinery/pkg/util/wait"
129
"github.com/coreos/go-semver/semver"
1310
"github.com/stretchr/testify/require"
11+
appsv1 "k8s.io/api/apps/v1"
1412
corev1 "k8s.io/api/core/v1"
13+
rbacv1 "k8s.io/api/rbac/v1"
14+
extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
15+
"k8s.io/apimachinery/pkg/api/errors"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17+
"k8s.io/apimachinery/pkg/util/wait"
1618

1719
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
1820
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha2"
@@ -78,6 +80,7 @@ func checkOperatorGroupAnnotations(obj metav1.Object, op *v1alpha2.OperatorGroup
7880

7981
func TestOperatorGroup(t *testing.T) {
8082
// Create namespace with specific label
83+
// Create CRD
8184
// Create CSV in operator namespace
8285
// Create operator group that watches namespace and uses specific label
8386
// Verify operator group status contains correct status
@@ -103,8 +106,16 @@ func TestOperatorGroup(t *testing.T) {
103106

104107
oldCommand := patchOlmDeployment(t, c, otherNamespaceName)
105108

109+
t.Log("Creating CRD")
110+
mainCRDPlural := genName("ins")
111+
apiGroup := "cluster.com"
112+
mainCRDName := mainCRDPlural + "." + apiGroup
113+
mainCRD := newCRD(mainCRDName, testNamespace, mainCRDPlural)
114+
cleanupCRD, err := createCRD(c, mainCRD)
115+
require.NoError(t, err)
116+
106117
t.Log("Creating CSV")
107-
aCSV := newCSV(csvName, testNamespace, "", *semver.New("0.0.0"), nil, nil, newNginxInstallStrategy("operator-deployment", nil, nil))
118+
aCSV := newCSV(csvName, testNamespace, "", *semver.New("0.0.0"), []extv1beta1.CustomResourceDefinition{mainCRD}, nil, newNginxInstallStrategy("operator-deployment", nil, nil))
108119
createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(&aCSV)
109120
require.NoError(t, err)
110121

@@ -119,6 +130,7 @@ func TestOperatorGroup(t *testing.T) {
119130
MatchLabels: matchingLabel,
120131
},
121132
},
133+
//ServiceAccountName: "default-sa",
122134
}
123135
_, err = crc.OperatorsV1alpha2().OperatorGroups(testNamespace).Create(&operatorGroup)
124136
require.NoError(t, err)
@@ -133,12 +145,36 @@ func TestOperatorGroup(t *testing.T) {
133145
return false, fetchErr
134146
}
135147
if len(fetched.Status.Namespaces) > 0 {
136-
require.Equal(t, expectedOperatorGroupStatus.Namespaces[0].Name, fetched.Status.Namespaces[0].Name)
148+
require.EqualValues(t, expectedOperatorGroupStatus.Namespaces[0].Name, fetched.Status.Namespaces[0].Name)
137149
return true, nil
138150
}
139151
return false, nil
140152
})
141153

154+
t.Log("Checking for proper RBAC permissions in target namespace")
155+
roleList, err := c.KubernetesInterface().RbacV1().ClusterRoles().List(metav1.ListOptions{})
156+
for _, item := range roleList.Items {
157+
role, err := c.GetClusterRole(item.GetName())
158+
require.NoError(t, err)
159+
switch roleName := item.GetName(); roleName {
160+
case "owned-crd-manager-another-csv":
161+
managerPolicyRules := []rbacv1.PolicyRule{
162+
rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
163+
}
164+
require.Equal(t, managerPolicyRules, role.Rules)
165+
case "e2e-operator-group-edit":
166+
editPolicyRules := []rbacv1.PolicyRule{
167+
rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
168+
}
169+
require.Equal(t, editPolicyRules, role.Rules)
170+
case "e2e-operator-group-view":
171+
viewPolicyRules := []rbacv1.PolicyRule{
172+
rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
173+
}
174+
require.Equal(t, viewPolicyRules, role.Rules)
175+
}
176+
}
177+
142178
t.Log("Waiting for operator namespace csv to have annotations")
143179
err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
144180
fetchedCSV, fetchErr := crc.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(csvName, metav1.GetOptions{})
@@ -171,8 +207,8 @@ func TestOperatorGroup(t *testing.T) {
171207
require.NoError(t, err)
172208
require.EqualValues(t, v1alpha1.CSVReasonCopied, fetchedCSV.Status.Reason)
173209
// also check name and spec
174-
require.Equal(t, createdCSV.Name, fetchedCSV.Name)
175-
require.Equal(t, createdCSV.Spec, fetchedCSV.Spec)
210+
require.EqualValues(t, createdCSV.Name, fetchedCSV.Name)
211+
require.EqualValues(t, createdCSV.Spec, fetchedCSV.Spec)
176212

177213
t.Log("Waiting on deployment to have correct annotations")
178214
err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
@@ -218,6 +254,8 @@ func TestOperatorGroup(t *testing.T) {
218254
}
219255
require.NoError(t, err)
220256

257+
cleanupCRD()
258+
221259
err = c.KubernetesInterface().CoreV1().Namespaces().Delete(otherNamespaceName, &metav1.DeleteOptions{})
222260
require.NoError(t, err)
223261
err = crc.OperatorsV1alpha2().OperatorGroups(testNamespace).Delete(operatorGroup.Name, &metav1.DeleteOptions{})

0 commit comments

Comments
 (0)