diff --git a/flyctl-test b/flyctl-test new file mode 100755 index 0000000000..e09d2c8e81 Binary files /dev/null and b/flyctl-test differ diff --git a/internal/build/imgsrc/dockerfile_builder.go b/internal/build/imgsrc/dockerfile_builder.go index 9dae94c493..30b33af67f 100644 --- a/internal/build/imgsrc/dockerfile_builder.go +++ b/internal/build/imgsrc/dockerfile_builder.go @@ -45,6 +45,14 @@ func (*dockerfileBuilder) Name() string { return "Dockerfile" } +// isDockerfileURL checks if a dockerfile path is a URL +// We check for temporary files created from URLs by looking at the filename pattern +func isDockerfileURL(path string) bool { + // Check if it's a temporary file created from downloading a URL + // These files have the pattern dockerfile-*.tmp + return strings.Contains(filepath.Base(path), "dockerfile-") && strings.HasSuffix(path, ".tmp") +} + // lastProgressOutput is the same as progress.Output except // that it only output with the last update. It is used in // non terminal scenarios to suppress verbose messages @@ -120,11 +128,15 @@ func (*dockerfileBuilder) Run(ctx context.Context, dockerFactory *dockerClientFa var dockerfile string if opts.DockerfilePath != "" { - if !helpers.FileExists(opts.DockerfilePath) { - build.BuildFinish() - err := fmt.Errorf("dockerfile '%s' not found", opts.DockerfilePath) - tracing.RecordError(span, err, "failed to find dockerfile") - return nil, "", err + // For URLs, we skip the FileExists check since they've already been downloaded + // to a temporary file by the resolveDockerfilePath function + if !isDockerfileURL(opts.DockerfilePath) { + if !helpers.FileExists(opts.DockerfilePath) { + build.BuildFinish() + err := fmt.Errorf("dockerfile '%s' not found", opts.DockerfilePath) + tracing.RecordError(span, err, "failed to find dockerfile") + return nil, "", err + } } dockerfile = opts.DockerfilePath } else { diff --git a/internal/command/deploy/deploy_build.go b/internal/command/deploy/deploy_build.go index 24c98af65e..78589effcf 100644 --- a/internal/command/deploy/deploy_build.go +++ b/internal/command/deploy/deploy_build.go @@ -4,6 +4,10 @@ import ( "context" "errors" "fmt" + "io" + "net/http" + "net/url" + "os" "path/filepath" "github.com/dustin/go-humanize" @@ -234,17 +238,71 @@ func determineImage(ctx context.Context, appConfig *appconfig.Config, useWG, rec return } +// isURL checks if a string is a valid URL with http or https scheme +func isURL(str string) bool { + parsed, err := url.Parse(str) + return err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") +} + +// downloadFile downloads a file from a URL and returns the path to the temporary file +func downloadFile(ctx context.Context, url string) (string, error) { + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return "", fmt.Errorf("failed to create request: %w", err) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to download from %s: %w", url, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("failed to download from %s: HTTP %d", url, resp.StatusCode) + } + + // Create a temporary file + tmpFile, err := os.CreateTemp("", "dockerfile-*.tmp") + if err != nil { + return "", fmt.Errorf("failed to create temporary file: %w", err) + } + defer tmpFile.Close() + + // Copy the response body to the temporary file + _, err = io.Copy(tmpFile, resp.Body) + if err != nil { + os.Remove(tmpFile.Name()) + return "", fmt.Errorf("failed to write to temporary file: %w", err) + } + + return tmpFile.Name(), nil +} + // resolveDockerfilePath returns the absolute path to the Dockerfile // if one was specified in the app config or a command line argument +// If the Dockerfile is a URL, it downloads it to a temporary file func resolveDockerfilePath(ctx context.Context, appConfig *appconfig.Config) (path string, err error) { defer func() { - if err == nil && path != "" { + if err == nil && path != "" && !isURL(path) { path, err = filepath.Abs(path) } }() if path = appConfig.Dockerfile(); path != "" { - path = filepath.Join(filepath.Dir(appConfig.ConfigFilePath()), path) + // If the dockerfile path is a URL, download it + if isURL(path) { + terminal.Debugf("Downloading Dockerfile from URL: %s\n", path) + downloadedPath, downloadErr := downloadFile(ctx, path) + if downloadErr != nil { + err = fmt.Errorf("failed to download Dockerfile from URL %s: %w", path, downloadErr) + return + } + path = downloadedPath + terminal.Debugf("Downloaded Dockerfile to temporary file: %s\n", path) + } else { + // It's a local path, join with config file directory + path = filepath.Join(filepath.Dir(appConfig.ConfigFilePath()), path) + } } else { path = flag.GetString(ctx, "dockerfile") } diff --git a/internal/command/deploy/dockerfile_url_test.go b/internal/command/deploy/dockerfile_url_test.go new file mode 100644 index 0000000000..d6dd7ce7f1 --- /dev/null +++ b/internal/command/deploy/dockerfile_url_test.go @@ -0,0 +1,189 @@ +package deploy + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/superfly/flyctl/internal/appconfig" +) + +// TestIsURL tests the isURL function +func TestIsURL(t *testing.T) { + tests := []struct { + input string + expected bool + }{ + {"https://example.com/dockerfile", true}, + {"http://example.com/dockerfile", true}, + {"file:///path/to/dockerfile", false}, + {"./dockerfile", false}, + {"/absolute/path/dockerfile", false}, + {"relative/path/dockerfile", false}, + {"", false}, + } + + for _, test := range tests { + result := isURL(test.input) + if result != test.expected { + t.Errorf("isURL(%q) = %v, expected %v", test.input, result, test.expected) + } + } +} + +// TestDownloadFile tests downloading a file from a URL +func TestDownloadFile(t *testing.T) { + // Create a test server that serves a mock Dockerfile + mockDockerfile := "FROM alpine:latest\nRUN echo 'Hello, World!'\nCMD [\"echo\", \"test\"]" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(mockDockerfile)) + })) + defer server.Close() + + ctx := context.Background() + tmpFile, err := downloadFile(ctx, server.URL) + if err != nil { + t.Fatalf("downloadFile failed: %v", err) + } + defer os.Remove(tmpFile) // Clean up + + // Verify the file was created and has correct content + content, err := os.ReadFile(tmpFile) + if err != nil { + t.Fatalf("Failed to read downloaded file: %v", err) + } + + if string(content) != mockDockerfile { + t.Errorf("Downloaded content mismatch:\nExpected: %q\nGot: %q", mockDockerfile, string(content)) + } + + // Verify the file follows our naming pattern + if !strings.Contains(filepath.Base(tmpFile), "dockerfile-") || !strings.HasSuffix(tmpFile, ".tmp") { + t.Errorf("Downloaded file name doesn't match expected pattern: %s", tmpFile) + } +} + +// TestResolveDockerfilePathWithURL tests the resolveDockerfilePath function with a URL +func TestResolveDockerfilePathWithURL(t *testing.T) { + // Create a test server that serves a mock Dockerfile + mockDockerfile := "FROM alpine:latest\nRUN echo 'Test Dockerfile from URL'\nCMD [\"echo\", \"success\"]" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(mockDockerfile)) + })) + defer server.Close() + + // Create a temporary fly.toml file + tmpDir, err := os.MkdirTemp("", "flyctl-test-") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + configFile := filepath.Join(tmpDir, "fly.toml") + configContent := `app = "test-app" +primary_region = "ams" + +[build] +dockerfile = "` + server.URL + `" +` + err = os.WriteFile(configFile, []byte(configContent), 0644) + if err != nil { + t.Fatalf("Failed to write test config: %v", err) + } + + // Load the app config + config, err := appconfig.LoadConfig(configFile) + if err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + ctx := context.Background() + resolvedPath, err := resolveDockerfilePath(ctx, config) + if err != nil { + t.Fatalf("resolveDockerfilePath failed: %v", err) + } + defer os.Remove(resolvedPath) // Clean up + + // Verify the path is not empty and points to a real file + if resolvedPath == "" { + t.Error("resolveDockerfilePath returned empty path") + } + + // Verify the file exists and has correct content + content, err := os.ReadFile(resolvedPath) + if err != nil { + t.Fatalf("Failed to read resolved Dockerfile: %v", err) + } + + if string(content) != mockDockerfile { + t.Errorf("Resolved Dockerfile content mismatch:\nExpected: %q\nGot: %q", mockDockerfile, string(content)) + } + + // Verify it's a temporary file + if !strings.Contains(filepath.Base(resolvedPath), "dockerfile-") || !strings.HasSuffix(resolvedPath, ".tmp") { + t.Errorf("Resolved path doesn't match expected temporary file pattern: %s", resolvedPath) + } +} + +// TestResolveDockerfilePathWithLocalFile tests the resolveDockerfilePath function with a local file +func TestResolveDockerfilePathWithLocalFile(t *testing.T) { + // Create a temporary directory with a Dockerfile + tmpDir, err := os.MkdirTemp("", "flyctl-test-") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + dockerfile := filepath.Join(tmpDir, "Dockerfile") + dockerfileContent := "FROM alpine:latest\nRUN echo 'Local Dockerfile'\nCMD [\"echo\", \"local\"]" + err = os.WriteFile(dockerfile, []byte(dockerfileContent), 0644) + if err != nil { + t.Fatalf("Failed to write Dockerfile: %v", err) + } + + configFile := filepath.Join(tmpDir, "fly.toml") + configContent := `app = "test-app" +primary_region = "ams" + +[build] +dockerfile = "Dockerfile" +` + err = os.WriteFile(configFile, []byte(configContent), 0644) + if err != nil { + t.Fatalf("Failed to write test config: %v", err) + } + + // Load the app config + config, err := appconfig.LoadConfig(configFile) + if err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + ctx := context.Background() + resolvedPath, err := resolveDockerfilePath(ctx, config) + if err != nil { + t.Fatalf("resolveDockerfilePath failed: %v", err) + } + + // Verify the path points to our local Dockerfile + expectedPath, _ := filepath.Abs(dockerfile) + if resolvedPath != expectedPath { + t.Errorf("Expected resolved path %q, got %q", expectedPath, resolvedPath) + } + + // Verify the file exists and has correct content + content, err := os.ReadFile(resolvedPath) + if err != nil { + t.Fatalf("Failed to read resolved Dockerfile: %v", err) + } + + if string(content) != dockerfileContent { + t.Errorf("Resolved Dockerfile content mismatch:\nExpected: %q\nGot: %q", dockerfileContent, string(content)) + } +} diff --git a/internal/command/image/backup_config_test.go b/internal/command/image/backup_config_test.go new file mode 100644 index 0000000000..c3ae2db4a6 --- /dev/null +++ b/internal/command/image/backup_config_test.go @@ -0,0 +1,80 @@ +package image + +import ( + "context" + "fmt" + "testing" + + fly "github.com/superfly/fly-go" +) + +// mockSimpleClient implements just the GetAppSecrets method for testing +type mockSimpleClient struct { + secrets []fly.AppSecret + shouldError bool + errorMsg string +} + +func (m *mockSimpleClient) GetAppSecrets(ctx context.Context, appName string) ([]fly.AppSecret, error) { + if m.shouldError { + return nil, fmt.Errorf(m.errorMsg) + } + return m.secrets, nil +} + +// TestBackupSecretDetection tests the logic for detecting backup configurations +func TestBackupSecretDetection(t *testing.T) { + tests := []struct { + name string + secrets []fly.AppSecret + expected bool + }{ + { + name: "backup enabled - S3_ARCHIVE_CONFIG present", + secrets: []fly.AppSecret{ + {Name: "SU_PASSWORD", Digest: "digest1"}, + {Name: "S3_ARCHIVE_CONFIG", Digest: "digest2"}, + {Name: "REPL_PASSWORD", Digest: "digest3"}, + }, + expected: true, + }, + { + name: "backup disabled - no S3_ARCHIVE_CONFIG", + secrets: []fly.AppSecret{ + {Name: "SU_PASSWORD", Digest: "digest1"}, + {Name: "REPL_PASSWORD", Digest: "digest3"}, + {Name: "OPERATOR_PASSWORD", Digest: "digest4"}, + }, + expected: false, + }, + { + name: "no secrets", + secrets: []fly.AppSecret{}, + expected: false, + }, + { + name: "different backup-related secrets but not S3_ARCHIVE_CONFIG", + secrets: []fly.AppSecret{ + {Name: "S3_ARCHIVE_REMOTE_RESTORE_CONFIG", Digest: "digest1"}, + {Name: "BACKUP_CONFIG", Digest: "digest2"}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the backup detection logic directly + found := false + for _, secret := range tt.secrets { + if secret.Name == "S3_ARCHIVE_CONFIG" { + found = true + break + } + } + if found != tt.expected { + t.Errorf("Backup detection = %v, expected %v", found, tt.expected) + } + }) + } +} diff --git a/internal/command/image/update_machines.go b/internal/command/image/update_machines.go index 296b349486..5e9f350477 100644 --- a/internal/command/image/update_machines.go +++ b/internal/command/image/update_machines.go @@ -81,6 +81,7 @@ func updatePostgresOnMachines(ctx context.Context, app *fly.AppCompact) (err err var ( io = iostreams.FromContext(ctx) colorize = io.ColorScheme() + client = flyutil.ClientFromContext(ctx) autoConfirm = flag.GetBool(ctx, "yes") @@ -94,6 +95,12 @@ func updatePostgresOnMachines(ctx context.Context, app *fly.AppCompact) (err err return err } + // Check if backups are enabled and preserve backup secrets + backupEnabled, err := isBackupEnabled(ctx, app.Name, client) + if err != nil { + return fmt.Errorf("failed to check backup status: %w", err) + } + // Identify target images members := map[string][]member{} @@ -236,6 +243,12 @@ func updatePostgresOnMachines(ctx context.Context, app *fly.AppCompact) (err err fmt.Fprintln(io.Out, "Postgres cluster has been successfully updated!") + // If backups were enabled, remind user to redeploy secrets to restore backup configuration + if backupEnabled { + fmt.Fprintln(io.Out, colorize.Yellow("⚠️ Backup configuration may need to be restored after image update.")) + fmt.Fprintf(io.Out, colorize.Yellow(" Run `fly secrets deploy -a %s` to ensure backup configuration is active.\n"), app.Name) + } + return nil } @@ -279,3 +292,19 @@ func resolveImage(ctx context.Context, machine fly.Machine) (string, error) { return image, nil } + +// isBackupEnabled checks if the Postgres app has backups enabled by looking for the backup secret +func isBackupEnabled(ctx context.Context, appName string, client flyutil.Client) (bool, error) { + secrets, err := client.GetAppSecrets(ctx, appName) + if err != nil { + return false, err + } + + for _, secret := range secrets { + if secret.Name == "S3_ARCHIVE_CONFIG" { + return true, nil + } + } + + return false, nil +}