feat: add deploy-kernel action and Dockerfile validation#22
feat: add deploy-kernel action and Dockerfile validation#22
Conversation
Static validation of kernel Dockerfiles before submission to API. Checks: required qbraid.kernel.* LABELs, EXPOSE 8888, CMD/ENTRYPOINT referencing jupyter kernelgateway, no root USER, no privileged flags, kernel.json presence, and kernel name format validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Course validator now fetches the kernel catalog from qbook-staging.k8s.qbraid.com/api/kernelspecs and verifies that every kernelName in chapters/sections exists. Gracefully skips validation if catalog is unreachable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New composite action at deploy-kernel/ for deploying custom Jupyter kernel images to qBraid. Called as qBraid/upload-course-action/deploy-kernel@main. Inputs: api-key, dockerfile-path, kernel-name, language, display-name, context-dir, api-base-url. Flow: validate Dockerfile structure -> base64 encode + submit to API -> poll Cloud Build status until active/failed. Outputs: kernel-name, image-uri, status, build-id. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lint job was failing on a stray blank line between the import block and the module-level constants. Auto-fixed via isort. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new deploy-kernel/ composite GitHub Action for deploying custom Jupyter kernel images to qBraid, alongside a shared static Dockerfile validator and additional course validation to ensure referenced kernels exist in a catalog.
Changes:
- Added a shared
validate_dockerfile.pyimplementation plus a deploy-kernel wrapper that delegates to it. - Added
deploy-kernelcomposite action + deployment script with polling and GitHub Actions outputs. - Extended
validate_course.pyto validatekernelNamereferences against a kernel catalog, and updated tests/examples accordingly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/scripts/validate_dockerfile.py |
Adds shared static Dockerfile validation logic used by the action. |
deploy-kernel/src/scripts/validate_dockerfile.py |
Wrapper that reuses the shared validator from src/scripts. |
deploy-kernel/src/scripts/deploy_kernel.py |
Implements kernel deploy API call + polling and writes action outputs. |
deploy-kernel/action.yml |
Defines the composite action steps (setup python, install deps, validate, deploy). |
deploy-kernel/requirements.txt |
Declares Python deps for the deploy-kernel action runtime. |
src/scripts/validate_course.py |
Adds kernel catalog lookup to validate kernelName references. |
test/unit/test_validate_dockerfile.py |
New unit tests covering Dockerfile validator behavior and helpers. |
test/unit/test_deploy_kernel.py |
New unit tests for deploy script API handling, polling, and outputs. |
test/unit/test_deploy_kernel_validate_dockerfile_wrapper.py |
Verifies wrapper validator matches shared validator behavior. |
test/unit/test_validate_course.py |
Updates course fixture values to align with kernel catalog validation. |
test/e2e/test_action_flow.py |
Updates kernel IDs/names used in end-to-end action flow test data. |
test/e2e/test_nasty_inputs.py |
Updates kernel IDs/names used in nasty-input E2E test data. |
test/conftest.py |
Updates shared course fixture kernel name/id used across tests. |
examples/Dockerfile.kernel |
Adds an example Dockerfile for custom kernel deployment. |
examples/requirements.kernel.txt |
Adds example requirements file for kernel image builds. |
examples/deploy-kernel-workflow.yml |
Adds example workflow demonstrating how to call the new action. |
CHANGELOG.md |
Notes new behavior/fixes related to kernel deployment and validator reuse. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review: PR #22 — Deploy Kernel GitHub ActionCRITICALC1. API key visible in process listing and potentially in logs Fix: Pass via environment variable instead: env:
QBRAID_API_KEY: ${{ inputs.api-key }}Have the script read from C2. Fix: Reuse HIGHH1. Fix: Add a deny-list ( H2. No input validation on Fix: Validate H3. Dockerfile labels and action inputs can diverge silently Fix: Cross-validate that action inputs match Dockerfile labels, or extract metadata from the labels. H4. Fix: Add MEDIUMM1. Polling sleeps 30s before the first check Fix: Move sleep to after the poll response check. M2. Hardcoded 30-minute timeout with no user override Fix: Add an optional M3. Fix: Test with pre-joined lines (matching real usage) and assert both keys. M4. LOW
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| poll_resp = requests.get( | ||
| poll_url, headers=headers, timeout=REQUEST_TIMEOUT_SECONDS | ||
| ) | ||
| except requests.RequestException as e: | ||
| logger.warning(f"Poll request failed: {e}") | ||
| continue | ||
|
|
||
| if poll_resp.status_code != 200: | ||
| error_code, error_message = _parse_error_response(poll_resp) | ||
| log_message = ( | ||
| f"Poll returned {poll_resp.status_code}" | ||
| f"{f' ({error_code})' if error_code else ''}: {error_message}" | ||
| ) | ||
| if poll_resp.status_code in FATAL_POLL_STATUS_CODES: | ||
| logger.error(log_message) | ||
| write_github_output("status", "failed") | ||
| sys.exit(1) | ||
| logger.warning(log_message) | ||
| continue |
There was a problem hiding this comment.
The polling loop does continue on request exceptions and non-fatal non-200 responses without sleeping, so it can rapidly hammer the API and burn through MAX_POLL_ATTEMPTS almost instantly. Consider ensuring time.sleep(POLL_INTERVAL_SECONDS) (or a backoff) happens between attempts even on transient failures.
| # Poll for completion | ||
| poll_url = f"{api_base_url}/kernels/deploy/{build_id}" | ||
| deadline = time.monotonic() + timeout_seconds | ||
| for attempt in range(1, MAX_POLL_ATTEMPTS + 1): | ||
| logger.info(f"Polling build status (attempt {attempt}/{MAX_POLL_ATTEMPTS})...") |
There was a problem hiding this comment.
timeout_seconds is used to compute deadline, but the polling loop is also capped by MAX_POLL_ATTEMPTS. If a caller sets timeout-seconds > (MAX_POLL_ATTEMPTS * POLL_INTERVAL_SECONDS), the action will still time out early, making the input misleading. Consider deriving max attempts from timeout_seconds (or looping until deadline only).
| shell: bash | ||
| env: | ||
| QBRAID_API_KEY: ${{ inputs.api-key }} | ||
| QBRAID_DEPLOY_TIMEOUT_SECONDS: ${{ inputs.timeout-seconds }} |
There was a problem hiding this comment.
The composite action exports QBRAID_DEPLOY_TIMEOUT_SECONDS, but deploy_kernel.py doesn't read this environment variable (timeout is passed via --timeout-seconds). This extra env var can confuse users—either remove it or wire the script to honor it when the CLI arg isn't provided.
| QBRAID_DEPLOY_TIMEOUT_SECONDS: ${{ inputs.timeout-seconds }} |
| - name: Deploy kernel to qBraid | ||
| id: deploy-kernel | ||
| uses: qBraid/upload-course-action/deploy-kernel@kernel-deployment-v0.0-beta.0.1 # ✅ repo/subdir@tag | ||
| with: |
There was a problem hiding this comment.
The PR description says workflows should reference qBraid/upload-course-action/deploy-kernel@main, but this example pins a pre-release tag. Please align the example with the intended guidance (either update the example ref or adjust the PR description).
|
|
||
| # 1. Check required LABEL directives | ||
| labels = _parse_labels(lines) | ||
| for required_label in REQUIRED_LABELS: |
There was a problem hiding this comment.
REQUIRED_LABELS is a frozenset, so iterating it can produce a non-deterministic error ordering across runs/Python versions. For more stable logs (and easier testing/debugging), consider iterating over sorted(REQUIRED_LABELS) when emitting errors.
| for required_label in REQUIRED_LABELS: | |
| for required_label in sorted(REQUIRED_LABELS): |
Summary
deploy-kernel/composite action for deploying custom Jupyter kernel images to qBraidsrc/scripts/validate_dockerfile.pyfor static Dockerfile validationvalidate_course.py(checks kernelName exists in catalog)Usage
Referenced as
qBraid/upload-course-action/deploy-kernel@mainin workflows.Inputs:
api-key,dockerfile-path,kernel-name,language,display-name,context-dirTest plan
🤖 Generated with Claude Code