Kind e2e tests#188
Conversation
Add a reusable e2e-tests-kind.yml workflow that creates a kind cluster and runs the e2e test suite against it. The roxie binary is extracted from the already-built image. OLM tests are skipped since kind has no OLM. Wire the new job into both PR and main-push workflows, running in parallel with the existing GKE-based e2e tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a new "E2E Tests (kind)" reusable GitHub Actions workflow wired into pr.yml and main-push.yml, introduces a Tiny ResourceProfile with corresponding resource constants and operator deployment logic, extends DeepMerge to concatenate compatible slices with type-mismatch error handling, and updates e2e test helpers and workflows to configure the resource profile via environment variable. ChangesCI Workflow
Tiny Resource Profile
DeepMerge Slice Appending
E2E Test Helpers
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PR as pr.yml / main-push.yml
participant Kind as e2e-tests-kind.yml
participant Quay as Quay.io
participant KindCluster as kind cluster
PR->>Kind: workflow_call with built image
Kind->>Quay: docker login
Kind->>Kind: extract roxie binary from image
Kind->>Kind: install roxctl (checksum verified)
Kind->>KindCluster: create kind cluster
Kind->>KindCluster: run make run-test-e2e
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🧹 Nitpick comments (3)
.github/workflows/e2e-tests-kind.yml (2)
24-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout.This job doesn't need to push using the checked-out credentials; persisting them unnecessarily widens the blast radius if the runner is compromised.
🔒 Proposed fix
- name: Checkout uses: actions/checkout@v6 with: fetch-depth: 0 ref: ${{ github.event.pull_request.head.sha || github.sha }} + persist-credentials: false🤖 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 @.github/workflows/e2e-tests-kind.yml around lines 24 - 28, The Checkout step in the e2e-tests-kind workflow is persisting GitHub credentials unnecessarily. Update the actions/checkout usage in the Checkout job to set persist-credentials to false alongside the existing fetch-depth and ref settings, since this job only needs to read the repository and should not retain push-capable credentials on the runner.Source: Linters/SAST tools
55-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
roxctlinstall step.This block is copy-pasted verbatim from
e2e-tests.yml(same version, sha256, and commands). Consider extracting into a composite action to avoid drift if the pinned version/sha needs updating in only one place.🤖 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 @.github/workflows/e2e-tests-kind.yml around lines 55 - 64, The roxctl installation block in the e2e-tests-kind workflow is duplicated from the other workflow, so update the shared setup by extracting this logic into a reusable composite action or shared step definition. Replace the repeated install commands in the roxctl install step with a call to that shared action, keeping the pinned version and sha256 managed in one place to prevent drift..github/workflows/main-push.yml (1)
38-44: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider scoping down permissions for the reusable workflow call.
No explicit
permissions:block is set on this job, so the reusable workflow inherits default (potentially broad)GITHUB_TOKENpermissions. Since this job only needs to build/run tests against a kind cluster, restricting to e.g.contents: readat the call site would tighten the token's blast radius.🤖 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 @.github/workflows/main-push.yml around lines 38 - 44, The reusable workflow call in e2e-tests-kind currently relies on the default GITHUB_TOKEN scope, which is broader than needed. Add an explicit permissions block on this job in main-push.yml and restrict it to the minimum required, such as contents: read, since the job only invokes the e2e-tests-kind reusable workflow to build and run tests against kind.Source: Linters/SAST tools
🤖 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 @.github/workflows/e2e-tests-kind.yml:
- Around line 43-48: The “Extract roxie binary from image” step in
e2e-tests-kind.yml is expanding inputs.image directly inside the run script,
which should be avoided. Move the image reference into an environment variable
for that step and have the docker create command use the env var instead, so the
shell treats it as data; update the workflow step using the existing
inputs.image reference and the docker create/docker cp/docker rm sequence.
In `@internal/deployer/deploy_via_operator.go`:
- Around line 762-767: The initContainer resource request in
deploy_via_operator.go uses the wrong memory unit for the crs init container.
Update the patch value in the deploy logic that targets
spec.template.spec.initContainers[name:crs].resources.requests so the memory
request uses 150Mi instead of 150m, keeping the CPU request unchanged.
In `@internal/helpers/helpers.go`:
- Around line 133-162: The DeepMerge helper currently treats only []interface{}
and []map[string]interface{} as slices, so typed slices like
spec.imagePullSecrets seeded from config can’t be merged and fall into the
mismatch error path. Update toSlice in helpers.go to recognize the typed slice
used by imagePullSecrets (and any equivalent typed map slice) by converting it
into []interface{} before merging, so DeepMerge can append overlays instead of
rejecting them. Use the existing DeepMerge and toSlice symbols to keep the fix
localized.
---
Nitpick comments:
In @.github/workflows/e2e-tests-kind.yml:
- Around line 24-28: The Checkout step in the e2e-tests-kind workflow is
persisting GitHub credentials unnecessarily. Update the actions/checkout usage
in the Checkout job to set persist-credentials to false alongside the existing
fetch-depth and ref settings, since this job only needs to read the repository
and should not retain push-capable credentials on the runner.
- Around line 55-64: The roxctl installation block in the e2e-tests-kind
workflow is duplicated from the other workflow, so update the shared setup by
extracting this logic into a reusable composite action or shared step
definition. Replace the repeated install commands in the roxctl install step
with a call to that shared action, keeping the pinned version and sha256 managed
in one place to prevent drift.
In @.github/workflows/main-push.yml:
- Around line 38-44: The reusable workflow call in e2e-tests-kind currently
relies on the default GITHUB_TOKEN scope, which is broader than needed. Add an
explicit permissions block on this job in main-push.yml and restrict it to the
minimum required, such as contents: read, since the job only invokes the
e2e-tests-kind reusable workflow to build and run tests against kind.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 87eda6e8-22a2-4032-b925-a1673f18af85
📒 Files selected for processing (12)
.github/workflows/e2e-tests-kind.yml.github/workflows/e2e-tests.yml.github/workflows/main-push.yml.github/workflows/pr.ymlinternal/clusterdefaults/clusterdefaults.gointernal/deployer/constants.gointernal/deployer/deploy_via_operator.gointernal/deployer/deployer.gointernal/helpers/helpers.gointernal/helpers/helpers_test.gointernal/types/resources.gotests/e2e/helpers.go
| - name: Extract roxie binary from image | ||
| run: | | ||
| docker create --name roxie-extract "${{ inputs.image }}" | ||
| docker cp roxie-extract:/usr/local/bin/roxie "$GITHUB_WORKSPACE/roxie" | ||
| docker rm roxie-extract | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid direct template expansion inside run: block.
"${{ inputs.image }}" is expanded directly into the shell command before execution, which zizmor flags as a template-injection risk. Prefer passing it through an environment variable so the shell treats it as data rather than as part of the script.
🔒 Proposed fix
- name: Extract roxie binary from image
+ env:
+ IMAGE_REF: ${{ inputs.image }}
run: |
- docker create --name roxie-extract "${{ inputs.image }}"
+ docker create --name roxie-extract "$IMAGE_REF"
docker cp roxie-extract:/usr/local/bin/roxie "$GITHUB_WORKSPACE/roxie"
docker rm roxie-extract📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Extract roxie binary from image | |
| run: | | |
| docker create --name roxie-extract "${{ inputs.image }}" | |
| docker cp roxie-extract:/usr/local/bin/roxie "$GITHUB_WORKSPACE/roxie" | |
| docker rm roxie-extract | |
| - name: Extract roxie binary from image | |
| env: | |
| IMAGE_REF: ${{ inputs.image }} | |
| run: | | |
| docker create --name roxie-extract "$IMAGE_REF" | |
| docker cp roxie-extract:/usr/local/bin/roxie "$GITHUB_WORKSPACE/roxie" | |
| docker rm roxie-extract |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 45-45: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/e2e-tests-kind.yml around lines 43 - 48, The “Extract
roxie binary from image” step in e2e-tests-kind.yml is expanding inputs.image
directly inside the run script, which should be avoided. Move the image
reference into an environment variable for that step and have the docker create
command use the env var instead, so the shell treats it as data; update the
workflow step using the existing inputs.image reference and the docker
create/docker cp/docker rm sequence.
Source: Linters/SAST tools
| "patches": []map[string]interface{}{ | ||
| { | ||
| "path": "spec.template.spec.initContainers[name:crs].resources.requests", | ||
| "value": `{"cpu":"80m", "memory": "150m"}`, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Kubernetes resource quantity "m" suffix for memory
💡 Result:
In Kubernetes, the "m" suffix when applied to memory resource quantities denotes "millibytes," where 1m equals 1/1000 of a byte [1]. Because Kubernetes measures memory resources in whole bytes, the "m" suffix is not useful for specifying memory requirements [1]. Specifying memory with this suffix, such as "400m", results in a fractional value (e.g., 0.4 bytes), which is generally not a valid or intended configuration for memory requests or limits [1]. It is critical to distinguish between the case-sensitive suffixes: - "M" stands for megabytes (10^6 bytes) [1]. - "m" stands for millibytes (10^-3 bytes) [1]. For memory, users should typically use power-of-two suffixes (e.g., Mi, Gi) or decimal SI suffixes (e.g., M, G) to represent the intended integer number of bytes [1][2][3]. Using "m" in the context of memory is often a mistake by users who intended to specify a standard unit like mebibytes (Mi) or megabytes (M) [1].
Citations:
- 1: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
- 2: https://kubernetes.io/docs/tasks/configure-pod-container/assign-memory-resource/
- 3: https://github.com/kubernetes/design-proposals-archive/blob/main/scheduling/resources.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section around the reported lines.
sed -n '730,780p' internal/deployer/deploy_via_operator.go | cat -n
# Find other occurrences of the same resource value or nearby resource patches.
rg -n '"150m"|"80m"|initContainers\[name:crs\]|resources.requests' internal/deployer/deploy_via_operator.goRepository: stackrox/roxie
Length of output: 2138
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the resource profiles and the sensor resource definitions to see whether this is a request-only tweak.
rg -n 'securedClusterSensorResourcesTiny|securedClusterSensorResourcesSmall|resources:\s*securedClusterSensorResources' internal/deployer/deploy_via_operator.go
sed -n '1,140p' internal/deployer/deploy_via_operator.go | cat -nRepository: stackrox/roxie
Length of output: 5477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the resource constants and any companion limits.
rg -n 'securedClusterSensorResourcesTiny|securedClusterSensorResourcesSmall|requests|limits' internal/deployer/deploy_via_operator.go
sed -n '1,140p' internal/deployer/deploy_via_operator.go | cat -nRepository: stackrox/roxie
Length of output: 5564
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the tiny/small resource constants to see whether limits are set elsewhere.
sed -n '760,840p' internal/deployer/deploy_via_operator.go | cat -n
rg -n 'securedClusterSensorResourcesTiny|securedClusterSensorResourcesSmall' internal/deployer/deploy_via_operator.goRepository: stackrox/roxie
Length of output: 2945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact resource quantities used for the sensor and any related initContainer resources.
sed -n '780,860p' internal/deployer/deploy_via_operator.go | cat -nRepository: stackrox/roxie
Length of output: 2769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the tiny/small resource definitions and the surrounding overlay patch in one slice.
sed -n '780,860p' internal/deployer/deploy_via_operator.go | cat -nRepository: stackrox/roxie
Length of output: 2769
Use 150Mi for the initContainer memory request. m is millibytes for memory, so 150m is effectively zero and will skew scheduling for crs; 150Mi looks like the intended unit.
🐛 Proposed fix
- "value": `{"cpu":"80m", "memory": "150m"}`,
+ "value": `{"cpu":"80m", "memory": "150Mi"}`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "patches": []map[string]interface{}{ | |
| { | |
| "path": "spec.template.spec.initContainers[name:crs].resources.requests", | |
| "value": `{"cpu":"80m", "memory": "150m"}`, | |
| }, | |
| }, | |
| "patches": []map[string]interface{}{ | |
| { | |
| "path": "spec.template.spec.initContainers[name:crs].resources.requests", | |
| "value": `{"cpu":"80m", "memory": "150Mi"}`, | |
| }, | |
| }, |
🤖 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 `@internal/deployer/deploy_via_operator.go` around lines 762 - 767, The
initContainer resource request in deploy_via_operator.go uses the wrong memory
unit for the crs init container. Update the patch value in the deploy logic that
targets spec.template.spec.initContainers[name:crs].resources.requests so the
memory request uses 150Mi instead of 150m, keeping the CPU request unchanged.
| if baseSlice, ok := toSlice(baseVal); ok { | ||
| if overlaySlice, ok := toSlice(v); ok { | ||
| base[k] = append(baseSlice, overlaySlice...) | ||
| continue | ||
| } | ||
| return fmt.Errorf("incompatible types for key %q: slice vs. %T", k, v) | ||
| } | ||
| if _, ok := toSlice(v); ok { | ||
| return fmt.Errorf("incompatible types for key %q: %T vs. slice", k, baseVal) | ||
| } | ||
| } | ||
| // Override with overlay value | ||
|
|
||
| base[k] = v | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func toSlice(v interface{}) ([]interface{}, bool) { | ||
| if s, ok := v.([]interface{}); ok { | ||
| return s, true | ||
| } | ||
| if s, ok := v.([]map[string]interface{}); ok { | ||
| out := make([]interface{}, len(s)) | ||
| for i, e := range s { | ||
| out[i] = e | ||
| } | ||
| return out, true | ||
| } | ||
| return nil, false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify DeepMerge/MergeMaps call sites that may pass typed slices through map[string]interface{} values.
rg -n --type=go -C4 '\b(DeepMerge|MergeMaps)\s*\(' .
sed -n '133,162p' internal/helpers/helpers.goRepository: stackrox/roxie
Length of output: 9429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the merge callers and the data they feed into helpers.DeepMerge/MergeMaps.
for f in internal/deployer/feature_flags.go internal/deployer/config.go internal/helpers/helpers.go internal/helpers/helpers_test.go; do
echo "===== $f ====="
wc -l "$f"
done
echo "===== feature_flags.go outline ====="
ast-grep outline internal/deployer/feature_flags.go --view expanded || true
echo "===== config.go outline ====="
ast-grep outline internal/deployer/config.go --view expanded || true
echo "===== helpers.go outline ====="
ast-grep outline internal/helpers/helpers.go --view expanded || true
echo "===== featureFlagsToOverrides ====="
rg -n -C 30 'func featureFlagsToOverrides|ToUnstructuredConfig|DeepMerge|MergeMaps|toSlice|Unstructured' internal/deployer internal/helpers
echo "===== typed slice builders in repo ====="
rg -n -C 3 '(\[\]string|\[\]int|\[\]bool|\[\]float64|\[\]interface\{\}|\[\]map\[string\]interface\{\})' internal | sed -n '1,220p'Repository: stackrox/roxie
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for typed slices being inserted into map[string]interface{} values that can reach DeepMerge/MergeMaps.
rg -n -C 3 'map\[string\]interface\{\}\{[^}]*\[\][A-Za-z_][A-Za-z0-9_]*\{' internal || true
echo "-----"
rg -n -C 3 'featureFlagsToOverrides|ToUnstructuredConfig|ToUnstructured|UnstructuredConfig|mapstructure|yaml.Unmarshal|json.Unmarshal' internal || true
echo "-----"
sed -n '1,220p' internal/deployer/feature_flags.go
echo "-----"
sed -n '1,240p' internal/deployer/config.goRepository: stackrox/roxie
Length of output: 23172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,160p' internal/types/exposure.go
echo "-----"
rg -n -C 2 'imagePullSecrets|labels": map\[string\]string|map\[string\]string|[]map\[string\]string' internal/deployer internal/types internal/helpersRepository: stackrox/roxie
Length of output: 1972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect every occurrence of imagePullSecrets and any DeepMerge tests around slice handling.
rg -n -C 3 'imagePullSecrets|DeepMergeAppendsSlices|DeepMergeSliceTypeMismatch|toSlice\(' internal/deployer internal/helpers internal -g '!**/*vendor*'Repository: stackrox/roxie
Length of output: 8051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show all imagePullSecrets occurrences and the exact slice handling tests.
rg -n -C 3 'imagePullSecrets|DeepMergeAppendsSlices|DeepMergeSliceTypeMismatch|func toSlice' internal/deployer internal/helpers internalRepository: stackrox/roxie
Length of output: 6255
Broaden toSlice to accept typed slices — spec.imagePullSecrets is seeded as []map[string]string in internal/deployer/config.go, but DeepMerge only recognizes []interface{} and []map[string]interface{}. An overlay for that key will hit the slice mismatch path instead of appending.
🤖 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 `@internal/helpers/helpers.go` around lines 133 - 162, The DeepMerge helper
currently treats only []interface{} and []map[string]interface{} as slices, so
typed slices like spec.imagePullSecrets seeded from config can’t be merged and
fall into the mismatch error path. Update toSlice in helpers.go to recognize the
typed slice used by imagePullSecrets (and any equivalent typed map slice) by
converting it into []interface{} before merging, so DeepMerge can append
overlays instead of rejecting them. Use the existing DeepMerge and toSlice
symbols to keep the fix localized.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests