diff --git a/.ci/pipelines/cluster/aks/aks-operator-deployment.sh b/.ci/pipelines/cluster/aks/aks-operator-deployment.sh index 0424160bca..b99f6640e3 100644 --- a/.ci/pipelines/cluster/aks/aks-operator-deployment.sh +++ b/.ci/pipelines/cluster/aks/aks-operator-deployment.sh @@ -48,7 +48,6 @@ initiate_rbac_aks_operator_deployment() { namespace::configure "${namespace}" # deploy_test_backstage_customization_provider "${namespace}" # Doesn't work on K8s - config::create_conditional_policies_operator /tmp/conditional-policies.yaml config::prepare_operator_app_config "${DIR}/resources/config_map/app-config-rhdh-rbac.yaml" apply_yaml_files "${DIR}" "${namespace}" "${rhdh_base_url}" diff --git a/.ci/pipelines/cluster/eks/eks-operator-deployment.sh b/.ci/pipelines/cluster/eks/eks-operator-deployment.sh index 977181b7e4..e15a0bcaaa 100644 --- a/.ci/pipelines/cluster/eks/eks-operator-deployment.sh +++ b/.ci/pipelines/cluster/eks/eks-operator-deployment.sh @@ -45,7 +45,6 @@ initiate_rbac_eks_operator_deployment() { namespace::configure "${namespace}" # deploy_test_backstage_customization_provider "${namespace}" # Doesn't work on K8s - config::create_conditional_policies_operator /tmp/conditional-policies.yaml config::prepare_operator_app_config "${DIR}/resources/config_map/app-config-rhdh-rbac.yaml" apply_yaml_files "${DIR}" "${namespace}" "${rhdh_base_url}" diff --git a/.ci/pipelines/cluster/gke/gke-operator-deployment.sh b/.ci/pipelines/cluster/gke/gke-operator-deployment.sh index 08b118d0bc..71eeb7bfb9 100644 --- a/.ci/pipelines/cluster/gke/gke-operator-deployment.sh +++ b/.ci/pipelines/cluster/gke/gke-operator-deployment.sh @@ -50,7 +50,6 @@ initiate_rbac_gke_operator_deployment() { namespace::configure "${namespace}" # deploy_test_backstage_customization_provider "${namespace}" # Doesn't work on K8s - config::create_conditional_policies_operator /tmp/conditional-policies.yaml config::prepare_operator_app_config "${DIR}/resources/config_map/app-config-rhdh-rbac.yaml" apply_yaml_files "${DIR}" "${namespace}" "${rhdh_base_url}" apply_gke_frontend_config "${namespace}" diff --git a/.ci/pipelines/jobs/ocp-operator.sh b/.ci/pipelines/jobs/ocp-operator.sh index 63be4f77c1..06b329394a 100644 --- a/.ci/pipelines/jobs/ocp-operator.sh +++ b/.ci/pipelines/jobs/ocp-operator.sh @@ -30,7 +30,6 @@ initiate_operator_deployments() { log::warn "Skipping orchestrator plugins and workflows deployment on Operator $NAME_SPACE deployment" namespace::configure "${NAME_SPACE_RBAC}" - config::create_conditional_policies_operator /tmp/conditional-policies.yaml config::prepare_operator_app_config "${DIR}/resources/config_map/app-config-rhdh-rbac.yaml" local rbac_rhdh_base_url="https://backstage-${RELEASE_NAME_RBAC}-${NAME_SPACE_RBAC}.${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE_RBAC}" "${rbac_rhdh_base_url}" @@ -66,7 +65,6 @@ initiate_operator_deployments_osd_gcp() { log::warn "Skipping orchestrator plugins and workflows deployment on OSD-GCP environment" namespace::configure "${NAME_SPACE_RBAC}" - config::create_conditional_policies_operator /tmp/conditional-policies.yaml config::prepare_operator_app_config "${DIR}/resources/config_map/app-config-rhdh-rbac.yaml" local rbac_rhdh_base_url="https://backstage-${RELEASE_NAME_RBAC}-${NAME_SPACE_RBAC}.${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE_RBAC}" "${rbac_rhdh_base_url}" diff --git a/.ci/pipelines/lib/README.md b/.ci/pipelines/lib/README.md index c6307bac69..bc7b74ffac 100644 --- a/.ci/pipelines/lib/README.md +++ b/.ci/pipelines/lib/README.md @@ -61,8 +61,7 @@ Functions: `namespace::configure`, `namespace::delete`, `namespace::force_delete Configuration management for ConfigMaps, dynamic plugins, and app configuration. Functions: `config::create_app_config_map`, `config::select_config_map_file`, -`config::create_dynamic_plugins_config`, `config::create_conditional_policies_operator`, -`config::prepare_operator_app_config` +`config::create_dynamic_plugins_config`, `config::prepare_operator_app_config` ### `testing.sh` diff --git a/.ci/pipelines/lib/config.sh b/.ci/pipelines/lib/config.sh index c086834564..aff007ca23 100644 --- a/.ci/pipelines/lib/config.sh +++ b/.ci/pipelines/lib/config.sh @@ -102,27 +102,6 @@ EOF # Operator Configuration # ============================================================================== -# Create conditional policies file for RBAC operator deployment -# Args: -# $1 - destination_file: Path for the generated policies file -# Returns: -# 0 - Success -config::create_conditional_policies_operator() { - local destination_file=$1 - - if [[ -z "$destination_file" ]]; then - log::error "Missing required parameter: destination_file" - log::info "Usage: config::create_conditional_policies_operator " - return 1 - fi - - yq '.upstream.backstage.initContainers[0].command[2]' "${DIR}/value_files/values_showcase-rbac.yaml" \ - | head -n -4 \ - | tail -n +2 > "$destination_file" - common::sed_inplace 's/\\\$/\$/g' "$destination_file" - return $? -} - # Prepare app configuration for operator deployment with RBAC # Args: # $1 - config_file: Path to the app configuration file to modify diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 800b252983..802b0f24fb 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -314,15 +314,9 @@ apply_yaml_files() { common::create_configmap_from_file "dynamic-plugins-config" "$project" \ "dynamic-plugins-config.yaml" "$dir/resources/config_map/dynamic-plugins-config.yaml" - if [[ "$JOB_NAME" == *operator* ]] && [[ "${project}" == *rbac* ]]; then - common::create_configmap_from_files "rbac-policy" "$project" \ - "rbac-policy.csv=$dir/resources/config_map/rbac-policy.csv" \ - "conditional-policies.yaml=/tmp/conditional-policies.yaml" - else - common::create_configmap_from_files "rbac-policy" "$project" \ - "rbac-policy.csv=$dir/resources/config_map/rbac-policy.csv" \ - "conditional-policies.yaml=$dir/resources/config_map/conditional-policies.yaml" - fi + common::create_configmap_from_files "rbac-policy" "$project" \ + "rbac-policy.csv=$dir/resources/config_map/rbac-policy.csv" \ + "conditional-policies.yaml=$dir/resources/config_map/conditional-policies.yaml" # configuration for testing global floating action button. common::create_configmap_from_file "dynamic-global-floating-action-button-config" "$project" \ diff --git a/e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts b/e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts index d94b9f9e58..c4a8d75f10 100644 --- a/e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts +++ b/e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts @@ -1,4 +1,4 @@ -import { test } from "@playwright/test"; +import { test, expect } from "@playwright/test"; import { Common, setupBrowser } from "../../utils/common"; import { RBAC_API, @@ -240,12 +240,24 @@ test.describe("Auditor check for RBAC Plugin", () => { for (const s of conditionRead) { test(`condition-read → ${s.name}`, async () => { - await s.call(); + const response = await s.call(); + let status: "succeeded" | "failed"; + if (s.name === "by-id" && response.status() === 404) { + // Condition by-id may return 404 if no conditions exist (e.g., + // empty conditional-policies.yaml). The audit log still records the + // event but with status "failed" instead of "succeeded". + status = "failed"; + } else { + expect(response.ok()).toBe(true); + status = "succeeded"; + } await validateRbacLogEvent( "condition-read", USER_ENTITY_REF, { method: "GET", url: s.url }, s.meta, + undefined, + status, ); }); } diff --git a/e2e-tests/playwright/e2e/audit-log/log-utils.ts b/e2e-tests/playwright/e2e/audit-log/log-utils.ts index a01e3b06d8..a24b5f58d1 100644 --- a/e2e-tests/playwright/e2e/audit-log/log-utils.ts +++ b/e2e-tests/playwright/e2e/audit-log/log-utils.ts @@ -189,7 +189,7 @@ export class LogUtils { retryDelay: number = 2000, ): Promise { const deploySelector = getBackstageDeploySelector(); - const tailNumber = 100; + const tailNumber = 500; // Resolve the deployment by its metadata labels, then fetch logs from it. // This works for both Helm and Operator since both set app.kubernetes.io/name diff --git a/e2e-tests/playwright/e2e/configuration-test/config-map.spec.ts b/e2e-tests/playwright/e2e/configuration-test/config-map.spec.ts index 7d5aeee8c6..b98674abd6 100644 --- a/e2e-tests/playwright/e2e/configuration-test/config-map.spec.ts +++ b/e2e-tests/playwright/e2e/configuration-test/config-map.spec.ts @@ -25,10 +25,9 @@ test.describe("Change app-config at e2e test runtime", () => { // Start with a common name, but let KubeClient find the actual ConfigMap const configMapName = "app-config-rhdh"; - // eslint-disable-next-line playwright/no-conditional-in-test + const namespace = process.env.NAME_SPACE_RUNTIME || "showcase-runtime"; const deploymentName = - // eslint-disable-next-line playwright/no-conditional-in-test (process.env.RELEASE_NAME || "rhdh") + "-developer-hub"; const kubeUtils = new KubeClient(); diff --git a/e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts b/e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts index 98eef0d816..068ae21500 100644 --- a/e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts +++ b/e2e-tests/playwright/e2e/plugins/rbac/rbac.spec.ts @@ -390,6 +390,8 @@ test.describe("Test RBAC", () => { await saveButton.click(); await uiHelper.verifyText( "Role role:default/test-role updated successfully", + true, + 15000, ); await page.getByPlaceholder("Filter").waitFor({ @@ -447,24 +449,33 @@ test.describe("Test RBAC", () => { await expect(nextButton2).toBeEnabled(); await nextButton2.click(); // Wait for Save button which only appears on the review step - await expect(page.getByRole("button", { name: "Save" })).toBeVisible({ + const saveButton1 = page.getByRole("button", { name: "Save" }); + await expect(saveButton1).toBeVisible({ timeout: 15000, }); - await uiHelper.clickButton("Save"); + // Dismiss quickstart overlay if visible — it can intercept button clicks + await uiHelper.hideQuickstartIfVisible(); + await expect(saveButton1).toBeEnabled(); + await saveButton1.click(); await uiHelper.verifyText( "Role role:default/test-role1 updated successfully", + true, + 15000, ); await uiHelper.verifyHeading(rbacPo.regexpShortUsersAndGroups(1, 1)); - await page - .getByTestId(ROLE_OVERVIEW_COMPONENTS_TEST_ID.updatePolicies) - .click(); + // Wait for the permissions section update button to be available + const updatePoliciesButton = page.getByTestId( + ROLE_OVERVIEW_COMPONENTS_TEST_ID.updatePolicies, + ); + await expect(updatePoliciesButton).toBeVisible({ timeout: 15000 }); + await updatePoliciesButton.click(); await uiHelper.verifyHeading("Edit Role"); await rbacPo.selectPluginsCombobox.click(); await rbacPo.selectOption("scaffolder"); // Close the plugins dropdown to access the permissions table - await page.getByRole("button", { name: "Close" }).click(); + await page.keyboard.press("Escape"); // Expand the Scaffolder row to access its permissions await page @@ -480,9 +491,13 @@ test.describe("Test RBAC", () => { await expect(page.getByRole("button", { name: "Save" })).toBeVisible({ timeout: 15000, }); + // Dismiss quickstart overlay if visible — it can intercept button clicks + await uiHelper.hideQuickstartIfVisible(); await uiHelper.clickButton("Save"); await uiHelper.verifyText( "Role role:default/test-role1 updated successfully", + true, + 15000, ); await uiHelper.verifyHeading("2 permissions"); @@ -873,6 +888,8 @@ test.describe("Test RBAC", () => { await saveButton.click(); await uiHelper.verifyText( "Role role:default/test-role updated successfully", + true, + 15000, ); await page.getByPlaceholder("Filter").waitFor({ diff --git a/e2e-tests/playwright/support/api/rbac-api.ts b/e2e-tests/playwright/support/api/rbac-api.ts index 832e690105..2c2d282f08 100644 --- a/e2e-tests/playwright/support/api/rbac-api.ts +++ b/e2e-tests/playwright/support/api/rbac-api.ts @@ -15,7 +15,7 @@ export default class RhdhRbacApi { Authorization: string; }; private myContext: APIRequestContext; - private readonly roleRegex = /^[a-zA-Z]+\/[a-zA-Z_]+$/; + private readonly roleRegex = /^[a-zA-Z0-9_-]+\/[a-zA-Z0-9_-]+$/; private constructor(private readonly token: string) { this.authHeader = { @@ -113,6 +113,14 @@ export default class RhdhRbacApi { return await this.myContext.get(`roles/conditions/${id}`); } + public async deleteConditionById(id: number): Promise { + return await this.myContext.delete(`roles/conditions/${id}`); + } + + public async dispose(): Promise { + await this.myContext.dispose(); + } + private checkRoleFormat(role: string) { if (!this.roleRegex.test(role)) throw Error( diff --git a/e2e-tests/playwright/support/page-objects/rbac-po.ts b/e2e-tests/playwright/support/page-objects/rbac-po.ts index e4d655124b..17a99186a5 100644 --- a/e2e-tests/playwright/support/page-objects/rbac-po.ts +++ b/e2e-tests/playwright/support/page-objects/rbac-po.ts @@ -6,6 +6,7 @@ import { ROLES_PAGE_COMPONENTS, } from "./page-obj"; import { type RoleBasedPolicy } from "@backstage-community/plugin-rbac-common"; +import RhdhRbacApi from "../api/rbac-api"; type PermissionPolicyType = "anyOf" | "not"; @@ -281,23 +282,40 @@ export class RbacPo extends PageObject { await this.verifyPermissionPoliciesHeader(policies.length); await this.create(); - // Check for error alert first + // Wait for either success message or error alert. + // Wrap both waitFor calls so the losing promise cannot reject unhandled. + const successLocator = this.page + .getByText(`Role role:default/${name} created successfully`, { + exact: true, + }) + .first(); const errorAlert = this.page .getByRole("alert") .filter({ hasText: /error/i }); - const errorCount = await errorAlert.count(); - if (errorCount > 0) { + const outcome = await Promise.race([ + successLocator + .waitFor({ state: "visible", timeout: 30000 }) + .then(() => "success" as const) + .catch(() => "success_timeout" as const), + errorAlert + .waitFor({ state: "visible", timeout: 30000 }) + .then(() => "error" as const) + .catch(() => "error_timeout" as const), + ]); + + if (outcome === "error") { const errorMessage = await errorAlert.textContent(); throw new Error( - `Failed to create role: ${errorMessage}. This may indicate insufficient permissions.`, + `Failed to create role: ${errorMessage}. This may indicate insufficient permissions or a leftover role from a previous test run.`, ); } - // Wait for success message before proceeding to roles list - await this.uiHelper.verifyText( - `Role role:default/${name} created successfully`, - ); + if (outcome !== "success") { + throw new Error( + `Role creation timed out: neither success message nor error alert appeared within 30s.`, + ); + } // Now we should be on the roles list page await this.page.getByPlaceholder("Filter").waitFor({ state: "visible" }); @@ -364,6 +382,8 @@ export class RbacPo extends PageObject { await this.uiHelper.clickButton("Create"); await this.uiHelper.verifyText( `Role role:default/${name} created successfully`, + true, + 15000, ); } else if (permissionPolicyType === "not") { // Conditional Scenario 2: Permission policies using Not @@ -391,18 +411,61 @@ export class RbacPo extends PageObject { } async tryDeleteRole(name: string): Promise { - await this.page.goto("/rbac"); - await this.uiHelper.searchInputAriaLabel(name); - const deleteButton = this.page.locator( - ROLES_PAGE_COMPONENTS.deleteRole(name), - ); - if ((await deleteButton.count()) > 0) { - await deleteButton.click(); - await this.uiHelper.verifyHeading("Delete this role?"); - await this.page.fill(DELETE_ROLE_COMPONENTS.roleName, name); - await this.uiHelper.clickButton("Delete"); - await this.uiHelper.verifyText(`Role ${name} deleted successfully`); + // Use the RBAC REST API for reliable cleanup — the UI-based approach + // can silently fail if the page hasn't fully loaded or the filter + // doesn't match, leaving a leftover role that blocks recreation. + const rbacApi = await RhdhRbacApi.buildRbacApi(this.page); + try { + // name is fully qualified like "role:default/test-role1" + // The API expects just "default/test-role1" + const apiRoleName = name.replace(/^role:/, ""); + + // Delete policies associated with the role first + const policiesResponse = await rbacApi.getPoliciesByRole(apiRoleName); + if (policiesResponse.ok()) { + const policies = await policiesResponse.json(); + if (policies.length > 0) { + await rbacApi.deletePolicy(apiRoleName, policies); + console.log( + `Deleted ${policies.length} leftover policies for ${name} via API`, + ); + } + } + + // Delete conditions associated with the role + const conditionsResponse = await rbacApi.getConditionByQuery({ + roleEntityRef: name, + }); + if (conditionsResponse.ok()) { + const conditions = await conditionsResponse.json(); + for (const condition of conditions) { + const delResponse = await rbacApi.deleteConditionById(condition.id); + if (delResponse.ok()) { + console.log( + `Deleted leftover condition ${condition.id} for ${name} via API`, + ); + } + } + } + + // Delete the role itself + const response = await rbacApi.deleteRole(apiRoleName); + if (response.ok()) { + console.log(`Successfully deleted leftover role ${name} via API`); + } else if (response.status() === 404) { + console.log(`Role ${name} does not exist, no cleanup needed`); + } else { + console.warn( + `Unexpected status ${response.status()} when deleting role ${name} via API`, + ); + } + } catch (error) { + console.warn(`API cleanup of role ${name} failed: ${error}`); + } finally { + await rbacApi.dispose(); } + // Navigate to RBAC page for the subsequent test steps + await this.page.goto("/rbac"); } async deleteRole(name: string, header: string = "All roles (0)") { diff --git a/e2e-tests/playwright/support/pages/adoption-insights.ts b/e2e-tests/playwright/support/pages/adoption-insights.ts index 73c7bcad7b..9e23c6fa60 100644 --- a/e2e-tests/playwright/support/pages/adoption-insights.ts +++ b/e2e-tests/playwright/support/pages/adoption-insights.ts @@ -59,11 +59,11 @@ export class TestHelper { async populateMissingPanelData( page: Page, uiHelper: UIhelper, - templatesFirstLast: string[], - catalogEntitiesFirstLast: string[], - techdocsFirstLast: string[], + templatesFirstLast: string[] | undefined, + catalogEntitiesFirstLast: string[] | undefined, + techdocsFirstLast: string[] | undefined, ): Promise { - if (templatesFirstLast.length === 0) { + if (!templatesFirstLast?.length) { await page.getByRole("link", { name: "Self-service" }).click(); await page .getByText("Templates", { exact: true }) @@ -103,9 +103,10 @@ export class TestHelper { await page .getByText("Run of Create a tekton CI") .waitFor({ state: "visible" }); + await page.waitForTimeout(5000); // wait for the flush interval to be sure } - if (catalogEntitiesFirstLast.length === 0) { + if (!catalogEntitiesFirstLast?.length) { // Visit a catalog entity await uiHelper.clickLink("Catalog"); await uiHelper.clickLink("Red Hat Developer Hub"); @@ -113,10 +114,11 @@ export class TestHelper { await expect(page.getByText("Red Hat Developer Hub")).toBeVisible(); } - if (techdocsFirstLast.length === 0) { + if (!techdocsFirstLast?.length) { // Visit docs await page.goto("/docs"); await uiHelper.clickLink("Red Hat Developer Hub"); + await page.waitForTimeout(5000); // wait for the flush interval to be sure await uiHelper.openSidebarButton("Administration"); } } @@ -141,7 +143,10 @@ export class TestHelper { // Wait for the expected API call to succeed await this.waitUntilApiCallSucceeds(newpage); - await newpage.getByText(expectedText).first().waitFor({ state: "visible" }); + await newpage + .getByText(expectedText) + .first() + .waitFor({ state: "visible", timeout: 30000 }); await newpage.waitForTimeout(5000); // wait for the flush interval to be sure await newpage.close(); } diff --git a/e2e-tests/playwright/utils/ui-helper.ts b/e2e-tests/playwright/utils/ui-helper.ts index b62347ffbd..5cfff41996 100644 --- a/e2e-tests/playwright/utils/ui-helper.ts +++ b/e2e-tests/playwright/utils/ui-helper.ts @@ -427,20 +427,25 @@ export class UIhelper { await this.page.waitForSelector(`text=${text}`, { state: "detached" }); } - async verifyText(text: string | RegExp, exact: boolean = true) { - await this.verifyTextInLocator("", text, exact); + async verifyText( + text: string | RegExp, + exact: boolean = true, + timeout: number = 5000, + ) { + await this.verifyTextInLocator("", text, exact, timeout); } private async verifyTextInLocator( locator: string, text: string | RegExp, exact: boolean, + timeout: number = 5000, ) { const elementLocator = locator ? this.page.locator(locator).getByText(text, { exact }).first() : this.page.getByText(text, { exact }).first(); - await elementLocator.waitFor({ state: "visible" }); + await elementLocator.waitFor({ state: "visible", timeout }); await elementLocator.waitFor({ state: "attached" }); try {