Skip to content

Commit

Permalink
fix(olm): fix RBAC cluster role creation
Browse files Browse the repository at this point in the history 
specifically in this case the resource names must use the plural form
  • Loading branch information
Jeff Peeler committed Nov 13, 2018
1 parent 4570fa5 commit 5933442
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 27 deletions.
13 changes: 6 additions & 7 deletions pkg/controller/operators/olm/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
// Create the new operator
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake)
op := &Operator{
Operator: queueOperator,
client: clientFake,
lister: operatorlister.NewLister(),
resolver: resolver,
Operator: queueOperator,
client: clientFake,
lister: operatorlister.NewLister(),
resolver: resolver,
csvQueues: make(map[string]workqueue.RateLimitingInterface),
recorder: eventRecorder,
}
Expand Down Expand Up @@ -2175,13 +2175,12 @@ func TestSyncOperatorGroups(t *testing.T) {
},
},
},

clientObjs: []runtime.Object{
csv("csv1",
testNS,
"",
installStrategy("csv1-dep1", nil, nil),
[]*v1beta1.CustomResourceDefinition{crd("c1", "v1")},
[]*v1beta1.CustomResourceDefinition{crd("c1.fake.api.group", "v1")},
[]*v1beta1.CustomResourceDefinition{},
v1alpha1.CSVPhaseSucceeded,
),
Expand Down Expand Up @@ -2232,7 +2231,7 @@ func TestSyncOperatorGroups(t *testing.T) {
testNS,
"",
installStrategy("csv1-dep1", nil, nil),
[]*v1beta1.CustomResourceDefinition{crd("c1", "v1")},
[]*v1beta1.CustomResourceDefinition{crd("c1.fake.api.group", "v1")},
[]*v1beta1.CustomResourceDefinition{},
v1alpha1.CSVPhaseSucceeded,
),
Expand Down
22 changes: 9 additions & 13 deletions pkg/controller/operators/olm/operatorgroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,22 +180,18 @@ func (a *Operator) ensureClusterRoles(op *v1alpha2.OperatorGroup) error {
apiEditPolicyRules := []rbacv1.PolicyRule{}
apiViewPolicyRules := []rbacv1.PolicyRule{}
for _, owned := range csv.Spec.CustomResourceDefinitions.Owned {
resourceNames := []string{}
for _, resource := range owned.Resources {
resourceNames = append(resourceNames, resource.Name)
nameGroupPair := strings.SplitN(owned.Name, ".", 2) // -> etcdclusters etcd.database.coreos.com
if len(nameGroupPair) != 2 {
return fmt.Errorf("Invalid parsing of name '%v', got %v", owned.Name, nameGroupPair)
}
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Name}, Resources: resourceNames})
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Name}, Resources: []string{owned.Kind}})
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Name}, Resources: []string{owned.Kind}})
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{nameGroupPair[1]}, Resources: []string{nameGroupPair[0]}})
}
for _, owned := range csv.Spec.APIServiceDefinitions.Owned {
resourceNames := []string{}
for _, resource := range owned.Resources {
resourceNames = append(resourceNames, resource.Name)
}
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Group}, Resources: resourceNames})
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Kind}})
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Kind}})
managerPolicyRules = append(managerPolicyRules, rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
apiEditPolicyRules = append(apiEditPolicyRules, rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
apiViewPolicyRules = append(apiViewPolicyRules, rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{owned.Group}, Resources: []string{owned.Name}})
}

clusterRole := &rbacv1.ClusterRole{
Expand Down
52 changes: 45 additions & 7 deletions test/e2e/operator_groups_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import (
"strings"
"testing"

appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"
"github.com/coreos/go-semver/semver"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"

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

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

oldCommand := patchOlmDeployment(t, c, otherNamespaceName)

t.Log("Creating CRD")
mainCRDPlural := genName("ins")
apiGroup := "cluster.com"
mainCRDName := mainCRDPlural + "." + apiGroup
mainCRD := newCRD(mainCRDName, testNamespace, mainCRDPlural)
cleanupCRD, err := createCRD(c, mainCRD)
require.NoError(t, err)

t.Log("Creating CSV")
aCSV := newCSV(csvName, testNamespace, "", *semver.New("0.0.0"), nil, nil, newNginxInstallStrategy("operator-deployment", nil, nil))
aCSV := newCSV(csvName, testNamespace, "", *semver.New("0.0.0"), []extv1beta1.CustomResourceDefinition{mainCRD}, nil, newNginxInstallStrategy("operator-deployment", nil, nil))
createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(&aCSV)
require.NoError(t, err)

Expand All @@ -119,6 +130,7 @@ func TestOperatorGroup(t *testing.T) {
MatchLabels: matchingLabel,
},
},
//ServiceAccountName: "default-sa",
}
_, err = crc.OperatorsV1alpha2().OperatorGroups(testNamespace).Create(&operatorGroup)
require.NoError(t, err)
Expand All @@ -133,12 +145,36 @@ func TestOperatorGroup(t *testing.T) {
return false, fetchErr
}
if len(fetched.Status.Namespaces) > 0 {
require.Equal(t, expectedOperatorGroupStatus.Namespaces[0].Name, fetched.Status.Namespaces[0].Name)
require.EqualValues(t, expectedOperatorGroupStatus.Namespaces[0].Name, fetched.Status.Namespaces[0].Name)
return true, nil
}
return false, nil
})

t.Log("Checking for proper RBAC permissions in target namespace")
roleList, err := c.KubernetesInterface().RbacV1().ClusterRoles().List(metav1.ListOptions{})
for _, item := range roleList.Items {
role, err := c.GetClusterRole(item.GetName())
require.NoError(t, err)
switch roleName := item.GetName(); roleName {
case "owned-crd-manager-another-csv":
managerPolicyRules := []rbacv1.PolicyRule{
rbacv1.PolicyRule{Verbs: []string{"*"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
}
require.Equal(t, managerPolicyRules, role.Rules)
case "e2e-operator-group-edit":
editPolicyRules := []rbacv1.PolicyRule{
rbacv1.PolicyRule{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
}
require.Equal(t, editPolicyRules, role.Rules)
case "e2e-operator-group-view":
viewPolicyRules := []rbacv1.PolicyRule{
rbacv1.PolicyRule{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
}
require.Equal(t, viewPolicyRules, role.Rules)
}
}

t.Log("Waiting for operator namespace csv to have annotations")
err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
fetchedCSV, fetchErr := crc.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(csvName, metav1.GetOptions{})
Expand Down Expand Up @@ -171,8 +207,8 @@ func TestOperatorGroup(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, v1alpha1.CSVReasonCopied, fetchedCSV.Status.Reason)
// also check name and spec
require.Equal(t, createdCSV.Name, fetchedCSV.Name)
require.Equal(t, createdCSV.Spec, fetchedCSV.Spec)
require.EqualValues(t, createdCSV.Name, fetchedCSV.Name)
require.EqualValues(t, createdCSV.Spec, fetchedCSV.Spec)

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

cleanupCRD()

err = c.KubernetesInterface().CoreV1().Namespaces().Delete(otherNamespaceName, &metav1.DeleteOptions{})
require.NoError(t, err)
err = crc.OperatorsV1alpha2().OperatorGroups(testNamespace).Delete(operatorGroup.Name, &metav1.DeleteOptions{})
Expand Down

0 comments on commit 5933442

Please sign in to comment.