feat(proxy): run multiple local shops in parallel behind a shared proxy - #1208
feat(proxy): run multiple local shops in parallel behind a shared proxy#1208Tomasz Turkowski (tturkowski) wants to merge 38 commits into
Conversation
4b3b422 to
ab10413
Compare
|
Whats definitively missing here is:
|
Dev watchers through the shared proxyA quick summary of how the admin/storefront watchers can work behind the shared reverse proxy. Admin watcher — works as-is, no code changesThe admin is Vite-only. Vite works out its own HMR connection from the page it's loaded on, so all that was needed:
You open Storefront watcher (now) — webpack + a small runtime patchThe storefront's classic watcher (HMR + webpack, Rather than patching vendor file, we inject a tiny preload script when launching the watcher (Node Result: the storefront watcher runs fully through the proxy - multiple shops in parallel, clean port-free hostnames, no exposed ports, and no change to Shopware or the shop. You browse Tradeoff: it's a runtime patch — clever but hidden, and it leans on the internals of Storefront watcher (future) — Vite, the clean pathFrom 6.7.11 the storefront also ships a Vite dev server. To make that work behind a reverse proxy we need a small, fully backward-compatible contribution to
With that, the storefront watcher - once enabled, works at the shop's own URL ( Plan proposal:
|
|
btw because of excactly those REASONS I DONT WANT TO have those watchers directly inside Shopware. we're like now screwed |
54c9305 to
cae4216
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new shopware-cli project proxy command group and supporting infrastructure to run multiple local Shopware projects in parallel behind a shared Traefik reverse proxy, using stable per-project hostnames with local DNS resolution and trusted HTTPS.
Changes:
- Adds a new
internal/proxysubsystem (DNS daemon, resolver configuration, Traefik management, verification, trust store integration, registry/settings state). - Implements proxy-mode Docker Compose overrides (marker-guarded
compose.override.yaml) to remove fixed host ports and route by hostname via Traefik. - Integrates proxy awareness into
project create,project dev, the dev TUI overview, and storefront watcher routing.
Reviewed changes
Copilot reviewed 69 out of 70 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/dev/tab_overview_health.go | Adds proxy-related setup health checks to the TUI overview health panel. |
| internal/tui/dev/tab_overview_format_test.go | Adds tests for overview formatting helpers used by the TUI. |
| internal/tui/dev/model.go | Adds proxy fallback handling and an interactive proxy setup execution path in the TUI. |
| internal/system/dockername.go | Introduces Docker Compose project-name validation helper. |
| internal/proxy/wsl_resolver_test.go | Adds unit tests for WSL DNS resolver guidance text. |
| internal/proxy/windows_access.go | Adds WSL/Windows browser access guidance and hostname list generation. |
| internal/proxy/windows_access_test.go | Adds tests for Windows access guidance and hostname generation. |
| internal/proxy/verify.go | Implements proxy verify bottom-up checks (Docker → DNS → OS resolver → Traefik → trusted HTTPS). |
| internal/proxy/verify_windows.go | Windows stub for OS-level resolution check. |
| internal/proxy/verify_test.go | Adds tests for verification hints and probe-hostname generation. |
| internal/proxy/verify_linux.go | Linux OS-resolution check via getent hosts. |
| internal/proxy/verify_darwin.go | macOS OS-resolution check via dscacheutil. |
| internal/proxy/trust.go | Implements CA trust installation flow and user guidance (mkcert/truststore). |
| internal/proxy/trust_test.go | Adds tests for trust-blocked guidance content. |
| internal/proxy/traefik.go | Adds Traefik container/network lifecycle management and hostname alias reconciliation. |
| internal/proxy/traefik_test.go | Adds tests for Traefik dynamic config writing and alias helpers. |
| internal/proxy/stats.go | Adds proxy instance stats collection for the TUI (shop count + memory sum). |
| internal/proxy/stats_test.go | Adds tests for parsing Docker memory usage strings. |
| internal/proxy/statedir.go | Adds a shared state directory helper for proxy state files. |
| internal/proxy/settings.go | Adds machine-wide proxy settings (base domain) with validation and persistence. |
| internal/proxy/settings_test.go | Adds tests for settings validation and persistence round-trips. |
| internal/proxy/resolver.go | Adds resolver status types and Linux-without-systemd-resolved guidance text. |
| internal/proxy/resolver_windows.go | Windows resolver stubs reporting unsupported behavior. |
| internal/proxy/resolver_linux.go | Linux systemd-resolved split-DNS configuration (configure/unconfigure + guidance). |
| internal/proxy/resolver_linux_test.go | Adds tests for Linux resolver blocked guidance content. |
| internal/proxy/resolver_darwin.go | macOS /etc/resolver configuration (configure/unconfigure + guidance). |
| internal/proxy/resolver_darwin_test.go | Adds tests for macOS resolver blocked guidance content. |
| internal/proxy/registry.go | Adds registry state for registered projects and restore metadata. |
| internal/proxy/registry_test.go | Adds registry behavior tests (upsert/remove/find/round-trip). |
| internal/proxy/projectconfig.go | Adds comment-preserving YAML URL rewrite/restore logic for .shopware-project.yml. |
| internal/proxy/projectconfig_test.go | Adds tests for URL rewrite/restore semantics and missing-file behavior. |
| internal/proxy/hostname.go | Adds proxy hostname derivation (from config URL or directory name). |
| internal/proxy/hostname_test.go | Adds tests for hostname derivation edge cases. |
| internal/proxy/docker.go | Adds Docker Compose version check for !reset support and docker runner helper. |
| internal/proxy/dns.go | Adds embedded DNS server implementation and direct-query helper for verification/tests. |
| internal/proxy/dns_test.go | Adds tests for DNS zone behavior and garbage packet handling. |
| internal/proxy/dns_daemon.go | Adds non-Windows DNS daemon spawning/management via self re-exec and PID/state files. |
| internal/proxy/dns_daemon_windows.go | Adds Windows stubs and shared “not supported” error for DNS daemon operations. |
| internal/proxy/cert.go | Adds mkcert-compatible CA/cert management and SAN host list generation. |
| internal/proxy/cert_test.go | Adds tests for certificate creation, idempotency, and regeneration triggers. |
| internal/proxy/canonical.go | Adds canonical project-root resolution (symlink normalization). |
| internal/mkcert/mkcert.go | Adds BSD-licensed mkcert-derived CA/certificate implementation as an internal library. |
| internal/mkcert/mkcert_test.go | Adds tests for CAROOT behavior, CA creation/reuse, cert issuance, and keyless mode. |
| internal/mkcert/LICENSE | Adds the mkcert BSD license text for the adapted code. |
| internal/extension/storefront_watch.go | Adds proxy-mode support for the deprecated storefront hot-proxy watcher via env + Node preload. |
| internal/extension/storefront_hmr_patch.cjs | Adds a managed Node preload patch to rewrite webpack-dev-server websocket target behind the proxy. |
| internal/executor/docker.go | Improves watcher shutdown by SIGINTing the full in-container process tree (not just the wrapper process). |
| internal/envfile/upsert.go | Adds an env-file “upsert var” helper for surgical .env updates. |
| internal/envfile/upsert_test.go | Adds tests for env var upsert and read behavior. |
| internal/docker/compose.go | Adds a YAML boolean-node helper used by proxy compose override generation. |
| internal/docker/compose_test.go | Adds a regression test ensuring base compose output remains non-proxy (ports/labels absent). |
| internal/docker/compose_override.go | Adds generation + write/remove for marker-guarded proxy compose overrides with Traefik routes. |
| internal/docker/compose_override_test.go | Adds extensive tests for override content and safety checks (refuse user override files). |
| go.mod | Adds github.com/smallstep/truststore and an indirect plist dependency for trust installation. |
| go.sum | Adds checksums for new module dependencies. |
| docs/proxy.md | Adds end-to-end architecture/design documentation for the shared proxy feature set. |
| cmd/root.go | Treats ErrProxyNotRegistered as a user-facing error (exit 1 without extra logging). |
| cmd/project/project_storefront_watch.go | Routes storefront watcher through proxy hostname when the project is proxied. |
| cmd/project/project_proxy_verify.go | Adds project proxy verify command and shared output printer for verification steps. |
| cmd/project/project_proxy_test.go | Adds tests for local-domain choice resolution and Shopware command availability detection. |
| cmd/project/project_proxy_setup.go | Adds project proxy setup and teardown, including DNS/trust installation and verification. |
| cmd/project/project_proxy_list.go | Adds project proxy list and status commands with running-instance detection and links. |
| cmd/project/project_proxy_dns_serve.go | Adds hidden internal subcommand used as the DNS daemon re-exec target. |
| cmd/project/project_dev.go | Bootstraps proxy infra for proxy-mode projects with a non-blocking fallback to port mode. |
| cmd/project/project_dev_test.go | Adds tests for proxy-project detection and local-domain hostname normalization. |
| cmd/project/project_create.go | Adds --local-domain support, inline (prompted) one-time setup option, and base-domain lookup. |
| cmd/project/project_create_install.go | Writes proxy hostname URLs into the created project config and updates create summary output. |
| cmd/project/project_create_form.go | Extends interactive create form to prompt for local domains and optional one-time machine setup. |
Suppressed comments (1)
internal/proxy/hostname.go:33
- ProjectHostname can produce invalid DNS hostnames when the project directory contains underscores (Docker Compose allows them, DNS labels do not). Sanitizing underscores to dashes here keeps hostnames valid and matches the behavior in project create (localDomainHostname).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Works same way as it works on mac or windows, we do serve dns and in wsl we redirect domain traffic to our dns that resolves to 127.0.0.1 and then traefik takes care about the rest. WSL users who want to access WSL-running shops from Windows browser would need to one time setup Windows's hosts file.
Made them work, but it was a hard part. See comment above with details.
Made that work, containers will resolve from domain to the container, no need of container names usage. |
|
Follow-up from our chat:
|
232eb49 to
7c64ea1
Compare
7c64ea1 to
c26ca18
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds a shared local proxy flow for Shopware projects. It introduces proxy setup and lifecycle commands, stable local hostnames with HTTPS, proxy-aware Docker Compose generation, project URL switching and restoration, and proxy status in project creation, development commands, and the TUI overview. ChangesShared local proxy
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ProxyInfra
participant ProjectEnv
participant Shop
User->>CLI: run project proxy up
CLI->>ProxyInfra: prepare DNS, certs, Traefik
ProxyInfra-->>CLI: proxy ready
CLI->>ProjectEnv: write proxy compose and start services
CLI->>Shop: update app and sales-channel URLs
CLI-->>User: print local HTTPS URLs and guidance
sequenceDiagram
participant User
participant CreateFlow
participant ProxySetup
participant Installer
User->>CreateFlow: create project with local domain
CreateFlow->>ProxySetup: optional inline setup
ProxySetup-->>CreateFlow: resolver and trust status
CreateFlow->>Installer: scaffold project with proxy URL
Installer-->>User: show HTTPS shop and admin URLs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Hi Tomasz Turkowski (@tturkowski), I tested this end-to-end — it works well overall. Here's my QA report with a few issues I ran into along the way. 🙏 SummaryIt works. I ran three shops at the same time ( Both ways of using it work:
The setup step, the on/off commands, the status/list views and the teardown all behaved. The new-shop flow is smooth. The rough edges are almost all in the "turn the proxy on for an existing Issues I ran into (worst first)1. Turning the proxy on for an existing shop can quietly point it at an empty database 🔴 HighWhat I did: ran What I expected: the shop keeps using its own data. What happened: the shop opened fine, but later — while clicking around — it broke with Why it matters: it looks fine at first and only breaks later, in use — so it's easy to miss and 2. Turning the proxy on can fail with a confusing message and leave things half-started 🔴 HighWhat I did: ran What I expected: either it works, or it tells me clearly what's missing. What happened: it failed with Why it matters: the message gives the user nothing to act on, and the leftover half-started state is 3. Turning the proxy on is blocked on almost every real existing shop 🟠 MediumWhat I did: ran What I expected: it just turns the proxy on. What happened: it stopped with an error because the shop already has a Why it matters: this is the headline "turn it on for an existing project" feature, and it needs a 4. A running shop shows as "stopped" in the list 🟠 MediumWhat I did: created a new shop with What I expected: the list shows it as running (it opens fine in the browser and returns pages). What happened: the list showed it as stopped, even though it was clearly running and serving the Why it matters: the status display can't be trusted for shops started this way — it says "stopped" 5. On/off/teardown sometimes dumps a huge error page but keeps working 🟠 MediumWhat I did: ran What I expected: a short success message. What happened: it printed roughly 100 lines of red error/stack trace about a "duplicate entry" for Why it matters: it's alarming and looks broken, so a user is likely to think the command failed when 6. Health check looks like a failure when the proxy simply isn't running yet 🟡 LowWhat I did: ran What I expected: something like "proxy isn't running — start it first." What happened: it reported a red Why it matters: a first-time user could read this as a real fault when nothing is actually wrong. One note on my testingIssues 2, 3, 4 I reproduced cleanly from a normal starting point. Issues 1 and 5 appeared |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (22)
docs/proxy.md-20-21 (1)
20-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify the in-container HTTPS claim.
The TL;DR states that shops can reach each other over HTTPS from inside their containers. Line 234-239 and Line 350-353 state that PHP/curl does not trust the proxy CA by default. Restrict this claim to clients with the mounted trust configuration, or state the PHP/curl prerequisite here.
🤖 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/proxy.md` around lines 20 - 21, Update the in-container HTTPS claim in the TL;DR to qualify that it applies only to clients with the proxy CA trust configuration mounted, including the PHP/curl trust prerequisite stated elsewhere in the document.docs/proxy.md-233-233 (1)
233-233: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the reported Markdown lint errors.
- Keep
>on the blank line at Line 233, or remove the blank line, so the blockquote remains valid.- Add a language identifier to the fence at Line 319, such as
powershell.- Add a language identifier to the fence at Line 330, such as
text.Also applies to: 319-319, 330-330
🤖 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/proxy.md` at line 233, Fix the Markdown lint issues in docs/proxy.md: preserve the blockquote by keeping the `>` marker on the blank line near line 233 or removing that blank line, and add appropriate language identifiers to the code fences near lines 319 and 330, using powershell and text respectively.Source: Linters/SAST tools
docs/proxy.md-51-55 (1)
51-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the absolute “no half-state” claim.
proxy setup --skip-trustexplicitly permits DNS setup without CA trust, as documented at Line 177-179. Change this text to state that the default setup avoids the half-state, with--skip-trustas the documented exception.🤖 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/proxy.md` around lines 51 - 55, Update the proxy setup documentation paragraph to qualify the claim: state that the default setup configures trusted HTTPS together, while explicitly identifying proxy setup --skip-trust as the supported exception that permits DNS without CA trust.internal/tui/dev/tab_overview.go-416-423 (1)
416-423: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winShow a load failure instead of "No projects registered yet."
loadInstancesdiscards the error fromproxy.InstanceStats. If Docker is unavailable or the registry read fails,instancesis empty and the section states that no projects are registered. Keep the error ininstancesLoadedMsgand render it, so the user can tell a failure from an empty registry.Also applies to: 685-692
🤖 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/tui/dev/tab_overview.go` around lines 416 - 423, Update loadInstances to retain the error returned by proxy.InstanceStats and include it in instancesLoadedMsg. Extend the message handling and overview rendering to display that error instead of the empty-registry text when loading fails, while preserving “No projects registered yet.” for successful empty results.cmd/project/project_dev.go-22-52 (1)
22-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLowercase the host before the domain comparison.
url.URL.Hostname()preserves the case of the configured URL. A config value such ashttps://My-Shop.Shopware.localis a valid proxy URL for DNS, but the comparison againstbaseDomainfails, soproject devskips the proxy bootstrap. Compare lowercased values.🐛 Proposed fix
- host := parsed.Hostname() - return host == baseDomain || strings.HasSuffix(host, "."+baseDomain) + host := strings.ToLower(parsed.Hostname()) + baseDomain = strings.ToLower(baseDomain) + return host == baseDomain || strings.HasSuffix(host, "."+baseDomain)🤖 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 `@cmd/project/project_dev.go` around lines 22 - 52, Update isProxyProjectForDomain to lowercase the value returned by parsed.Hostname() before comparing it with baseDomain, ensuring both exact and subdomain checks handle mixed-case proxy URLs correctly.internal/tui/dev/tab_overview.go-1272-1288 (1)
1272-1288: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a Compose route for
rabbitmqor remove it fromknownServices. In proxy mode, the dashboard buildshttps://rabbitmq.<host>, but the override routeslavinmq, notrabbitmq; that URL has no router. Theadminer,mailer, andlavinmqlabels match.🤖 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/tui/dev/tab_overview.go` around lines 1272 - 1288, Update the proxy-mode URL construction in the discovered-services flow to use the Compose service name that is actually routed for RabbitMQ, such as the existing lavinmq identifier, while preserving the adminer and mailer routes; alternatively remove the RabbitMQ entry from knownServices so no unroutable rabbitmq.<host> URL is generated.cmd/project/project_proxy_setup.go-328-346 (1)
328-346: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTell the user what to do after a failed deregistration.
The loop prints each failure and continues. Teardown then stops Traefik and the DNS server. A project that failed to deregister keeps its proxy URL and its
compose.override.yaml, so it becomes unreachable with no further hint. Count the failures. If any occurred, print the recovery step, for example "run "shopware-cli project proxy down" in ", and finish with a non-zero exit.🤖 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 `@cmd/project/project_proxy_setup.go` around lines 328 - 346, Track the number of deregistration failures in the loop over reg.Projects, incrementing it whenever newProxyEnvironmentForRoot or env.down fails. After stopping Traefik and the DNS server, print a recovery instruction for each failed entry using its project path, then return a non-zero error when any failures occurred instead of reporting successful teardown.internal/proxy/trust.go-16-26 (1)
16-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
%qcorrupts Windows paths in the printed command.
%qproduces a Go-quoted string and escapes every backslash. On WindowscaPathcontains backslashes, so the printedcertutilcommand shows a doubled-backslash path that fails when a user copies it. Print the path with%sinside plain double quotes.🐛 Proposed fix
case "windows": - return fmt.Sprintf("certutil -addstore -f ROOT %q", caPath) + return fmt.Sprintf("certutil -addstore -f ROOT \"%s\"", caPath)🤖 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/proxy/trust.go` around lines 16 - 26, Update the Windows branch of TrustInstructions to format caPath with %s inside plain double quotes instead of using %q, so copied certutil commands preserve Windows backslashes. Leave the macOS and Unix command formatting unchanged.cmd/project/project_proxy_setup.go-21-24 (1)
21-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
SilenceErrorsso the sentinel error stays silent.
RunEreturnsErrProxyVerificationFailedat line 134. The declaration of that error incmd/project/project_proxy_verify.gostates that it exits non-zero without an extra message, andprojectProxyVerifyCmdsetsSilenceErrors: true.projectProxySetupCmddoes not, so Cobra prints "Error: proxy verification failed" after the printed check results.🐛 Proposed fix
var projectProxySetupCmd = &cobra.Command{ Use: "setup", SilenceUsage: true, + SilenceErrors: true, Short: "One-time machine setup for the shared proxy: DNS and HTTPS trust (needs sudo)",Note: with
SilenceErrorsevery error from this command becomes silent, so print the other failures explicitly, or return the sentinel through a wrapper that the root command recognizes.🤖 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 `@cmd/project/project_proxy_setup.go` around lines 21 - 24, Set SilenceErrors: true on projectProxySetupCmd so ErrProxyVerificationFailed does not produce Cobra’s duplicate error output. Audit the command’s RunE error paths and explicitly print any non-sentinel failures that must remain visible, preserving the existing verification-result output.cmd/project/project_proxy_setup.go-54-75 (1)
54-75: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStop
proxy setupearly on native Windows with the WSL2 guidance.On Windows,
ConfigureResolverreturnserrNotSupportedOnWindows, so setup exits throughResolverBlockedGuidancebeforeEnsureDNSServerRunningand shows no WSL2 pointer. Add a platform check before resolver work and print the hint used byproxy.Verify:Run shopware-cli inside WSL2 to use the proxy (see docs/proxy.md).🤖 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 `@cmd/project/project_proxy_setup.go` around lines 54 - 75, Add a native-Windows platform check before the resolver configuration block in the proxy setup flow, and print the WSL2 guidance used by proxy.Verify: “Run shopware-cli inside WSL2 to use the proxy (see docs/proxy.md).” Return immediately after displaying the hint so ConfigureResolver and EnsureDNSServerRunning are not invoked on Windows.internal/proxy/traefik.go-61-68 (1)
61-68: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRepresent
entryPointsas a YAML sequence. The file provider expects a list of strings. Use- websecureso Traefik loadsproxy-ping.🤖 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/proxy/traefik.go` around lines 61 - 68, Update the proxy-ping router YAML in the Traefik configuration template so entryPoints is represented as a sequence containing websecure, using the YAML list form required by the file provider. Preserve the existing router rule, TLS settings, and ping@internal service.internal/envfile/upsert.go-43-56 (1)
43-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStrip surrounding quotes from the returned value.
Symfony dotenv files allow quoted values, for example
APP_URL="http://127.0.0.1:8000".ReadEnvVarreturns the quotes as part of the value.cmd/project/project_proxy.gouses the result aspreviousAppURLand compares it againstproxyURL(lines 185-198), so a quoted value produces a wrong restore target and a wrongurlChangeddecision.🐛 Proposed fix
for _, l := range strings.Split(string(content), "\n") { if strings.HasPrefix(strings.TrimSpace(l), key+"=") { - return strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(l), key+"=")) + value := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(l), key+"=")) + if len(value) >= 2 && (value[0] == '"' || value[0] == '\'') && value[len(value)-1] == value[0] { + value = value[1 : len(value)-1] + } + + return value } }🤖 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/envfile/upsert.go` around lines 43 - 56, Update ReadEnvVar to remove one matching pair of surrounding single or double quotes from the extracted environment value before returning it, while preserving unquoted values and internal quote characters.cmd/project/project_proxy.go-605-620 (1)
605-620: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEscape backslashes as well before interpolating into the SQL literal.
MariaDB treats
\as an escape character inside string literals by default. A URL that ends with a backslash turns the doubled quote into an escaped quote and changes the statement. Escape\in addition to'.🔒️ Proposed fix
- esc := func(s string) string { return strings.ReplaceAll(s, "'", "''") } + esc := func(s string) string { + s = strings.ReplaceAll(s, `\`, `\\`) + return strings.ReplaceAll(s, "'", "''") + }🤖 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 `@cmd/project/project_proxy.go` around lines 605 - 620, Update the local esc function in repointSalesChannelViaSQL to escape backslashes as well as single quotes before interpolating fromURL and toURL into the SQL string literals, preserving the existing query construction and command execution.internal/proxy/windows_access.go-36-41 (1)
36-41: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winPrefer a user-owned path over
C:\Users\Publicfor the CA copy.
C:\Users\Publicis writable by every local account. Another local user can replaceshopware-cli-rootCA.pembetween step 1 and step 2, and the administrator then trusts a foreign CA root. The file holds only the public certificate, so no key leaks, but the trust step is the sensitive part. Point the copy at a path inside the user profile, for example%USERPROFILE%\shopware-cli-rootCA.pem, and keep the WSL mount path in sync.🤖 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/proxy/windows_access.go` around lines 36 - 41, Update the windowsCACopyPath constant to use a user-owned location under %USERPROFILE% instead of C:\Users\Public, and update wslWindowsCACopyMount to reference the corresponding WSL-mounted user-profile path. Keep both constants synchronized so the copy and trust commands target the same per-user certificate file.internal/proxy/hostname.go-32-38 (1)
32-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the derived label as a DNS label, not only as a Compose name.
system.ValidateDockerComposeNameaccepts names that are invalid DNS labels. A directory namedmy-shop-ormy_shop_maps tomy-shop-, which ends with a hyphen. A directory name longer than 63 characters also passes. Both produce a malformed hostname that Traefik routing and certificate matching reject later. The linked issue requires validation of generated domains.Add explicit label checks after the mapping.
🛡️ Proposed additional validation
name := strings.ReplaceAll(filepath.Base(projectRoot), "_", "-") if err := system.ValidateDockerComposeName(name); err != nil { return "", fmt.Errorf("cannot derive a hostname from directory name %q: %w", filepath.Base(projectRoot), err) } + if len(name) > 63 || strings.HasSuffix(name, "-") { + return "", fmt.Errorf("cannot derive a hostname from directory name %q: %q is not a valid DNS label", filepath.Base(projectRoot), name) + }🤖 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/proxy/hostname.go` around lines 32 - 38, Update the derived hostname label in the proxy hostname construction flow after the underscore-to-dash mapping and Compose-name validation. Add explicit DNS-label validation for an ASCII label: enforce the 63-character maximum, require alphanumeric start and end characters, and allow only alphanumeric characters or hyphens internally; return the existing hostname-derivation error for invalid labels before using the name.internal/proxy/registry_test.go-11-17 (1)
11-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRedirect the state directory on Windows too.
StateDirusesos.UserConfigDir. On Windows that function reads%AppData%and ignoresHOMEandXDG_CONFIG_HOME. A test run on Windows then reads and overwrites the real user registry at%AppData%\shopware-cli\proxy\registry.json.Set
AppDataas well.🛡️ Proposed fix
dir := t.TempDir() t.Setenv("HOME", dir) t.Setenv("XDG_CONFIG_HOME", dir) + t.Setenv("AppData", dir)🤖 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/proxy/registry_test.go` around lines 11 - 17, Update the test helper withTempStateDir to also set the AppData environment variable to the temporary directory, ensuring os.UserConfigDir resolves the isolated state directory on Windows while preserving the existing HOME and XDG_CONFIG_HOME setup.internal/mkcert/mkcert.go-50-69 (1)
50-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle an empty
LocalAppDataon Windows.If
LocalAppDatais empty,CAROOT()returns the relative pathmkcert.LoadOrCreateCA()then creates the CA in the current working directory instead of returning an error.🛡️ Proposed fix
case runtime.GOOS == "windows": dir = os.Getenv("LocalAppData") + if dir == "" { + return "" + }🤖 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/mkcert/mkcert.go` around lines 50 - 69, Update the Windows branch in CAROOT to validate that LocalAppData is non-empty before constructing the mkcert path; return the empty result used for missing base directories so LoadOrCreateCA does not create the CA in the current working directory.internal/proxy/hostname.go-21-30 (1)
21-30: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject configured hosts outside
baseDomain.
EnsureCertificateadds explicit SANs, but the embedded DNS server only answers names underbaseDomain. Without a manual/etc/hostsentry, an override such asshop.example.comdoes not resolve to Traefik.🤖 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/proxy/hostname.go` around lines 21 - 30, Update the configured-host handling in the hostname resolution function around cfg.URL and parsed.Hostname() to accept only hostnames within the configured baseDomain, including the base domain itself, while preserving the existing exclusions for localhost and IP addresses. Reject or ignore hosts outside baseDomain so the returned hostname always resolves through the embedded DNS server.internal/proxy/projectconfig.go-139-146 (1)
139-146: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse two-space indentation when rewriting the project config.
yaml.Marshaldefaults to four-space indentation, so rewriting.shopware-project.ymlcan reindent nested blocks. Useyaml.EncoderwithSetIndent(2).🤖 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/proxy/projectconfig.go` around lines 139 - 146, Update writeConfigDoc to serialize the YAML document through a yaml.Encoder configured with SetIndent(2) instead of yaml.Marshal, then write the encoded output to path while preserving existing error propagation and file permissions.internal/proxy/cert_test.go-92-105 (1)
92-105: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse
requireinreadTestCertificateto avoid a nil dereference panic.
assert.NoErrorandassert.NotNilmark the test failed but continue. Ifos.ReadFilefails orpem.Decodereturns no block, line 101 dereferences a nilblockand the test binary panics. The panic hides the real assertion message.🛠️ Proposed fix
content, err := os.ReadFile(path) - assert.NoError(t, err) + require.NoError(t, err) block, _ := pem.Decode(content) - assert.NotNil(t, block) + require.NotNil(t, block) cert, err := x509.ParseCertificate(block.Bytes) - assert.NoError(t, err) + require.NoError(t, err)Add the
github.com/stretchr/testify/requireimport.🤖 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/proxy/cert_test.go` around lines 92 - 105, Update readTestCertificate to use testify/require for the os.ReadFile error and pem.Decode result checks, replacing the corresponding assert calls so the test exits before dereferencing a nil block; add the require import.internal/proxy/dns.go-33-63 (1)
33-63: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose the UDP socket on every return path.
The watchdog goroutine closes
connonly whenctxis done. IfReadFromUDPfails for another reason,RunDNSServerreturns the error and leaves the socket open plus the goroutine blocked on<-ctx.Done(). A caller that restarts the server then fails to bind the port.🛠️ Proposed fix
conn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: dnsPort}) if err != nil { return err } + defer func() { _ = conn.Close() }()🤖 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/proxy/dns.go` around lines 33 - 63, Update RunDNSServer to defer closing conn immediately after a successful net.ListenUDP call, ensuring the socket is released on both normal shutdown and read errors. Retain the existing context-watcher behavior, while ensuring its repeated close is harmless and the goroutine does not prevent cleanup.internal/proxy/cert.go-113-132 (1)
113-132: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle IP literals in proxy certificate coverage
ValidateDomainaccepts127.0.0.1, soCertHostscan pass an IP literal toEnsureCertificate.mkcertstores it inIPAddresses, butcertCoverschecks onlyDNSNames. Each call then regenerates the certificate and restarts Traefik. Reject IP literals inValidateDomain, or compare them withcert.IPAddresses.🤖 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/proxy/cert.go` around lines 113 - 132, Update certCovers to handle IP literal hosts by parsing each host and comparing valid IPs against cert.IPAddresses while continuing to compare domain names against cert.DNSNames; preserve the existing expiry and certificate-read checks so covered certificates are not regenerated unnecessarily.
🧹 Nitpick comments (18)
internal/tui/dev/model.go (1)
306-311: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove the setup-state refresh off the update loop.
overviewSetupDonereadsregistry.json, loads proxy settings, and callsproxy.CheckResolverConfigured, which inspects the OS resolver. This runs synchronously insideUpdate, so the TUI freezes for the duration of that check. Return it as atea.Cmdand apply the result through a message, asloadSetupHealthalready does.🤖 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/tui/dev/model.go` around lines 306 - 311, The proxySetupDoneMsg branch in Update currently calls overviewSetupDone synchronously, blocking the TUI. Move that refresh into a tea.Cmd that returns a dedicated result message, then update m.overview.domainsSetupDone when handling that message; preserve the existing healthLoading and loadSetupHealth behavior.internal/tui/dev/tab_overview_health.go (1)
94-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated base-domain fallback logic. Both sites repeat the same
proxy.DefaultDomainplusLoadSettings().BaseDomain()fallback, whichcmd/project/project_create.goalso implements asproxyBaseDomain. Add one exported helper ininternal/proxyand call it from all three places.
internal/tui/dev/tab_overview_health.go#L94-L97: replace the local fallback block with the shared helper.internal/tui/dev/tab_overview.go#L342-L347: replace the identical block inoverviewSetupDonewith the same helper.🤖 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/tui/dev/tab_overview_health.go` around lines 94 - 97, Introduce one exported helper in internal/proxy that returns proxy.DefaultDomain unless proxy.LoadSettings succeeds, then returns settings.BaseDomain(). Replace the fallback blocks in internal/tui/dev/tab_overview_health.go:94-97 and internal/tui/dev/tab_overview.go:342-347 with calls to this helper, and update cmd/project/project_create.go’s proxyBaseDomain logic to use it as well.internal/tui/dev/tab_overview.go (1)
314-332: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueResolve the proxy hostname once in the constructor.
proxyHostname(projectRoot)runs three times here: at line 326, insideoverviewSetupDoneat line 327, and at line 328. Each call loadsregistry.jsonand resolves symlinks. Compute it once and pass it tooverviewSetupDone.♻️ Proposed refactor
func NewOverviewModel(envType, shopURL, username, password, projectRoot string, exec executor.Executor, shopCfg *shop.Config) OverviewModel { + proxyHost := proxyHostname(projectRoot) return OverviewModel{ ... - proxyHost: proxyHostname(projectRoot), - domainsSetupDone: overviewSetupDone(projectRoot), - instancesLoading: proxyHostname(projectRoot) != "", + proxyHost: proxyHost, + domainsSetupDone: proxyHost != "" && resolverConfigured(), + instancesLoading: proxyHost != "",🤖 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/tui/dev/tab_overview.go` around lines 314 - 332, Update NewOverviewModel to compute proxyHostname(projectRoot) once in a local variable, pass that value to overviewSetupDone, and reuse it for proxyHost and instancesLoading instead of invoking proxyHostname repeatedly.internal/proxy/dns_test.go (1)
20-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport the server error when startup fails.
The goroutine discards the
RunDNSServererror. The reserved port can be taken by another process betweenconn.Close()and the bind. The test then fails with "DNS server did not start" and hides the bind error. Capture the error and include it in the failure message.♻️ Proposed change
ctx, cancel := context.WithCancel(t.Context()) t.Cleanup(cancel) + errCh := make(chan error, 1) go func() { - _ = RunDNSServer(ctx, port, "shopware.local") + errCh <- RunDNSServer(ctx, port, "shopware.local") }() addr := fmt.Sprintf("127.0.0.1:%d", port) // Wait until the server answers. for range 50 { + select { + case err := <-errCh: + require.NoError(t, err, "DNS server stopped") + default: + } + if _, err := queryDNS(ctx, addr, "probe.shopware.local", dnsmessage.TypeA, 200*time.Millisecond); err == nil { return addr } time.Sleep(20 * time.Millisecond) }🤖 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/proxy/dns_test.go` around lines 20 - 45, Update the RunDNSServer startup goroutine to capture its returned error through a test-safe channel or shared state, then include that error in the t.Fatal message when the startup probe loop fails. Preserve the existing successful startup flow and cleanup behavior.internal/proxy/trust.go (1)
70-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the discarded
mkcert -installfailure.The
mkcerterror is dropped, and the fallback runs silently. If both paths fail, the returned message only describes thetruststorefailure, so the root cause stays hidden. Log themkcerterror through the context logger.♻️ Proposed change
if err := cmd.Run(); err == nil { return "The mkcert root CA is installed, certificates issued by it are trusted.", nil - } + } else { + logging.FromContext(ctx).Debugf("mkcert -install failed, falling back to the truststore library: %s", err) + } // mkcert failed (often: sudo blocked, or a broken mkcert install); // fall through to the library path, which explains itself on failure.As per coding guidelines: "Use structured logging through
go.uber.org/zap, obtain context-based loggers withlogging.FromContext(ctx), and report errors gracefully to users."🤖 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/proxy/trust.go` around lines 70 - 85, Capture the error returned by cmd.Run in the mkcert installation path and log it through the context logger obtained with logging.FromContext(ctx), using structured zap error logging before falling back to truststore.InstallFile. Preserve the existing fallback and returned error behavior.Source: Coding guidelines
cmd/project/project_proxy_setup.go (2)
307-313: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the teardown command into its own file.
projectProxyTeardownCmdis a separate subcommand but lives inproject_proxy_setup.go. Move it,confirmTeardown, and its flag registration intocmd/project/project_proxy_teardown.go.As per coding guidelines: "Organize Cobra commands with the main command in
cmd/[group]/[group].goand subcommands incmd/[group]/[group]_[subcommand].go".🤖 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 `@cmd/project/project_proxy_setup.go` around lines 307 - 313, Move the projectProxyTeardownCmd declaration, confirmTeardown, and teardown-specific flag registration from project_proxy_setup.go into cmd/project/project_proxy_teardown.go, preserving their behavior and command wiring. Keep setup-related symbols in the original file and follow the project_proxy_<subcommand>.go organization convention.Source: Coding guidelines
176-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared resolver and DNS step.
Lines 176-193 repeat lines 54-75 almost exactly: the same
CheckResolverConfiguredbranch, the sameErrNoSystemdResolvedhandling, the same success messages, and the sameEnsureDNSServerRunningcall. Two copies drift apart when the guidance text or the error handling changes. Extract one helper that both call, and keep only the domain-change reporting inRunE.🤖 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 `@cmd/project/project_proxy_setup.go` around lines 176 - 193, The resolver and DNS startup logic is duplicated between RunE and the earlier setup flow. Extract the shared CheckResolverConfigured, ConfigureResolver, ErrNoSystemdResolved handling, success output, and EnsureDNSServerRunning sequence into one helper, have both callers invoke it, and leave only domain-change reporting in RunE.internal/proxy/traefik.go (1)
315-331: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueTrim each output line before comparison.
docker psoutput can carry\rline endings on Windows hosts. The comparisonname == ContainerNamethen fails, and the proxy container appears in the instance list. Trim each line.♻️ Proposed change
var instances []Instance for _, name := range strings.Split(strings.TrimSpace(out), "\n") { + name = strings.TrimSpace(name) if name == "" || name == ContainerName { continue }🤖 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/proxy/traefik.go` around lines 315 - 331, Update RunningInstances to trim whitespace from each name yielded by the docker output before checking for empty values or comparing it with ContainerName, so carriage returns and other line-ending whitespace cannot include the proxy container as an instance.internal/envfile/upsert.go (1)
38-38: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCreate env files with 0600 instead of 0644.
UpsertEnvVartargets.env.local, which commonly holdsAPP_SECRET, database credentials and API keys. When the file does not exist yet, this call creates it world-readable. Use0o600for a newly created secrets file.🔒️ Proposed change
- return os.WriteFile(path, []byte(strings.Join(lines, "\n")+"\n"), 0o644) + return os.WriteFile(path, []byte(strings.Join(lines, "\n")+"\n"), 0o600)🤖 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/envfile/upsert.go` at line 38, Update the file mode passed to os.WriteFile in UpsertEnvVar from 0o644 to 0o600 so newly created .env.local files are accessible only by their owner.internal/extension/storefront_watch.go (1)
98-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
maps.Copyfor the merge.The manual loop can be replaced by the standard-library helper.
♻️ Proposed refactor
if opts.ProxyHostname != "" { proxyEnv, err := storefrontProxyEnv(projectRoot, cmdExecutor, opts.ProxyHostname) if err != nil { return nil, err } - for k, v := range proxyEnv { - env[k] = v - } + maps.Copy(env, proxyEnv) }Add
"maps"to the import block.As per coding guidelines: "Prefer Go 1.24 standard-library packages, such as
slices, when 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 `@internal/extension/storefront_watch.go` around lines 98 - 106, In the environment merge within the storefront proxy handling, replace the manual k/v iteration over proxyEnv with the Go standard-library maps.Copy helper, and add the maps import. Preserve the existing error return and merge behavior in the surrounding function.Source: Coding guidelines
internal/proxy/canonical.go (1)
8-15: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake the path absolute before resolving symlinks.
filepath.EvalSymlinkskeeps a relative input relative.ProjectEntry.ProjectRootis documented as the canonical absolute path and is used as the registry key (seeinternal/proxy/registry.go:11-23). If any caller passes a relative root, the registry gets a non-absolute key and lookups from another working directory miss. Addfilepath.Absfirst.♻️ Proposed hardening
func CanonicalProjectRoot(projectRoot string) string { - resolved, err := filepath.EvalSymlinks(projectRoot) + abs, err := filepath.Abs(projectRoot) + if err != nil { + abs = projectRoot + } + + resolved, err := filepath.EvalSymlinks(abs) if err != nil { - return projectRoot + return abs } return resolved }🤖 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/proxy/canonical.go` around lines 8 - 15, Update CanonicalProjectRoot to call filepath.Abs on projectRoot before filepath.EvalSymlinks, ensuring the returned canonical path is absolute. Preserve the existing fallback behavior on resolution errors, returning the absolute path when available.internal/proxy/projectconfig.go (1)
67-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that these two functions need an existing config file.
SetProjectConfigURLsandRestoreProjectConfigURLsreturn theos.ReadFileerror when the file is absent.ReadProjectConfigURLsinstead maps that case toHasFile=false. A caller that skips theHasFilecheck receives a raw "no such file or directory" error. State the precondition in both doc comments, or return early when the file is missing.🤖 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/proxy/projectconfig.go` around lines 67 - 104, Update the doc comments for SetProjectConfigURLs and RestoreProjectConfigURLs to state that configPath must reference an existing configuration file; preserve their current missing-file error behavior rather than adding new handling.internal/proxy/registry.go (1)
57-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace manual slice loops and
sortwith theslicespackage. Both sites use pre-generics slice handling where asliceshelper is clearer and shorter. The coding guidelines ask for Go 1.24 standard-library packages such asslices.
internal/proxy/registry.go#L57-L78: useslices.IndexFuncinUpsertandRemove, andslices.Deletefor the removal.internal/proxy/stats.go#L81-L84: replacesort.SliceStablewithslices.SortStableFuncand drop thesortimport.
As per coding guidelines: "Prefer Go 1.24 standard-library packages, such asslices, when 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 `@internal/proxy/registry.go` around lines 57 - 78, Replace the manual searches in internal/proxy/registry.go:57-78 with slices.IndexFunc in both Registry.Upsert and Registry.Remove, preserving the existing update and boolean-return behavior; use slices.Delete when removing the matched project. In internal/proxy/stats.go:81-84, replace sort.SliceStable with slices.SortStableFunc and remove the now-unused sort import.Source: Coding guidelines
internal/proxy/stats.go (2)
41-44: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueReport the
docker psfailure gracefully.
InstanceStatsreturns the rawdocker pserror. The development dashboard calls this function on refresh, so a stopped Docker daemon surfaces as a raw command error in the overview. Wrap the error with context, and log it throughlogging.FromContext(ctx)so the dashboard can present a readable state.🤖 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/proxy/stats.go` around lines 41 - 44, Update InstanceStats to wrap the error returned by runDocker for the "ps" command with descriptive context, then log the contextual error through logging.FromContext(ctx) before returning. Preserve the existing nil, zero, error return behavior.Source: Coding guidelines
105-111: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLimit
docker statsto the relevant containers.
docker stats --no-streamsamples every running container on the host, including containers unrelated to registered projects. Each sample takes hundreds of milliseconds per container, and the development dashboard callsInstanceStatson refresh. Pass the known container IDs so the cost scales with the registered projects.♻️ Proposed change
-func memoryByProject(ctx context.Context, projectOfContainer map[string]string) map[string]int64 { +func memoryByProject(ctx context.Context, projectOfContainer map[string]string, containerIDs []string) map[string]int64 { byProject := map[string]int64{} + if len(containerIDs) == 0 { + return byProject + } - out, err := runDocker(ctx, "stats", "--no-stream", "--format", "{{.Name}}\t{{.MemUsage}}") + args := append([]string{"stats", "--no-stream", "--format", "{{.Name}}\t{{.MemUsage}}"}, containerIDs...) + out, err := runDocker(ctx, args...) if err != nil { return byProject }Update the call site at Line 60 accordingly.
🤖 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/proxy/stats.go` around lines 105 - 111, Update memoryByProject and its call site in InstanceStats to pass the known container IDs to runDocker’s docker stats invocation. Build the container arguments from projectOfContainer keys and preserve the existing no-stream and format options, so stats samples only registered project containers.internal/proxy/verify_darwin.go (1)
21-23: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMatch the resolved address per line, not as a substring.
strings.Containsalso matchesip_address: 127.0.0.10orip_address: 127.0.0.1x. Compare the trimmed value of eachip_address:line instead.♻️ Proposed fix
- if !strings.Contains(string(out), "ip_address: 127.0.0.1") { - return fmt.Errorf("%s does not resolve to 127.0.0.1 via the system resolver", hostname) - } - - return nil + for _, line := range strings.Split(string(out), "\n") { + value, ok := strings.CutPrefix(strings.TrimSpace(line), "ip_address:") + if ok && strings.TrimSpace(value) == "127.0.0.1" { + return nil + } + } + + return fmt.Errorf("%s does not resolve to 127.0.0.1 via the system resolver", hostname)🤖 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/proxy/verify_darwin.go` around lines 21 - 23, Update the resolver validation around the ip_address check to inspect each output line, identify lines beginning with "ip_address:", and compare the trimmed value after the delimiter exactly to 127.0.0.1. Replace the substring-based strings.Contains check while preserving the existing error return when no exact match is found.internal/proxy/windows_access.go (1)
12-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one source of truth for routed subdomains.
internal/proxy/windows_access.goduplicates the subdomains defined ininternal/docker/compose_override.go. When a new subdomain route is added, update both lists or derive the Windows hosts list from the shared route definitions.🤖 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/proxy/windows_access.go` around lines 12 - 19, Update ProxyHostnames to use the shared routed-subdomain definitions from compose_override.go instead of maintaining its own hardcoded subdomains list. Preserve the existing conditional inclusion of AMQP and Elasticsearch routes while ensuring future route additions automatically apply to the Windows hosts list.internal/proxy/projectconfig_test.go (1)
97-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
countOccurrenceswithstrings.Count.The standard library provides this exact behavior. The helper adds a hand-written loop with no added value.
♻️ Proposed refactor
-func countOccurrences(s, sub string) int { - count := 0 - for i := 0; i+len(sub) <= len(s); i++ { - if s[i:i+len(sub)] == sub { - count++ - } - } - return count -}Then use
strings.Countat the call sites (lines 53, 63) and add thestringsimport.As per coding guidelines: "Prefer Go 1.24 standard-library packages, such as
slices, when 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 `@internal/proxy/projectconfig_test.go` around lines 97 - 105, Remove the custom countOccurrences helper and replace its call sites in the test with strings.Count, passing the same string and substring arguments. Add the strings import and preserve the existing assertions and counting behavior.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6695d98-b1fe-4668-bfc2-071186a4a0c1
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (70)
cmd/project/project_create.gocmd/project/project_create_form.gocmd/project/project_create_install.gocmd/project/project_dev.gocmd/project/project_dev_test.gocmd/project/project_proxy.gocmd/project/project_proxy_dns_serve.gocmd/project/project_proxy_list.gocmd/project/project_proxy_setup.gocmd/project/project_proxy_test.gocmd/project/project_proxy_verify.gocmd/project/project_storefront_watch.gocmd/root.godocs/proxy.mdgo.modinternal/docker/compose.gointernal/docker/compose_override.gointernal/docker/compose_override_test.gointernal/docker/compose_test.gointernal/envfile/upsert.gointernal/envfile/upsert_test.gointernal/executor/docker.gointernal/extension/storefront_hmr_patch.cjsinternal/extension/storefront_watch.gointernal/mkcert/LICENSEinternal/mkcert/mkcert.gointernal/mkcert/mkcert_test.gointernal/proxy/canonical.gointernal/proxy/cert.gointernal/proxy/cert_test.gointernal/proxy/dns.gointernal/proxy/dns_daemon.gointernal/proxy/dns_daemon_windows.gointernal/proxy/dns_test.gointernal/proxy/docker.gointernal/proxy/hostname.gointernal/proxy/hostname_test.gointernal/proxy/projectconfig.gointernal/proxy/projectconfig_test.gointernal/proxy/registry.gointernal/proxy/registry_test.gointernal/proxy/resolver.gointernal/proxy/resolver_darwin.gointernal/proxy/resolver_darwin_test.gointernal/proxy/resolver_linux.gointernal/proxy/resolver_linux_test.gointernal/proxy/resolver_windows.gointernal/proxy/settings.gointernal/proxy/settings_test.gointernal/proxy/statedir.gointernal/proxy/stats.gointernal/proxy/stats_test.gointernal/proxy/traefik.gointernal/proxy/traefik_test.gointernal/proxy/trust.gointernal/proxy/trust_test.gointernal/proxy/verify.gointernal/proxy/verify_darwin.gointernal/proxy/verify_linux.gointernal/proxy/verify_test.gointernal/proxy/verify_windows.gointernal/proxy/windows_access.gointernal/proxy/windows_access_test.gointernal/proxy/wsl_resolver_test.gointernal/system/dockername.gointernal/tui/dev/model.gointernal/tui/dev/model_view.gointernal/tui/dev/tab_overview.gointernal/tui/dev/tab_overview_format_test.gointernal/tui/dev/tab_overview_health.go
|
I was just browsing this repo and wanted to say my two cents: You can save yourself the work of an local DNS server, just use a real domain. Nothing (except the ticket at your infra team :D) prevents you from pointing *.local.dev.shopware.com to 127.0.0.1, we do this internally for our development defaults. Saves everyone from configuring their /etc/hosts file. You can see this in action via https://nip.io who provides this generically "for free". So Also, from my experience, a "real" DNS entry is significantly more reliable. It works in every container, on every device, VM whatever, it saves so much "why is this sh**t not resolving" pain. If you do decide to go the DNS server route, a setting to disable the DNS server and provide my own toplevel domain would be greatly appreciated. Or us nip.io, it's fine for dev, does the same thing as a custom showpare local entry, just less "brandy". |
|
We actually solved this problem in our development environments already and we are pretty happy with it. The solution is very similar to yours with the difference we actually needed direct database access to spin up staging environments for our shopware that we export via the dump command. So we created a include:
- path: ./compose.local.yaml
env_file: ./.env.localAnd then the Most importantly, this allows us to still access our services via the $BINDING_IPV4 (at least on linux you can just curl http://127.0.0.45 and have a good idea, you would have to test windows yourself) You also don't need |
|
One thing is important tho, so it get's its own comment. If you have a compose service, it adds the compose name (e.g. "traction-integration-extension_default": {
"IPAMConfig": null,
"Links": null,
"Aliases": [
"traction-integration-extension-mailer-1",
"mailer" // DANGEROUS
],
"DriverOpts": null,
"GwPriority": 0,
"NetworkID": "887b957011ceafd012f3cc69de243040c91a28689475a1d24730f4242ad5eb59",
"EndpointID": "d04048bbd12d464be74bc2015881964e6300af05996dfe4d56e76ed6e1539874",
"Gateway": "172.25.0.1",
"IPAddress": "172.25.0.4",
"MacAddress": "02:0d:ca:63:a9:b8",
"IPPrefixLen": 16,
"IPv6Gateway": "",
"GlobalIPv6Address": "",
"GlobalIPv6PrefixLen": 0,
"DNSNames": [
"traction-integration-extension-mailer-1",
"mailer", // DANGEROUS
"a2ab381e402d"
]
}This is outlined in https://docs.docker.com/compose/how-tos/networking/#connecting-multiple-compose-projects and it what you would expect to happen. And as long as the containers of a stack are all running, this will work fine. But take the I am honestly a little too lazy to read the implementation in detail but it looks somewhat vulnerable to this. All I can say is we had a staging system with a set of default passwords for every docker instance running on there and at some point someone shut down one of his services and we had mails and messages in instances we did not expect nor wanted. |
…cuit verify on Windows
…her port Drop the version-gated adminWatchLocalPort in favor of the shared extension.AdminDevServerPort (from #1288), which reads the ADMIN_VITE flag instead of guessing from the Shopware version — correct for a 6.6 shop that opted into Vite.
next tightened .golangci.yml (perfsprint): replace fmt.Sprintf/Errorf with strconv/errors/string concatenation in the proxy, docker-compose, and mkcert code.
…anging the OS resolver
…drop docker version gate
…ges, thin the cmd
…ction (any version)
…p the override file
aa572a4 to
05f8c6e
Compare
…f-PID, hostname, atomic registry, network rollback)
d0461ae to
dd10c4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/executor/docker.go (1)
127-203: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
composeArgsfor the two new compose invocations.
DockerExecutor.composeProjectNameexists so every compose command targets the same project even after the project.envchanges. Line 134 and line 169 builddocker compose ...directly and skip the-pflag. If Compose resolves a different project name at that moment,exec -T web printenvandcompose portaddress another stack or fail, andDatabaseConnectionthen returns wrong or no credentials.🐛 Proposed fix
- cmd := exec.CommandContext(ctx, "docker", "compose", "exec", "-T", "web", "printenv", "DATABASE_URL") + cmd := exec.CommandContext(ctx, "docker", d.composeArgs("exec", "-T", "web", "printenv", "DATABASE_URL")...)- cmd := exec.CommandContext(ctx, "docker", "compose", "port", conn.Host, conn.Port) + cmd := exec.CommandContext(ctx, "docker", d.composeArgs("port", conn.Host, conn.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 `@internal/executor/docker.go` around lines 127 - 203, Update DatabaseConnection and resolvePublishedPort to build both Docker Compose commands through the existing composeArgs helper, including the appropriate subcommand arguments, so composeProjectName and the shared project selection are preserved. Replace the direct docker compose argument lists used for reading DATABASE_URL and resolving the published port; leave command execution and error handling unchanged.
🧹 Nitpick comments (2)
cmd/project/project_proxy_test.go (1)
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider running each case as a subtest.
t.Run(c.name, ...)isolates failures and names them in the test output. The current loop reports all failures under one test name.♻️ Proposed change
for _, c := range cases { - gotLocal, gotSetup := resolveLocalDomainChoice(c.useDocker, c.wantLocal, c.promptShown, c.setupDone, c.answer) - assert.Equal(t, c.wantUseLocal, gotLocal, c.name+" (useLocalDomain)") - assert.Equal(t, c.wantSetupNow, gotSetup, c.name+" (setupProxyNow)") + t.Run(c.name, func(t *testing.T) { + t.Parallel() + gotLocal, gotSetup := resolveLocalDomainChoice(c.useDocker, c.wantLocal, c.promptShown, c.setupDone, c.answer) + assert.Equal(t, c.wantUseLocal, gotLocal, "useLocalDomain") + assert.Equal(t, c.wantSetupNow, gotSetup, "setupProxyNow") + }) }🤖 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 `@cmd/project/project_proxy_test.go` around lines 27 - 32, Update the test loop around resolveLocalDomainChoice to run each case through t.Run using c.name, placing the existing assertions inside the subtest so failures are isolated and reported with the individual case name.internal/executor/docker.go (1)
217-238: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueLog cleanup-command failures at debug level. The configured
ghcr.io/shopware/docker-devimage contains/usr/bin/pgrep. Otherdocker compose execfailures remain hidden by_ = killCmd.Run().🤖 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/executor/docker.go` around lines 217 - 238, Update the stop callback’s killCmd.Run handling to log cleanup-command failures at debug level instead of discarding them. Preserve the existing process signal and nil-error return behavior, and use the surrounding executor logging mechanism.
🤖 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 `@cmd/project/project_create_test.go`:
- Around line 84-89: Update the test case invoking applyNonInteractiveDefaults
to initialize createOptions with setupProxyNow: true, while preserving the
existing assertion that opts.setupProxyNow is false so the test verifies the
field is reset for non-interactive project creation.
In `@cmd/project/project_dev.go`:
- Around line 50-53: Update ensureProxyForDevProjectWithFallback so failures
from dockerpkg.WriteComposeFile are returned instead of ignored, and adjust both
callers to propagate the error. Only report the local-port fallback and continue
startup after fixed-port Compose generation succeeds; otherwise stop before
starting the environment.
- Around line 51-52: Handle the error returned by dockerpkg.WriteComposeFile in
the fallback path before returning true, so a failed Compose write is not
silently ignored and the proxy-mode file is not left active. Log the bootstrap
error through logging.FromContext(ctx).Error with zap.Error(err), while
preserving the existing local-port fallback output for successful writes.
In `@cmd/project/project_proxy_setup.go`:
- Around line 136-140: Update the automatic verification branch around
runProxyVerification to honor the --skip-trust setting: when trust installation
is skipped, do not fail setup on the trusted-HTTPS verification, while
preserving the existing verification and ErrProxyVerificationFailed behavior
when trust is enabled.
- Around line 444-464: The teardown flow around newProxyEnvironmentForRoot and
env.down must report partial project failures instead of returning success. Keep
processing all projects, collect any deregistration errors, complete
proxy.StopTraefik and proxy.StopDNSContainer cleanup, then return an aggregated
non-nil error if any project failed; only print the success message and return
nil when all project teardowns succeed.
- Around line 90-96: Update the change handling around
configureResolverAutomatically so change.commit or change.persist runs only
after proxy.CheckResolverConfigured(change.requested) succeeds. Preserve the
prior resolver settings when automatic setup returns proxy.ErrNoSystemdResolved
or validation fails, and only activate the domain after the new resolver route
is confirmed.
In `@cmd/project/project_proxy_verify.go`:
- Around line 35-36: Update runProxyVerification and its callers in the project
proxy verification flow to distinguish an inactive shared proxy from failed
resolver, certificate, or runtime checks. Report the inactive proxy as stopped
and return success for that state, while preserving nonzero exits for actual
verification failures; update the handling around settings.BaseDomain() and the
related lines 46-74 accordingly.
In `@cmd/project/project_proxy.go`:
- Around line 329-378: Update ensureHostnameResolves and hostsFileContains to
use the platform-appropriate hosts-file path, specifically
C:\Windows\System32\drivers\etc\hosts on native Windows, and print that same
path in the manual-entry guidance instead of /etc/hosts. Preserve the existing
Unix behavior and detection logic.
In `@docs/proxy.md`:
- Around line 379-382: Update the watcher-support limitation in the
documentation to acknowledge the implemented admin watcher routing and
storefront HMR runtime patch. Describe only any remaining limitations, and
remove the claim that admin-watch and storefront-watch are unavailable
end-to-end.
- Around line 183-187: Update the certificate behavior description near the
proxy up documentation to state that setup restarts Traefik when certificate
regeneration changes the certificate, replacing the claim that Traefik reloads
the files without restarting.
- Line 247: Fix the Markdown lint violations in docs/proxy.md: at lines 247-247,
remove the empty blockquote separator or merge the adjacent blockquote
paragraphs; at lines 334-336, add an appropriate language identifier to the
Windows terminal code fence; and at lines 345-348, add an appropriate language
identifier to the hosts-file code fence.
- Line 263: Remove the obsolete embedded-DNS design-decision row mentioning
golang.org/x/net/dns/dnsmessage and miekg/dns from the design table, or replace
it with an accurate entry describing the current CoreDNS container
implementation without contradicting the existing CoreDNS description.
In `@internal/proxy/dns_container_test.go`:
- Around line 21-30: Update the DNS container test around
EnsureDNSContainerRunning to skip when dnsContainerExists(ctx) reports the fixed
shared container already exists, preventing reuse or replacement of another
process’s resource. In the t.Cleanup callback, derive a bounded context from
context.Background() instead of t.Context(), call StopDNSContainer with it, and
assert any cleanup error rather than ignoring it.
In `@internal/proxy/dns_container.go`:
- Around line 63-101: Add a state-directory lock at the start of
EnsureDNSContainerRunning, before writeDNSCorefile and all Docker state checks,
and release it on every return path. After acquiring the lock, re-check the
container state before starting or creating it, preserving the existing behavior
while preventing concurrent docker run name conflicts.
In `@internal/proxy/hostname.go`:
- Around line 22-30: Update ProjectHostname to select
cfg.Environments["local"].URL before cfg.URL, matching the effective-URL logic
used by IsProxyProjectForDomain; parse and derive the hostname from that
selected URL while preserving existing validation and fallback behavior. Add a
regression test covering a localhost cfg.URL with a custom local environment URL
and assert the custom hostname is returned.
In `@internal/proxy/resolver_darwin.go`:
- Around line 31-40: Update the resolver configuration validation in the Darwin
resolver path to require both nameserver 127.0.0.1 and the expected DNSPort
before returning Configured: true; otherwise return the existing
misconfiguration status. Add a test covering a different nameserver using the
same port and assert it is reported as unconfigured.
---
Outside diff comments:
In `@internal/executor/docker.go`:
- Around line 127-203: Update DatabaseConnection and resolvePublishedPort to
build both Docker Compose commands through the existing composeArgs helper,
including the appropriate subcommand arguments, so composeProjectName and the
shared project selection are preserved. Replace the direct docker compose
argument lists used for reading DATABASE_URL and resolving the published port;
leave command execution and error handling unchanged.
---
Nitpick comments:
In `@cmd/project/project_proxy_test.go`:
- Around line 27-32: Update the test loop around resolveLocalDomainChoice to run
each case through t.Run using c.name, placing the existing assertions inside the
subtest so failures are isolated and reported with the individual case name.
In `@internal/executor/docker.go`:
- Around line 217-238: Update the stop callback’s killCmd.Run handling to log
cleanup-command failures at debug level instead of discarding them. Preserve the
existing process signal and nil-error return behavior, and use the surrounding
executor logging mechanism.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: bb0720f3-d0d4-445c-bc3d-31885867882b
📒 Files selected for processing (40)
cmd/project/project_create.gocmd/project/project_create_form.gocmd/project/project_create_install.gocmd/project/project_create_test.gocmd/project/project_dev.gocmd/project/project_proxy.gocmd/project/project_proxy_down.gocmd/project/project_proxy_list.gocmd/project/project_proxy_setup.gocmd/project/project_proxy_test.gocmd/project/project_proxy_up.gocmd/project/project_proxy_verify.godocs/proxy.mdinternal/docker/compose.gointernal/docker/compose_proxy.gointernal/docker/compose_proxy_test.gointernal/docker/compose_test.gointernal/executor/docker.gointernal/proxy/dns.gointernal/proxy/dns_container.gointernal/proxy/dns_container_test.gointernal/proxy/dns_test.gointernal/proxy/docker.gointernal/proxy/hostname.gointernal/proxy/hostname_test.gointernal/proxy/infra.gointernal/proxy/infra_test.gointernal/proxy/registry.gointernal/proxy/resolver.gointernal/proxy/resolver_darwin.gointernal/proxy/resolver_linux.gointernal/proxy/resolver_test.gointernal/proxy/resolver_windows.gointernal/proxy/statedir.gointernal/proxy/stats.gointernal/proxy/traefik.gointernal/proxy/verify.gointernal/tui/dev/model.gointernal/tui/dev/model_commands.gointernal/tui/dev/model_update.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/docker/compose_test.go
- cmd/project/project_create_form.go
- internal/proxy/statedir.go
- cmd/project/project_create.go
- internal/proxy/verify.go
- internal/proxy/registry.go
- internal/proxy/resolver_linux.go
…ethod, addVolumes, fold project-url patching into shop config) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Write APP_URL into .env.local before the container starts and drop the pinned environment: APP_URL, so editing .env.local is no longer silently overridden. Sales channel repoint stays after start (needs the DB). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…m store Mount a combined CA bundle (the image's public CAs plus the proxy CA) over /etc/ssl/certs/ca-certificates.crt, so PHP and curl trust the proxy for Shopware's own APP_URL reachability self-call — the bare CA under /usr/local/share/ca-certificates did nothing (image runs as www-data, never runs update-ca-certificates). Node keeps trusting it via NODE_EXTRA_CA_CERTS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough report 🙏 Status after this round:
The new-shop flow you liked is unchanged. |
…idle up now refuses early (before starting shared infra or rewriting compose) when the project isn't a shopware-cli Docker dev environment, with a clear message — avoids the opaque "operation not supported" + half-started proxy, and stops a differently-managed shop being pointed at an empty database (QA #1, #2). proxy verify now renders not-running layers (DNS/Traefik) as a calm "not started yet" and exits 0 instead of a red error page when the proxy is simply idle, e.g. after teardown (QA #6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR #1208 Re-test QA report — run multiple local shops in parallel behind a shared proxySummaryIt works. I ran multiple shops at the same time, each opening in the browser over HTTPS at its own I tested twice: once on the first version, and again after the update that was pushed to address the Status after re-test (updated version):
Issue #1 — still present (details)Turning the proxy on can point the shop at the wrong (empty) database 🔴 HighWhat I did: ran a shop whose data lives in a database with a non-default name (set correctly in the What I expected: the shop uses the database I configured. What happened: the shop opened, then broke while I used it with Why it matters: it looks fine at first and only breaks later, in use — easy to miss. Nothing warns Re-test on the updated version: still happens. (The update did make the shop's web address How to reproduce it (verified):
What you see side by side when it happens:
Scope: only affects shops using a non-default database name. Shops made the normal way with One more thing to watch (new, minor)On the updated version, the step that updates the shop's web address reaches the database through a port |
What changed?
New command group
shopware-cli project proxy— run any number of local shops in parallel under stable hostnames, instead of everyone fighting over127.0.0.1:8000.proxy setup--domain,--skip-trust)proxy up/downproxy list/statusproxy verifyproxy teardownUnder the hood: one shared Traefik container routes by hostname (shops publish no host ports at all), a tiny DNS server embedded in the binary answers
*.shopware.local → 127.0.0.1, and an mkcert-compatible local CA provides trusted HTTPS. Proxy mode is a marker-guardedcompose.override.yaml— the basecompose.yamlstays untouched, soproject devand manualdocker composekeep working.uppointsAPP_URL, the sales-channel domain and the project config at the proxy;downrestores every value exactly.➡️ Architecture, design decisions and trade-offs:
docs/proxy.mdWhy?
The dev environment publishes fixed host ports, so a second shop can't start — anyone working on multiple projects juggles ports or stops shops. Routing by hostname removes the conflict by construction, and trusted HTTPS matters for testing payment providers locally.
How was this tested?
go test ./...green,golangci-lint run ./...— 0 issuesup/down/teardowncycles with byte-identical restore of.shopware-project.yml,.env.localand the sales-channel domain;verifyladder validated against a real corporate sudo-block scenarioRelated issue or discussion
Closes #1094, related: #939
Summary by CodeRabbit
New Features
project proxy setup,up,down,teardown,verify,status, andlistcommands.Bug Fixes