diff --git a/cmd/root.go b/cmd/root.go index d3ece66e..76e0fdbf 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -104,7 +104,7 @@ func init() { RootCmd.PersistentFlags().Bool("all-commands", false, "display all available commands and their flags in tree format") RootCmd.PersistentFlags().Bool("skip-auth", false, "skip authentication checks (for advanced users)") RootCmd.PersistentFlags().Bool("force-interactive", false, "force setup wizards to run even if already completed") - RootCmd.PersistentFlags().Duration("plugin-discovery-timeout", 2*time.Second, "timeout for plugin discovery (0s disables)") + RootCmd.PersistentFlags().Duration("plugin-discovery-timeout", 3*time.Second, "timeout for plugin discovery (0s disables)") // Make some of these flags available via Viper _ = viper.BindPFlag("config", RootCmd.PersistentFlags().Lookup("config")) diff --git a/docs/development/plugins.md b/docs/development/plugins.md index a20fb31a..05ed6a3e 100644 --- a/docs/development/plugins.md +++ b/docs/development/plugins.md @@ -30,7 +30,7 @@ Plugins are deduplicated by `manifest.name` (not by filename). If multiple binar ### Timeouts -- Overall discovery is bounded by the global flag `--plugin-discovery-timeout` (default `2s`). +- Overall discovery is bounded by the global flag `--plugin-discovery-timeout` (default `3s`). - Set to `0s` to disable plugin discovery entirely. - Manifest retrieval is bounded by `plugin.manifest_timeout_ms` (default `500ms`). diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 67df955f..fd77c208 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -115,7 +115,7 @@ dr templates list --verbose dr templates list --debug # Timeout for plugin discovery (0s disables discovery) -dr --plugin-discovery-timeout 2s --help +dr --plugin-discovery-timeout 3s --help ``` > [!WARNING] diff --git a/internal/plugin/cli_version_test.go b/internal/plugin/cli_version_test.go new file mode 100644 index 00000000..17a3a693 --- /dev/null +++ b/internal/plugin/cli_version_test.go @@ -0,0 +1,173 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugin + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCompatibleCLIVersion(t *testing.T) { + tests := []struct { + name string + cliVersion string + constraint string + compatible bool + expectErr bool + }{ + // No constraint - always compatible + { + name: "empty constraint is always compatible", + cliVersion: "1.0.0", + constraint: "", + compatible: true, + }, + // Dev version - always compatible + { + name: "dev CLI version is always compatible", + cliVersion: "dev", + constraint: ">=1.0.0", + compatible: true, + }, + // Latest CLI + plugin with no CLI constraints + { + name: "latest CLI with no constraints", + cliVersion: "2.0.0", + constraint: "", + compatible: true, + }, + // Latest CLI + plugin with CLI min version (using >= constraint) + { + name: "latest CLI satisfies min version constraint", + cliVersion: "2.0.0", + constraint: ">=1.0.0", + compatible: true, + }, + // Latest CLI + plugin with CLI max version (using < constraint) + { + name: "latest CLI within max version constraint", + cliVersion: "1.5.0", + constraint: "< 2.0.0", + compatible: true, + }, + { + name: "latest CLI exceeds max version constraint", + cliVersion: "2.0.0", + constraint: "< 2.0.0", + compatible: false, + }, + // Old CLI + plugin with no CLI constraints + { + name: "old CLI with no constraints", + cliVersion: "0.1.0", + constraint: "", + compatible: true, + }, + // Old CLI + plugin with CLI min version → incompatible + { + name: "old CLI below min version constraint", + cliVersion: "0.1.0", + constraint: ">=1.0.0", + compatible: false, + }, + // Old CLI + plugin with CLI max version + { + name: "old CLI within max version constraint", + cliVersion: "0.1.0", + constraint: "< 2.0.0", + compatible: true, + }, + // Semver constraint patterns + { + name: "caret constraint compatible", + cliVersion: "1.5.0", + constraint: "^1.0.0", + compatible: true, + }, + { + name: "caret constraint incompatible major bump", + cliVersion: "2.0.0", + constraint: "^1.0.0", + compatible: false, + }, + { + name: "tilde constraint compatible", + cliVersion: "1.0.5", + constraint: "~1.0.0", + compatible: true, + }, + { + name: "tilde constraint incompatible minor bump", + cliVersion: "1.1.0", + constraint: "~1.0.0", + compatible: false, + }, + { + name: "exact version match", + cliVersion: "1.2.3", + constraint: "1.2.3", + compatible: true, + }, + { + name: "exact version mismatch", + cliVersion: "1.2.4", + constraint: "1.2.3", + compatible: false, + }, + { + name: "range constraint compatible", + cliVersion: "1.5.0", + constraint: ">= 1.0.0, < 2.0.0", + compatible: true, + }, + { + name: "range constraint incompatible", + cliVersion: "2.0.0", + constraint: ">= 1.0.0, < 2.0.0", + compatible: false, + }, + // Error cases + { + name: "invalid constraint syntax", + cliVersion: "1.0.0", + constraint: "@invalid", + compatible: false, + expectErr: true, + }, + { + name: "invalid CLI version", + cliVersion: "not-semver", + constraint: ">=1.0.0", + compatible: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := compatibleCLIVersion(tt.cliVersion, tt.constraint) + + if tt.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.compatible, result) + }) + } +} diff --git a/internal/plugin/discover.go b/internal/plugin/discover.go index 3368142c..9845c2ec 100644 --- a/internal/plugin/discover.go +++ b/internal/plugin/discover.go @@ -25,8 +25,10 @@ import ( "strings" "time" + "github.com/Masterminds/semver/v3" "github.com/datarobot/cli/internal/log" "github.com/datarobot/cli/internal/repo" + "github.com/datarobot/cli/internal/version" "github.com/spf13/viper" ) @@ -160,6 +162,21 @@ func loadManagedPlugin(dir, name string, seen map[string]bool) (*DiscoveredPlugi return nil, nil } + compatible, err := compatibleCLIVersion(version.Version, manifest.CLIVersion) + if err != nil { + log.Warn("Cannot parse cli version constraint for plugin", + "name", manifest.Name, + "installed", version.Version, + "constraint", manifest.CLIVersion) + } else if !compatible { + log.Warn("Plugin is incompatible with DataRobot CLI version", + "name", manifest.Name, + "installed", version.Version, + "constraint", manifest.CLIVersion) + + return nil, nil + } + executable, err := resolvePlatformExecutable(pluginDir, &manifest) if err != nil { return nil, err @@ -205,7 +222,7 @@ func errMissingManifestField(field string) error { return errors.New("plugin manifest missing required field: " + field) } -func discoverInDir(dir string, seen map[string]bool) ([]DiscoveredPlugin, []error) { +func discoverInDir(dir string, seen map[string]bool) ([]DiscoveredPlugin, []error) { //nolint: cyclop plugins := make([]DiscoveredPlugin, 0) var errors []error @@ -242,7 +259,6 @@ func discoverInDir(dir string, seen map[string]bool) ([]DiscoveredPlugin, []erro manifest, err := getManifest(fullPath) if err != nil { errors = append(errors, err) - continue } @@ -255,6 +271,21 @@ func discoverInDir(dir string, seen map[string]bool) ([]DiscoveredPlugin, []erro continue } + compatible, err := compatibleCLIVersion(version.Version, manifest.CLIVersion) + if err != nil { + log.Warn("Cannot parse cli version constraint for plugin", + "name", manifest.Name, + "installed", version.Version, + "constraint", manifest.CLIVersion) + } else if !compatible { + log.Warn("Plugin is incompatible with DataRobot CLI version", + "name", manifest.Name, + "installed", version.Version, + "constraint", manifest.CLIVersion) + + continue + } + seen[manifest.Name] = true plugins = append(plugins, DiscoveredPlugin{ @@ -299,3 +330,22 @@ func getManifest(executable string) (*PluginManifest, error) { return &manifest, nil } + +func compatibleCLIVersion(cliVersion, constraint string) (bool, error) { + if cliVersion == "dev" || constraint == "" { + return true, nil + } + + c, err := semver.NewConstraint(constraint) + if err != nil { + return false, err + } + + v, err := semver.NewVersion(cliVersion) + if err != nil { + return false, err + } + + // Check if the version meets the constraints. + return c.Check(v), nil +} diff --git a/internal/plugin/integration_test.go b/internal/plugin/integration_test.go index 37fdd6ca..738aa6d4 100644 --- a/internal/plugin/integration_test.go +++ b/internal/plugin/integration_test.go @@ -88,9 +88,9 @@ func TestLivePluginManifests(t *testing.T) { // Validate version format assert.Regexp(t, `^v?\d+\.\d+\.\d+`, manifest.Version, "Manifest %s version must be semver", manifestPath) - // Validate minCLIVersion format if present - if manifest.MinCLIVersion != "" { - assert.Regexp(t, `^v?\d+\.\d+\.\d+`, manifest.MinCLIVersion, "Manifest %s minCLIVersion must be semver", manifestPath) + // Validate CLIVersion format if present + if manifest.CLIVersion != "" { + assert.Regexp(t, `^v?\d+\.\d+\.\d+`, manifest.CLIVersion, "Manifest %s CLIVersion must be semver", manifestPath) } }) } diff --git a/internal/plugin/remote.go b/internal/plugin/remote.go index fae0221b..313109bd 100644 --- a/internal/plugin/remote.go +++ b/internal/plugin/remote.go @@ -30,7 +30,6 @@ import ( "time" "github.com/Masterminds/semver/v3" - "github.com/codeclysm/extract/v4" "github.com/datarobot/cli/internal/config" "github.com/datarobot/cli/internal/log" diff --git a/internal/plugin/remote_test.go b/internal/plugin/remote_test.go index 381a628d..bce763e2 100644 --- a/internal/plugin/remote_test.go +++ b/internal/plugin/remote_test.go @@ -109,7 +109,7 @@ func TestPluginManifestSchema(t *testing.T) { "name":"test", "version":"1.0.0", "description":"Test plugin", - "minCLIVersion":"0.2.0", + "cliVersion":"0.2.0", "authentication":true, "scripts":{ "posix":"scripts/test.sh", @@ -120,7 +120,7 @@ func TestPluginManifestSchema(t *testing.T) { assert.Equal(t, "test", m.Name) assert.Equal(t, "1.0.0", m.Version) assert.Equal(t, "Test plugin", m.Description) - assert.Equal(t, "0.2.0", m.MinCLIVersion) + assert.Equal(t, "0.2.0", m.CLIVersion) assert.True(t, m.Authentication) require.NotNil(t, m.Scripts) assert.Equal(t, "scripts/test.sh", m.Scripts.Posix) diff --git a/internal/plugin/types.go b/internal/plugin/types.go index b632d282..a05ea23a 100644 --- a/internal/plugin/types.go +++ b/internal/plugin/types.go @@ -40,8 +40,8 @@ type BasicPluginManifest struct { // Embeds BasicPluginManifest and adds additional fields for managed plugins. type PluginManifest struct { BasicPluginManifest - Scripts *PluginScripts `json:"scripts,omitempty"` // Platform-specific script paths - MinCLIVersion string `json:"minCLIVersion,omitempty"` // Minimum CLI version required + Scripts *PluginScripts `json:"scripts,omitempty"` // Platform-specific script paths + CLIVersion string `json:"cliVersion,omitempty"` // Minimum CLI version required } // RegistryVersion represents a specific version in the plugin registry diff --git a/internal/plugin/validation.go b/internal/plugin/validation.go index 7265c737..89df79d1 100644 --- a/internal/plugin/validation.go +++ b/internal/plugin/validation.go @@ -29,7 +29,7 @@ import ( ) // ValidatePluginScript validates that a plugin script outputs a manifest matching the expected manifest. -// All fields must match exactly, including Scripts and MinCLIVersion for managed plugins. +// All fields must match exactly, including Scripts and CLIVersion for managed plugins. func ValidatePluginScript(pluginDir string, expectedManifest PluginManifest) error { if err := ValidateLicense(pluginDir); err != nil { return err @@ -70,9 +70,9 @@ func ValidateLicense(pluginDir string) error { // validateManifests compares two manifests and returns an error if they differ. func validateManifests(expected, actual PluginManifest) error { opts := cmp.Options{ - // Ignore Scripts and MinCLIVersion - they're optional managed plugin fields + // Ignore Scripts and CLIVersion - they're optional managed plugin fields cmp.FilterPath(func(p cmp.Path) bool { - return p.String() == "Scripts" || p.String() == "MinCLIVersion" + return p.String() == "Scripts" || p.String() == "CLIVersion" }, cmp.Ignore()), } diff --git a/internal/plugin/validation_test.go b/internal/plugin/validation_test.go index a8f621d8..fd21e12c 100644 --- a/internal/plugin/validation_test.go +++ b/internal/plugin/validation_test.go @@ -33,7 +33,7 @@ func TestValidateManifests_Matching(t *testing.T) { Description: "Test plugin", Authentication: true, }, - MinCLIVersion: "1.0.0", + CLIVersion: "1.0.0", } err := validateManifests(manifest, manifest) @@ -49,7 +49,7 @@ func TestValidateManifests_Mismatch(t *testing.T) { Description: "Test plugin", Authentication: true, }, - MinCLIVersion: "1.0.0", + CLIVersion: "1.0.0", } actual := PluginManifest{ @@ -59,7 +59,7 @@ func TestValidateManifests_Mismatch(t *testing.T) { Description: "Different description", Authentication: false, }, - MinCLIVersion: "1.0.0", + CLIVersion: "1.0.0", } err := validateManifests(expected, actual) @@ -83,7 +83,7 @@ func TestValidateManifests_ManagedPlugin(t *testing.T) { Posix: "dr-apps.sh", Windows: "dr-apps.ps1", }, - MinCLIVersion: "0.2.0", + CLIVersion: "0.2.0", } err := validateManifests(manifest, manifest) @@ -92,7 +92,7 @@ func TestValidateManifests_ManagedPlugin(t *testing.T) { } func TestValidateManifests_MissingScripts(t *testing.T) { - // Scripts and MinCLIVersion are now ignored in validation + // Scripts and CLIVersion are now ignored in validation // This allows managed plugins to work as PATH plugins expected := PluginManifest{ BasicPluginManifest: BasicPluginManifest{ @@ -105,7 +105,7 @@ func TestValidateManifests_MissingScripts(t *testing.T) { Posix: "dr-apps.sh", Windows: "dr-apps.ps1", }, - MinCLIVersion: "0.2.0", + CLIVersion: "0.2.0", } actual := PluginManifest{ @@ -115,13 +115,13 @@ func TestValidateManifests_MissingScripts(t *testing.T) { Description: "Host custom applications in DataRobot", Authentication: true, }, - Scripts: nil, // Different but ignored - MinCLIVersion: "", // Different but ignored + Scripts: nil, // Different but ignored + CLIVersion: "", // Different but ignored } err := validateManifests(expected, actual) - assert.NoError(t, err, "Scripts and MinCLIVersion differences should be ignored") + assert.NoError(t, err, "Scripts and CLIVersion differences should be ignored") } func TestValidateManifests_AllowExtraFields(t *testing.T) { @@ -147,7 +147,7 @@ func TestValidateManifests_AllowExtraFields(t *testing.T) { Posix: "dr-apps.sh", Windows: "dr-apps.ps1", }, - MinCLIVersion: "0.2.0", + CLIVersion: "0.2.0", } // Should pass - actual can have more fields than expected @@ -246,7 +246,7 @@ func TestExecPluginManifest(t *testing.T) { Description: "Test plugin", Authentication: true, }, - MinCLIVersion: "1.0.0", + CLIVersion: "1.0.0", } scriptPath := createTestScript(t, tempDir, expected)