Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions cmd/downloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,117 @@ func addTarballToCollectionFromStdin(cmd *cobra.Command, args []string) {
ops.DisplayTarball(tarballDesc)
}

func addTarballToCollectionFromUrl(cmd *cobra.Command, args []string) error {
if len(args) < 1 {
return fmt.Errorf("command 'add-url' requires a URL")
}
tarballUrl := args[0]
if !common.IsUrl(tarballUrl) {
return fmt.Errorf("argument %q does not look like a valid URL (must start with http:// or https://)", tarballUrl)
}

flags := cmd.Flags()
overrideOS, _ := flags.GetString(globals.OSLabel)
overrideArch, _ := flags.GetString(globals.ArchLabel)
overrideFlavor, _ := flags.GetString(globals.FlavorLabel)
overrideVersion, _ := flags.GetString(globals.VersionLabel)
overrideShortVersion, _ := flags.GetString(globals.ShortVersionLabel)
overrideMinimal, _ := flags.GetBool(globals.MinimalLabel)
overwrite, _ := flags.GetBool(globals.OverwriteLabel)
skipVerify, _ := flags.GetBool(globals.SkipVerifyUrlLabel)

// Parse the filename from the URL
tarballDesc, err := downloads.ParseTarballUrlInfo(tarballUrl)
if err != nil {
return fmt.Errorf("error parsing tarball URL: %s\n"+
"Use --version, --OS, --arch, --flavor flags to provide metadata manually", err)
}

// Apply any user overrides
if overrideOS != "" {
tarballDesc.OperatingSystem = overrideOS
}
if overrideArch != "" {
tarballDesc.Arch = overrideArch
}
if overrideFlavor != "" {
tarballDesc.Flavor = overrideFlavor
}
if overrideVersion != "" {
tarballDesc.Version = overrideVersion
}
if overrideShortVersion != "" {
tarballDesc.ShortVersion = overrideShortVersion
}
if overrideMinimal {
tarballDesc.Minimal = true
Comment on lines +641 to +642
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The --minimal override currently can only force Minimal=true. If the filename auto-detect sets Minimal=true, there is no way to override it back to false (even --minimal=false won’t work) because the code only applies the override when overrideMinimal is true. Consider checking whether the flag was explicitly set (Flags().Lookup(...).Changed) and then assigning tarballDesc.Minimal = overrideMinimal.

Suggested change
if overrideMinimal {
tarballDesc.Minimal = true
// Only override the Minimal field if the --minimal flag was explicitly set.
if minimalFlag := cmd.Flags().Lookup("minimal"); minimalFlag != nil && minimalFlag.Changed {
tarballDesc.Minimal = overrideMinimal

Copilot uses AI. Check for mistakes.
}

// Validate required fields
if tarballDesc.Version == "" {
return fmt.Errorf("could not detect version from filename; use --version to specify it")
}
if tarballDesc.ShortVersion == "" {
return fmt.Errorf("could not detect short version from filename; use --short-version to specify it")
}
if tarballDesc.OperatingSystem == "" {
return fmt.Errorf("could not detect OS from filename; use --OS to specify it")
}
if tarballDesc.Arch == "" {
return fmt.Errorf("could not detect architecture from filename; use --arch to specify it")
}
if tarballDesc.Flavor == "" {
return fmt.Errorf("could not detect flavor from filename; use --flavor to specify it")
}

// Verify the URL is accessible
if !skipVerify {
fmt.Printf("Verifying URL accessibility: %s\n", tarballUrl)
size, err := downloads.CheckRemoteUrl(tarballUrl)
if err != nil {
return fmt.Errorf("URL is not accessible: %s\nUse --skip-verify-url to bypass this check", err)
}
tarballDesc.Size = size
if size > 0 {
fmt.Printf("URL is accessible (size: %s)\n", humanize.Bytes(uint64(size)))
} else {
fmt.Printf("URL is accessible (size unknown)\n")
}
}

tarballDesc.Notes = fmt.Sprintf("added with version %s", common.VersionDef)
tarballDesc.DateAdded = time.Now().Format("2006-01-02 15:04")

// Load existing collection and add the new entry
var tarballCollection = downloads.DefaultTarballRegistry

existingTarball, findErr := downloads.FindTarballByName(tarballDesc.Name)
if findErr == nil {
if overwrite {
var newList []downloads.TarballDescription
newList, err = downloads.DeleteTarball(tarballCollection.Tarballs, tarballDesc.Name)
if err != nil {
return fmt.Errorf("error removing existing tarball %s from list: %s", tarballDesc.Name, err)
}
tarballCollection.Tarballs = newList
} else {
ops.DisplayTarball(existingTarball)
fmt.Println()
return fmt.Errorf("tarball %s already in the list; use --overwrite to replace it", tarballDesc.Name)
}
}

tarballCollection.Tarballs = append(tarballCollection.Tarballs, tarballDesc)

err = downloads.WriteTarballFileInfo(tarballCollection)
if err != nil {
return fmt.Errorf("error writing tarball list: %s", err)
}
fmt.Printf("Tarball below added to %s\n", downloads.TarballFileRegistry)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a discrepancy between the implementation and the PR description. The description states that custom tarballs are saved to ~/.dbdeployer/custom_tarballs.json, but the code uses downloads.TarballFileRegistry, which points to tarball-list.json. This means custom entries are mixed into the main registry file, which might lead to them being lost or causing conflicts if the main registry is updated or reset.

ops.DisplayTarball(tarballDesc)
Comment on lines +680 to +706
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says custom URLs are saved to ~/.dbdeployer/custom_tarballs.json and merged with the embedded registry at runtime, but this implementation writes to downloads.TarballFileRegistry (tarball-list.json) via WriteTarballFileInfo and relies on replacing DefaultTarballRegistry on startup. Either update the PR description/command help to match the existing tarball-list behavior, or implement a separate custom registry file + merge logic so users don’t pin themselves to an old full registry snapshot.

Copilot uses AI. Check for mistakes.
return nil
}

func removeTarballFromCollection(cmd *cobra.Command, args []string) {
if len(args) < 1 {
common.Exit(1, "command 'delete' requires a tarball name")
Expand Down Expand Up @@ -753,6 +864,27 @@ var downloadsAddStdinCmd = &cobra.Command{
Run: addTarballToCollectionFromStdin,
}

var downloadsAddUrlCmd = &cobra.Command{
Use: "add-url URL",
Short: "Adds a remote tarball to the list by URL",
Long: `Adds a tarball entry to the local registry using a direct URL.

The filename is parsed to auto-detect version, OS, architecture, flavor, and
whether the tarball is minimal. Use the override flags if auto-detection fails
or produces wrong results.

The URL is validated with an HTTP HEAD request before the entry is saved.
Use --skip-verify-url to bypass this check.
`,
Example: `
$ dbdeployer downloads add-url https://example.com/mysql-8.4.8-linux-glibc2.17-x86_64.tar.xz
$ dbdeployer downloads add-url https://example.com/Percona-Server-8.0.35-27-Linux.x86_64.glibc2.17-minimal.tar.gz
$ dbdeployer downloads add-url https://example.com/mysql-8.4.8-macos15-arm64.tar.gz --OS=darwin --arch=arm64
$ dbdeployer downloads add-url https://example.com/mysql-8.4.8-linux-glibc2.17-x86_64-minimal.tar.xz --overwrite
`,
RunE: addTarballToCollectionFromUrl,
}

var downloadsCmd = &cobra.Command{
Use: "downloads",
Short: "Manages remote tarballs",
Expand Down Expand Up @@ -788,6 +920,7 @@ func init() {
downloadsCmd.AddCommand(downloadsAddStdinCmd)
downloadsCmd.AddCommand(downloadsDeleteCmd)
downloadsCmd.AddCommand(downloadsTreeCmd)
downloadsCmd.AddCommand(downloadsAddUrlCmd)

downloadsListCmd.Flags().BoolP(globals.ShowUrlLabel, "", false, "Show the URL")
downloadsListCmd.Flags().String(globals.FlavorLabel, "", "Which flavor will be listed")
Expand Down Expand Up @@ -837,6 +970,15 @@ func init() {

downloadsAddStdinCmd.Flags().BoolP(globals.OverwriteLabel, "", false, "Overwrite existing entry")

downloadsAddUrlCmd.Flags().String(globals.OSLabel, "", "Override the detected OS (e.g. linux, darwin)")
downloadsAddUrlCmd.Flags().String(globals.ArchLabel, "", "Override the detected architecture (e.g. amd64, arm64)")
downloadsAddUrlCmd.Flags().String(globals.FlavorLabel, "", "Override the detected flavor (e.g. mysql, percona)")
downloadsAddUrlCmd.Flags().String(globals.VersionLabel, "", "Override the detected version (e.g. 8.4.8)")
downloadsAddUrlCmd.Flags().String(globals.ShortVersionLabel, "", "Override the detected short version (e.g. 8.4)")
downloadsAddUrlCmd.Flags().BoolP(globals.MinimalLabel, "", false, "Mark the tarball as minimal")
downloadsAddUrlCmd.Flags().BoolP(globals.OverwriteLabel, "", false, "Overwrite existing entry")
downloadsAddUrlCmd.Flags().BoolP(globals.SkipVerifyUrlLabel, "", false, "Skip URL accessibility check")

downloadsExportCmd.Flags().BoolP(globals.AddEmptyItemLabel, "", false, "Add an empty item to the tarballs list")

downloadsImportCmd.Flags().Int64P(globals.RetriesOnFailureLabel, "", 0, "How many times retry a download if a failure occurs on first try")
Expand Down
89 changes: 89 additions & 0 deletions downloads/remote_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,95 @@ func checkRemoteUrl(remoteUrl string) (int64, error) {
return size, nil
}

// CheckRemoteUrl validates that a URL is accessible and returns its content size.
// It uses an HTTP HEAD request first; if that fails (some servers don't support HEAD),
// it falls back to a GET request.
func CheckRemoteUrl(remoteUrl string) (int64, error) {
// Try HEAD first to avoid downloading the file
// #nosec G107
resp, err := http.Head(remoteUrl)
if err == nil {
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
if resp.StatusCode == http.StatusOK {
var size int64
for key := range resp.Header {
if strings.EqualFold(key, "Content-Length") && len(resp.Header[key]) > 0 {
size, _ = strconv.ParseInt(resp.Header[key][0], 10, 0)
}
}
return size, nil
Comment on lines +581 to +588
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In CheckRemoteUrl, Content-Length extraction iterates headers and uses strconv.ParseInt(..., 10, 0). Two improvements: (1) use resp.ContentLength (already parsed by net/http) or Header.Get("Content-Length") instead of looping, and (2) parse with bitSize 64 to avoid int-size-dependent overflow. This also reduces duplicated logic vs checkRemoteUrl().

Copilot uses AI. Check for mistakes.
}
}
// Fall back to GET if HEAD failed or returned non-200
return checkRemoteUrl(remoteUrl)
Comment on lines +573 to +592
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckRemoteUrl (and the underlying checkRemoteUrl fallback) uses the default http client (http.Head/http.Get) with no timeout, so downloads add-url can hang indefinitely on slow/stalled connections. Consider using an http.Client with a reasonable Timeout (or request context with deadline) and reusing it for both HEAD/GET attempts.

Copilot uses AI. Check for mistakes.
}
Comment on lines +573 to +593
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The CheckRemoteUrl function uses the default HTTP client which has no timeout, potentially causing the CLI to hang on slow or unresponsive servers. Additionally, manually parsing the Content-Length header is unnecessary and error-prone (especially with bitSize set to 0 in ParseInt, which can overflow on 32-bit systems for files > 2GB). It is recommended to use a client with a timeout and leverage resp.ContentLength directly.

func CheckRemoteUrl(remoteUrl string) (int64, error) {
	client := &http.Client{
		Timeout: 10 * time.Second,
	}
	// Try HEAD first to avoid downloading the file
	// #nosec G107
	resp, err := client.Head(remoteUrl)
	if err == nil {
		defer func(Body io.ReadCloser) {
			_ = Body.Close()
		}(resp.Body)
		if resp.StatusCode == http.StatusOK {
			return resp.ContentLength, nil
		}
	}
	// Fall back to GET if HEAD failed or returned non-200
	return checkRemoteUrl(remoteUrl)
}


// ParseTarballUrlInfo parses a tarball URL/filename and returns a partially-filled
// TarballDescription with auto-detected fields. The caller should override any
// fields that were incorrectly detected.
func ParseTarballUrlInfo(tarballUrl string) (TarballDescription, error) {
fileName := common.BaseName(tarballUrl)
if fileName == "" {
return TarballDescription{}, fmt.Errorf("could not determine filename from URL: %s", tarballUrl)
}
Comment on lines +598 to +602
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParseTarballUrlInfo uses common.BaseName(tarballUrl) (filepath.Base) to extract the filename. For URLs with query strings (e.g. signed URLs like .../mysql-8.4.8.tar.xz?token=...) this will include the query and break FindTarballInfo parsing. Consider parsing with net/url and using path.Base(u.Path) (and/or stripping ? / #) before attempting tarball detection.

Copilot uses AI. Check for mistakes.

flavor, version, shortVersion, err := common.FindTarballInfo(fileName)
if err != nil {
return TarballDescription{}, fmt.Errorf("could not parse version from filename %q: %s", fileName, err)
}
Comment on lines +604 to +607
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Returning an error here when FindTarballInfo fails prevents the user from using the override flags (--version, --flavor, etc.) for URLs with non-standard filenames. Since the caller already validates the required fields after applying overrides, this function should be more lenient and allow the process to continue even if auto-detection fails.

Suggested change
flavor, version, shortVersion, err := common.FindTarballInfo(fileName)
if err != nil {
return TarballDescription{}, fmt.Errorf("could not parse version from filename %q: %s", fileName, err)
}
flavor, version, shortVersion, _ := common.FindTarballInfo(fileName)


OS, arch := detectOSArchFromFilename(fileName)
minimal := strings.Contains(strings.ToLower(fileName), "minimal")

return TarballDescription{
Name: fileName,
Url: tarballUrl,
Flavor: flavor,
Version: version,
ShortVersion: shortVersion,
OperatingSystem: OS,
Arch: arch,
Minimal: minimal,
}, nil
}

// detectOSArchFromFilename attempts to detect the OS and architecture from a tarball filename.
// It handles common MySQL/Percona/MariaDB naming conventions.
func detectOSArchFromFilename(fileName string) (OS, arch string) {
lower := strings.ToLower(fileName)

// Detect OS
switch {
case strings.Contains(lower, "linux") || strings.Contains(lower, "glibc"):
OS = "Linux"
case strings.Contains(lower, "macos") || strings.Contains(lower, "osx") || strings.Contains(lower, "darwin"):
OS = "Darwin"
case strings.Contains(lower, "windows") || strings.Contains(lower, "winx64"):
OS = "Windows"
default:
OS = runtime.GOOS
if OS == "darwin" {
OS = "Darwin"
} else if OS == "linux" {
OS = "Linux"
}
}
Comment on lines +638 to +644
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The default OS detection returns lowercase runtime.GOOS (e.g., "windows"), which is inconsistent with the explicit check for "Windows" (Title case) at line 635 and the general convention in this project for other operating systems like "Linux" and "Darwin".

	default:
		OS = runtime.GOOS
		if strings.EqualFold(OS, "darwin") {
			OS = "Darwin"
		} else if strings.EqualFold(OS, "linux") {
			OS = "Linux"
		} else if strings.EqualFold(OS, "windows") {
			OS = "Windows"
		}


// Detect architecture
switch {
case strings.Contains(lower, "arm64") || strings.Contains(lower, "aarch64"):
arch = "arm64"
case strings.Contains(lower, "x86_64") || strings.Contains(lower, "x86-64") || strings.Contains(lower, "amd64"):
arch = "amd64"
default:
arch = runtime.GOARCH
}

return OS, arch
}

func CheckTarballList(tarballList []TarballDescription) error {
uniqueNames := make(map[string]bool)
uniqueCombinations := make(map[string]bool)
Expand Down
102 changes: 102 additions & 0 deletions downloads/remote_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,105 @@ func TestMergeCollection(t *testing.T) {
})
}
}

func TestParseTarballUrlInfo(t *testing.T) {
tests := []struct {
name string
url string
wantName string
wantVersion string
wantShort string
wantOS string
wantArch string
wantFlavor string
wantMinimal bool
wantErr bool
}{
{
name: "mysql-linux-amd64",
url: "https://cdn.mysql.com/Downloads/MySQL-8.4/mysql-8.4.8-linux-glibc2.17-x86_64.tar.xz",
wantName: "mysql-8.4.8-linux-glibc2.17-x86_64.tar.xz",
wantVersion: "8.4.8",
wantShort: "8.4",
wantOS: "Linux",
wantArch: "amd64",
wantFlavor: "mysql",
wantMinimal: false,
},
{
name: "mysql-linux-amd64-minimal",
url: "https://cdn.mysql.com/Downloads/MySQL-8.4/mysql-8.4.8-linux-glibc2.17-x86_64-minimal.tar.xz",
wantName: "mysql-8.4.8-linux-glibc2.17-x86_64-minimal.tar.xz",
wantVersion: "8.4.8",
wantShort: "8.4",
wantOS: "Linux",
wantArch: "amd64",
wantFlavor: "mysql",
wantMinimal: true,
},
{
name: "mysql-macos-arm64",
url: "https://cdn.mysql.com/Downloads/MySQL-8.4/mysql-8.4.8-macos15-arm64.tar.gz",
wantName: "mysql-8.4.8-macos15-arm64.tar.gz",
wantVersion: "8.4.8",
wantShort: "8.4",
wantOS: "Darwin",
wantArch: "arm64",
wantFlavor: "mysql",
wantMinimal: false,
},
{
name: "percona-linux-amd64-minimal",
url: "https://downloads.percona.com/downloads/Percona-Server-8.0/Percona-Server-8.0.35-27/binary/tarball/Percona-Server-8.0.35-27-Linux.x86_64.glibc2.17-minimal.tar.gz",
wantName: "Percona-Server-8.0.35-27-Linux.x86_64.glibc2.17-minimal.tar.gz",
wantVersion: "8.0.35",
wantShort: "8.0",
wantOS: "Linux",
wantArch: "amd64",
wantFlavor: "percona",
wantMinimal: true,
},
{
name: "invalid-no-version",
url: "https://example.com/some-tarball-without-version.tar.gz",
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseTarballUrlInfo(tt.url)
if (err != nil) != tt.wantErr {
t.Errorf("ParseTarballUrlInfo() error = %v, wantErr %v", err, tt.wantErr)
return
}
if tt.wantErr {
return
}
if got.Name != tt.wantName {
t.Errorf("Name = %q, want %q", got.Name, tt.wantName)
}
if got.Version != tt.wantVersion {
t.Errorf("Version = %q, want %q", got.Version, tt.wantVersion)
}
if got.ShortVersion != tt.wantShort {
t.Errorf("ShortVersion = %q, want %q", got.ShortVersion, tt.wantShort)
}
if got.OperatingSystem != tt.wantOS {
t.Errorf("OperatingSystem = %q, want %q", got.OperatingSystem, tt.wantOS)
}
if got.Arch != tt.wantArch {
t.Errorf("Arch = %q, want %q", got.Arch, tt.wantArch)
}
if got.Flavor != tt.wantFlavor {
t.Errorf("Flavor = %q, want %q", got.Flavor, tt.wantFlavor)
}
if got.Minimal != tt.wantMinimal {
t.Errorf("Minimal = %v, want %v", got.Minimal, tt.wantMinimal)
}
if got.Url != tt.url {
t.Errorf("Url = %q, want %q", got.Url, tt.url)
}
})
}
}
1 change: 1 addition & 0 deletions globals/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ const (
DeleteAfterUnpackLabel = "delete-after-unpack"
MaxItemsLabel = "max-items"
ChangeUserAgentLabel = "change-user-agent"
SkipVerifyUrlLabel = "skip-verify-url"

// Instantiated in cmd/admin.go
VerboseLabel = "verbose"
Expand Down
Loading