e2e: run e2e testing with the operator as well#173
Conversation
📝 WalkthroughWalkthroughAdds a METHOD selector (operator|helm) propagated through CI, Makefiles, e2e scripts, and deploy helpers; injects dynamic AUTH_CONFIG for operator deployments; adds AutoProvisioning to the operator API/CRD and wires the Jumpstarter CR to controller provisioning behavior and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions (matrix)
participant Make as Makefile
participant Setup as e2e/setup-e2e.sh
participant DeployScript as controller/hack/deploy_with_operator.sh
participant Controller as Operator/Helm deploy
participant Cluster as Kubernetes
GH->>Make: run targets with METHOD
Make->>Setup: e2e-setup (METHOD)
Setup->>DeployScript: request controller deploy (METHOD)
alt METHOD == operator
DeployScript->>Controller: apply operator manifests (inject AUTH_CONFIG)
else METHOD == helm
DeployScript->>Controller: helm install (values file)
end
Controller->>Cluster: create resources
Cluster-->>Controller: readiness / endpoint info
Controller-->>Setup: return endpoint/status
Setup->>Make: write .e2e-setup-complete marker (includes METHOD)
Make->>Setup: e2e-run (METHOD)
Setup->>Cluster: execute tests against endpoint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
|
ahh, the deploy with operator part is not setting up dex yet, so it blocks on the OIDC logins. e2e tests need some timeout I guess. |
![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
🤖 Fix all issues with AI agents
In `@controller/hack/deploy_with_operator.sh`:
- Around line 116-152: The JWT YAML block built into JWT_AUTH_CONFIG has
incorrect indentation (the `jwt:` key and its nested `audiences` lines are not
indented relative to each other), which yields invalid CR; update the string
construction for JWT_AUTH_CONFIG (and where it's injected into AUTH_CONFIG) so
that all `jwt`-scoped entries (issuer, url, audiences list items,
audienceMatchPolicy, certificateAuthority, claimMappings, etc.) are indented one
additional level relative to `jwt:` and the `audiences` list items are indented
as YAML list elements (ensure the lines produced by DEX_CA_CONTENT via sed keep
the correct extra indentation as well); fix the indentation where
JWT_AUTH_CONFIG is concatenated into AUTH_CONFIG so the final YAML under the
`authentication:` block is properly nested.
In `@e2e/setup-e2e.sh`:
- Around line 315-334: The endpoint resolution currently masks failures because
command substitutions are used directly in local/ export assignments; change to
run the kubectl/helm commands separately, capture their output into variables
(e.g., call kubectl get jumpstarter ... and assign to a temp variable before
setting BASEDOMAIN), check their exit status, then set BASEDOMAIN and export
ENDPOINT (for operator branch use BASEDOMAIN and port 8082; for helm branch run
helm get values ... | jq -r ... into a variable, verify success, then export
ENDPOINT). Update the code paths referencing METHOD, JS_NAMESPACE, BASEDOMAIN,
and ENDPOINT so failures from kubectl or helm propagate (allow set -e to fail)
instead of being swallowed by command substitution.
In `@e2e/tests.bats`:
- Around line 63-75: The parallel kubectl waits
(exporters.jumpstarter.dev/test-exporter-oidc, -sa, -legacy) are started into
pids pid1/pid2/pid3 but using a single "wait $pid1 $pid2 $pid3" masks earlier
errors; capture each wait exit code individually (wait $pid1 -> rc1=$?; wait
$pid2 -> rc2=$?; wait $pid3 -> rc3=$?) and then aggregate (if any rcN != 0 then
fail/exit with non-zero and print which exporter failed) so all failures are
reported instead of only the last one.
|
ok, auto provisioning is disabled: (in test file e2e/tests.bats, line 207)`jmp shell --client test-client-oidc-provisioning --selector example.com/board=oidc j power on' failedThis fails because the user is not created in jumpstarter, but we are not enabling auto-provisioning of users in the operator/controller |
we don't even have the option ... nice |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/deploy/operator/api/v1alpha1/jumpstarter_types.go (1)
343-360:⚠️ Potential issue | 🟠 MajorRegenerate CRD manifests to include autoProvisioning field.
The new
spec.authentication.autoProvisioningfield was added to the Go types but is missing from the generated CRD schema atcontroller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml. Without this,kubectl applywill reject or silently drop the field when users specify it in their Jumpstarter resources. Run the CRD generation command (typicallycontroller-gen) to update the manifest.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/e2e.yaml:
- Around line 15-31: The changes job’s path filtering lacks a base for git diff
and sufficient checkout history; update the job referenced as changes to: (1)
pass a base parameter to the dorny/paths-filter step (set base to the target
branch, e.g., "main") so merge_group/git-based diffs work, (2) set
actions/checkout@v4 to use fetch-depth: 0 (or at least 20) so merge-base
detection can find history, and (3) broaden the job if condition to run on
manual dispatch by changing the if for the changes job to include "||
github.event_name == 'workflow_dispatch'"; apply these edits around the steps
using actions/checkout and the dorny/paths-filter step with id: filter.
mangelajo
force-pushed
the
e2e-with-operator
branch
from
222b9b4 to
13ef492
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/e2e.yaml:
- Around line 15-34: The workflow grants only contents: read but
dorny/paths-filter@v3 (step id: filter) needs pull-requests: read when running
on pull_request events; add a workflow-level permissions block that includes
pull-requests: read (and keep contents: read) so the filter step can call the
REST API, then re-run to ensure outputs.should_run from the filter step works
correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@e2e/run-e2e.sh`:
- Around line 101-103: The bats invocation currently runs with -x (bats -x ...)
which enables trace output and can leak secrets; make tracing opt-in by checking
an env flag (e.g., E2E_TRACE or RUN_E2E_TRACE) before adding -x: update the code
around log_info and the bats invocation so that by default you run bats without
-x, and only include the -x and --show-output-of-passing-tests flags when the
env flag is truthy; reference the existing log_info call and the bats invocation
to locate where to conditionally build the bats command and execute it.
| # Run bats tests | ||
| log_info "Running bats tests..." | ||
| bats --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats | ||
| bats -x --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats |
There was a problem hiding this comment.
Guard bats -x to avoid leaking secrets in CI logs.
Tracing can print command lines and env-expanded secrets (OIDC tokens, kubeconfig, etc.) into logs. Make tracing opt-in via an env flag.
🔧 Suggested change (opt‑in trace)
- bats -x --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats
+ BATS_TRACE_FLAG=()
+ if [[ -n "${BATS_TRACE:-}" ]]; then
+ BATS_TRACE_FLAG=(-x)
+ fi
+ bats "${BATS_TRACE_FLAG[@]}" --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats📝 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.
| # Run bats tests | |
| log_info "Running bats tests..." | |
| bats --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats | |
| bats -x --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats | |
| # Run bats tests | |
| log_info "Running bats tests..." | |
| BATS_TRACE_FLAG=() | |
| if [[ -n "${BATS_TRACE:-}" ]]; then | |
| BATS_TRACE_FLAG=(-x) | |
| fi | |
| bats "${BATS_TRACE_FLAG[@]}" --show-output-of-passing-tests --verbose-run "$SCRIPT_DIR"/tests.bats |
🤖 Prompt for AI Agents
In `@e2e/run-e2e.sh` around lines 101 - 103, The bats invocation currently runs
with -x (bats -x ...) which enables trace output and can leak secrets; make
tracing opt-in by checking an env flag (e.g., E2E_TRACE or RUN_E2E_TRACE) before
adding -x: update the code around log_info and the bats invocation so that by
default you run bats without -x, and only include the -x and
--show-output-of-passing-tests flags when the env flag is truthy; reference the
existing log_info call and the bats invocation to locate where to conditionally
build the bats command and execute it.
There was a problem hiding this comment.
that's ok, we don't have any of those in bats and makes debugging much easier and logs more understandable
There was a problem hiding this comment.
@mangelajo, understood! I appreciate the clarification. Keeping the tracing enabled unconditionally makes sense if it significantly improves the debugging experience and there are no secrets to worry about in the test environment.
✏️ Learnings added
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 173
File: e2e/run-e2e.sh:101-103
Timestamp: 2026-02-02T09:17:04.720Z
Learning: In the jumpstarter e2e tests (e2e/run-e2e.sh and e2e/tests.bats), bats tracing (-x flag) should remain unconditionally enabled because there are no secrets in the bats tests and it makes debugging much easier with more understandable logs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Thank you bella! |
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.