Skip to content

OPRUN-4056: [OTE] Migrate preflight checks from openshift/origin #423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,64 @@
[
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {services} are not specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {create} verb is not specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {ClusterRoleBindings} are not specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {ConfigMap:resourceNames} are not all specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {clusterextension/finalizer} is not specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks should report error when {escalate, bind} is not specified",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:olmv1",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[sig-olmv1] OLMv1 should pass a trivial sanity check",
"labels": {},
Expand Down
248 changes: 248 additions & 0 deletions openshift/tests-extension/test/olmv1-preflight.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
package test

import (
"context"
"fmt"
"time"

//nolint:staticcheck // ST1001: dot-imports for readability
. "github.com/onsi/ginkgo/v2"
//nolint:staticcheck // ST1001: dot-imports for readability
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
"sigs.k8s.io/controller-runtime/pkg/client"

olmv1 "github.com/operator-framework/operator-controller/api/v1"

"github/operator-framework-operator-controller/openshift/tests-extension/pkg/env"
"github/operator-framework-operator-controller/openshift/tests-extension/pkg/helpers"
)

var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMPreflightPermissionChecks][Skipped:Disconnected] OLMv1 operator preflight checks", func() {
var (
namespace string
k8sClient client.Client
)
BeforeEach(func() {
helpers.RequireOLMv1CapabilityOnOpenshift()
k8sClient = env.Get().K8sClient
namespace = "preflight-test-ns-" + rand.String(4)

By(fmt.Sprintf("creating namespace %s", namespace))
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
},
}
Expect(k8sClient.Create(context.Background(), ns)).To(Succeed(), "failed to create test namespace")
DeferCleanup(func() {
_ = k8sClient.Delete(context.Background(), ns)
})
})

It("should report error when {services} are not specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 0, namespace)
})

It("should report error when {create} verb is not specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 1, namespace)
})

It("should report error when {ClusterRoleBindings} are not specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 2, namespace)
})

It("should report error when {ConfigMap:resourceNames} are not all specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 3, namespace)
})

It("should report error when {clusterextension/finalizer} is not specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 4, namespace)
})

It("should report error when {escalate, bind} is not specified", func(ctx SpecContext) {
runNegativePreflightTest(ctx, 5, namespace)
})
})

