Integrate Wildside with Nile Valley previews#358
Open
leynos wants to merge 17 commits into
Open
Conversation
Document the approved-gate plan for hardening Wildside's health server, container image, Helm chart, and local k3d preview workflow before any implementation work begins. Capture the current repository discoveries, Corbusier and Nile Valley prior art, validation strategy, and the CodeRabbit rate-limit blocker from the planning milestone.
Contributor
WalkthroughDomain health model moves liveness and readiness behind a ChangesBackend Health Contract and Local Kubernetes Preview
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds a detailed ExecPlan document describing the future integration of the Wildside backend with Nile Valley previews and GitOps workflows, including health semantics refactors, container and Helm hardening, local k3d orchestration, and documentation updates, but does not change any implementation code yet. Flow diagram for local k3d preview workflow (local_k8s up)flowchart LR
A["Developer runs
make local-k8s-up"] --> B["uv run scripts/local_k8s.py up"]
B --> C["Validate tools: k3d, kubectl, Helm, Docker"]
C -->|ok| D["Create or reuse k3d cluster
(loopback ingress port)"]
D --> E["Docker build backend image
from deploy/docker/backend.Dockerfile"]
E --> F["Import image into k3d cluster"]
F --> G["helm upgrade --install wildside
using deploy/charts/wildside"]
G --> H["Wildside Pod exposes
/health/live and /health/ready"]
C -->|missing tool| X["Fail with preflight error
and exit"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add a domain-owned process health model and observation port so HTTP health routes map domain state instead of owning liveness and readiness semantics. Cover the health contract with focused `rstest` unit tests and an `rstest-bdd` feature for externally observable probe responses.
Update the Nile Valley integration ExecPlan with the committed health milestone, gate results, and CodeRabbit outcome before starting the next implementation milestone.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Update the backend Dockerfile to use an edition-2024-capable Rust builder, a non-root Debian slim runtime, explicit runtime libraries, and a stable `/health/live` liveness probe. Add a root `.dockerignore` to keep local build and dependency artefacts out of container contexts, and record the local Docker CLI validation blocker in the execution plan.
Update the execution plan with the committed container image hardening milestone and the successful CodeRabbit review result before starting the Helm chart work.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add ExternalSecret rendering, service account support, local preview values, hostless ingress handling, and schema coverage for deployment values used by Nile Valley overlays. Keep live Secret lookup validation opt-in so GitOps and offline renders work without a cluster, while startup and liveness probes use `/health/live` consistently.
Update the execution plan with the committed Helm chart alignment milestone and the successful CodeRabbit review result before starting the local preview workflow.
Add a Cyclopts and `uv` based local Kubernetes CLI for creating, inspecting, logging, and tearing down a Wildside k3d preview. Wire Makefile targets to the CLI, add local validation helpers, and record the missing local `k3d` and `kubectl` environment blocker in the execution plan.
Record the local k3d preview commit and CodeRabbit review outcome in the execution plan before moving to the documentation milestone.
Add the local k3d preview design and user-facing server guide, then link the workflow from developer, architecture, repository, contents, and roadmap documentation. Record the documentation validation evidence in the execution plan and mark the Nile Valley preview integration roadmap item complete.
Record final validation, milestone review outcomes, residual local tool limitations, and implementation results in the Nile Valley integration execution plan.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. scripts/local_k8s/deployment.py Comment on file """Split a Docker image reference into Helm repository and tag values."""
repository, separator, tag = image_name.rpartition(":")
if not separator or "/" in tag or not repository or not tag:❌ New issue: Complex Conditional |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Move the local preview image reference validation condition behind a named helper so the parser satisfies CodeScene�s complex conditional threshold without changing behaviour.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Code Duplicationbackend/tests/health_probes_bdd.rs: What lead to degradation?The module contains 2 functions with similar structure: HealthProbeWorld.request_liveness,HealthProbeWorld.request_readiness Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
This comment was marked as resolved.
This comment was marked as resolved.
Share the Actix test request setup between readiness and liveness probe steps so the BDD world avoids duplicated request code without changing scenario behaviour.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Return a stable JSON health envelope from liveness and readiness probes and cover both pass and fail states with `insta` JSON snapshots. This protects the operator-facing response contract while keeping the domain health policy behind the existing health observer port.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Update the pnpm override for `qs` to 6.15.2 so the transitive `style-dictionary` dependency path uses the patched release for GHSA-q8mj-m7cp-5q26.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc93169661
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ], | ||
| ) | ||
| print(pods.stdout.strip() or "pods: none") | ||
| services = run("kubectl", ["-n", config.namespace, "get", "service", config.release_name, "--ignore-not-found"]) |
There was a problem hiding this comment.
print_kubernetes_status fetches the Service by config.release_name, but the chart names the Service with include "wildside.fullname" (see templates/service.yaml), which becomes <release>-wildside for non-default release names. In that common override case (WILDSIDE_HELM_RELEASE not equal to chart name), local_k8s status will incorrectly report service: none even though the release is healthy, which makes status output misleading and harder to trust for troubleshooting.
Useful? React with 👍 / 👎.
| def deploy_preview(config: PreviewConfig, *, skip_build: bool) -> None: | ||
| """Build the image and install or upgrade the Wildside Helm release.""" | ||
|
|
||
| require_tools(("docker", "helm", "k3d", "kubectl")) |
There was a problem hiding this comment.
deploy_preview always requires docker before it checks skip_build. As a result, local_k8s up --skip-build still fails immediately on machines without Docker even though no build is attempted, defeating the purpose of the flag for prebuilt images and blocking a documented workflow variant.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 @.dockerignore:
- Line 3: Remove or replace the incorrect literal `.node_modules` entry in
.dockerignore: delete the `.node_modules` line (the literal name matches
nothing) since `**/node_modules` already covers standard node_modules folders,
or if you intended to ignore a hidden folder specifically, change it to an
explicit pattern that matches that target; update the `.node_modules` entry
accordingly so only the correct node_modules patterns remain.
In `@deploy/charts/wildside/values.schema.json`:
- Around line 157-159: Tighten the externalSecret.data item schema in
deploy/charts/wildside/values.schema.json so the array cannot contain
empty/unshaped objects: replace the current "data": { "type": "array", "items":
{ "type": "object" } } with an item schema that mirrors
external-secrets.io/v1beta1 spec.data[] (e.g. items: { "type":"object",
"properties": { "secretKey": { "type":"string" }, "remoteRef": {
"type":"object", "properties": { "key": { "type":"string" }, "property": {
"type":"string" }, "propertyRegex": { "type":"string" }, "version": {
"type":"string" } }, "required":["key"], "additionalProperties": false } },
"required":["secretKey","remoteRef"], "additionalProperties": false }) so
templates/externalsecret.yaml rendering of .Values.externalSecret.data will
reject {}/malformed entries.
In `@docs/execplans/backend-nile-valley-integration.md`:
- Around line 721-723: Edit the rationale paragraph in the users-guide rationale
and replace the incorrect noun phrase "a global users guide" with the corrected
phrase "a global user guide" so the sentence reads "...the repository does not
currently have a global user guide covering server and deployment behaviour.";
search for the exact phrase "a global users guide" in the document to locate the
spot to update.
In `@docs/repository-structure.md`:
- Around line 488-493: Rewrap the Markdown paragraph that mentions the
developer-local k3d preview helper so no line exceeds 80 columns: break the long
line containing "`scripts/local_k8s.py` and `scripts/local_k8s/*`" (and the
clause about importing the backend image and installing the Wildside chart with
`deploy/charts/wildside/values.local.yaml`) into shorter lines so the paragraph
conforms to the 80-column limit while preserving wording and inline code
segments.
In `@package.json`:
- Line 55: In package.json, add the "qs": "6.15.2" entry into the top-level
"overrides" object (not just under "pnpm.overrides") so npm and pnpm override
parity is maintained; update the overrides block to include the same version pin
as the existing "pnpm.overrides" entry and add a short comment or adjacent
package.json field (e.g., "overridesNotes" or update package.json README)
documenting that qs@6.15.2 is the first patched version for the GitHub advisory
fixing the DoS affecting >=6.11.1 <=6.15.1 so reviewers can see the security
rationale.
In `@scripts/local_k8s.py`:
- Around line 23-27: The _run function currently accepts func: object and
silently no-ops if func isn't callable; change its signature to a strict
callable type (e.g., typing.Callable[[], None] or Callable[..., Any]) and remove
the callable() guard so the code directly calls func() and lets type checkers
and runtime errors surface invalid call sites; update imports to include
Callable and keep the existing LocalK8sError except block and invocation of
func() inside the try.
In `@scripts/local_k8s/__init__.py`:
- Line 1: Replace the single-line docstring with a short triple-quoted module
docstring for scripts.local_k8s that explains the package purpose (local
Kubernetes preview helpers for Wildside), lists provided utilities/entry points
(how to invoke CLI entry points or functions used to start the preview), and
shows a minimal usage example and invocation pattern (e.g., example CLI call or
calling main() if present); make sure it follows coding guidelines by describing
purpose, utility, and usage including an example invocation.
In `@scripts/local_k8s/commands.py`:
- Line 1: Update the module docstring and the public docstrings for
CommandResult and run to NumPy style: replace the short module string with a
NumPy-structured summary including Purpose, Usage (brief example), and Notes;
expand the CommandResult class docstring to include a short Summary, Attributes
(document fields like returncode, stdout, stderr, succeeded), and Examples;
expand the run function docstring to NumPy sections: Summary, Parameters
(document parameters and types), Returns (describe returned CommandResult),
Raises (exceptions raised), and Examples; ensure the symbol names CommandResult
and run are referenced and that types and example usage are present in each
docstring.
In `@scripts/local_k8s/k3d.py`:
- Around line 63-66: The code currently calls json.loads(result.stdout or "[]")
and will let json.JSONDecodeError propagate; wrap the parse in a try/except that
catches json.JSONDecodeError and raises LocalK8sError (including the original
exception message/context) so callers get a stable local-preview error; update
the block around json.loads, keep the subsequent type check for list and the
final any(...) check (referencing json.loads, LocalK8sError, and cluster_name)
and ensure the raised LocalK8sError preserves the original error text.
In `@scripts/local_k8s/k8s.py`:
- Around line 10-21: The short one-line docstrings for the public functions
ensure_namespace and print_kubernetes_status must be replaced with full
NumPy-style docstrings: for each function add a Parameters section documenting
the config parameter (type PreviewConfig and brief description), a Returns
section stating None, and a Raises section if the function can propagate
exceptions (e.g., from require_tools or run) — update the docstrings for
ensure_namespace and print_kubernetes_status accordingly to follow the numpy
style guide.
In `@scripts/local_k8s/unittests/test_validation.py`:
- Around line 20-23: The test test_validate_port_rejects_invalid_values
currently only asserts the exception type; update it to assert the error message
using a regex by passing the match parameter to pytest.raises (or by capturing
the exception and asserting re.search) so the message contract from
validate_port is verified for each raw_value; reference the validate_port
function and the test name test_validate_port_rejects_invalid_values when making
the change and ensure the regex covers the expected phrasing (e.g., mentions
invalid port or the given raw_value) for each input.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 07548c6b-a8aa-4ea7-a0d4-77f52c9dbf2d
⛔ Files ignored due to path filters (5)
backend/tests/snapshots/health_probes_bdd__health_probe_health_live_200.snapis excluded by!**/*.snapbackend/tests/snapshots/health_probes_bdd__health_probe_health_live_503.snapis excluded by!**/*.snapbackend/tests/snapshots/health_probes_bdd__health_probe_health_ready_200.snapis excluded by!**/*.snapbackend/tests/snapshots/health_probes_bdd__health_probe_health_ready_503.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
.dockerignoreMakefilebackend/src/domain/health.rsbackend/src/domain/mod.rsbackend/src/domain/ports/health_observer.rsbackend/src/domain/ports/mod.rsbackend/src/inbound/http/health.rsbackend/src/lib.rsbackend/src/tests.rsbackend/tests/features/health_probes.featurebackend/tests/health_probes_bdd.rsdeploy/charts/wildside/templates/_helpers.tpldeploy/charts/wildside/templates/deployment.yamldeploy/charts/wildside/templates/externalsecret.yamldeploy/charts/wildside/templates/ingress.yamldeploy/charts/wildside/templates/serviceaccount.yamldeploy/charts/wildside/values.local.yamldeploy/charts/wildside/values.schema.jsondeploy/charts/wildside/values.yamldeploy/docker/backend.Dockerfiledocs/backend-roadmap.mddocs/contents.mddocs/developers-guide.mddocs/execplans/backend-nile-valley-integration.mddocs/local-k8s-preview-design.mddocs/repository-structure.mddocs/users-guide.mddocs/wildside-backend-architecture.mdpackage.jsonscripts/local_k8s.pyscripts/local_k8s/__init__.pyscripts/local_k8s/commands.pyscripts/local_k8s/config.pyscripts/local_k8s/deployment.pyscripts/local_k8s/k3d.pyscripts/local_k8s/k8s.pyscripts/local_k8s/unittests/test_validation.pyscripts/local_k8s/validation.py
| @@ -0,0 +1,12 @@ | |||
| .git | |||
| .gitignore | |||
| .node_modules | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for any .node_modules directory in the repository
fd -HI -t d '^\.node_modules$' .Repository: leynos/wildside
Length of output: 41
Fix the .dockerignore dependency ignore pattern.
- Replace or remove the
.node_modulesentry on.dockerignoreline 3: it only matches a literal root folder named.node_modules(none exists), while**/node_moduleson line 8 already ignores the standardnode_modulesdirectory everywhere in the build context.
🤖 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 @.dockerignore at line 3, Remove or replace the incorrect literal
`.node_modules` entry in .dockerignore: delete the `.node_modules` line (the
literal name matches nothing) since `**/node_modules` already covers standard
node_modules folders, or if you intended to ignore a hidden folder specifically,
change it to an explicit pattern that matches that target; update the
`.node_modules` entry accordingly so only the correct node_modules patterns
remain.
Comment on lines
+157
to
+159
| "data": { "type": "array", "items": { "type": "object" } } | ||
| }, | ||
| "required": ["enabled", "refreshInterval", "secretStoreRef", "data"], |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate where externalSecret.data is defined in values schema
rg -n '"data"\s*:\s*\{|\bexternalSecret\b' deploy/charts/wildside/values.schema.json
# Print the specific region around the reported lines (~157-159)
sed -n '130,190p' deploy/charts/wildside/values.schema.json
# Find how externalSecret.data is consumed in templates
rg -n "externalSecret\.data|spec\.data|toYaml \.Values\.externalSecret\.data|remoteRef|secretKey" deploy/charts/wildside/templates
# Show the externalsecret template section that renders spec.data
rg -n "spec:\s*$|data:" deploy/charts/wildside/templates/externalsecret.yaml || true
sed -n '1,220p' deploy/charts/wildside/templates/externalsecret.yamlRepository: leynos/wildside
Length of output: 3991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect externalsecret template in full (small file)
sed -n '1,220p' deploy/charts/wildside/templates/externalsecret.yaml
# Inspect helper logic that mentions $secretKey and $data/$stringData
sed -n '1,160p' deploy/charts/wildside/templates/_helpers.tpl
# Inspect deployment template where secretKeyRef is built from $sec
sed -n '60,140p' deploy/charts/wildside/templates/deployment.yaml
# Search for where externalSecret.data is shaped/expected (remoteRef/secretKey/keys)
rg -n --context 2 "externalSecret\.data|remoteRef|secretKey" deploy/charts/wildside/templatesRepository: leynos/wildside
Length of output: 9929
Tighten externalSecret.data item validation in deploy/charts/wildside/values.schema.json
deploy/charts/wildside/templates/externalsecret.yaml renders spec.data verbatim from externalSecret.data (toYaml .Values.externalSecret.data), while the schema currently allows empty/unshaped objects:
"data": { "type": "array", "items": { "type": "object" } }
},
"required": ["enabled", "refreshInterval", "secretStoreRef", "data"],Reject malformed entries by defining an item schema for each element (use required fields matching external-secrets.io/v1beta1 spec.data[], and set additionalProperties: false, so {} is invalid).
🤖 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 `@deploy/charts/wildside/values.schema.json` around lines 157 - 159, Tighten
the externalSecret.data item schema in deploy/charts/wildside/values.schema.json
so the array cannot contain empty/unshaped objects: replace the current "data":
{ "type": "array", "items": { "type": "object" } } with an item schema that
mirrors external-secrets.io/v1beta1 spec.data[] (e.g. items: { "type":"object",
"properties": { "secretKey": { "type":"string" }, "remoteRef": {
"type":"object", "properties": { "key": { "type":"string" }, "property": {
"type":"string" }, "propertyRegex": { "type":"string" }, "version": {
"type":"string" } }, "required":["key"], "additionalProperties": false } },
"required":["secretKey","remoteRef"], "additionalProperties": false }) so
templates/externalsecret.yaml rendering of .Values.externalSecret.data will
reject {}/malformed entries.
Comment on lines
+721
to
+723
| Rationale: the user explicitly requested that file, and the repository does | ||
| not currently have a global users guide covering server and deployment | ||
| behaviour. |
Contributor
There was a problem hiding this comment.
Fix the noun phrase in the users-guide rationale.
Replace “a global users guide” with “a global user guide”.
Triage: [type:grammar] [type:docstyle]
As per coding guidelines: "Follow documentation style guide conventions for clear and consistent documentation".
🧰 Tools
🪛 LanguageTool
[grammar] ~722-~722: The plural noun “users” cannot be used with the article “a”. Did you mean “a global user” or “global users”?
Context: ...he repository does not currently have a global users guide covering server and deployment ...
(A_NNS)
🤖 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 `@docs/execplans/backend-nile-valley-integration.md` around lines 721 - 723,
Edit the rationale paragraph in the users-guide rationale and replace the
incorrect noun phrase "a global users guide" with the corrected phrase "a global
user guide" so the sentence reads "...the repository does not currently have a
global user guide covering server and deployment behaviour."; search for the
exact phrase "a global users guide" in the document to locate the spot to
update.
Comment on lines
+488
to
+493
| This repository also provides a developer-local k3d preview helper: | ||
| `scripts/local_k8s.py` and `scripts/local_k8s/*`. The helper builds the | ||
| backend image, imports it into a local `k3d` cluster, and installs the Wildside | ||
| chart with `deploy/charts/wildside/values.local.yaml`. It validates the | ||
| application packaging contract; Nile Valley remains the owner of shared | ||
| cluster and GitOps automation. |
Contributor
There was a problem hiding this comment.
Rewrap line 490 to comply with 80-column limit.
Line 490 exceeds the 80-column wrapping requirement for Markdown paragraphs.
Triage: [type:docstyle]
📏 Proposed rewrap
This repository also provides a developer-local k3d preview helper:
-`scripts/local_k8s.py` and `scripts/local_k8s/*`. The helper builds the
-backend image, imports it into a local `k3d` cluster, and installs the Wildside
-chart with `deploy/charts/wildside/values.local.yaml`. It validates the
+`scripts/local_k8s.py` and `scripts/local_k8s/*`. The helper builds the backend
+image, imports it into a local `k3d` cluster, and installs the Wildside chart
+with `deploy/charts/wildside/values.local.yaml`. It validates the
application packaging contract; Nile Valley remains the owner of shared
cluster and GitOps automation.As per coding guidelines, Markdown paragraphs must be wrapped at 80 columns.
🤖 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 `@docs/repository-structure.md` around lines 488 - 493, Rewrap the Markdown
paragraph that mentions the developer-local k3d preview helper so no line
exceeds 80 columns: break the long line containing "`scripts/local_k8s.py` and
`scripts/local_k8s/*`" (and the clause about importing the backend image and
installing the Wildside chart with `deploy/charts/wildside/values.local.yaml`)
into shorter lines so the paragraph conforms to the 80-column limit while
preserving wording and inline code segments.
| "lodash-es": "4.18.1", | ||
| "pino": "9.13.1", | ||
| "qs": "6.14.2", | ||
| "qs": "6.15.2", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify override parity
node ./scripts/check-overrides-parity.mjsRepository: leynos/wildside
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check for qs security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "qs") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Repository: leynos/wildside
Length of output: 2376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate qs and overrides blocks in package.json
FILE="package.json"
echo "== Locations of \"qs\" in $FILE =="
rg -n '"qs"\s*:' "$FILE" || true
echo
echo "== Extract around first match (approx) =="
# Print a small window around the first match
line=$(rg -n '"qs"\s*:' "$FILE" | head -n1 | cut -d: -f1)
start=$((line-20))
end=$((line+20))
sed -n "${start},${end}p" "$FILE" | cat -n
echo
echo "== Show overrides sections (limited) =="
# Show any overrides-related sections
rg -n '"overrides"\s*:' "$FILE" || true
rg -n '"pnpm"\s*:' "$FILE" || true
# Print a broader region around "overrides" keys
for l in $(rg -n '"overrides"\s*:' "$FILE" | cut -d: -f1); do
start=$((l-10)); end=$((l+80));
echo "--- overrides region around line $l ---"
sed -n "${start},${end}p" "$FILE" | nl -ba
doneRepository: leynos/wildside
Length of output: 1641
Mirror the qs override into root overrides and link 6.15.2 to the security fix.
"qs": "6.15.2",
- Add
qsto the top-leveloverridesblock too: it currently exists only underpnpm.overrides, while rootoverridesdoes not includeqs, so npm/pnpm override parity is not maintained for this package. - Document that
qs@6.15.2is the first patched version for the GitHub MODERATE remotely triggerable DoS advisory affecting>= 6.11.1, <= 6.15.1.
🤖 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 `@package.json` at line 55, In package.json, add the "qs": "6.15.2" entry into
the top-level "overrides" object (not just under "pnpm.overrides") so npm and
pnpm override parity is maintained; update the overrides block to include the
same version pin as the existing "pnpm.overrides" entry and add a short comment
or adjacent package.json field (e.g., "overridesNotes" or update package.json
README) documenting that qs@6.15.2 is the first patched version for the GitHub
advisory fixing the DoS affecting >=6.11.1 <=6.15.1 so reviewers can see the
security rationale.
| @@ -0,0 +1 @@ | |||
| """Local Kubernetes preview helpers for Wildside.""" | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Expand the module docstring to include purpose, utility, and usage.
Replace the single-line package docstring with a short module overview that states
what the package provides and how entry points are invoked.
As per coding guidelines, "Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate."
🤖 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 `@scripts/local_k8s/__init__.py` at line 1, Replace the single-line docstring
with a short triple-quoted module docstring for scripts.local_k8s that explains
the package purpose (local Kubernetes preview helpers for Wildside), lists
provided utilities/entry points (how to invoke CLI entry points or functions
used to start the preview), and shows a minimal usage example and invocation
pattern (e.g., example CLI call or calling main() if present); make sure it
follows coding guidelines by describing purpose, utility, and usage including an
example invocation.
| @@ -0,0 +1,34 @@ | |||
| """Command execution primitives for the local preview workflow.""" | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace public docstrings with NumPy-structured docstrings.
Expand Line 15 (CommandResult) and Line 22 (run) to full NumPy sections, and enrich the module docstring at Line 1 with utility/usage context.
🛠️ Proposed docstring update
-"""Command execution primitives for the local preview workflow."""
+"""Command execution primitives for the local preview workflow.
+
+This module runs local tooling commands and normalizes execution failures into
+`LocalK8sError` for CLI workflows.
+"""
@@
`@dataclass`(frozen=True, slots=True)
class CommandResult:
- """Captured stdout and stderr from an external command."""
+ """Captured external command output.
+
+ Attributes
+ ----------
+ stdout
+ Captured standard output stream.
+ stderr
+ Captured standard error stream.
+ """
@@
def run(command: str, args: Sequence[str], *, cwd: str | None = None) -> CommandResult:
- """Run a command and raise a local preview error on failure."""
+ """Run a local command and capture output.
+
+ Parameters
+ ----------
+ command
+ Executable name on `PATH`.
+ args
+ Positional arguments passed to the executable.
+ cwd
+ Working directory for execution. Use the current directory when unset.
+
+ Returns
+ -------
+ CommandResult
+ Captured command stdout and stderr streams.
+
+ Raises
+ ------
+ LocalK8sError
+ Raised when process execution fails.
+ """As per coding guidelines: "Docstrings must follow the numpy style guide... full structured docs for all public interfaces."
Also applies to: 15-24
🤖 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 `@scripts/local_k8s/commands.py` at line 1, Update the module docstring and the
public docstrings for CommandResult and run to NumPy style: replace the short
module string with a NumPy-structured summary including Purpose, Usage (brief
example), and Notes; expand the CommandResult class docstring to include a short
Summary, Attributes (document fields like returncode, stdout, stderr,
succeeded), and Examples; expand the run function docstring to NumPy sections:
Summary, Parameters (document parameters and types), Returns (describe returned
CommandResult), Raises (exceptions raised), and Examples; ensure the symbol
names CommandResult and run are referenced and that types and example usage are
present in each docstring.
Comment on lines
+63
to
+66
| clusters = json.loads(result.stdout or "[]") | ||
| if not isinstance(clusters, list): | ||
| raise LocalK8sError("unexpected k3d cluster list JSON shape") | ||
| return any(isinstance(cluster, dict) and cluster.get("name") == cluster_name for cluster in clusters) |
Contributor
There was a problem hiding this comment.
Wrap JSON parse failures in LocalK8sError.
Handle invalid k3d JSON output and raise a stable local-preview error instead of leaking
json.JSONDecodeError.
Proposed fix
def _cluster_exists(cluster_name: str) -> bool:
result = run("k3d", ["cluster", "list", "--output", "json"])
- clusters = json.loads(result.stdout or "[]")
+ try:
+ clusters = json.loads(result.stdout or "[]")
+ except json.JSONDecodeError as exc:
+ message = "unexpected k3d cluster list JSON payload"
+ raise LocalK8sError(message) from exc
if not isinstance(clusters, list):
raise LocalK8sError("unexpected k3d cluster list JSON shape")
return any(isinstance(cluster, dict) and cluster.get("name") == cluster_name for cluster in clusters)🧰 Tools
🪛 Ruff (0.15.14)
[warning] 65-65: Avoid specifying long messages outside the exception class
(TRY003)
🤖 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 `@scripts/local_k8s/k3d.py` around lines 63 - 66, The code currently calls
json.loads(result.stdout or "[]") and will let json.JSONDecodeError propagate;
wrap the parse in a try/except that catches json.JSONDecodeError and raises
LocalK8sError (including the original exception message/context) so callers get
a stable local-preview error; update the block around json.loads, keep the
subsequent type check for list and the final any(...) check (referencing
json.loads, LocalK8sError, and cluster_name) and ensure the raised LocalK8sError
preserves the original error text.
Comment on lines
+10
to
+21
| def ensure_namespace(config: PreviewConfig) -> None: | ||
| """Create the preview namespace when it does not already exist.""" | ||
|
|
||
| require_tools(("kubectl",)) | ||
| result = run("kubectl", ["get", "namespace", config.namespace, "--ignore-not-found"]) | ||
| if result.stdout.strip(): | ||
| return | ||
| run("kubectl", ["create", "namespace", config.namespace]) | ||
|
|
||
|
|
||
| def print_kubernetes_status(config: PreviewConfig) -> None: | ||
| """Print namespace, service, and pod status for the preview release.""" |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use full NumPy docstrings for public functions.
Replace the single-line docstrings on Line 11 and Line 21 with structured NumPy sections (Parameters, Returns, and Raises where relevant).
🛠️ Proposed docstring update
def ensure_namespace(config: PreviewConfig) -> None:
- """Create the preview namespace when it does not already exist."""
+ """Create the preview namespace when absent.
+
+ Parameters
+ ----------
+ config
+ Preview configuration carrying the target namespace.
+
+ Returns
+ -------
+ None
+ This function mutates cluster state only when the namespace is missing.
+
+ Raises
+ ------
+ LocalK8sError
+ Raised when `kubectl` is unavailable or command execution fails.
+ """
@@
def print_kubernetes_status(config: PreviewConfig) -> None:
- """Print namespace, service, and pod status for the preview release."""
+ """Print Kubernetes status for the preview release.
+
+ Parameters
+ ----------
+ config
+ Preview configuration containing namespace and Helm release name.
+
+ Returns
+ -------
+ None
+ Status is printed to standard output.
+
+ Raises
+ ------
+ LocalK8sError
+ Raised when `kubectl` is unavailable or command execution fails.
+ """As per coding guidelines: "Docstrings must follow the numpy style guide... full structured docs for all public interfaces."
🤖 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 `@scripts/local_k8s/k8s.py` around lines 10 - 21, The short one-line docstrings
for the public functions ensure_namespace and print_kubernetes_status must be
replaced with full NumPy-style docstrings: for each function add a Parameters
section documenting the config parameter (type PreviewConfig and brief
description), a Returns section stating None, and a Raises section if the
function can propagate exceptions (e.g., from require_tools or run) — update the
docstrings for ensure_namespace and print_kubernetes_status accordingly to
follow the numpy style guide.
Comment on lines
+20
to
+23
| @pytest.mark.parametrize("raw_value", ["0", "65536", "not-a-port"]) | ||
| def test_validate_port_rejects_invalid_values(raw_value: str) -> None: | ||
| with pytest.raises(LocalK8sError): | ||
| validate_port(raw_value, default=8088, name="WILDSIDE_K3D_PORT") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'pytest\.raises\(\s*LocalK8sError\s*\)' scripts/local_k8s/unittests/test_validation.pyRepository: leynos/wildside
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore -S "def validate_port|validate_port\(" scripts/local_k8s
rg -n --hidden --no-ignore -S "class LocalK8sError|LocalK8sError\(" scripts/local_k8sRepository: leynos/wildside
Length of output: 1402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba scripts/local_k8s/unittests/test_validation.py | sed -n '1,120p'Repository: leynos/wildside
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n scripts/local_k8s/unittests/test_validation.py | sed -n '1,120p'Repository: leynos/wildside
Length of output: 2195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n scripts/local_k8s/validation.py | sed -n '1,120p'Repository: leynos/wildside
Length of output: 1622
Constrain invalid-port exception assertions with regex.
Assert the invalid-port error-message contract for each input; the test currently checks only the exception type.
Patch
-@pytest.mark.parametrize("raw_value", ["0", "65536", "not-a-port"])
-def test_validate_port_rejects_invalid_values(raw_value: str) -> None:
- with pytest.raises(LocalK8sError):
+@pytest.mark.parametrize(
+ ("raw_value", "message_pattern"),
+ [
+ ("0", r"must be between 1 and 65535"),
+ ("65536", r"must be between 1 and 65535"),
+ ("not-a-port", r"must be an integer TCP port"),
+ ],
+)
+def test_validate_port_rejects_invalid_values(raw_value: str, message_pattern: str) -> None:
+ with pytest.raises(LocalK8sError, match=message_pattern):
validate_port(raw_value, default=8088, name="WILDSIDE_K3D_PORT")🤖 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 `@scripts/local_k8s/unittests/test_validation.py` around lines 20 - 23, The
test test_validate_port_rejects_invalid_values currently only asserts the
exception type; update it to assert the error message using a regex by passing
the match parameter to pytest.raises (or by capturing the exception and
asserting re.search) so the message contract from validate_port is verified for
each raw_value; reference the validate_port function and the test name
test_validate_port_rejects_invalid_values when making the change and ensure the
regex covers the expected phrasing (e.g., mentions invalid port or the given
raw_value) for each input.
Resolve the Helm fullname used for the Wildside Service before running the status lookup. This keeps `local_k8s status` accurate when `WILDSIDE_HELM_RELEASE` is set to a non-default release name.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Make the local k3d deployment preflight match the selected build mode. `local_k8s up --skip-build` now checks only Helm, k3d, and kubectl, while the image-building path still requires Docker.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrates Wildside with Nile Valley preview and GitOps workflows by delivering the approved execution plan in
docs/execplans/backend-nile-valley-integration.md.Implemented:
/health/liveand/health/readyadapters./health/livehealth check defaults.uvlocal k3d preview workflow with Makefile lifecycle targets.Review walkthrough
docs/execplans/backend-nile-valley-integration.mdfor the approved plan, final progress, decisions, validation evidence, and residual environment limitations.backend/src/domain/health.rs,backend/src/domain/ports/health_observer.rs, andbackend/src/inbound/http/health.rsfor the health boundary.deploy/docker/backend.Dockerfile,.dockerignore, anddeploy/charts/wildside/for packaging and deployment changes.scripts/local_k8s.pyandscripts/local_k8s/for the local preview helper.docs/local-k8s-preview-design.mdanddocs/users-guide.mdfor the user-facing and design documentation.Validation
Passed:
make check-fmtmake lintmake testmake fmtmake markdownlintmake nixiehelm lint deploy/charts/wildsidehelm template deploy/charts/wildside --kube-version 1.31.0coderabbit review --agentafter each major milestone and final closeout, all with zero findingsEnvironment-limited checks:
k3dandkubectlare not installed in this environment.Both blockers are recorded in the execplan and covered by CLI preflight checks.
References
docs/execplans/backend-nile-valley-integration.md