-
Notifications
You must be signed in to change notification settings - Fork 154
Allow specifying CLI version constraints required to run the bundle #1320
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.
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
Changes from all commits
aa747de
f38c6a0
652b973
bcbf648
d02c9df
c308dc4
3cd8962
79c35c6
189f9ba
37a3655
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package mutator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "regexp" | ||
|
|
||
| semver "github.com/Masterminds/semver/v3" | ||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/internal/build" | ||
| "github.com/databricks/cli/libs/diag" | ||
| ) | ||
|
|
||
| func VerifyCliVersion() bundle.Mutator { | ||
| return &verifyCliVersion{} | ||
| } | ||
|
|
||
| type verifyCliVersion struct { | ||
| } | ||
|
|
||
| func (v *verifyCliVersion) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { | ||
| // No constraints specified, skip the check. | ||
| if b.Config.Bundle.DatabricksCliVersion == "" { | ||
| return nil | ||
| } | ||
|
|
||
| constraint := b.Config.Bundle.DatabricksCliVersion | ||
| if err := validateConstraintSyntax(constraint); err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| currentVersion := build.GetInfo().Version | ||
| c, err := semver.NewConstraint(constraint) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
|
|
||
| version, err := semver.NewVersion(currentVersion) | ||
| if err != nil { | ||
| return diag.Errorf("parsing CLI version %q failed", currentVersion) | ||
| } | ||
|
|
||
| if !c.Check(version) { | ||
| return diag.Errorf("Databricks CLI version constraint not satisfied. Required: %s, current: %s", constraint, currentVersion) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (v *verifyCliVersion) Name() string { | ||
| return "VerifyCliVersion" | ||
| } | ||
|
|
||
| // validateConstraintSyntax validates the syntax of the version constraint. | ||
| func validateConstraintSyntax(constraint string) error { | ||
| r := generateConstraintSyntaxRegexp() | ||
| if !r.MatchString(constraint) { | ||
| return fmt.Errorf("invalid version constraint %q specified. Please specify the version constraint in the format (>=) 0.0.0(, <= 1.0.0)", constraint) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // Generate regexp which matches the supported version constraint syntax. | ||
| func generateConstraintSyntaxRegexp() *regexp.Regexp { | ||
| // We intentionally only support the format supported by requirements.txt: | ||
| // 1. 0.0.0 | ||
| // 2. >= 0.0.0 | ||
| // 3. <= 0.0.0 | ||
| // 4. > 0.0.0 | ||
| // 5. < 0.0.0 | ||
| // 6. != 0.0.0 | ||
| // 7. 0.0.* | ||
| // 8. 0.* | ||
| // 9. >= 0.0.0, <= 1.0.0 | ||
| // 10. 0.0.0-0 | ||
| // 11. 0.0.0-beta | ||
| // 12. >= 0.0.0-0, <= 1.0.0-0 | ||
|
|
||
| matchVersion := `(\d+\.\d+\.\d+(\-\w+)?|\d+\.\d+.\*|\d+\.\*)` | ||
| matchOperators := `(>=|<=|>|<|!=)?` | ||
| return regexp.MustCompile(fmt.Sprintf(`^%s ?%s(, %s %s)?$`, matchOperators, matchVersion, matchOperators, matchVersion)) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| package mutator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/internal/build" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| type testCase struct { | ||
| currentVersion string | ||
| constraint string | ||
| expectedError string | ||
| } | ||
|
|
||
| func TestVerifyCliVersion(t *testing.T) { | ||
| testCases := []testCase{ | ||
| { | ||
| currentVersion: "0.0.1", | ||
| }, | ||
| { | ||
| currentVersion: "0.0.1", | ||
| constraint: "0.100.0", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: 0.100.0, current: 0.0.1", | ||
| }, | ||
| { | ||
| currentVersion: "0.0.1", | ||
| constraint: ">= 0.100.0", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: >= 0.100.0, current: 0.0.1", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.0", | ||
| constraint: "0.100.0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.1", | ||
| constraint: "0.100.0", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: 0.100.0, current: 0.100.1", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.1", | ||
| constraint: ">= 0.100.0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.0", | ||
| constraint: "<= 1.0.0", | ||
| }, | ||
| { | ||
| currentVersion: "1.0.0", | ||
| constraint: "<= 1.0.0", | ||
| }, | ||
| { | ||
| currentVersion: "1.0.0", | ||
| constraint: "<= 0.100.0", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: <= 0.100.0, current: 1.0.0", | ||
| }, | ||
| { | ||
| currentVersion: "0.99.0", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: >= 0.100.0, <= 0.100.2, current: 0.99.0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.0", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.1", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.2", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| }, | ||
| { | ||
| currentVersion: "0.101.0", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: >= 0.100.0, <= 0.100.2, current: 0.101.0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.0-beta", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: >= 0.100.0, <= 0.100.2, current: 0.100.0-beta", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.0-beta", | ||
| constraint: ">= 0.100.0-0, <= 0.100.2-0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.1-beta", | ||
| constraint: ">= 0.100.0-0, <= 0.100.2-0", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.3-beta", | ||
| constraint: ">= 0.100.0, <= 0.100.2", | ||
| expectedError: "Databricks CLI version constraint not satisfied. Required: >= 0.100.0, <= 0.100.2, current: 0.100.3-beta", | ||
| }, | ||
| { | ||
| currentVersion: "0.100.123", | ||
| constraint: "0.100.*", | ||
andrewnester marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, | ||
| { | ||
| currentVersion: "0.100.123", | ||
| constraint: "^0.100", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should just stick to the operators of requirementst.txt? (https://pip.pypa.io/en/stable/reference/requirements-file-format/)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a strong opinion here but since it's allowed by library we use for version check I'd prefer to have it as it's pretty common syntax in other languages / tools as well |
||
| expectedError: "invalid version constraint \"^0.100\" specified. Please specify the version constraint in the format (>=) 0.0.0(, <= 1.0.0)", | ||
| }, | ||
| } | ||
|
|
||
| t.Cleanup(func() { | ||
| // Reset the build version to the default version | ||
| // so that it doesn't affect other tests | ||
| // It doesn't really matter what we configure this to when testing | ||
| // as long as it is a valid semver version. | ||
| build.SetBuildVersion(build.DefaultSemver) | ||
andrewnester marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }) | ||
|
|
||
| for i, tc := range testCases { | ||
| t.Run(fmt.Sprintf("testcase #%d", i), func(t *testing.T) { | ||
| build.SetBuildVersion(tc.currentVersion) | ||
| b := &bundle.Bundle{ | ||
| Config: config.Root{ | ||
| Bundle: config.Bundle{ | ||
| DatabricksCliVersion: tc.constraint, | ||
| }, | ||
| }, | ||
| } | ||
| diags := bundle.Apply(context.Background(), b, VerifyCliVersion()) | ||
| if tc.expectedError != "" { | ||
| require.NotEmpty(t, diags) | ||
| require.Equal(t, tc.expectedError, diags.Error().Error()) | ||
| } else { | ||
| require.Empty(t, diags) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateConstraint(t *testing.T) { | ||
| testCases := []struct { | ||
| constraint string | ||
| expected bool | ||
| }{ | ||
| {"0.0.0", true}, | ||
| {">= 0.0.0", true}, | ||
| {"<= 0.0.0", true}, | ||
| {"> 0.0.0", true}, | ||
| {"< 0.0.0", true}, | ||
| {"!= 0.0.0", true}, | ||
| {"0.0.*", true}, | ||
| {"0.*", true}, | ||
| {">= 0.0.0, <= 1.0.0", true}, | ||
| {">= 0.0.0-0, <= 1.0.0-0", true}, | ||
| {"0.0.0-0", true}, | ||
| {"0.0.0-beta", true}, | ||
| {"^0.0.0", false}, | ||
| {"~0.0.0", false}, | ||
| {"0.0.0 1.0.0", false}, | ||
| {"> 0.0.0 < 1.0.0", false}, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| t.Run(tc.constraint, func(t *testing.T) { | ||
| err := validateConstraintSyntax(tc.constraint) | ||
| if tc.expected { | ||
| require.NoError(t, err) | ||
| } else { | ||
| require.Error(t, err) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.