Skip to content

NO-JIRA: perses-dev with 6th user#80804

Open
etmurasaki wants to merge 1 commit into
openshift:mainfrom
etmurasaki:etmura-perses
Open

NO-JIRA: perses-dev with 6th user#80804
etmurasaki wants to merge 1 commit into
openshift:mainfrom
etmurasaki:etmura-perses

Conversation

@etmurasaki

@etmurasaki etmurasaki commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

This PR updates OpenShift CI configuration for the Observability “Monitoring Plugin” Perses UI Cypress test coverage.

  • Perses dev UI tests: Adjusts the Perses dev UI test user generation by increasing the htpasswd/IDP user count from 5 to 6, which expands the CYPRESS_LOGIN_USERS set used by the Cypress suite.
  • Perses UI IVT (integration) coverage: Adds a new IVT test step and end-to-end job (monitoring-plugin-tests-perses-ui-ivt / e2e-perses-ivt) including:
    • Step-registry definition with resource requests and Cypress configuration knobs
    • A runner script that provisions htpasswd + patches the oauth ClusterOperator with an HTPasswd identity provider, waits for OAuth to converge, configures Cypress (kubeconfig/base URL and login env vars), and collects Cypress artifacts/screenshots/videos on exit
    • Prow job wiring and Slack reporting
    • A new periodic cron entry scheduled to run the IVT job
    • Ownership/review metadata for the new step
  • Documentation text updates: Updates step descriptions for existing Perses dev UI and Perses UI step refs to clarify they run the corresponding Perses UI Cypress tests.

@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@etmurasaki: This pull request explicitly references no jira issue.

Details

In response to this:

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 19, 2026
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR introduces a new Perses UI IVT (In-Vitro Testing) test with complete step-registry definition, test script, and pipeline job integration for both standard and periodic schedules. It increments the perses-dev-ui user generation loop from 5 to 6 users, removes HTPasswd identity provider setup from the perses-ui test script, and corrects step documentation strings to accurately describe Perses UI Cypress tests.

Changes

Perses UI Testing Infrastructure

Layer / File(s) Summary
IVT step registry definition and ownership
ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml, ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json, ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
Step-registry YAML defines the Cypress test interface with resource requests/limits, timeout/grace period, environment variables for test configuration (install/skip toggles, image overrides), and dependency wiring to parent container images. Metadata JSON and OWNERS file establish path metadata and team ownership.
IVT test script implementation
ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh
Bash script bootstraps strict shell modes and clears empty Cypress variables, loads kubeadmin credentials from KUBEADMIN_PASSWORD_FILE, sources proxy configuration when available, gates execution on console availability, registers an EXIT trap for artifact collection, configures Cypress runtime environment with kubeconfig path and login credentials, and executes the Cypress test suite.
Pipeline job configuration
ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml, ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
Main and periodic job entries add optional e2e-perses-ivt test jobs using the OpenShift AWS cluster profile, ipi-aws workflow, and Slack reporter integration to #observability-ui-qe channel with templated job-state reporting.
Existing test updates and documentation
ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh, ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml, ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-commands.sh, ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
Perses-dev-ui user generation loop increments from 5 to 6 users. Step documentation for perses-dev-ui and perses-ui are corrected to reference Perses UI Cypress tests. HTPasswd identity provider setup and test user generation are removed from perses-ui test script.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

lgtm, ok-to-test

Suggested reviewers

  • jgbernalp
🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'NO-JIRA: perses-dev with 6th user' directly describes the main change across the pull request—adding support for a 6th test user to the perses-dev configuration and related test infrastructure.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed This PR modifies CI configuration files (YAML, bash scripts, OWNERS) and contains no Ginkgo test code or test name definitions. The check is not applicable to CI configuration repositories.
Test Structure And Quality ✅ Passed PR contains no Ginkgo test code—only Bash scripts, YAML configs, and metadata files. Check is not applicable and thus passes by inapplicability.
Microshift Test Compatibility ✅ Passed This PR does not add any Ginkgo e2e tests. It contains only CI configuration files (YAML, bash scripts) for Cypress UI tests. The check is not applicable to this PR.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR adds Cypress UI tests and CI configuration, not Ginkgo e2e tests. The SNO compatibility check applies only to Ginkgo tests, which are not present.
Topology-Aware Scheduling Compatibility ✅ Passed PR contains only CI/CD test infrastructure changes (job configs, test scripts, metadata), not deployment manifests, operator code, or scheduling constraints subject to topology checks.
Ote Binary Stdout Contract ✅ Passed The PR contains only CI configuration (YAML), shell scripts, and metadata files—no Go source code or OTE binaries that could violate stdout contract requirements.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The custom check applies to new Ginkgo e2e tests, but this PR adds Cypress tests (JavaScript UI tests that run npm run test-cypress-perses-ivt), not Ginkgo tests. The check is not applicable.
No-Weak-Crypto ✅ Passed No weak cryptography patterns (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB) detected. Passwords use /dev/urandom and bcrypt hashing. No custom crypto or non-constant-time secret comparisons found.
Container-Privileges ✅ Passed No privileged container settings found in PR changes. YAML step registry definitions, CI config files, and shell scripts contain no privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capabi...
No-Sensitive-Data-In-Logs ✅ Passed Scripts do not contain direct logging of passwords, tokens, API keys, or PII. Credentials are stored in environment variables (without explicit echo/logging) and passed to test runners—no sensitive...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 19, 2026
@openshift-ci openshift-ci Bot requested review from PeterYurkovich and zhuje June 19, 2026 23:09
@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: etmurasaki

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Around line 37-45: The script currently exits with status 0 (success) when
encountering errors in the KUBEADMIN_PASSWORD_FILE prerequisite checks, which
masks failures and allows the test to be skipped while appearing successful.
Change both exit 0 statements (one in the variable existence check and one in
the file existence check) to exit 1 to properly signal failure when these
critical prerequisites are missing.
- Around line 104-114: The while loop controlled by the wait_auth variable has
no timeout mechanism and can hang indefinitely if the OAuth deployment rollout
never converges. Add a timeout to this polling loop by either tracking elapsed
time with a start timestamp and breaking if the elapsed time exceeds a defined
maximum (such as 300 seconds), or by maintaining an iteration counter and
breaking after a maximum number of retries. Ensure the timeout is checked within
the loop condition or at the beginning of each iteration to prevent unbounded
waiting.

