fix: add operator-sdk to Cypress test runner image#966
Conversation
When CYPRESS_CUSTOM_COO_BUNDLE_IMAGE is set, the test setup calls `operator-sdk run bundle` which fails with exit code 127 because the binary is not present in the image. Co-authored-by: Cursor <cursoragent@cursor.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: DavidRajnoha The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThe Cypress test Docker container is updated to include ChangesCypress test container dependencies
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/cypress/Dockerfile (1)
1-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd a USER instruction to run as non-root.
The Dockerfile does not specify a USER instruction, which means the container runs as root by default. Running containers as root is a critical security risk.
Add a USER instruction after setting up the filesystem to ensure the container runs with a non-root user.
🔒 Proposed fix: Add non-root user
COPY . /tmp/monitoring-plugin RUN chmod -R 777 /tmp/monitoring-plugin + +# Create and use non-root user +RUN useradd -u 1001 -m -s /bin/bash cypress +USER 1001 + WORKDIR /tmp/monitoring-plugin/web/cypressNote: The
chmod -R 777on line 8 may also need adjustment—consider more restrictive permissions that still allow the non-root user to access required files.As per coding guidelines: "USER non-root; never run as root"
🤖 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 `@web/cypress/Dockerfile` around lines 1 - 9, The Dockerfile currently leaves the container running as root after COPY/RUN/WORKDIR steps; add a USER instruction after the filesystem setup (after RUN chmod -R 777 /tmp/monitoring-plugin and before/after WORKDIR /tmp/monitoring-plugin/web/cypress) to switch to a non-root account (either create a dedicated user/group earlier in the Dockerfile or switch to a provided non-root UID/GID), and tighten file permissions (replace or relax chmod -R 777) so the chosen non-root user has only the required access; reference the COPY, RUN chmod -R 777, and WORKDIR instructions when applying the change.
🧹 Nitpick comments (1)
web/cypress/Dockerfile (1)
1-9: 💤 Low valueConsider adding a HEALTHCHECK instruction.
The Dockerfile does not define a HEALTHCHECK instruction, which is recommended by the container security guidelines. While less critical for a test runner than for long-running services, a health check can still help orchestration systems detect when the container is ready or has failed.
Example HEALTHCHECK for Cypress runner
WORKDIR /tmp/monitoring-plugin/web/cypress + +# Basic health check to verify container is operational +HEALTHCHECK --interval=30s --timeout=3s \ + CMD test -f /tmp/monitoring-plugin/web/cypress/package.json || exit 1As per coding guidelines: "HEALTHCHECK defined"
🤖 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 `@web/cypress/Dockerfile` around lines 1 - 9, Add a HEALTHCHECK instruction to the Dockerfile (after WORKDIR /tmp/monitoring-plugin/web/cypress) that performs a lightweight readiness probe for the Cypress test runner; implement the command to return non-zero on failure (e.g., a small shell/curl/command that verifies the container can run Cypress or that required resources are present), and include sensible options like --interval, --timeout, --start-period and --retries so orchestrators can detect unhealthy containers; reference the HEALTHCHECK keyword and place it in the Dockerfile near the existing WORKDIR line.
🤖 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 `@web/cypress/Dockerfile`:
- Line 2: The Dockerfile uses a non-approved floating base image in the FROM
instruction ("quay.io/redhat-distributed-tracing-qe/cypress-base:latest");
replace that FROM line with an approved image from catalog.redhat.com (UBI
minimal or distroless) and avoid the :latest floating tag by pinning to a stable
Red Hat tag or digest; if you need a Cypress-specific base, create and reference
a custom image you build FROM a UBI minimal/distroless image instead and pin
that image by tag or digest.
- Line 7: The Dockerfile currently uses COPY . /tmp/monitoring-plugin which
copies the entire build context; change it to copy only the required files and
directories for Cypress tests (for example COPY package.json package-lock.json
/tmp/monitoring-plugin/ and COPY web/ /tmp/monitoring-plugin/web/ and COPY
cypress/ /tmp/monitoring-plugin/cypress/ or other specific fixtures) so
unnecessary files (like .git, dev tooling, build artifacts) are excluded; update
the Dockerfile's COPY commands to list the specific paths needed for test
execution and adjust any subsequent paths/WORKDIR references that rely on
/tmp/monitoring-plugin accordingly.
---
Outside diff comments:
In `@web/cypress/Dockerfile`:
- Around line 1-9: The Dockerfile currently leaves the container running as root
after COPY/RUN/WORKDIR steps; add a USER instruction after the filesystem setup
(after RUN chmod -R 777 /tmp/monitoring-plugin and before/after WORKDIR
/tmp/monitoring-plugin/web/cypress) to switch to a non-root account (either
create a dedicated user/group earlier in the Dockerfile or switch to a provided
non-root UID/GID), and tighten file permissions (replace or relax chmod -R 777)
so the chosen non-root user has only the required access; reference the COPY,
RUN chmod -R 777, and WORKDIR instructions when applying the change.
---
Nitpick comments:
In `@web/cypress/Dockerfile`:
- Around line 1-9: Add a HEALTHCHECK instruction to the Dockerfile (after
WORKDIR /tmp/monitoring-plugin/web/cypress) that performs a lightweight
readiness probe for the Cypress test runner; implement the command to return
non-zero on failure (e.g., a small shell/curl/command that verifies the
container can run Cypress or that required resources are present), and include
sensible options like --interval, --timeout, --start-period and --retries so
orchestrators can detect unhealthy containers; reference the HEALTHCHECK keyword
and place it in the Dockerfile near the existing WORKDIR line.
🪄 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: f80a7a86-69a9-4152-a6eb-509627e6546a
📒 Files selected for processing (1)
web/cypress/Dockerfile
|
/cc @etmurasaki |
|
/retest |
1 similar comment
|
/retest |
|
@DavidRajnoha: The following test 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. |
1 participant
Adds
operator-sdkbinary to the Cypress test runner image (web/cypress/Dockerfile).Fixes
operator-sdk: command not found(exit 127) whenCYPRESS_CUSTOM_COO_BUNDLE_IMAGEis set in CI periodic jobs.Made with Cursor
Summary by CodeRabbit