Skip to content

Add provider validation to deployment commands #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
renecannao merged 2 commits into from
Mar 24, 2026
Merged

Add provider validation to deployment commands #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db96554
feat: add provider validation to single sandbox deployment
renecannao Mar 24, 2026
c84d199
feat: add provider validation to replication and multiple deployments
renecannao Mar 24, 2026
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
10 changes: 10 additions & 0 deletions cmd/multiple.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd
import (
"github.com/ProxySQL/dbdeployer/common"
"github.com/ProxySQL/dbdeployer/globals"
"github.com/ProxySQL/dbdeployer/providers"
"github.com/ProxySQL/dbdeployer/sandbox"
"github.com/spf13/cobra"
)
Expand All @@ -28,6 +29,15 @@ func multipleSandbox(cmd *cobra.Command, args []string) {
flags := cmd.Flags()
sd, err := fillSandboxDefinition(cmd, args, false)
common.ErrCheckExitf(err, 1, "error filling sandbox definition")
// Validate version with provider
// TODO: Phase 2b — determine provider from sd.Flavor instead of hardcoding "mysql"
p, provErr := providers.DefaultRegistry.Get("mysql")
if provErr != nil {
common.Exitf(1, "provider error: %s", provErr)
}
if provErr = p.ValidateVersion(sd.Version); provErr != nil {
common.Exitf(1, "version validation failed: %s", provErr)
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The failure message version validation failed doesn’t include which provider and version were being validated. Including both (e.g., provider name and sd.Version) would make CLI failures actionable, especially once multiple providers are supported.

Suggested change
common.Exitf(1, "version validation failed: %s", provErr)
common.Exitf(1, "version validation failed for provider %s, version %s: %s", "mysql", sd.Version, provErr)

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

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

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

medium

This provider validation logic is duplicated in cmd/replication.go and cmd/single.go. To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a common helper function. For example, you could create a validateProvider(sd sandbox.SandboxDef) function in cmd/single.go (alongside fillSandboxDefinition) and call it from all three command files.

Comment on lines +32 to +40
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The provider lookup + version validation logic is duplicated across multiple deployment commands (single/replication/multiple). Consider extracting this into a shared helper (e.g., validateProviderVersion(sd) in cmd) to reduce drift and make Phase 2b (provider-from-flavor) a smaller change.

Copilot uses AI. Check for mistakes.
nodes, _ := flags.GetInt(globals.NodesLabel)
sd.SBType = "multiple"
origin := args[0]
Expand Down
10 changes: 10 additions & 0 deletions cmd/replication.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd
import (
"github.com/ProxySQL/dbdeployer/common"
"github.com/ProxySQL/dbdeployer/globals"
"github.com/ProxySQL/dbdeployer/providers"
"github.com/ProxySQL/dbdeployer/sandbox"
"github.com/spf13/cobra"
)
Expand All @@ -28,6 +29,15 @@ func replicationSandbox(cmd *cobra.Command, args []string) {
common.CheckOrigin(args)
sd, err := fillSandboxDefinition(cmd, args, false)
common.ErrCheckExitf(err, 1, "error filling sandbox definition : %s", err)
// Validate version with provider
// TODO: Phase 2b — determine provider from sd.Flavor instead of hardcoding "mysql"
p, provErr := providers.DefaultRegistry.Get("mysql")
if provErr != nil {
common.Exitf(1, "provider error: %s", provErr)
}
if provErr = p.ValidateVersion(sd.Version); provErr != nil {
common.Exitf(1, "version validation failed: %s", provErr)
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The failure message version validation failed doesn’t include which provider and version were being validated. Including both (e.g., provider name and sd.Version) would make CLI failures actionable, especially once multiple providers are supported.

Suggested change
common.Exitf(1, "version validation failed: %s", provErr)
common.Exitf(1, "version validation failed for provider %s, version %s: %s", "mysql", sd.Version, provErr)

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

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

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

medium

This provider validation logic is duplicated in cmd/multiple.go and cmd/single.go. To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a common helper function. For example, you could create a validateProvider(sd sandbox.SandboxDef) function in cmd/single.go (alongside fillSandboxDefinition) and call it from all three command files.

Comment on lines +32 to +40
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The provider lookup + version validation logic is duplicated across multiple deployment commands (single/replication/multiple). Consider extracting this into a shared helper (e.g., validateProviderVersion(sd) in cmd) to reduce drift and make Phase 2b (provider-from-flavor) a smaller change.

Copilot uses AI. Check for mistakes.
if sd.Flavor == common.TiDbFlavor {
common.Exitf(1, "flavor '%s' is not suitable to create replication sandboxes", common.TiDbFlavor)
}
Expand Down
10 changes: 10 additions & 0 deletions cmd/single.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/ProxySQL/dbdeployer/common"
"github.com/ProxySQL/dbdeployer/defaults"
"github.com/ProxySQL/dbdeployer/globals"
"github.com/ProxySQL/dbdeployer/providers"
"github.com/ProxySQL/dbdeployer/sandbox"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -445,6 +446,15 @@ func singleSandbox(cmd *cobra.Command, args []string) {
if err != nil {
common.Exitf(1, "error while filling the sandbox definition: %+v", err)
}
// Validate version with provider
// TODO: Phase 2b — determine provider from sd.Flavor instead of hardcoding "mysql"
p, provErr := providers.DefaultRegistry.Get("mysql")
if provErr != nil {
common.Exitf(1, "provider error: %s", provErr)
}
if provErr = p.ValidateVersion(sd.Version); provErr != nil {
common.Exitf(1, "version validation failed: %s", provErr)
Comment on lines +451 to +456
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The failure message version validation failed doesn’t include which provider and version were being validated. Including both (e.g., provider name and sd.Version) would make CLI failures actionable, especially once multiple providers are supported.

Suggested change
p, provErr := providers.DefaultRegistry.Get("mysql")
if provErr != nil {
common.Exitf(1, "provider error: %s", provErr)
}
if provErr = p.ValidateVersion(sd.Version); provErr != nil {
common.Exitf(1, "version validation failed: %s", provErr)
providerName := "mysql"
p, provErr := providers.DefaultRegistry.Get(providerName)
if provErr != nil {
common.Exitf(1, "provider error: %s", provErr)
}
if provErr = p.ValidateVersion(sd.Version); provErr != nil {
common.Exitf(1, "version validation failed for provider %q and version %q: %s", providerName, sd.Version, provErr)

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

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

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

medium

This provider validation logic is duplicated in cmd/multiple.go and cmd/replication.go. To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a common helper function. For example, you could create a validateProvider(sd sandbox.SandboxDef) function in this file (alongside fillSandboxDefinition) and call it from all three command files.

Comment on lines +449 to +457
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The provider lookup + version validation logic is duplicated across multiple deployment commands (single/replication/multiple). Consider extracting this into a shared helper (e.g., validateProviderVersion(sd) in cmd) to reduce drift and make Phase 2b (provider-from-flavor) a smaller change.

Copilot uses AI. Check for mistakes.
// When deploying a single sandbox, we disable concurrency
sd.RunConcurrently = false
err = sandbox.CreateStandaloneSandbox(sd)
Expand Down
Loading