In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml`:
- Around line 64-65: The documentation field at lines 64-65 contains misleading
text that references "monitoring-plugin-tests-monitoring-ui" when the actual
step is "monitoring-plugin-tests-perses-ui-ivt". Update the documentation text
to replace the reference to "monitoring-ui" with "perses-ui-ivt" so that the
description accurately matches the step name and what it actually does.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 058d41cc-3745-4b2c-bc89-43517aefc58c

📥 Commits

Reviewing files that changed from the base of the PR and between ce2db99 and f0f7c20.

⛔ Files ignored due to path filters (2)
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-periodics.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (7)
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
✅ Files skipped from review due to trivial changes (2)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh (2)

27-34: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider defensive handling for indirect expansion with nounset.

With set -o nounset, if any variable in vars is completely unset (not merely empty), the ${!var} indirect expansion will fail before the -z test runs. If ci-operator guarantees these are always set (even to empty strings) via the ref.yaml env definitions, this is fine. Otherwise, using ${!var:-} provides safer handling.

 for var in "${vars[@]}"; do
-  if [[ -z "${!var}" ]]; then
+  if [[ -z "${!var:-}" ]]; then
     unset "$var"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`
around lines 27 - 34, The indirect variable expansion ${!var} used in the loop
will fail with set -o nounset if any variable in the vars array is completely
unset, before the -z test can evaluate it. Replace the unsafe indirect
expansions ${!var} with the safer pattern ${!var:-} in both the -z check and the
echo statement within the loop, which provides a default empty value if the
variable doesn't exist, ensuring compatibility with nounset and preventing
script failure on unset variables.

137-142: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Remove debug ls -ltr command.

Line 138 lists directory contents but the output isn't used for anything. This appears to be a leftover debug command that adds noise to CI logs.

 # Install npm modules