// runNegativePreflightTest creates a deficient ClusterRole and a ClusterExtension that
// relies on it, then waits for the expected preflight failure.
func runNegativePreflightTest(ctx context.Context, scenario int, namespace string) {
k8sClient := env.Get().K8sClient
unique := rand.String(8)

// Define names
crName := fmt.Sprintf("install-test-cr-%s", unique)
saName := fmt.Sprintf("install-test-sa-%s", unique)
crbName := fmt.Sprintf("install-test-crb-%s", unique)
ceName := fmt.Sprintf("install-test-ce-%s", unique)

// Step 1: Create deficient ClusterRole
defCR := createDeficientClusterRole(scenario, crName, ceName)
Expect(k8sClient.Create(ctx, defCR)).To(Succeed(), "failed to create ClusterRole")
DeferCleanup(func(ctx SpecContext) {
_ = k8sClient.Delete(ctx, defCR)
})

// Step 2: Create matching ServiceAccount
sa := helpers.NewServiceAccount(saName, namespace)
Expect(k8sClient.Create(ctx, sa)).To(Succeed(), "failed to create ServiceAccount")
DeferCleanup(func(ctx SpecContext) {
_ = k8sClient.Delete(ctx, sa)
})

// Step 3: Bind SA to the deficient ClusterRole
crb := helpers.NewClusterRoleBinding(crbName, crName, saName, namespace)
Expect(k8sClient.Create(ctx, crb)).To(Succeed(), "failed to create ClusterRoleBinding")
DeferCleanup(func(ctx SpecContext) {
_ = k8sClient.Delete(ctx, crb)
})

// Step 4: Create ClusterExtension referencing that SA
ce := helpers.NewClusterExtensionObject("openshift-pipelines-operator-rh", "1.15.0", ceName, saName, namespace)
Expect(k8sClient.Create(ctx, ce)).To(Succeed(), "failed to create ClusterExtension")
DeferCleanup(func(ctx SpecContext) {
_ = k8sClient.Delete(ctx, ce)
})

// Step 5: Wait for failure
By("waiting for ClusterExtension to report preflight failure")
Eventually(func(g Gomega) {
latest := &olmv1.ClusterExtension{}
err := k8sClient.Get(ctx, client.ObjectKey{Name: ce.Name}, latest)
g.Expect(err).NotTo(HaveOccurred())

c := meta.FindStatusCondition(latest.Status.Conditions, "Progressing")
g.Expect(c).NotTo(BeNil())
g.Expect(c.Status).To(Equal(metav1.ConditionTrue))
g.Expect(c.Message).To(ContainSubstring("pre-authorization failed"))

c = meta.FindStatusCondition(latest.Status.Conditions, "Installed")
g.Expect(c).NotTo(BeNil())
g.Expect(c.Status).To(Equal(metav1.ConditionFalse))
}).WithTimeout(5 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
}

// createDeficientClusterRole returns a modified ClusterRole according to the test scenario.
func createDeficientClusterRole(scenario int, name, ceName string) *rbacv1.ClusterRole {
baseRules := []rbacv1.PolicyRule{
{
APIGroups: []string{"olm.operatorframework.io"},
Resources: []string{"clusterextensions/finalizers"},
Verbs: []string{"update"},
ResourceNames: []string{ceName},
},
{
APIGroups: []string{""},
Resources: []string{"nodes"},
Verbs: []string{"list"},
},
{
APIGroups: []string{""},
Resources: []string{"pods", "pods/finalizers", "services", "services/finalizers", "endpoints", "endpoints/finalizers", "persistentvolumeclaims", "persistentvolumeclaims/finalizers", "events", "events/finalizers", "configmaps", "configmaps/finalizers", "secrets", "secrets/finalizers", "pods/log", "limitranges", "limitranges/finalizers", "namespaces", "namespaces/finalizers", "serviceaccounts", "serviceaccounts/finalizers"},
Verbs: []string{"delete", "deletecollection", "create", "patch", "get", "list", "update", "watch"},
},
{
APIGroups: []string{"rbac.authorization.k8s.io"},
Resources: []string{"clusterroles", "clusterroles/finalizers", "roles", "roles/finalizers", "clusterrolebindings", "clusterrolebindings/finalizers", "rolebindings", "rolebindings/finalizers"},
Verbs: []string{"delete", "deletecollection", "create", "patch", "get", "list", "update", "watch", "bind", "escalate"},
},
}

// Copy rules to avoid mutation
rules := make([]rbacv1.PolicyRule, len(baseRules))
copy(rules, baseRules)

switch scenario {
case 0:
// Remove 'services' and 'services/finalizers'
for i, r := range rules {
if r.APIGroups[0] == "" {
filtered := []string{}
for _, res := range r.Resources {
if res != "services" && res != "services/finalizers" {
filtered = append(filtered, res)
}
}
rules[i].Resources = filtered
}
}
case 1:
// Remove 'create' verb
for i, r := range rules {
if r.APIGroups[0] == "" {
filtered := []string{}
for _, v := range r.Verbs {
if v != "create" {
filtered = append(filtered, v)
}
}
rules[i].Verbs = filtered
}
}
case 2:
// Remove 'clusterrolebindings'
for i, r := range rules {
if r.APIGroups[0] == "rbac.authorization.k8s.io" {
filtered := []string{}
for _, res := range r.Resources {
if res != "clusterrolebindings" && res != "clusterrolebindings/finalizers" {
filtered = append(filtered, res)
}
}
rules[i].Resources = filtered
}
}
case 3:
// Restrict configmaps to named subset (resourceNames)
for i, r := range rules {
if r.APIGroups[0] == "" {
filtered := []string{}
for _, res := range r.Resources {
if res != "configmaps" && res != "configmaps/finalizers" {
filtered = append(filtered, res)
}
}
rules[i].Resources = filtered
rules = append(rules, rbacv1.PolicyRule{
APIGroups: []string{""},
Resources: []string{"configmaps"},
Verbs: r.Verbs,
ResourceNames: []string{"config-logging", "tekton-config-defaults", "tekton-config-observability"},
})
}
}
case 4:
// Remove olm.operatorframework.io permission for finalizers
filtered := []rbacv1.PolicyRule{}
for _, r := range rules {
if len(r.APIGroups) != 1 || r.APIGroups[0] != "olm.operatorframework.io" {
filtered = append(filtered, r)
}
}
rules = filtered
case 5:
// Remove 'bind' and 'escalate' verbs
for i, r := range rules {
if r.APIGroups[0] == "rbac.authorization.k8s.io" {
filtered := []string{}
for _, v := range r.Verbs {
if v != "bind" && v != "escalate" {
filtered = append(filtered, v)
}
}
rules[i].Verbs = filtered
}
}
}

return &rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{Name: name},
Rules: rules,
}
}