feat(agent): Albacore lifecycle fixes plus an out-of-process plugin framework#164
Conversation
Setting a service image or updating compose now validates a relative env_file against the deployment directory, so deployments using ./.env no longer fail validation. Additional domains route to the deployment's own container instead of an arbitrary container that happens to share a service name, fixing intermittent cross-deployment responses. Long hostnames no longer break nginx after an upgrade now that the hash sizing is applied to managed configs. Deployment and service actions can force-recreate containers, rebuild without cache, and pull fresh images, so updated environment variables and images actually take effect without dropping to the terminal. A deployment's primary service can be pinned and is reported per service, and the pin survives compose updates and re-discovery. Adds Access Groups, a built-in app that models per-deployment east-west and egress rules. Rules are persisted and previewable, with enforcement still to come.
…n apps Plugins can now be standalone binaries that the agent launches as subprocesses and reverse-proxies, so an app ships its own API and UI without being compiled into the agent or run as a container. The plugin contract is split into small capability interfaces, so an app implements only what it needs: a firewall has no services to start or stop. The built-in apps, a host firewall and the DNS providers, now register through one registry and list uniformly alongside any installed plugins. The marketplace catalog is fetched through the agent rather than the browser, which removes the cross-origin restriction that blocked it; the upstream is configurable. Drops the per-deployment access-groups app, which only duplicated the network isolation Docker already provides between deployments by default.
Code Review SummaryThis PR introduces a robust out-of-process plugin framework (Albacore) and several lifecycle fixes for deployments. It transitions from a monolithic plugin interface to a capability-based one, allowing for more flexible service extensions like the new Firewall module. 🚀 Key Improvements
💡 Minor Suggestions
|
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| pluginapi.EnvAgentURL+"="+h.agentURL, | ||
| pluginapi.EnvToken+"="+h.token, | ||
| ) | ||
| cmd.Stdout = os.Stdout |
There was a problem hiding this comment.
Redirecting both Stdout and Stderr to the parent process's Stdout can make debugging plugin crashes difficult as error messages will be interleaved with standard logs without distinction. Consider using a prefixed logger.
| cmd.Stdout = os.Stdout | |
| cmd.Stdout = log.Writer() | |
| cmd.Stderr = log.Writer() |
| if resp.StatusCode != http.StatusOK { | ||
| return pluginapi.Info{}, fmt.Errorf("info returned %d", resp.StatusCode) | ||
| } | ||
| if resp.Header.Get(pluginapi.HandshakeHeader) != handshake { |
There was a problem hiding this comment.
The handshake check uses a simple string comparison. While sufficient for the current implementation, using subtle.ConstantTimeCompare is a better security practice for sensitive handshake/token comparisons to prevent timing attacks.
| if resp.Header.Get(pluginapi.HandshakeHeader) != handshake { | |
| if subtle.ConstantTimeCompare([]byte(resp.Header.Get(pluginapi.HandshakeHeader)), []byte(handshake)) != 1 { |
| return out, err | ||
| } | ||
| } | ||
| args := applyUpFlags([]string{"up", "-d", "--no-deps"}, ro) |
There was a problem hiding this comment.
When fresh_pull is requested, appending --pull always is correct, but ensure this doesn't conflict with existing logic in runCompose if it also tries to manage pull flags.
| args := applyUpFlags([]string{"up", "-d", "--no-deps"}, ro) | |
| args = applyUpFlags([]string{"up", "-d", "--no-deps", "--pull", "always"}, ro) |
An additional domain that points at a deployment's own primary service produced a proxy upstream with the deployment name repeated, which does not resolve, so the deployment became unreachable and requests timed out. Such a domain now routes to the correct container.
| if resp.StatusCode != http.StatusOK { | ||
| return pluginapi.Info{}, fmt.Errorf("info returned %d", resp.StatusCode) | ||
| } | ||
| if resp.Header.Get(pluginapi.HandshakeHeader) != handshake { |
There was a problem hiding this comment.
While existing review comments suggested subtle.ConstantTimeCompare, a simple equality check is still being used here. For a per-launch random handshake cookie, standard equality is likely fine, but ensure the HandshakeHeader value is not empty to avoid accidental matches if a plugin misbehaves.
| if resp.Header.Get(pluginapi.HandshakeHeader) != handshake { | |
| if h := resp.Header.Get(pluginapi.HandshakeHeader); h == "" || h != handshake { |
| } | ||
|
|
||
| // A stale socket file from a previous run would block Listen. | ||
| _ = os.Remove(socket) |
There was a problem hiding this comment.
The plugin SDK removes the socket file before listening. This is good practice for Unix sockets, but consider checking if the file exists and is indeed a socket to avoid accidentally deleting unrelated files if the environment variable is misconfigured.
| _ = os.Remove(socket) | |
| if fi, err := os.Stat(socket); err == nil && fi.Mode()&os.ModeSocket != 0 { | |
| _ = os.Remove(socket) | |
| } |
1 participant
Albacore (#158)
env_fileresolves against the deployment directory, so setting a service image or updating compose stops failing validation.Plugins
Firewall models and validates rules but does not enforce them yet.