-ls -ltr
 npm install
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`
around lines 137 - 142, Remove the standalone `ls -ltr` command that appears
after the "# Install npm modules" comment. This debug command serves no
functional purpose in the script and only adds unnecessary noise to the CI logs.
Simply delete the line containing `ls -ltr` so that the npm install command
directly follows the install modules comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Line 56: The KUBECONFIG variable in the oc get clusteroperator command is
unquoted, which can cause word splitting issues if the path contains spaces.
Wrap the variable reference ${KUBECONFIG} with double quotes so it becomes
"${KUBECONFIG}" to ensure the entire path is treated as a single argument in the
oc command invocation.
- Line 121: In the cp command on line 121, the $KUBECONFIG variable is unquoted
which can cause word splitting if the path contains spaces or special
characters. Quote the $KUBECONFIG variable by changing $KUBECONFIG to
"$KUBECONFIG" in the cp -L command to prevent word splitting issues.

---

Nitpick comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Around line 27-34: The indirect variable expansion ${!var} used in the loop
will fail with set -o nounset if any variable in the vars array is completely
unset, before the -z test can evaluate it. Replace the unsafe indirect
expansions ${!var} with the safer pattern ${!var:-} in both the -z check and the
echo statement within the loop, which provides a default empty value if the
variable doesn't exist, ensuring compatibility with nounset and preventing
script failure on unset variables.
- Around line 137-142: Remove the standalone `ls -ltr` command that appears
after the "# Install npm modules" comment. This debug command serves no
functional purpose in the script and only adds unnecessary noise to the CI logs.
Simply delete the line containing `ls -ltr` so that the npm install command
directly follows the install modules comment.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: c5046786-807f-4f6a-8c81-c83b09aa8583

📥 Commits

Reviewing files that changed from the base of the PR and between f0f7c20 and dbdb674.

⛔ Files ignored due to path filters (2)
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-periodics.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (9)
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
✅ Files skipped from review due to trivial changes (3)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (5)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Around line 99-101: The OAuth test IDP setup needs to be made idempotent and
non-destructive. First, replace the `oc create secret` command with `oc apply`
to make secret creation idempotent so it succeeds on reruns without failing if
the secret already exists. Second, the OAuth patch that adds the HTPasswd
identity provider to `/spec/identityProviders` is destructive because it
replaces the entire array, potentially removing existing providers. Instead,
check if the HTPasswd provider named "uiauto-htpasswd-idp" already exists, and
use a JSON patch operation like "add" with an index or "test" operation to
verify the array exists before appending, ensuring existing identity providers
are preserved and the new test IDP is only added once.
- Around line 27-32: The indirect expansion ${!var} in the for loop will cause
the script to abort when the nounset option is enabled and any target variable
is unset, preventing the cleanup logic from executing properly. Modify the
indirect expansion to use a default value syntax such as ${!var:-} or ${!var-}
to gracefully handle cases where the target variable is unset, allowing the loop
to continue and properly unset the variables instead of aborting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5c735e85-0110-458c-9637-2c223e27fc5f

📥 Commits

Reviewing files that changed from the base of the PR and between dbdb674 and 79aac8f.

⛔ Files ignored due to path filters (2)
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-periodics.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (9)
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
✅ Files skipped from review due to trivial changes (4)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh (1)

27-34: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Use a default for indirect expansion under nounset.

With set -o nounset (line 3), ${!var} on line 28 will abort when any target CYPRESS_* variable is completely unset (not just empty), preventing the cleanup loop from functioning as intended.

Proposed fix
 for var in "${vars[@]}"; do
-  if [[ -z "${!var}" ]]; then
+  if [[ -z "${!var:-}" ]]; then
     unset "$var"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`
around lines 27 - 34, The indirect variable expansion ${!var} used in the
conditional check on line 28 will cause the script to abort when any CYPRESS_*
variable is completely unset due to the nounset setting at line 3. Fix this by
applying a default value to the indirect expansion syntax (such as ${!var:-})
when checking if the variable is empty in the if condition, allowing the cleanup
loop to safely handle unset variables without aborting the script.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Around line 87-91: The CYPRESS_LOGIN_USERS export statement references an
undefined users variable which will cause script failure under set -o nounset.
Remove the undefined ${users} reference and only include the kubeadmin
authentication in the CYPRESS_LOGIN_USERS assignment. Additionally, the
CYPRESS_LOGIN_IDP_DEV_USER variable references an unconfigured
uiauto-htpasswd-idp identity provider, so either remove this export entirely or
replace it with an identity provider that is actually configured in the script.

---

Duplicate comments:
In
`@ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh`:
- Around line 27-34: The indirect variable expansion ${!var} used in the
conditional check on line 28 will cause the script to abort when any CYPRESS_*
variable is completely unset due to the nounset setting at line 3. Fix this by
applying a default value to the indirect expansion syntax (such as ${!var:-})
when checking if the variable is empty in the if condition, allowing the cleanup
loop to safely handle unset variables without aborting the script.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 1c1081a9-b807-436e-967a-3ca676998a99

📥 Commits

Reviewing files that changed from the base of the PR and between 79aac8f and 97b1fff.

⛔ Files ignored due to path filters (2)
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-periodics.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/openshift/monitoring-plugin/openshift-monitoring-plugin-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (10)
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/OWNERS
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
💤 Files with no reviewable changes (1)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-commands.sh
✅ Files skipped from review due to trivial changes (4)
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui/monitoring-plugin-tests-perses-ui-ref.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-dev-ui/monitoring-plugin-tests-perses-dev-ui-commands.sh
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main__periodics.yaml
  • ci-operator/step-registry/monitoring-plugin/tests/perses-ui-ivt/monitoring-plugin-tests-perses-ui-ivt-ref.metadata.json
  • ci-operator/config/openshift/monitoring-plugin/openshift-monitoring-plugin-main.yaml

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@etmurasaki: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-monitoring-plugin-main-e2e-perses-ivt openshift/monitoring-plugin presubmit Presubmit changed
pull-ci-openshift-monitoring-plugin-main-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-5.1-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-5.0-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-4.23-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-4.22-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-ocp-4.22-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-ocp-4.19-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-0.5-e2e-perses-dev openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-main-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-5.1-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-5.0-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-4.23-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-4.22-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-ocp-4.22-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-ocp-4.19-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
pull-ci-openshift-monitoring-plugin-release-coo-0.5-e2e-perses openshift/monitoring-plugin presubmit Registry content changed
periodic-ci-openshift-monitoring-plugin-main-periodics-e2e-perses N/A periodic Registry content changed
periodic-ci-openshift-monitoring-plugin-main-periodics-e2e-perses-ivt N/A periodic Periodic changed
periodic-ci-openshift-monitoring-plugin-main-periodics-e2e-perses-dev N/A periodic Registry content changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@etmurasaki: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants