diff --git a/go/apps/ctrl/services/build/backend/depot/create_build.go b/go/apps/ctrl/services/build/backend/depot/create_build.go index ad5559a683..2e22321078 100644 --- a/go/apps/ctrl/services/build/backend/depot/create_build.go +++ b/go/apps/ctrl/services/build/backend/depot/create_build.go @@ -19,6 +19,7 @@ import ( corev1 "buf.build/gen/go/depot/api/protocolbuffers/go/depot/core/v1" cliv1 "github.com/depot/depot-go/proto/depot/cli/v1" + builderrors "github.com/unkeyed/unkey/go/apps/ctrl/services/build/errors" ctrlv1 "github.com/unkeyed/unkey/go/gen/proto/ctrl/v1" "github.com/unkeyed/unkey/go/pkg/assert" "github.com/unkeyed/unkey/go/pkg/db" @@ -203,8 +204,7 @@ func (s *Depot) CreateBuild( "build_id", buildResp.ID, "depot_project_id", depotProjectID, "unkey_project_id", unkeyProjectID) - return nil, connect.NewError(connect.CodeInternal, - fmt.Errorf("build failed: %w", buildErr)) + return nil, builderrors.ClassifyBuildError(buildErr, dockerfilePath) } s.logger.Info("Build completed successfully", diff --git a/go/apps/ctrl/services/build/backend/depot/create_build_test.go b/go/apps/ctrl/services/build/backend/depot/create_build_test.go new file mode 100644 index 0000000000..5f75bac49a --- /dev/null +++ b/go/apps/ctrl/services/build/backend/depot/create_build_test.go @@ -0,0 +1,111 @@ +package depot + +import ( + "errors" + "strings" + "testing" + + "connectrpc.com/connect" + builderrors "github.com/unkeyed/unkey/go/apps/ctrl/services/build/errors" +) + +func TestClassifyBuildError(t *testing.T) { + tests := []struct { + name string + buildError error + dockerfilePath string + expectedCode connect.Code + shouldContain []string + }{ + { + name: "dockerfile not found error - buildkit", + buildError: errors.New("failed to solve: failed to read dockerfile: open Dockerfile: no such file or directory"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInvalidArgument, + shouldContain: []string{ + "dockerfile not found", + "Dockerfile", + "does not exist", + }, + }, + { + name: "dockerfile not found with frontend error", + buildError: errors.New("failed to solve with frontend dockerfile.v0: failed to read dockerfile: open Dockerfile.prod: no such file or directory"), + dockerfilePath: "Dockerfile.prod", + expectedCode: connect.CodeInvalidArgument, + shouldContain: []string{ + "dockerfile not found", + "Dockerfile.prod", + }, + }, + { + name: "custom dockerfile path not found", + buildError: errors.New("no such file or directory: docker/Dockerfile"), + dockerfilePath: "docker/Dockerfile", + expectedCode: connect.CodeInvalidArgument, + shouldContain: []string{ + "dockerfile not found", + "docker/Dockerfile", + }, + }, + { + name: "permission denied error", + buildError: errors.New("permission denied: cannot read dockerfile"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodePermissionDenied, + shouldContain: []string{ + "permission denied", + }, + }, + { + name: "generic build error", + buildError: errors.New("build failed: image pull failed"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInternal, + shouldContain: []string{ + "build failed", + }, + }, + { + name: "network error", + buildError: errors.New("failed to fetch base image: network timeout"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInternal, + shouldContain: []string{ + "build failed", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := builderrors.ClassifyBuildError(tt.buildError, tt.dockerfilePath) + + // Check error is not nil + if err == nil { + t.Error("expected error, got nil") + return + } + + // Check it's a connect error + connectErr, ok := err.(*connect.Error) + if !ok { + t.Errorf("expected *connect.Error, got %T", err) + return + } + + // Check error code + if connectErr.Code() != tt.expectedCode { + t.Errorf("expected code %v, got %v", tt.expectedCode, connectErr.Code()) + } + + // Check error message contains expected strings + errorMsg := connectErr.Message() + for _, contain := range tt.shouldContain { + if !strings.Contains(errorMsg, contain) { + t.Errorf("error message should contain %q, got: %s", contain, errorMsg) + } + } + }) + } +} diff --git a/go/apps/ctrl/services/build/backend/docker/create_build.go b/go/apps/ctrl/services/build/backend/docker/create_build.go index f7bdf0160f..91e8cb0d36 100644 --- a/go/apps/ctrl/services/build/backend/docker/create_build.go +++ b/go/apps/ctrl/services/build/backend/docker/create_build.go @@ -11,6 +11,7 @@ import ( "connectrpc.com/connect" "github.com/docker/docker/api/types/build" "github.com/docker/docker/client" + builderrors "github.com/unkeyed/unkey/go/apps/ctrl/services/build/errors" ctrlv1 "github.com/unkeyed/unkey/go/gen/proto/ctrl/v1" "github.com/unkeyed/unkey/go/pkg/assert" "github.com/unkeyed/unkey/go/pkg/uid" @@ -142,7 +143,7 @@ func (d *Docker) CreateBuild( } if buildError != nil { - return nil, connect.NewError(connect.CodeInternal, buildError) + return nil, builderrors.ClassifyBuildError(buildError, dockerfilePath) } buildID := uid.New(uid.BuildPrefix) diff --git a/go/apps/ctrl/services/build/backend/docker/create_build_test.go b/go/apps/ctrl/services/build/backend/docker/create_build_test.go new file mode 100644 index 0000000000..deda11d8d5 --- /dev/null +++ b/go/apps/ctrl/services/build/backend/docker/create_build_test.go @@ -0,0 +1,101 @@ +package docker + +import ( + "errors" + "strings" + "testing" + + "connectrpc.com/connect" + builderrors "github.com/unkeyed/unkey/go/apps/ctrl/services/build/errors" +) + +func TestClassifyBuildError(t *testing.T) { + tests := []struct { + name string + buildError error + dockerfilePath string + expectedCode connect.Code + shouldContain []string + }{ + { + name: "dockerfile not found error", + buildError: errors.New("failed to solve with frontend dockerfile.v0: failed to read dockerfile: open Dockerfile: no such file or directory"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInvalidArgument, + shouldContain: []string{ + "dockerfile not found", + "Dockerfile", + "does not exist", + }, + }, + { + name: "custom dockerfile not found", + buildError: errors.New("failed to read dockerfile: open Dockerfile.prod: no such file or directory"), + dockerfilePath: "Dockerfile.prod", + expectedCode: connect.CodeInvalidArgument, + shouldContain: []string{ + "dockerfile not found", + "Dockerfile.prod", + }, + }, + { + name: "permission denied error", + buildError: errors.New("permission denied: cannot access build context"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodePermissionDenied, + shouldContain: []string{ + "permission denied", + }, + }, + { + name: "generic build error", + buildError: errors.New("build failed: unknown error occurred"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInternal, + shouldContain: []string{ + "build failed", + }, + }, + { + name: "syntax error in dockerfile", + buildError: errors.New("failed to parse dockerfile: invalid instruction"), + dockerfilePath: "Dockerfile", + expectedCode: connect.CodeInternal, + shouldContain: []string{ + "build failed", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := builderrors.ClassifyBuildError(tt.buildError, tt.dockerfilePath) + + // Check error is not nil + if err == nil { + t.Error("expected error, got nil") + return + } + + // Check it's a connect error + connectErr, ok := err.(*connect.Error) + if !ok { + t.Errorf("expected *connect.Error, got %T", err) + return + } + + // Check error code + if connectErr.Code() != tt.expectedCode { + t.Errorf("expected code %v, got %v", tt.expectedCode, connectErr.Code()) + } + + // Check error message contains expected strings + errorMsg := connectErr.Message() + for _, contain := range tt.shouldContain { + if !strings.Contains(errorMsg, contain) { + t.Errorf("error message should contain %q, got: %s", contain, errorMsg) + } + } + }) + } +} diff --git a/go/apps/ctrl/services/build/errors/classify.go b/go/apps/ctrl/services/build/errors/classify.go new file mode 100644 index 0000000000..f8b83b0abb --- /dev/null +++ b/go/apps/ctrl/services/build/errors/classify.go @@ -0,0 +1,32 @@ +package errors + +import ( + "fmt" + "strings" + + "connectrpc.com/connect" +) + +// ClassifyBuildError analyzes build errors and returns appropriate error codes and messages +func ClassifyBuildError(buildError error, dockerfilePath string) error { + errorMsg := buildError.Error() + + // Check for Dockerfile-related errors + if strings.Contains(errorMsg, "failed to solve with frontend dockerfile.v0") || + strings.Contains(errorMsg, "failed to read dockerfile") || + strings.Contains(errorMsg, "failed to solve: failed to read dockerfile") || + strings.Contains(errorMsg, "no such file or directory") && strings.Contains(errorMsg, dockerfilePath) { + return connect.NewError(connect.CodeInvalidArgument, + fmt.Errorf("dockerfile not found: the file '%s' does not exist in the build context. Please check the dockerfile path and ensure it exists", dockerfilePath)) + } + + // Check for permission errors + if strings.Contains(errorMsg, "permission denied") { + return connect.NewError(connect.CodePermissionDenied, + fmt.Errorf("permission denied: unable to access dockerfile or build context. Please check file permissions")) + } + + // Default to internal error for other build failures + return connect.NewError(connect.CodeInternal, + fmt.Errorf("build failed: %w", buildError)) +} diff --git a/go/cmd/deploy/control_plane.go b/go/cmd/deploy/control_plane.go index 45f87c7885..fc7b127512 100644 --- a/go/cmd/deploy/control_plane.go +++ b/go/cmd/deploy/control_plane.go @@ -59,6 +59,16 @@ func NewControlPlaneClient(opts DeployOptions) *ControlPlaneClient { // UploadBuildContext uploads the build context to S3 and returns the context key func (c *ControlPlaneClient) UploadBuildContext(ctx context.Context, contextPath string) (string, error) { + // Validate Dockerfile exists before uploading + dockerfilePath := c.opts.Dockerfile + if dockerfilePath == "" { + dockerfilePath = "Dockerfile" + } + + if err := validateDockerfile(contextPath, dockerfilePath); err != nil { + return "", err + } + uploadReq := connect.NewRequest(&ctrlv1.GenerateUploadURLRequest{ UnkeyProjectId: c.opts.ProjectID, }) @@ -133,6 +143,29 @@ func uploadToPresignedURL(ctx context.Context, presignedURL, filePath string) er return nil } +// validateDockerfile checks if the Dockerfile exists in the context path +func validateDockerfile(contextPath, dockerfilePath string) error { + absContextPath, err := filepath.Abs(contextPath) + if err != nil { + return fmt.Errorf("failed to get absolute context path: %w", err) + } + + fullDockerfilePath := filepath.Join(absContextPath, dockerfilePath) + info, err := os.Stat(fullDockerfilePath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("dockerfile not found at '%s'\n\nPlease ensure:\n 1. The Dockerfile exists in your build context directory\n 2. The --dockerfile flag points to the correct path (default: 'Dockerfile')\n 3. The --context flag points to the correct directory (default: '.')", fullDockerfilePath) + } + return fmt.Errorf("failed to check dockerfile: %w", err) + } + + if info.IsDir() { + return fmt.Errorf("dockerfile path '%s' is a directory, not a file", fullDockerfilePath) + } + + return nil +} + // createContextTar creates a tar.gz from the given directory path // and returns the absolute path to the created tar file func createContextTar(contextPath string) (string, error) { diff --git a/go/cmd/deploy/control_plane_test.go b/go/cmd/deploy/control_plane_test.go new file mode 100644 index 0000000000..7d80ffd8c4 --- /dev/null +++ b/go/cmd/deploy/control_plane_test.go @@ -0,0 +1,205 @@ +package deploy + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestValidateDockerfile(t *testing.T) { + // Create temporary test directories + tempDir := t.TempDir() + + tests := []struct { + name string + setup func() (contextPath, dockerfilePath string) + expectedError string + shouldContain []string + shouldNotError bool + }{ + { + name: "valid dockerfile exists", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "valid") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + dockerfilePath := filepath.Join(dir, "Dockerfile") + if err := os.WriteFile(dockerfilePath, []byte("FROM alpine\n"), 0644); err != nil { + t.Fatal(err) + } + return dir, "Dockerfile" + }, + shouldNotError: true, + }, + { + name: "missing dockerfile with default name", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "missing") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + return dir, "Dockerfile" + }, + shouldContain: []string{ + "dockerfile not found", + "Please ensure:", + "Dockerfile exists in your build context directory", + "--dockerfile flag", + "--context flag", + }, + }, + { + name: "missing custom dockerfile", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "missing-custom") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + return dir, "Dockerfile.prod" + }, + shouldContain: []string{ + "dockerfile not found", + "Dockerfile.prod", + }, + }, + { + name: "dockerfile path is a directory", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "dir-instead") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + dockerfileDir := filepath.Join(dir, "Dockerfile") + if err := os.MkdirAll(dockerfileDir, 0755); err != nil { + t.Fatal(err) + } + return dir, "Dockerfile" + }, + shouldContain: []string{ + "is a directory, not a file", + }, + }, + { + name: "valid custom dockerfile", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "custom") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + dockerfilePath := filepath.Join(dir, "Dockerfile.custom") + if err := os.WriteFile(dockerfilePath, []byte("FROM alpine\n"), 0644); err != nil { + t.Fatal(err) + } + return dir, "Dockerfile.custom" + }, + shouldNotError: true, + }, + { + name: "dockerfile in subdirectory", + setup: func() (string, string) { + dir := filepath.Join(tempDir, "subdir") + if err := os.MkdirAll(filepath.Join(dir, "docker"), 0755); err != nil { + t.Fatal(err) + } + dockerfilePath := filepath.Join(dir, "docker", "Dockerfile") + if err := os.WriteFile(dockerfilePath, []byte("FROM alpine\n"), 0644); err != nil { + t.Fatal(err) + } + return dir, "docker/Dockerfile" + }, + shouldNotError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + contextPath, dockerfilePath := tt.setup() + err := validateDockerfile(contextPath, dockerfilePath) + + if tt.shouldNotError { + if err != nil { + t.Errorf("expected no error, got: %v", err) + } + return + } + + if err == nil { + t.Error("expected error, got nil") + return + } + + errorMsg := err.Error() + for _, contain := range tt.shouldContain { + if !strings.Contains(errorMsg, contain) { + t.Errorf("error message should contain %q, got: %s", contain, errorMsg) + } + } + }) + } +} + +func TestValidateDockerfileWithRealPaths(t *testing.T) { + // Test with the actual test directories we created + tests := []struct { + name string + contextPath string + dockerfilePath string + shouldError bool + errorContains string + }{ + { + name: "missing dockerfile directory", + contextPath: "/tmp/test-dockerfile-validation/missing-dockerfile", + dockerfilePath: "Dockerfile", + shouldError: true, + errorContains: "dockerfile not found", + }, + { + name: "existing dockerfile directory", + contextPath: "/tmp/test-dockerfile-validation/with-dockerfile", + dockerfilePath: "Dockerfile", + shouldError: false, + }, + { + name: "custom dockerfile wrong path", + contextPath: "/tmp/test-dockerfile-validation/custom-dockerfile", + dockerfilePath: "Dockerfile", + shouldError: true, + errorContains: "dockerfile not found", + }, + { + name: "custom dockerfile correct path", + contextPath: "/tmp/test-dockerfile-validation/custom-dockerfile", + dockerfilePath: "Dockerfile.custom", + shouldError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Skip if the test directory doesn't exist + if _, err := os.Stat(tt.contextPath); os.IsNotExist(err) { + t.Skipf("test directory %s does not exist", tt.contextPath) + return + } + + err := validateDockerfile(tt.contextPath, tt.dockerfilePath) + + if tt.shouldError { + if err == nil { + t.Error("expected error, got nil") + return + } + if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("error should contain %q, got: %s", tt.errorContains, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error, got: %v", err) + } + } + }) + } +}