Skip to content

Commit

Permalink
refactor: improve code quality and fix linting issues
Browse files Browse the repository at this point in the history
  • Loading branch information
bnema committed Nov 18, 2024
1 parent b146b1d commit 67afead
Show file tree
Hide file tree
Showing 15 changed files with 316 additions and 294 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ linters:
- misspell
- gosec
- unconvert
- deadcode
- gocyclo

linters-settings:
Expand Down
39 changes: 12 additions & 27 deletions internal/cli/cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,12 @@ func deployImage(a *cli.App, reader io.Reader, imageName, port, targetDomain str
return fmt.Errorf("conflict check failed: %w", err)
}

// Shorten the ContainerID to 12 characters
shortID := conflictResp.ContainerID[:12]
var shortID string
if len(conflictResp.ContainerID) >= 12 {
shortID = conflictResp.ContainerID[:12]
} else {
shortID = conflictResp.ContainerID // Use full ID if less than 12 chars
}

// If there's a conflict, handle it before proceeding
if !conflictResp.Success && conflictResp.ContainerID != "" {
Expand Down Expand Up @@ -141,7 +145,9 @@ func deployImage(a *cli.App, reader io.Reader, imageName, port, targetDomain str
if deployResponse.ContainerID != "" {
log.Warn("Container already exists")
// Retry the deployment after handling the conflict
imageReader.Seek(0, 0) // Reset reader to beginning
if _, err := imageReader.Seek(0, 0); err != nil {
return fmt.Errorf("failed to reset image reader: %w", err)
}
resp, err = chunkedClient.SendFileAsChunks(ctx, "/deploy", headers, imageReader, size, imageName)
if err != nil {
return fmt.Errorf("deployment failed after container removal: %w", err)
Expand Down Expand Up @@ -207,7 +213,9 @@ func validateInputs(imageName, port, targetDomain string) error {
return err
}

handler.EnsureImageTag(&imageName)
if err := handler.EnsureImageTag(&imageName); err != nil {
return fmt.Errorf("failed to ensure image tag: %w", err)
}

if port == "" {
return fmt.Errorf("port is required")
Expand All @@ -216,29 +224,6 @@ func validateInputs(imageName, port, targetDomain string) error {
return handler.ValidateTargetDomain(targetDomain)
}

func handleDeployError(a *cli.App, err error, imageName, port, targetDomain string, reader io.Reader) error {
var deployErr *common.DeploymentError
if errors.As(err, &deployErr) {
var deployResponse common.DeployResponse
var conflictResp common.ConflictCheckResponse
if jsonErr := json.Unmarshal([]byte(deployErr.RawResponse), &deployResponse); jsonErr == nil {
if deployErr.StatusCode == http.StatusConflict && deployResponse.ContainerID != "" {
if err := handler.HandleExistingContainer(a, &conflictResp); err != nil {
if strings.Contains(err.Error(), "cancelled by user") {
log.Warn("Deployment cancelled by user")
return nil
}
log.Error("Failed to handle existing container", "error", err)
return err
}
// Retry deployment
return deployImage(a, reader, imageName, port, targetDomain)
}
}
}
return err
}

func checkDeployConflict(a *cli.App, targetDomain string, port string) (*common.ConflictCheckResponse, error) {

// Create request to check-conflict endpoint with both domain and port parameters
Expand Down
37 changes: 15 additions & 22 deletions internal/cli/handler/chunked_transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,13 @@ type UploadProgress struct {

// ProgressDisplay manages the upload progress display
type ProgressDisplay struct {
mu sync.Mutex
model *uploadModel
program *tea.Program
updateChan chan UploadProgress
done chan struct{}
interrupted bool
paused bool
pauseChan chan bool
mu sync.Mutex
model *uploadModel
program *tea.Program
updateChan chan UploadProgress
done chan struct{}
paused bool
pauseChan chan bool
}

type uploadModel struct {
Expand Down Expand Up @@ -80,7 +79,7 @@ func (pd *ProgressDisplay) Start() {

// Run the UI in a goroutine
go func() {
if err := pd.program.Start(); err != nil {
if _, err := pd.program.Run(); err != nil {
fmt.Printf("Error running progress display: %v\n", err)
}
}()
Expand Down Expand Up @@ -199,14 +198,13 @@ func (m *uploadModel) View() string {
type ChunkMetadata = common.ChunkMetadata

type ChunkedClient struct {
client *http.Client
baseURL string
token string
chunkSize int64
app *cli.App
progress *ProgressDisplay
cancelFunc context.CancelFunc
conflictAction chan common.ConflictAction
client *http.Client
baseURL string
token string
chunkSize int64
app *cli.App
progress *ProgressDisplay
cancelFunc context.CancelFunc
}

type transferStore struct {
Expand Down Expand Up @@ -562,11 +560,6 @@ func verifyChunkIntegrity(chunk []byte, metadata common.ChunkMetadata) error {
return nil
}

// isAuthError checks if a response is an authentication error
func isAuthError(resp *Response) bool {
return resp != nil && resp.StatusCode == http.StatusUnauthorized
}

// cleanupClientTransfer cleans up a client transfer
func cleanupClientTransfer(transferID string) {
clientChunkStore.mu.Lock()
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/handler/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func createRequest(apiUrl, endpoint, method string, rp *common.RequestPayload, t
}

// Set the authorization header for all request types
req.Header.Set("Authorization", "Bearer "+token)
setAuthRequestHeaders(req, token)

return req, nil
}
69 changes: 65 additions & 4 deletions internal/common/inputs.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,72 @@
package common

import "fmt"
import (
"bufio"
"fmt"
"log"
"os"
"strings"
)

// ReadUserInput reads a string input from the user with a prompt.
// It handles multi-word inputs and properly checks for errors.
func ReadUserInput(prompt string) string {
fmt.Println(prompt)
var input string
fmt.Scanln(&input)
fmt.Print(prompt + " ")

// Use bufio.NewReader for better input handling
reader := bufio.NewReader(os.Stdin)

// ReadString reads until newline, handling spaces in input
input, err := reader.ReadString('\n')
if err != nil {
log.Printf("Error reading input: %v", err)
return ""
}

// Trim spaces and newline characters
return strings.TrimSpace(input)
}

// ReadUserInputWithValidation reads user input and validates it using a provided function.
// It keeps prompting until valid input is received.
func ReadUserInputWithValidation(prompt string, validate func(string) bool) string {
for {
input := ReadUserInput(prompt)
if validate(input) {
return input
}
fmt.Println("Invalid input. Please try again.")
}
}

// ReadUserInputWithDefault reads user input with a default value if input is empty.
func ReadUserInputWithDefault(prompt string, defaultValue string) string {
fmt.Printf("%s (default: %s) ", prompt, defaultValue)

reader := bufio.NewReader(os.Stdin)
input, err := reader.ReadString('\n')
if err != nil {
log.Printf("Error reading input: %v", err)
return defaultValue
}

input = strings.TrimSpace(input)
if input == "" {
return defaultValue
}
return input
}

// ReadConfirmation reads a yes/no confirmation from the user.
func ReadConfirmation(prompt string) bool {
input := ReadUserInputWithValidation(
prompt+" (y/n)",
func(s string) bool {
s = strings.ToLower(s)
return s == "y" || s == "n" || s == "yes" || s == "no"
},
)

input = strings.ToLower(input)
return input == "y" || input == "yes"
}
2 changes: 1 addition & 1 deletion internal/httpserve/handlers/endpoint_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func PostDeploy(c echo.Context, a *server.App) error {
})
}

_, err = saveAndImportDeployImage(c, a, payload)
_, err = saveAndImportImage(c, a, payload)
if err != nil {
// Debug
log.Error("Failed to save and import image", "error", err)
Expand Down
81 changes: 29 additions & 52 deletions internal/httpserve/handlers/endpoint_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"net/http"
"os"
"regexp"
"sync"
"time"
Expand Down Expand Up @@ -132,66 +131,59 @@ func validateAndPrepareDeployPayload(c echo.Context) (*common.DeployPayload, err
return payload, nil
}

// saveAndImportPushImage saves the image to the storage and imports it to the Docker engine, updating the payload with the image ID and returning the path to the saved image
func saveAndImportPushImage(c echo.Context, a *server.App, payload *common.PushPayload) (string, error) {
imageReader := c.Request().Body
defer imageReader.Close()

imageFileName := sanitizeImageFileName(payload.ImageName)

imagePath, err := store.SaveImageToStorage(&a.Config, imageFileName, imageReader)
if err != nil {
return "", fmt.Errorf("failed to save image: %v", err)
}

imageID, err := docker.ImportImageToEngine(imagePath)
if err != nil {
store.RemoveFromStorage(imagePath)
return "", fmt.Errorf("failed to import image: %v", err)
// ssaveAndImportImage saves and imports an image from the request body
func saveAndImportImage(c echo.Context, a *server.App, payload interface{}) (string, error) {
var imageName, imageID string

// Determine payload type and extract image name
switch p := payload.(type) {
case *common.PushPayload:
imageName = p.ImageName
defer func() { p.ImageID = imageID }()
case *common.DeployPayload:
imageName = p.ImageName
defer func() { p.ImageID = imageID }()
default:
return "", fmt.Errorf("unsupported payload type: %T", payload)
}

if imageID == "" {
return "", errors.New("imported image ID is empty")
}

err = store.RemoveFromStorage(imagePath)
if err != nil {
return "", fmt.Errorf("failed to remove temporary image file: %v", err)
}

payload.ImageID = imageID
return imagePath, nil
}

// saveAndImportDeployImage saves the image to the storage and imports it to the Docker engine, updating the payload with the image ID and returning the path to the saved image
func saveAndImportDeployImage(c echo.Context, a *server.App, payload *common.DeployPayload) (string, error) {

imageReader := c.Request().Body
defer imageReader.Close()

imageFileName := sanitizeImageFileName(payload.ImageName)
imageFileName := sanitizeImageFileName(imageName)

imagePath, err := store.SaveImageToStorage(&a.Config, imageFileName, imageReader)
if err != nil {
return "", fmt.Errorf("failed to save image: %v", err)
}

imageID, err := docker.ImportImageToEngine(imagePath)
imageID, err = docker.ImportImageToEngine(imagePath)
if err != nil {
store.RemoveFromStorage(imagePath)
if removeErr := store.RemoveFromStorage(imagePath); removeErr != nil {
log.Error("Failed to remove temporary image file after import failure",
"error", removeErr,
"path", imagePath)
}
return "", fmt.Errorf("failed to import image: %v", err)
}

if imageID == "" {
if removeErr := store.RemoveFromStorage(imagePath); removeErr != nil {
log.Error("Failed to remove temporary image file after empty ID",
"error", removeErr,
"path", imagePath)
}
return "", errors.New("imported image ID is empty")
}

err = store.RemoveFromStorage(imagePath)
if err != nil {
log.Error("Failed to remove temporary image file after successful import",
"error", err,
"path", imagePath)
return "", fmt.Errorf("failed to remove temporary image file: %v", err)
}

payload.ImageID = imageID
return imagePath, nil
}

Expand Down Expand Up @@ -357,21 +349,6 @@ func extractContainerID(errorMessage string) string {
return ""
}

// verifyFileContents checks if the file size matches the expected size
func verifyFileContents(file *os.File, expectedSize int64) error {
actualSize, err := file.Seek(0, 2) // Seek to end
if err != nil {
return fmt.Errorf("failed to verify file size: %w", err)
}

if actualSize != expectedSize {
return fmt.Errorf("final file size mismatch: got %d, expected %d", actualSize, expectedSize)
}

_, err = file.Seek(0, 0) // Reset to beginning
return err
}

// storeIDMapping stores a mapping between a short ID and a full ID
func storeIDMapping(shortID, fullID string) {
safelyInteractWithIDMap(Update, shortID, fullID)
Expand Down
Loading

0 comments on commit 67afead

Please sign in to comment.