diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go index 09fe06b..55fb24b 100644 --- a/pkg/manager/operations.go +++ b/pkg/manager/operations.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "llamactl/pkg/instance" - "llamactl/pkg/validation" "log" ) @@ -58,12 +57,7 @@ func (im *instanceManager) CreateInstance(name string, options *instance.Options return nil, fmt.Errorf("instance options cannot be nil") } - name, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - - err = options.BackendOptions.ValidateInstanceOptions() + err := options.BackendOptions.ValidateInstanceOptions() if err != nil { return nil, err } diff --git a/pkg/manager/remote.go b/pkg/manager/remote.go index ab021f2..292b39f 100644 --- a/pkg/manager/remote.go +++ b/pkg/manager/remote.go @@ -8,7 +8,6 @@ import ( "io" "llamactl/pkg/config" "llamactl/pkg/instance" - "llamactl/pkg/validation" "net/http" "net/url" "sync" @@ -139,16 +138,9 @@ func parseRemoteResponse(resp *http.Response, result any) error { // --- Remote CRUD operations --- -// CreateInstance creates a new instance on a remote node. +// createInstance creates a new instance on a remote node. func (rm *remoteManager) createInstance(ctx context.Context, node *config.NodeConfig, name string, opts *instance.Options) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/", apiBasePath, escapedName) @@ -165,16 +157,10 @@ func (rm *remoteManager) createInstance(ctx context.Context, node *config.NodeCo return &inst, nil } -// GetInstance retrieves an instance by name from a remote node. +// getInstance retrieves an instance by name from a remote node. func (rm *remoteManager) getInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/", apiBasePath, escapedName) resp, err := rm.makeRemoteRequest(ctx, node, "GET", path, nil) @@ -190,16 +176,10 @@ func (rm *remoteManager) getInstance(ctx context.Context, node *config.NodeConfi return &inst, nil } -// UpdateInstance updates an existing instance on a remote node. +// updateInstance updates an existing instance on a remote node. func (rm *remoteManager) updateInstance(ctx context.Context, node *config.NodeConfig, name string, opts *instance.Options) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/", apiBasePath, escapedName) @@ -216,16 +196,10 @@ func (rm *remoteManager) updateInstance(ctx context.Context, node *config.NodeCo return &inst, nil } -// DeleteInstance deletes an instance from a remote node. +// deleteInstance deletes an instance from a remote node. func (rm *remoteManager) deleteInstance(ctx context.Context, node *config.NodeConfig, name string) error { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/", apiBasePath, escapedName) resp, err := rm.makeRemoteRequest(ctx, node, "DELETE", path, nil) @@ -236,16 +210,10 @@ func (rm *remoteManager) deleteInstance(ctx context.Context, node *config.NodeCo return parseRemoteResponse(resp, nil) } -// StartInstance starts an instance on a remote node. +// startInstance starts an instance on a remote node. func (rm *remoteManager) startInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/start", apiBasePath, escapedName) resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil) @@ -261,16 +229,10 @@ func (rm *remoteManager) startInstance(ctx context.Context, node *config.NodeCon return &inst, nil } -// StopInstance stops an instance on a remote node. +// stopInstance stops an instance on a remote node. func (rm *remoteManager) stopInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/stop", apiBasePath, escapedName) resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil) @@ -286,16 +248,9 @@ func (rm *remoteManager) stopInstance(ctx context.Context, node *config.NodeConf return &inst, nil } -// RestartInstance restarts an instance on a remote node. +// restartInstance restarts an instance on a remote node. func (rm *remoteManager) restartInstance(ctx context.Context, node *config.NodeConfig, name string) (*instance.Instance, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/restart", apiBasePath, escapedName) resp, err := rm.makeRemoteRequest(ctx, node, "POST", path, nil) @@ -311,16 +266,10 @@ func (rm *remoteManager) restartInstance(ctx context.Context, node *config.NodeC return &inst, nil } -// GetInstanceLogs retrieves logs for an instance from a remote node. +// getInstanceLogs retrieves logs for an instance from a remote node. func (rm *remoteManager) getInstanceLogs(ctx context.Context, node *config.NodeConfig, name string, numLines int) (string, error) { - // Validate instance name to prevent injection attacks - validatedName, err := validation.ValidateInstanceName(name) - if err != nil { - return "", err - } - // URL-encode the validated instance name to safely include it in the URL path - escapedName := url.PathEscape(validatedName) + escapedName := url.PathEscape(name) path := fmt.Sprintf("%s%s/logs?lines=%d", apiBasePath, escapedName, numLines) resp, err := rm.makeRemoteRequest(ctx, node, "GET", path, nil) diff --git a/pkg/server/handlers_instances.go b/pkg/server/handlers_instances.go index 9e41190..dfe9971 100644 --- a/pkg/server/handlers_instances.go +++ b/pkg/server/handlers_instances.go @@ -5,6 +5,7 @@ import ( "fmt" "llamactl/pkg/instance" "llamactl/pkg/manager" + "llamactl/pkg/validation" "net/http" "net/http/httputil" "net/url" @@ -55,8 +56,10 @@ func (h *Handler) ListInstances() http.HandlerFunc { func (h *Handler) CreateInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } @@ -66,7 +69,7 @@ func (h *Handler) CreateInstance() http.HandlerFunc { return } - inst, err := h.InstanceManager.CreateInstance(name, &options) + inst, err := h.InstanceManager.CreateInstance(validatedName, &options) if err != nil { http.Error(w, "Failed to create instance: "+err.Error(), http.StatusInternalServerError) return @@ -95,12 +98,14 @@ func (h *Handler) CreateInstance() http.HandlerFunc { func (h *Handler) GetInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - inst, err := h.InstanceManager.GetInstance(name) + inst, err := h.InstanceManager.GetInstance(validatedName) if err != nil { http.Error(w, "Invalid instance: "+err.Error(), http.StatusBadRequest) return @@ -130,8 +135,10 @@ func (h *Handler) GetInstance() http.HandlerFunc { func (h *Handler) UpdateInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } @@ -141,7 +148,7 @@ func (h *Handler) UpdateInstance() http.HandlerFunc { return } - inst, err := h.InstanceManager.UpdateInstance(name, &options) + inst, err := h.InstanceManager.UpdateInstance(validatedName, &options) if err != nil { http.Error(w, "Failed to update instance: "+err.Error(), http.StatusInternalServerError) return @@ -169,12 +176,14 @@ func (h *Handler) UpdateInstance() http.HandlerFunc { func (h *Handler) StartInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - inst, err := h.InstanceManager.StartInstance(name) + inst, err := h.InstanceManager.StartInstance(validatedName) if err != nil { // Check if error is due to maximum running instances limit if _, ok := err.(manager.MaxRunningInstancesError); ok { @@ -208,12 +217,14 @@ func (h *Handler) StartInstance() http.HandlerFunc { func (h *Handler) StopInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - inst, err := h.InstanceManager.StopInstance(name) + inst, err := h.InstanceManager.StopInstance(validatedName) if err != nil { http.Error(w, "Failed to stop instance: "+err.Error(), http.StatusInternalServerError) return @@ -241,12 +252,14 @@ func (h *Handler) StopInstance() http.HandlerFunc { func (h *Handler) RestartInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - inst, err := h.InstanceManager.RestartInstance(name) + inst, err := h.InstanceManager.RestartInstance(validatedName) if err != nil { http.Error(w, "Failed to restart instance: "+err.Error(), http.StatusInternalServerError) return @@ -273,12 +286,14 @@ func (h *Handler) RestartInstance() http.HandlerFunc { func (h *Handler) DeleteInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - if err := h.InstanceManager.DeleteInstance(name); err != nil { + if err := h.InstanceManager.DeleteInstance(validatedName); err != nil { http.Error(w, "Failed to delete instance: "+err.Error(), http.StatusInternalServerError) return } @@ -302,8 +317,10 @@ func (h *Handler) DeleteInstance() http.HandlerFunc { func (h *Handler) GetInstanceLogs() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } @@ -319,7 +336,7 @@ func (h *Handler) GetInstanceLogs() http.HandlerFunc { } // Use the instance manager which handles both local and remote instances - logs, err := h.InstanceManager.GetInstanceLogs(name, numLines) + logs, err := h.InstanceManager.GetInstanceLogs(validatedName, numLines) if err != nil { http.Error(w, "Failed to get logs: "+err.Error(), http.StatusInternalServerError) return @@ -345,12 +362,14 @@ func (h *Handler) GetInstanceLogs() http.HandlerFunc { func (h *Handler) ProxyToInstance() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") - if name == "" { - http.Error(w, "Instance name cannot be empty", http.StatusBadRequest) + + validatedName, err := validation.ValidateInstanceName(name) + if err != nil { + http.Error(w, "Invalid instance name: "+err.Error(), http.StatusBadRequest) return } - inst, err := h.InstanceManager.GetInstance(name) + inst, err := h.InstanceManager.GetInstance(validatedName) if err != nil { http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError) return @@ -358,7 +377,7 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc { // Check if this is a remote instance if inst.IsRemote() { - h.RemoteInstanceProxy(w, r, name, inst) + h.RemoteInstanceProxy(w, r, validatedName, inst) return } @@ -375,7 +394,7 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc { } // Strip the "/api/v1/instances//proxy" prefix from the request URL - prefix := fmt.Sprintf("/api/v1/instances/%s/proxy", name) + prefix := fmt.Sprintf("/api/v1/instances/%s/proxy", validatedName) r.URL.Path = strings.TrimPrefix(r.URL.Path, prefix) // Update the last request time for the instance