chore: breaking change - remove wrappers...#4909
Conversation
…-256, RHDHPLAN-934) Signed-off-by: Nick Boldt <nboldt@redhat.com>
|
The container image build workflow finished with status: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4909 +/- ##
==========================================
- Coverage 55.82% 55.25% -0.58%
==========================================
Files 121 109 -12
Lines 2350 2132 -218
Branches 562 536 -26
==========================================
- Hits 1312 1178 -134
+ Misses 1032 953 -79
+ Partials 6 1 -5
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
before removing wrappers from image, we need to have a catalog image that is not referencing any wrappers. So we need to update |
…unt point for plugins Signed-off-by: Nick Boldt <nboldt@redhat.com>
|
|
The container image build workflow finished with status: |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
On further reflection we have decided to hold on removing wrappers until July. See discussion in https://docs.google.com/document/d/1Kr9qb1jLmngX3G7Qq7dpWVSQMehreIGQuNLL30URkzY/edit?tab=t.0 TL;DR: we want to think through what this will mean for SBOM creation, prod sec exceptions to use the non-advisory pipeline, talk to David about why we can’t just consume npm packages more directly (ie., non-bundled wrappers within oci artifacts), and more. @jon Koops will start some spike work once he and David are back from PTO in late June; then in early July we’ll all meet back to discuss the plan and if possible we’ll remove wrappers at the same time we explore moving to BS 1.53 (due Jul 14 2026). |
|
This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days. |
|
This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days. |
The runtime deployment uses a local ./dynamic-plugins/dist/ path for the homepage plugin. Add a comment referencing redhat-developer#4909 so the OCI migration sweep can find this spot. Assisted-by: OpenCode
|
@nickboldt: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…#5027) * refactor(e2e): move about:blank cleanup from try/finally to test.afterEach Move the WebSocket teardown workaround (page.goto('about:blank')) from per-test try/finally blocks into test.afterEach hooks. This is cleaner: the cleanup is declared once per describe block and applies to all tests that use the page fixture, without wrapping test bodies. Assisted-by: OpenCode * fix(e2e): fix external DB tests for operator deployments For operator deployments, patching POSTGRES_* env vars directly onto the deployment is reverted by operator reconciliation. The external DB tests (Azure DB, RDS) configure postgres-cred secret with connection details, but those values never reached the RHDH container as env vars. Fix: - Add postgres-cred to extraEnvs.secrets in the Backstage CR so the operator injects all its keys as env vars automatically - Skip direct deployment env var patching in prepareForExternalDatabase for operator installs (same pattern as schema-mode setup) Assisted-by: OpenCode * fix(ci): write pessimistic test status before Playwright execution When Prow kills a job (2h timeout) or Playwright hangs, the mark_test_result call at the end of testing::run_tests never executes. This leaves STATUS_TEST_FAILED.txt and STATUS_NUMBER_OF_TEST_FAILED.txt with fewer entries than STATUS_DEPLOYMENT_NAMESPACE.txt, creating misaligned arrays that break downstream reporting (Slack notifications). Fix: write a pessimistic default (failed=true, count=N/A) immediately after registering the test run. The real result overwrites it after Playwright completes. If the job is killed mid-test, the STATUS files still have entries for all registered test runs. To support this, save_status_test_failed and save_status_number_of_test_failed now regenerate the file from the in-memory array instead of appending, making them safe to call multiple times for the same deployment ID. Assisted-by: OpenCode * docs(ci): document same-process requirement for _regenerate_status_file The regenerate-from-array pattern requires all callers to run in the same shell process because bash associative arrays are not inherited by child processes. Add a comment documenting this constraint so it travels with the code. Also note the bash >= 4.3 requirement for local -n (nameref). Assisted-by: OpenCode * refactor(ci): unify unknown failure count sentinel value Replace the two different sentinel strings ('N/A' and 'some') with a single UNKNOWN_FAILURE_COUNT constant defined in test-run-tracker.sh. Both meant 'failed but count unknown' — the downstream Slack consumer now only has to handle one sentinel format. Assisted-by: OpenCode * refactor(e2e): extract BACKSTAGE_CR_EXTRA_ENV_SECRETS constant The secrets list in the Backstage CR merge-patch (postgres-config.ts) was hardcoded separately from generateBackstageCR (runtime-config.ts). If a secret were added to one but not the other, the patch would silently strip it mid-test. Extract the base list into a shared constant so the two stay in sync. Assisted-by: OpenCode * refactor(e2e): add mergePatchCustomObject helper to KubeClient Centralise the JSON merge-patch Content-Type header in a KubeClient method, matching the existing jsonPatchDeployment and updateSecret patterns. Simplifies the call site in addPostgresCredToBackstageCR by removing the trailing positional undefined args. Assisted-by: OpenCode * docs(e2e): note intentional afterEach page fixture overhead The suite-level afterEach requests the page fixture for every test, including non-UI ones. This is an intentional tradeoff — the minor overhead of an extra browser context is acceptable vs per-test conditional logic. Assisted-by: OpenCode * docs(e2e): note local dist path for homepage plugin package The runtime deployment uses a local ./dynamic-plugins/dist/ path for the homepage plugin. Add a comment referencing #4909 so the OCI migration sweep can find this spot. Assisted-by: OpenCode * refactor(e2e): remove redundant try/catch in config-map test The try/catch only logged and rethrew — it existed to pair with the finally block that was removed when about:blank cleanup moved to afterEach. Playwright already reports errors with full stacks, so the wrapper adds no value and costs an indent level. Assisted-by: OpenCode * docs(e2e): explain explicit form in getReleaseName The verbose !== undefined && !== '' check is used instead of || because oxlint's strict-boolean-expressions and prefer-nullish-coalescing rules (pedantic category, set to error) reject || on string operands. Assisted-by: OpenCode
|
This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days. |
4 participants
What does this PR do?
chore: breaking change - remove wrappers from RHDH entirely (RHDHPLAN-256, RHDHPLAN-934)
Signed-off-by: Nick Boldt nboldt@redhat.com
Screenshot/screencast of this PR
N/A
What issues does this PR fix or reference?
N/A (or see commit message above for issue number)
How to test this PR?
N/A
PR Checklist
As the author of this Pull Request I made sure that:
Reviewers
Reviewers, please comment how you tested the PR when approving it.