diff --git a/pkg/provider/dir.go b/pkg/provider/dir.go index 1a922c8a1..c42fa46cd 100644 --- a/pkg/provider/dir.go +++ b/pkg/provider/dir.go @@ -12,6 +12,8 @@ import ( "github.com/devsy-org/devsy/pkg/config" config2 "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/id" + "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/providers" ) const ( @@ -303,9 +305,39 @@ func LoadProviderConfig(context, provider string) (*ProviderConfig, error) { return nil, err } + // Refresh the exec commands of built-in providers from embedded YAML: a CLI + // change (e.g. renaming the command wrapper) must not be shadowed by a stale + // provider.json. Only the exec blocks are overlaid so user customizations + // (custom --name, resolved options) survive. + if providerConfig.Source.Internal { + if embedded := embeddedProviderConfig(providerConfig.Source.Raw); embedded != nil { + providerConfig.Exec = embedded.Exec + providerConfig.Agent.Exec = embedded.Agent.Exec + } + } + return providerConfig, nil } +// embeddedProviderConfig parses the built-in provider definition for the given +// source key (e.g. "pro"), returning nil when it is not built-in or the YAML +// fails to parse. The built-in map is keyed by source id, which differs from +// the provider Name for some providers (pro -> name "devsy-pro"). +func embeddedProviderConfig(sourceID string) *ProviderConfig { + raw := providers.GetBuiltInProviders()[sourceID] + if raw == "" { + return nil + } + parsed, err := ParseProvider(strings.NewReader(raw)) + if err != nil { + // Embedded YAML failing to parse is a build-time regression; log rather + // than silently fall back to the stale stored config this refresh fixes. + log.Errorf("built-in provider %q failed to parse: %v", sourceID, err) + return nil + } + return parsed +} + func LoadMachineConfig(context, machineID string) (*Machine, error) { machineDir, err := GetMachineDir(context, machineID) if err != nil { diff --git a/pkg/provider/load_internal_test.go b/pkg/provider/load_internal_test.go new file mode 100644 index 000000000..5cc0053f7 --- /dev/null +++ b/pkg/provider/load_internal_test.go @@ -0,0 +1,135 @@ +package provider + +import ( + "strings" + "testing" + + "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/types" + "github.com/stretchr/testify/require" +) + +// staleHelperShCommand is the removed "helper sh" exec.command wrapper that +// stale provider.json files carry before a refresh rewrites them. +const staleHelperShCommand = `"${DEVSY}" helper sh -c "${COMMAND}"` + +const staleVersion = "v0.0.0-stale" + +// TestLoadProviderConfig_RefreshesInternalProvider verifies that a stored +// built-in provider config with a stale exec.command (e.g. the removed +// "helper sh" wrapper baked in before a CLI rename) has its exec block +// refreshed from the embedded provider definition on load. +func TestLoadProviderConfig_RefreshesInternalProvider(t *testing.T) { + setupTestHome(t) + + stale := &ProviderConfig{ + Name: DockerDriver, + Version: staleVersion, + Source: ProviderSource{Internal: true, Raw: DockerDriver}, + Exec: ProviderCommands{ + Command: []string{staleHelperShCommand}, + }, + } + require.NoError(t, SaveProviderConfig(config.DefaultContext, stale)) + + loaded, err := LoadProviderConfig(config.DefaultContext, DockerDriver) + require.NoError(t, err) + require.Len(t, loaded.Exec.Command, 1) + require.NotContains(t, loaded.Exec.Command[0], "helper sh", + "internal provider must be refreshed from embedded yaml, not the stale stored config") + require.True(t, strings.Contains(loaded.Exec.Command[0], "internal sh"), + "refreshed command should use the current 'internal sh' wrapper") +} + +// TestLoadProviderConfig_PreservesCustomizations verifies the exec refresh +// overlays only the embedded exec block, leaving a user's custom Name and +// resolved Options intact (a built-in added with `--name`/`--option`). +func TestLoadProviderConfig_PreservesCustomizations(t *testing.T) { + setupTestHome(t) + + stored := &ProviderConfig{ + Name: "my-docker", + Version: staleVersion, + Source: ProviderSource{Internal: true, Raw: DockerDriver}, + Options: map[string]*types.Option{ + "DOCKER_PATH": {Default: "podman"}, + }, + Exec: ProviderCommands{ + Command: []string{staleHelperShCommand}, + }, + } + require.NoError(t, SaveProviderConfig(config.DefaultContext, stored)) + + loaded, err := LoadProviderConfig(config.DefaultContext, "my-docker") + require.NoError(t, err) + require.Equal(t, "my-docker", loaded.Name, "custom provider name must survive refresh") + require.Equal(t, "v0.0.0-stale", loaded.Version, "stored version must survive refresh") + require.Equal(t, "podman", loaded.Options["DOCKER_PATH"].Default, + "resolved options must survive refresh") + require.NotContains(t, loaded.Exec.Command[0], "helper sh", + "exec block must still be refreshed from embedded yaml") +} + +// TestLoadProviderConfig_RefreshesProBySourceID guards the case where a +// built-in's provider Name differs from its source id: pro is stored as +// "devsy-pro" but keyed in the embedded map as "pro". Refresh must key off +// Source.Raw (the source id), not Name, or the pro provider keeps a stale config. +func TestLoadProviderConfig_RefreshesProBySourceID(t *testing.T) { + setupTestHome(t) + + stale := &ProviderConfig{ + Name: "devsy-pro", + Version: staleVersion, + Source: ProviderSource{Internal: true, Raw: "pro"}, + Exec: ProviderCommands{Daemon: &DaemonCommands{Start: []string{"stale start"}}}, + } + require.NoError(t, SaveProviderConfig(config.DefaultContext, stale)) + + loaded, err := LoadProviderConfig(config.DefaultContext, "devsy-pro") + require.NoError(t, err) + require.NotNil(t, loaded.Exec.Daemon, "pro exec must be refreshed from embedded yaml") + require.NotEqual(t, []string{"stale start"}, []string(loaded.Exec.Daemon.Start), + "pro provider exec must be refreshed from embedded yaml via Source.Raw") +} + +// TestLoadProviderConfig_UnknownInternalFallsBack ensures an internal provider +// whose name is not a built-in (e.g. a removed/renamed provider lingering on +// disk) falls back to the stored config rather than failing or returning empty. +func TestLoadProviderConfig_UnknownInternalFallsBack(t *testing.T) { + setupTestHome(t) + + stored := &ProviderConfig{ + Name: "retired-provider", + Version: "v0.0.1", + Source: ProviderSource{Internal: true, Raw: "retired-provider"}, + Exec: ProviderCommands{ + Command: []string{`"${DEVSY}" internal sh -c "${COMMAND}"`}, + }, + } + require.NoError(t, SaveProviderConfig(config.DefaultContext, stored)) + + loaded, err := LoadProviderConfig(config.DefaultContext, "retired-provider") + require.NoError(t, err) + require.Equal(t, stored.Exec.Command, loaded.Exec.Command) + require.Equal(t, "v0.0.1", loaded.Version) +} + +// TestLoadProviderConfig_PreservesExternalProvider ensures non-internal +// providers are loaded verbatim from disk (no embedded refresh). +func TestLoadProviderConfig_PreservesExternalProvider(t *testing.T) { + setupTestHome(t) + + external := &ProviderConfig{ + Name: DockerDriver, + Version: "v0.0.1", + Source: ProviderSource{Github: "some-org/some-provider"}, + Exec: ProviderCommands{ + Command: []string{staleHelperShCommand}, + }, + } + require.NoError(t, SaveProviderConfig(config.DefaultContext, external)) + + loaded, err := LoadProviderConfig(config.DefaultContext, DockerDriver) + require.NoError(t, err) + require.Equal(t, external.Exec.Command, loaded.Exec.Command) +} diff --git a/providers/docker/provider.yaml b/providers/docker/provider.yaml index 90037881a..75085908c 100644 --- a/providers/docker/provider.yaml +++ b/providers/docker/provider.yaml @@ -1,5 +1,5 @@ name: docker -version: v0.0.1 +version: v1.0.0 icon: https://devsy.sh/assets/docker.svg home: https://github.com/devsy-org/devsy description: |- diff --git a/providers/kubernetes/provider.yaml b/providers/kubernetes/provider.yaml index be120bd83..965c968cd 100644 --- a/providers/kubernetes/provider.yaml +++ b/providers/kubernetes/provider.yaml @@ -1,5 +1,5 @@ name: kubernetes -version: v0.0.1 +version: v1.0.0 icon: https://devsy.sh/assets/kubernetes.svg home: https://github.com/devsy-org/devsy description: |- diff --git a/providers/podman/provider.yaml b/providers/podman/provider.yaml index f8533ffaa..60ba8f810 100644 --- a/providers/podman/provider.yaml +++ b/providers/podman/provider.yaml @@ -1,5 +1,5 @@ name: podman -version: v0.0.1 +version: v1.0.0 icon: https://devsy.sh/assets/podman.svg home: https://github.com/devsy-org/devsy description: |- diff --git a/providers/pro/provider.yaml b/providers/pro/provider.yaml index d7fa41c9a..fbe4401bc 100644 --- a/providers/pro/provider.yaml +++ b/providers/pro/provider.yaml @@ -1,5 +1,5 @@ name: devsy-pro -version: v0.0.1 +version: v1.0.0 icon: https://devsy.sh/assets/devsy.svg home: https://github.com/devsy-org/devsy description: Devsy Pro