ci(nightly): Replace the operator image by its Sealights-instrumented counterpart if SL is enabled [RHDHBUGS-2050]

[rm3l]

Description

• Introduce a new repo variable (SEALIGHTS_ENABLED) allowing to skip Sealights completely if needed
• Replace the operator image by its Sealights-instrumented counterpart if SL is enabled

Which issue(s) does this PR fix or relate to

• Fixes https://issues.redhat.com/browse/RHDHBUGS-2050

PR acceptance criteria

• Tests
• Documentation

How to test changes / Special notes to the reviewer

Checked locally that this works by running a simple E2E test:

$ SEALIGHTS_ENABLED=true OPERATOR_MANIFEST=`pwd`/dist/rhdh/install.yaml ginkgo -v -nodes=1 -focus 'examples/bs1.yaml' tests/e2e

[...]
[SynchronizedAfterSuite] PASSED [7.015 seconds]                                                                                                                                            
------------------------------                                                                                                                                                             
                                                                                                                                                                                           
Ran 1 of 8 Specs in 267.467 seconds                                                                                                                                                        
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 7 Skipped                                                                                                                                    
PASS                                                                                                                                                                                       
                                                                                                                                                                                           
Ginkgo ran 1 suite in 4m30.209109555s                                                                                                                                                      
Test Suite Passed

Summary by Sourcery

Enable optional SeaLights integration in nightly CI workflows and E2E tests by introducing a SEALIGHTS_ENABLED toggle, swapping the operator image for its instrumented variant, and configuring imagePullSecrets when needed.

New Features:

• Introduce SEALIGHTS_ENABLED variable to toggle SeaLights instrumentation.
• Replace the operator image with its SeaLights-instrumented counterpart when enabled.

Enhancements:

• Add DownloadFile and AddPullSecretToDeployment helpers for file retrieval and Kubernetes deployment patching.

CI:

• Gate all SeaLights steps in the nightly GitHub Actions workflow on the SEALIGHTS_ENABLED flag.

Tests:

• Modify E2E suite to download operator manifests, swap images based on SEALIGHTS_ENABLED, and patch deployments with a pull secret for private SeaLights images.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nickboldt for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ RHDHBUGS-2050 - PR Code Verified Compliant requirements: E2E tests should use the Sealights-instrumented operator image instead of the standard operator image. Provide a mechanism to enable/disable Sealights usage (repo-level variable/flag). Ensure necessary auth (pull secret) is handled for the private Sealights image in E2E environment. Integrate Sealights steps in CI only when enabled, without breaking existing flows. Requires further human verification: Validate on CI that the operator is indeed pulled from the Sealights repository when SEALIGHTS_ENABLED=true and that coverage is reported in Sealights dashboards. Confirm that non-SL runs (SEALIGHTS_ENABLED not 'true') behave exactly as before across branches.

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue Error variable 'err' reused before check: in the Sealights branch, 'data, err := os.ReadFile(p)' assigns 'err' but 'err' is only checked after the write. A failed read would be ignored. Should check read error before using 'data'. data, err := os.ReadFile(p) updated := strings.ReplaceAll(string(data), "quay.io/rhdh/rhdh-rhel9-operator", "quay.io/rhdh/rhdh-rhel9-operator-sealights") err = os.WriteFile(p, []byte(updated), 0644) Expect(err).ShouldNot(HaveOccurred()) } Robustness DownloadFile performs http.Get without context/timeout and does not close resp.Body on non-200 paths via early returns. Consider using a client with timeout and ensure body is closed in all paths; also handle large files and set User-Agent if required by source. func DownloadFile(url string) (string, error) { var in io.ReadCloser var err error if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { resp, err := http.Get(url) if err != nil { return "", fmt.Errorf("failed to GET remote file: %w", err) } if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("bad status: %s", resp.Status) } in = resp.Body } else { in, err = os.Open(url) if err != nil { return "", fmt.Errorf("failed to open local file: %w", err) } } defer in.Close() // Create a temporary file in system temp dir tmpFile, err := os.CreateTemp("", "download-*"+filepath.Ext(url)) if err != nil { return "", fmt.Errorf("failed to create temp file: %w", err) } defer tmpFile.Close() _, err = io.Copy(tmpFile, in) if err != nil { return "", fmt.Errorf("copy failed: %w", err) } return tmpFile.Name(), nil } Security Hygiene 'Remove SeaLights secrets' step now uses 'rm -rf sltoken.txt || true'. Consider using a more explicit secure deletion or ensure no logs contain token; also confirm secrets aren't echoed elsewhere. Minor, but worth validating. if: always() && vars.SEALIGHTS_ENABLED == 'true' && steps.operator-image-existence-checker.outputs.OPERATOR_IMAGE_EXISTS == 'true' run: | echo "[SeaLights] Cleaning up after SeaLights run" rm -rf sltoken.txt || true

📄 References No matching references available

[qodo-code-review]

PR Type

Enhancement

---

Description

• Add conditional Sealights integration with SEALIGHTS_ENABLED variable

• Replace operator image with Sealights-instrumented version when enabled

• Add pull secret management for private Sealights images

• Enhance E2E test infrastructure with file download utilities

---

File Walkthrough

Relevant files

Enhancement

e2e_suite_test.go Add Sealights operator image replacement logic                      tests/e2e/e2e_suite_test.go Add installRhdhOperatorManifest function to handle operator manifest installation Replace operator image with Sealights version when SEALIGHTS_ENABLED=true Add pull secret patching for private Sealights images Refactor existing operator installation logic +33/-3    utils.go Add file download and deployment utilities                              tests/helper/utils.go Add DownloadFile function supporting both HTTP URLs and local files Add AddPullSecretToDeployment function for Kubernetes deployment patching Import additional packages for HTTP and file operations +50/-0    nightly.yaml Add conditional Sealights workflow integration                      .github/workflows/nightly.yaml Add conditional Sealights steps based on SEALIGHTS_ENABLED variable Create Kubernetes namespace and pull secret for Sealights images Pass SEALIGHTS_ENABLED environment variable to E2E tests Update cleanup step conditions +25/-10

