diff --git a/Makefile b/Makefile index 4bae4cbdb7b..4b06e2c67ff 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,7 @@ container: clean-e2e: kubectl delete crds --all + kubectl delete apiservices.apiregistration.k8s.io v1alpha1.packages.apps.redhat.com || true kubectl delete -f test/e2e/resources/0000_50_olm_00-namespace.yaml clean: diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go index 063071471dd..9b4cd41d830 100644 --- a/pkg/controller/operators/catalog/operator.go +++ b/pkg/controller/operators/catalog/operator.go @@ -388,7 +388,7 @@ func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) { return nil } - + logger.Debug("catsrc configmap state good, checking registry pod") } @@ -398,7 +398,6 @@ func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) { return fmt.Errorf("no reconciler for source type %s", catsrc.Spec.SourceType) } - // if registry pod hasn't been created or hasn't been updated since the last configmap update, recreate it if catsrc.Status.RegistryServiceStatus == nil || catsrc.Status.RegistryServiceStatus.CreatedAt.Before(&catsrc.Status.LastSync) { logger.Debug("registry server scheduled recheck") @@ -545,7 +544,7 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { subs, err := o.lister.OperatorsV1alpha1().SubscriptionLister().Subscriptions(namespace).List(labels.Everything()) if err != nil { - logger.WithError(err).Debug("couldn't list subscriptions") + logger.WithError(err).Debug("couldn't list subscriptions") return err } @@ -752,7 +751,7 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha } else { out.Status.State = v1alpha1.SubscriptionStateAtLatest } - + out.Status.InstalledCSV = sub.Status.CurrentCSV } diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go index bc98a1c0099..e80bb8b464b 100644 --- a/pkg/controller/operators/olm/operator_test.go +++ b/pkg/controller/operators/olm/operator_test.go @@ -2987,7 +2987,7 @@ func TestSyncOperatorGroups(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "csv-role", Namespace: operatorNamespace, - Labels: ownerutil.OwnerLabel(operatorCSV), + Labels: ownerutil.OwnerLabel(operatorCSV, v1alpha1.ClusterServiceVersionKind), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(operatorCSV)}, }, Rules: permissions[0].Rules, @@ -3001,7 +3001,7 @@ func TestSyncOperatorGroups(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "csv-rolebinding", Namespace: operatorNamespace, - Labels: ownerutil.OwnerLabel(operatorCSV), + Labels: ownerutil.OwnerLabel(operatorCSV, v1alpha1.ClusterServiceVersionKind), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(operatorCSV)}, }, Subjects: []rbacv1.Subject{ diff --git a/pkg/controller/operators/olm/operatorgroup.go b/pkg/controller/operators/olm/operatorgroup.go index af77c88bf8a..432466e08f4 100644 --- a/pkg/controller/operators/olm/operatorgroup.go +++ b/pkg/controller/operators/olm/operatorgroup.go @@ -249,30 +249,21 @@ func (a *Operator) ensureClusterRolesForCSV(csv *v1alpha1.ClusterServiceVersion, group := nameGroupPair[1] namePrefix := fmt.Sprintf("%s-%s-", owned.Name, owned.Version) - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, AdminSuffix, VerbsForSuffix[AdminSuffix], group, plural, nil); err != nil { - return err - } - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, EditSuffix, VerbsForSuffix[EditSuffix], group, plural, nil); err != nil { - return err - } - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, ViewSuffix, VerbsForSuffix[ViewSuffix], group, plural, nil); err != nil { - return err + for suffix, verbs := range VerbsForSuffix { + if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, suffix, verbs, group, plural, nil); err != nil { + return err + } } - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix+"-crd", ViewSuffix, []string{"get"}, "apiextensions.k8s.io", "customresourcedefinitions", []string{owned.Name}); err != nil { + if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix+"crd", ViewSuffix, []string{"get"}, "apiextensions.k8s.io", "customresourcedefinitions", []string{owned.Name}); err != nil { return err } } for _, owned := range csv.Spec.APIServiceDefinitions.Owned { namePrefix := fmt.Sprintf("%s-%s-", owned.Name, owned.Version) - - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, AdminSuffix, VerbsForSuffix[AdminSuffix], owned.Group, owned.Name, nil); err != nil { - return err - } - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, EditSuffix, VerbsForSuffix[EditSuffix], owned.Group, owned.Name, nil); err != nil { - return err - } - if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, ViewSuffix, VerbsForSuffix[ViewSuffix], owned.Group, owned.Name, nil); err != nil { - return err + for suffix, verbs := range VerbsForSuffix { + if err := a.ensureProvidedAPIClusterRole(operatorGroup, csv, namePrefix, suffix, verbs, owned.Group, owned.Name, nil); err != nil { + return err + } } } return nil @@ -351,6 +342,9 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C if err != nil { return err } + if len(ownedRoles) == 0 { + return fmt.Errorf("no owned roles found") + } for _, r := range ownedRoles { a.Log.Debug("processing role") @@ -363,7 +357,7 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C }, ObjectMeta: metav1.ObjectMeta{ Name: r.GetName(), - Labels: ownerutil.OwnerLabel(csv), + Labels: r.GetLabels(), }, Rules: r.Rules, } @@ -378,6 +372,9 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C if err != nil { return err } + if len(ownedRoleBindings) == 0 { + return fmt.Errorf("no owned rolebindings found") + } for _, r := range ownedRoleBindings { _, err := a.lister.RbacV1().ClusterRoleBindingLister().Get(r.GetName()) @@ -389,7 +386,7 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C }, ObjectMeta: metav1.ObjectMeta{ Name: r.GetName(), - Labels: ownerutil.OwnerLabel(csv), + Labels: r.GetLabels(), }, Subjects: r.Subjects, RoleRef: rbacv1.RoleRef{ diff --git a/pkg/controller/registry/resolver/rbac.go b/pkg/controller/registry/resolver/rbac.go index 3b84763018d..8d0608ddd95 100644 --- a/pkg/controller/registry/resolver/rbac.go +++ b/pkg/controller/registry/resolver/rbac.go @@ -85,7 +85,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri Name: generateName(csv.GetName()), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, - Labels: ownerutil.OwnerLabel(csv), + Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind), }, Rules: permission.Rules, } @@ -97,7 +97,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri Name: generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, - Labels: ownerutil.OwnerLabel(csv), + Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind), }, RoleRef: rbacv1.RoleRef{ Kind: "Role", @@ -128,7 +128,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri ObjectMeta: metav1.ObjectMeta{ Name: generateName(csv.GetName()), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, - Labels: ownerutil.OwnerLabel(csv), + Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind), }, Rules: permission.Rules, } @@ -140,7 +140,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri Name: generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, - Labels: ownerutil.OwnerLabel(csv), + Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind), }, RoleRef: rbacv1.RoleRef{ Kind: "ClusterRole", diff --git a/pkg/lib/ownerutil/util.go b/pkg/lib/ownerutil/util.go index 1f1b578509d..e2d3a034bc8 100644 --- a/pkg/lib/ownerutil/util.go +++ b/pkg/lib/ownerutil/util.go @@ -187,25 +187,29 @@ func NonBlockingOwner(owner Owner) metav1.OwnerReference { } // OwnerLabel returns a label added to generated objects for later querying -func OwnerLabel(owner Owner) map[string]string { +func OwnerLabel(owner Owner, kind string) map[string]string { return map[string]string{ OwnerKey: owner.GetName(), OwnerNamespaceKey: owner.GetNamespace(), - OwnerKind: owner.GetObjectKind().GroupVersionKind().Kind, + OwnerKind: kind, } } // AddOwnerLabels adds ownerref-like labels to an object -func AddOwnerLabels(object metav1.Object, owner Owner) { +func AddOwnerLabels(object metav1.Object, owner Owner) error { + err := InferGroupVersionKind(owner) + if err != nil { + return err + } labels := object.GetLabels() if labels == nil { labels = map[string]string{} } - for key, val := range OwnerLabel(owner) { + for key, val := range OwnerLabel(owner, owner.GetObjectKind().GroupVersionKind().Kind) { labels[key] = val } object.SetLabels(labels) - return + return nil } // IsOwnedByKindLabel returns whether or not a label exists on the object pointing to an owner of a particular kind @@ -241,7 +245,7 @@ func AdoptableLabels(labels map[string]string, checkName bool, targets ...Owner) // CSVOwnerSelector returns a label selector to find generated objects owned by owner func CSVOwnerSelector(owner *v1alpha1.ClusterServiceVersion) labels.Selector { - return labels.SelectorFromSet(OwnerLabel(owner)) + return labels.SelectorFromSet(OwnerLabel(owner, v1alpha1.ClusterServiceVersionKind)) } // AddOwner adds an owner to the ownerref list. diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go index 4f25b08b314..e9e691e92fe 100644 --- a/test/e2e/installplan_e2e_test.go +++ b/test/e2e/installplan_e2e_test.go @@ -12,6 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/pkg/apis/rbac" @@ -711,11 +712,36 @@ func TestCreateInstallPlanWithPermissions(t *testing.T) { t.Logf("%v, %v: %v && %v", key, expected, strings.HasPrefix(key.Name, expected.Name), key.Kind == expected.Kind) } } + + // This operator was installed into a global operator group, so the roles should have been lifted to clusterroles + if step.Resource.Kind == "Role" { + err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { + _, err = c.GetClusterRole(step.Resource.Name) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil + }) + } + if step.Resource.Kind == "RoleBinding" { + err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { + _, err = c.GetClusterRoleBinding(step.Resource.Name) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil + }) + } } // Should have removed every matching step require.Equal(t, 0, len(expectedSteps), "Actual resource steps do not match expected: %#v", expectedSteps) - } func TestInstallPlanCRDValidation(t *testing.T) { diff --git a/test/e2e/operator_groups_e2e_test.go b/test/e2e/operator_groups_e2e_test.go index 4870cac1f52..7be7ec00bb9 100644 --- a/test/e2e/operator_groups_e2e_test.go +++ b/test/e2e/operator_groups_e2e_test.go @@ -8,12 +8,14 @@ import ( "time" "github.com/coreos/go-semver/semver" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" @@ -1215,12 +1217,12 @@ func TestCSVCopyWatchingAllNamespaces(t *testing.T) { defer func() { c.DeleteServiceAccount(serviceAccount.GetNamespace(), serviceAccount.GetName(), metav1.NewDeleteOptions(0)) }() - _, err = c.CreateRole(role) + createdRole, err := c.CreateRole(role) require.NoError(t, err) defer func() { c.DeleteRole(role.GetNamespace(), role.GetName(), metav1.NewDeleteOptions(0)) }() - _, err = c.CreateRoleBinding(roleBinding) + createdRoleBinding, err := c.CreateRoleBinding(roleBinding) require.NoError(t, err) defer func() { c.DeleteRoleBinding(roleBinding.GetNamespace(), roleBinding.GetName(), metav1.NewDeleteOptions(0)) @@ -1234,10 +1236,47 @@ func TestCSVCopyWatchingAllNamespaces(t *testing.T) { createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(opGroupNamespace).Create(&aCSV) require.NoError(t, err) + ownerutil.AddOwnerLabels(createdRole, createdCSV) + _, err = c.UpdateRole(createdRole) + require.NoError(t, err) + + ownerutil.AddOwnerLabels(createdRoleBinding, createdCSV) + _, err = c.UpdateRoleBinding(createdRoleBinding) + require.NoError(t, err) + t.Log("wait for CSV to succeed") _, err = fetchCSV(t, crc, createdCSV.GetName(), opGroupNamespace, csvSucceededChecker) require.NoError(t, err) + t.Log("wait for roles to be promoted to clusterroles") + var fetchedRole *rbacv1.ClusterRole + err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { + fetchedRole, err = c.GetClusterRole(role.GetName()) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil + }) + require.EqualValues(t, role.Rules, fetchedRole.Rules) + var fetchedRoleBinding *rbacv1.ClusterRoleBinding + err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { + fetchedRoleBinding, err = c.GetClusterRoleBinding(roleBinding.GetName()) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return true, nil + }) + require.EqualValues(t, roleBinding.Subjects, fetchedRoleBinding.Subjects) + require.EqualValues(t, roleBinding.RoleRef.Name, fetchedRoleBinding.RoleRef.Name) + require.EqualValues(t, "rbac.authorization.k8s.io", fetchedRoleBinding.RoleRef.APIGroup) + require.EqualValues(t, "ClusterRole", fetchedRoleBinding.RoleRef.Kind) + t.Log("Waiting for operator namespace csv to have annotations") err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { fetchedCSV, fetchErr := crc.OperatorsV1alpha1().ClusterServiceVersions(opGroupNamespace).Get(csvName, metav1.GetOptions{})