feat(api): Add deploy endpoint, compose mount, and custom-image create#125
Conversation
Code Review SummaryThe PR introduces a unified deploy endpoint, support for dynamic volume mounting into Compose files, and allows custom images for deployments. These changes significantly improve the flexibility of the agent's API. 🚀 Key Improvements
💡 Minor Suggestions
|
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| @@ -2615,7 +2834,9 @@ type ComposeGenerateRequest struct { | |||
|
|
|||
| type PortConfig struct { | |||
There was a problem hiding this comment.
The PortConfig struct now has redundant fields (ContainerPort/Container and HostPort/Host). This likely stems from supporting different JSON naming conventions. It is cleaner to use standard tags and handle mapping in the logic or use a single naming convention.
| type PortConfig struct { | |
| type PortConfig struct { | |
| ContainerPort int `json:"container_port"` | |
| HostPort string `json:"host_port"` | |
| } |
| if len(image) > 255 { | ||
| return fmt.Errorf("image name is too long") | ||
| } | ||
| validImage := regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._:/@-]*$`) |
There was a problem hiding this comment.
The regexp is compiled inside the function. For better performance, move this to a global variable using regexp.MustCompile so it is only compiled once.
| validImage := regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._:/@-]*$`) | |
| var validImageRegex = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._:/@-]*$`) |
| }{ | ||
| Action: "restart", | ||
| } | ||
| if c.Request.Body != nil && c.Request.ContentLength != 0 { |
There was a problem hiding this comment.
The check c.Request.Body != nil is generally true in Gin; checking c.Request.ContentLength == 0 is okay but ShouldBindJSON handles empty bodies depending on the binding. However, since a default restart action is desired, ensure that if a body is present but doesn't contain the field, the default is kept.
| if c.Request.Body != nil && c.Request.ContentLength != 0 { | |
| if c.Request.ContentLength > 0 { |
| return "", fmt.Errorf("source_path is invalid") | ||
| } | ||
|
|
||
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") |
There was a problem hiding this comment.
When sourcePath starts with a /, it is converted to a relative path by prepending ./. However, the logic then duplicates the ./ check and trimming in a confusing way. Simplifying this to a single path.Join with a base or a more direct prefixing would be clearer.
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") | |
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") | |
| if strings.HasPrefix(sourcePath, "/") { | |
| sourcePath = "." + sourcePath | |
| } else { | |
| sourcePath = "./" + sourcePath | |
| } |
| HostPort: hostPort, | ||
| } | ||
| if len(ports) > 0 { | ||
| composeReq.ContainerPort = ports[0].ContainerPort |
There was a problem hiding this comment.
The logic to extract the first port from the ports slice is repetitive. Using the new PortConfig fields (Container and Host) alongside the existing ones without clear priority is messy. It is better to consolidate this into a helper or a more concise conditional block.
| composeReq.ContainerPort = ports[0].ContainerPort | |
| p := ports[0] | |
| composeReq.ContainerPort = p.ContainerPort | |
| if composeReq.ContainerPort == 0 { | |
| composeReq.ContainerPort = p.Container | |
| } | |
| host := p.HostPort | |
| if host == "" { | |
| host = p.Host | |
| } | |
| if host != "" { | |
| composeReq.MapPorts = true | |
| composeReq.HostPort = host | |
| } |
Adds POST /deployments/:name/deploy that bundles image pull and the requested lifecycle action in one request, so scripted rollouts no longer need a separate pull-then-restart sequence. Adds POST /deployments/:name/compose/mount that wires a deployment- directory path into one compose service as a bind mount, with traversal protection on the source and absolute-path enforcement on the target. The mount handler also honors an optional SELinux relabel flag for hosts where SELinux is enforcing, so the resulting bind mount stays readable inside the container on RHEL-family distributions. Lets deployment creation request a specific container image and exposes basic validation on it, so the custom-compose path is no longer hardcoded to nginx:alpine.
nfebe
force-pushed
the
feat/deployment-endpoints
branch
from
890498d to
981e9d7
Compare
| return "", fmt.Errorf("source_path is invalid") | ||
| } | ||
|
|
||
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") |
There was a problem hiding this comment.
The normalization logic for sourcePath is complex and slightly redundant with the subsequent path.Clean call. It can be simplified by relying on path.Join to ensure a consistent ./ prefix after cleaning.
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") | |
| sourcePath = strings.ReplaceAll(sourcePath, "\\", "/") | |
| if !strings.HasPrefix(sourcePath, "/") { | |
| sourcePath = "/" + sourcePath | |
| } | |
| cleaned := path.Clean(sourcePath) | |
| if cleaned == "/" { | |
| return ".", nil | |
| } | |
| cleaned = "." + cleaned |
1 participant
Adds POST /deployments/:name/deploy that bundles image pull and the requested lifecycle action in one request, so scripted rollouts no longer need a separate pull-then-restart sequence.
Adds POST /deployments/:name/compose/mount that wires a deployment-directory path into one compose service as a bind mount, with traversal protection on the source and absolute-path enforcement on the target. The mount handler also honors an optional SELinux relabel flag for RHEL-family hosts.
Lets deployment creation request a specific container image and exposes basic validation on it, so the custom-compose path is no longer hardcoded to nginx:alpine.
Closes #21