---

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `.github/workflows/nightly.yaml:217-223` </location>
<code_context>
             --selector=app.kubernetes.io/component=controller \
             --timeout=90s

+      - name: E2E prerequisites for Sealights
+        if: ${{ vars.SEALIGHTS_ENABLED == 'true' && steps.operator-image-existence-checker.outputs.OPERATOR_IMAGE_EXISTS == 'true' }}
+        env:
+          SEALIGHTS_BOT_QUAY_USERNAME: ${{secrets.SEALIGHTS_BOT_QUAY_USERNAME}}
+          SEALIGHTS_BOT_QUAY_PASSWORD: ${{secrets.SEALIGHTS_BOT_QUAY_PASSWORD}}
+        run: |
+          kubectl create namespace rhdh-operator
+          # Needed because the Operator Sealights image is private
</code_context>

<issue_to_address>
**suggestion:** Check for idempotency in namespace and secret creation.

If the namespace or secret already exists, these commands will fail. To improve robustness, use 'kubectl apply' or add logic to handle existing resources without error.

```suggestion
          kubectl get namespace rhdh-operator || kubectl create namespace rhdh-operator
          # Needed because the Operator Sealights image is private
          kubectl -n rhdh-operator create secret docker-registry rhdh-pull-secret \
            --docker-server=quay.io \
            --docker-username="$SEALIGHTS_BOT_QUAY_USERNAME" \
            --docker-password="$SEALIGHTS_BOT_QUAY_PASSWORD" \
            --docker-email=asoro@redhat.com || \
          kubectl -n rhdh-operator delete secret rhdh-pull-secret && \
          kubectl -n rhdh-operator create secret docker-registry rhdh-pull-secret \
            --docker-server=quay.io \
            --docker-username="$SEALIGHTS_BOT_QUAY_USERNAME" \
            --docker-password="$SEALIGHTS_BOT_QUAY_PASSWORD" \
            --docker-email=asoro@redhat.com
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix potential resource leak on error In the DownloadFile function, close resp.Body before returning an error for a non-OK HTTP status to prevent a resource leak. tests/helper/utils.go [164-173] if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { resp, err := http.Get(url) if err != nil { return "", fmt.Errorf("failed to GET remote file: %w", err) } if resp.StatusCode != http.StatusOK { + resp.Body.Close() return "", fmt.Errorf("bad status: %s", resp.Status) } in = resp.Body } else { -... [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a resource leak in the newly added DownloadFile function, where resp.Body is not closed on a non-200 status code, preventing a potential bug.	Medium
High-level	Modify manifest before applying it Instead of applying a manifest and then patching the deployment, modify the manifest content in-memory to include the Sealights image and imagePullSecrets before applying it. This prevents race conditions and ensures the deployment is created correctly from the start. Examples: tests/e2e/e2e_suite_test.go [38-68] func installRhdhOperatorManifest(operatorManifest string) { p, dErr := helper.DownloadFile(operatorManifest) Expect(dErr).ShouldNot(HaveOccurred()) defer os.Remove(p) fmt.Fprintf(GinkgoWriter, "Installing RHDH Operator Manifest: %q\n", p) withSL := os.Getenv("SEALIGHTS_ENABLED") == "true" if withSL { data, err := os.ReadFile(p) updated := strings.ReplaceAll(string(data), "quay.io/rhdh/rhdh-rhel9-operator", "quay.io/rhdh/rhdh-rhel9-operator-sealights") ... (clipped 21 lines) Solution Walkthrough: Before: func installRhdhOperatorManifest(operatorManifest string) { // ... if withSL { data, _ := os.ReadFile(p) updated := strings.ReplaceAll(string(data), "...", "quay.io/rhdh/rhdh-rhel9-operator-sealights") os.WriteFile(p, []byte(updated), 0644) } // Apply manifest, creating the deployment exec.Command(helper.GetPlatformTool(), "apply", "-f", p) if withSL { // Wait for deployment to exist Eventually(...).Should(Succeed()) // Patch the existing deployment with the pull secret helper.AddPullSecretToDeployment(_namespace, "rhdh-operator", "rhdh-pull-secret") } } After: func installRhdhOperatorManifest(operatorManifest string) { // ... manifestData, _ := os.ReadFile(p) if withSL { // Unmarshal YAML, modify the Deployment object in memory // to change the image and add imagePullSecrets. // This avoids string manipulation and is more robust. modifiedData := updateDeploymentInManifest(manifestData, "quay.io/rhdh/rhdh-rhel9-operator-sealights", "rhdh-pull-secret") manifestData = modifiedData } // Apply the fully-formed manifest from stdin cmd := exec.Command(helper.GetPlatformTool(), "apply", "-f", "-") cmd.Stdin = bytes.NewReader(manifestData) helper.Run(cmd) } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a race condition where pods might fail with ImagePullBackOff before the deployment is patched, proposing a more robust, atomic approach to improve test reliability.	Medium
Security	Avoid hardcoding personal email addresses Replace the hardcoded personal email asoro@redhat.com in the docker secret creation command with a generic, non-personal email address. .github/workflows/nightly.yaml [223] ---docker-email=asoro@redhat.com +--docker-email=bot@example.com Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a hardcoded personal email address in a CI/CD configuration, which is a minor security and maintenance concern.	Low
Update

[rm3l]

Merging so we can trigger the nightly workflow and see how it goes..