diff --git a/cmd/ci/ci.go b/cmd/ci/ci.go index afcf8d518..4c5c55949 100644 --- a/cmd/ci/ci.go +++ b/cmd/ci/ci.go @@ -67,6 +67,13 @@ exit code of "devsy ci".`, } func (cmd *CICmd) registerFlags(ciCmd *cobra.Command) { + cmd.registerRunFlags(ciCmd) + cmd.registerBuildFlags(ciCmd) + cmd.registerWorkspaceFlags(ciCmd) + cmd.registerSecretsFlags(ciCmd) +} + +func (cmd *CICmd) registerRunFlags(ciCmd *cobra.Command) { cliflags.Add(ciCmd, cliflags.String(&cmd.RunCmdString, runCmdFlag, "", "Shell command to run inside the container, executed via sh -c "+ @@ -75,14 +82,15 @@ func (cmd *CICmd) registerFlags(ciCmd *cobra.Command) { "Environment variables to set in the container at run time (KEY=VALUE, repeatable)"), cliflags.Bool(&cmd.Keep, keepFlag, false, "Keep the workspace after running instead of tearing it down (useful for debugging)"), + ) +} + +func (cmd *CICmd) registerBuildFlags(ciCmd *cobra.Command) { + cliflags.Add(ciCmd, cliflags.String(&cmd.DevContainerSource, names.DevContainer, "", "Select the devcontainer config source, overriding project discovery: "+ `"none" (ignore the project config), "image:" (use only that image), `+ `"id:" (a named .devcontainer/ profile), or a path to a devcontainer.json`), - cliflags.StringSlice(&cmd.ProviderOptions, names.ProviderOption, nil, - "Provider option in the form KEY=VALUE"), - cliflags.String(&cmd.Machine, names.Machine, "", - "The machine to use for this workspace. The machine needs to exist beforehand"), cliflags.Bool(&cmd.NoCache, names.NoCache, false, "Do not use the build cache when building the image"), cliflags.String(&cmd.RunPlatform, names.Platform, "", @@ -95,6 +103,18 @@ func (cmd *CICmd) registerFlags(ciCmd *cobra.Command) { cliflags.StringArray(&cmd.CacheFrom, names.CacheFrom, nil, "Cache sources for the build (e.g., myregistry.io/cache:latest). "+ "Reuse a pre-built image to warm the build"), + ) + cliflags.RegisterDevContainerModifierFlags(ciCmd.Flags(), cliflags.DevContainerModifierFlags{ + Features: &cmd.AdditionalFeatures, + }) +} + +func (cmd *CICmd) registerWorkspaceFlags(ciCmd *cobra.Command) { + cliflags.Add(ciCmd, + cliflags.StringSlice(&cmd.ProviderOptions, names.ProviderOption, nil, + "Provider option in the form KEY=VALUE"), + cliflags.String(&cmd.Machine, names.Machine, "", + "The machine to use for this workspace. The machine needs to exist beforehand"), cliflags.StringArray(&cmd.WorkspaceEnv, names.WorkspaceEnv, nil, "Env variables available at build/lifecycle time (KEY=VALUE, repeatable)"), cliflags.StringSlice(&cmd.WorkspaceEnvFile, names.WorkspaceEnvFile, nil, @@ -105,6 +125,11 @@ func (cmd *CICmd) registerFlags(ciCmd *cobra.Command) { nil, "Extra env variables to inject during workspace initialization (KEY=VALUE, repeatable)", ), + ) +} + +func (cmd *CICmd) registerSecretsFlags(ciCmd *cobra.Command) { + cliflags.Add(ciCmd, cliflags.String( &cmd.SecretsFile, names.SecretsFile, @@ -127,9 +152,6 @@ func (cmd *CICmd) registerFlags(ciCmd *cobra.Command) { cliflags.String(&cmd.GitTokenUsername, names.GitTokenUsername, "", "Username for --git-token (default inferred from the repo host)"), ) - cliflags.RegisterDevContainerModifierFlags(ciCmd.Flags(), cliflags.DevContainerModifierFlags{ - Features: &cmd.AdditionalFeatures, - }) } func splitArgs( diff --git a/cmd/flags/flags.go b/cmd/flags/flags.go index bb6280133..a8e8284ec 100644 --- a/cmd/flags/flags.go +++ b/cmd/flags/flags.go @@ -26,12 +26,32 @@ type GlobalFlags struct { func SetGlobalFlags(flags *flag.FlagSet) *GlobalFlags { globalFlags := &GlobalFlags{} + registerCoreFlags(flags, globalFlags) + registerOutputFlags(flags, globalFlags) + registerVerbosityFlags(flags, globalFlags) + registerHiddenFlags(flags, globalFlags) + bindGlobalEnvVars(flags) + + return globalFlags +} + +func registerCoreFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { flags.StringVar( &globalFlags.DevsyHome, names.Home, "", "If defined will override the default devsy home", ) + flags.StringVar(&globalFlags.Context, names.Context, "", "The context to use") + flags.StringVar( + &globalFlags.Provider, + names.Provider, + "", + "The provider to use. Needs to be configured for the selected context", + ) +} + +func registerOutputFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { flags.StringVar( &globalFlags.ResultFormat, names.ResultFormat, @@ -46,13 +66,9 @@ func SetGlobalFlags(flags *flag.FlagSet) *GlobalFlags { ) flags.StringVar(&globalFlags.LogOutput, names.LogFormat, "text", "Alias for --log-output") _ = flags.MarkHidden(names.LogFormat) - flags.StringVar(&globalFlags.Context, names.Context, "", "The context to use") - flags.StringVar( - &globalFlags.Provider, - names.Provider, - "", - "The provider to use. Needs to be configured for the selected context", - ) +} + +func registerVerbosityFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { flags.CountVarP( &globalFlags.Verbosity, names.Verbose, @@ -72,7 +88,9 @@ func SetGlobalFlags(flags *flag.FlagSet) *GlobalFlags { false, "Enable debug logging (equivalent to -vv)", ) +} +func registerHiddenFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { flags.Var(&globalFlags.Owner, names.Owner, "Show pro workspaces for owner") _ = flags.MarkHidden(names.Owner) flags.StringVar(&globalFlags.UID, names.UID, "", "Set UID for workspace") @@ -84,11 +102,11 @@ func SetGlobalFlags(flags *flag.FlagSet) *GlobalFlags { "The data folder where agent data is stored.", ) _ = flags.MarkHidden(names.AgentDir) +} +func bindGlobalEnvVars(flags *flag.FlagSet) { pkgflags.BindEnv(flags, names.Home) pkgflags.BindEnv(flags, names.Context) pkgflags.BindEnv(flags, names.Provider) pkgflags.BindEnv(flags, names.Debug) - - return globalFlags } diff --git a/cmd/internal/agentcontainer/setup.go b/cmd/internal/agentcontainer/setup.go index c0db3a416..fae899201 100644 --- a/cmd/internal/agentcontainer/setup.go +++ b/cmd/internal/agentcontainer/setup.go @@ -76,6 +76,52 @@ func NewSetupContainerCmd(globalFlags *flags.GlobalFlags) *cobra.Command { return cmd.Run(cobraCmd.Context()) }, } + cmd.registerFlags(setupContainerCmd) + + return setupContainerCmd +} + +type setupContext struct { + ctx context.Context + workspaceInfo *provider2.ContainerWorkspaceInfo + setupInfo *config.Result + tunnelClient tunnel.TunnelClient + secretsEnv []string +} + +// Run runs the command logic. +func (cmd *SetupContainerCmd) Run(ctx context.Context) error { + tunnelClient, err := cmd.initializeTunnelClient(ctx) + if err != nil { + return err + } + + workspaceInfo, setupInfo, err := cmd.parseWorkspaceAndSetupInfo() + if err != nil { + return err + } + + sctx := &setupContext{ + ctx: ctx, + workspaceInfo: workspaceInfo, + setupInfo: setupInfo, + tunnelClient: tunnelClient, + } + + if err := cmd.prepareWorkspace(sctx); err != nil { + return err + } + + return cmd.finalizeSetup(sctx) +} + +func (cmd *SetupContainerCmd) registerFlags(setupContainerCmd *cobra.Command) { + cmd.registerBehaviorFlags(setupContainerCmd) + cmd.registerWorkspaceInfoFlags(setupContainerCmd) + cmd.registerDotfilesFlags(setupContainerCmd) +} + +func (cmd *SetupContainerCmd) registerBehaviorFlags(setupContainerCmd *cobra.Command) { cliflags.Add( setupContainerCmd, cliflags.Bool( @@ -102,6 +148,12 @@ func NewSetupContainerCmd(globalFlags *flags.GlobalFlags) *cobra.Command { false, "If Devsy should inject git credentials during setup", ), + ) +} + +func (cmd *SetupContainerCmd) registerWorkspaceInfoFlags(setupContainerCmd *cobra.Command) { + cliflags.Add( + setupContainerCmd, cliflags.String( &cmd.ContainerWorkspaceInfo, names.ContainerWorkspaceInfo, @@ -112,6 +164,16 @@ func NewSetupContainerCmd(globalFlags *flags.GlobalFlags) *cobra.Command { cliflags.String(&cmd.AccessKey, names.AccessKey, "", "Access Key to use"), cliflags.String(&cmd.WorkspaceHost, names.WorkspaceHost, "", "Workspace hostname to use"), cliflags.String(&cmd.PlatformHost, names.PlatformHost, "", "Platform host"), + ) + _ = setupContainerCmd.MarkFlagRequired(names.SetupInfo) + + cliflags.BindEnv(setupContainerCmd.Flags(), names.AccessKey) + cliflags.BindEnv(setupContainerCmd.Flags(), names.PlatformHost) +} + +func (cmd *SetupContainerCmd) registerDotfilesFlags(setupContainerCmd *cobra.Command) { + cliflags.Add( + setupContainerCmd, cliflags.String(&cmd.DotfilesRepo, names.DotfilesRepo, "", "Dotfiles repository URL"), cliflags.String( &cmd.DotfilesScript, @@ -120,46 +182,6 @@ func NewSetupContainerCmd(globalFlags *flags.GlobalFlags) *cobra.Command { "Dotfiles install script path", ), ) - _ = setupContainerCmd.MarkFlagRequired(names.SetupInfo) - - cliflags.BindEnv(setupContainerCmd.Flags(), names.AccessKey) - cliflags.BindEnv(setupContainerCmd.Flags(), names.PlatformHost) - - return setupContainerCmd -} - -type setupContext struct { - ctx context.Context - workspaceInfo *provider2.ContainerWorkspaceInfo - setupInfo *config.Result - tunnelClient tunnel.TunnelClient - secretsEnv []string -} - -// Run runs the command logic. -func (cmd *SetupContainerCmd) Run(ctx context.Context) error { - tunnelClient, err := cmd.initializeTunnelClient(ctx) - if err != nil { - return err - } - - workspaceInfo, setupInfo, err := cmd.parseWorkspaceAndSetupInfo() - if err != nil { - return err - } - - sctx := &setupContext{ - ctx: ctx, - workspaceInfo: workspaceInfo, - setupInfo: setupInfo, - tunnelClient: tunnelClient, - } - - if err := cmd.prepareWorkspace(sctx); err != nil { - return err - } - - return cmd.finalizeSetup(sctx) } func (cmd *SetupContainerCmd) prepareWorkspace(sctx *setupContext) error { @@ -919,74 +941,96 @@ func streamMount( ) error { // if we have a platform workspace socket we connect directly to it if workspaceInfo.CLIOptions.Platform.Enabled { - // check if the runner proxy socket exists - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, + return streamMountFromPlatform(ctx, workspaceInfo, m) + } + + return streamMountFromTunnel(ctx, m, tunnelClient) +} + +func streamMountFromPlatform( + ctx context.Context, + workspaceInfo *provider2.ContainerWorkspaceInfo, + m *config.Mount, +) error { + log.Infof("Download %s into DevContainer %s", m.Source, m.Target) + req, err := buildPlatformDownloadRequest(ctx, workspaceInfo, m) + if err != nil { + return err + } + + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + //nolint:gosec // pre-existing, relocated by funlen extraction; out of scope for this PR + InsecureSkipVerify: true, }, - } + }, + } + resp, err := httpClient.Do(req) + if err != nil { + return fmt.Errorf("download workspace: %w", err) + } + defer func() { _ = resp.Body.Close() }() - // build the url - log.Infof("Download %s into DevContainer %s", m.Source, m.Target) - url := fmt.Sprintf( - "https://%s/kubernetes/management/apis/management.devsy.sh/v1/namespaces/%s/devsyworkspaceinstances/%s/download?path=%s", - ts.RemoveProtocol(workspaceInfo.CLIOptions.Platform.PlatformHost), - workspaceInfo.CLIOptions.Platform.InstanceNamespace, - workspaceInfo.CLIOptions.Platform.InstanceName, - url.QueryEscape(m.Source), - ) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return fmt.Errorf("create request: %w", err) - } - req.Header.Set( - "Authorization", - fmt.Sprintf("Bearer %s", workspaceInfo.CLIOptions.Platform.AccessKey), + // check if the response is ok + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf( + "download workspace: body = %s, status = %s", + string(body), + resp.Status, ) + } - // send the request - resp, err := httpClient.Do(req) - if err != nil { - return fmt.Errorf("download workspace: %w", err) - } - defer func() { _ = resp.Body.Close() }() - - // check if the response is ok - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf( - "download workspace: body = %s, status = %s", - string(body), - resp.Status, - ) - } + progressReader := &progressReader{ + Reader: resp.Body, + } - // create progress reader - progressReader := &progressReader{ - Reader: resp.Body, - } + if err := extract.Extract(progressReader, m.Target); err != nil { + return fmt.Errorf("stream mount %s: %w", m.String(), err) + } - // target folder - err = extract.Extract(progressReader, m.Target) - if err != nil { - return fmt.Errorf("stream mount %s: %w", m.String(), err) - } + return nil +} - return nil +// buildPlatformDownloadRequest builds the authenticated request to the +// runner proxy socket that serves m.Source for download. +func buildPlatformDownloadRequest( + ctx context.Context, + workspaceInfo *provider2.ContainerWorkspaceInfo, + m *config.Mount, +) (*http.Request, error) { + downloadURL := fmt.Sprintf( + "https://%s/kubernetes/management/apis/management.devsy.sh/v1/namespaces/%s/devsyworkspaceinstances/%s/download?path=%s", //nolint:lll // pre-existing, relocated by funlen extraction; out of scope for this PR + ts.RemoveProtocol(workspaceInfo.CLIOptions.Platform.PlatformHost), + workspaceInfo.CLIOptions.Platform.InstanceNamespace, + workspaceInfo.CLIOptions.Platform.InstanceName, + url.QueryEscape(m.Source), + ) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) + if err != nil { + return nil, fmt.Errorf("create request: %w", err) } + req.Header.Set( + "Authorization", + fmt.Sprintf("Bearer %s", workspaceInfo.CLIOptions.Platform.AccessKey), + ) + + return req, nil +} - // stream mount +func streamMountFromTunnel( + ctx context.Context, + m *config.Mount, + tunnelClient tunnel.TunnelClient, +) error { log.Infof("Copy %s into DevContainer %s", m.Source, m.Target) stream, err := tunnelClient.StreamMount(ctx, &tunnel.StreamMountRequest{Mount: m.String()}) if err != nil { return fmt.Errorf("init stream mount %s: %w", m.String(), err) } - // target folder - err = extract.Extract(tunnelserver.NewStreamReader(stream), m.Target) - if err != nil { + if err := extract.Extract(tunnelserver.NewStreamReader(stream), m.Target); err != nil { return fmt.Errorf("stream mount %s: %w", m.String(), err) } diff --git a/cmd/internal/runusercommands.go b/cmd/internal/runusercommands.go index 71608665e..5113a206e 100644 --- a/cmd/internal/runusercommands.go +++ b/cmd/internal/runusercommands.go @@ -54,6 +54,60 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { RunE: runE, } + cmd.registerFlags(runCmd) + + runCmd.MarkFlagsOneRequired(names.WorkspaceFolder, names.ContainerID) + + return runCmd +} + +// NewRunUserCommandsCmdAlias creates the hidden camelCase alias for devcontainer CLI compat. +func NewRunUserCommandsCmdAlias(f *flags.GlobalFlags) *cobra.Command { + primary := NewRunUserCommandsCmd(f) + primary.Use = "runUserCommands" + primary.Hidden = true + return primary +} + +const updateContentCommand = "updateContentCommand" + +// Run executes the run-user-commands logic. +func (cmd *RunUserCommandsCmd) Run(ctx context.Context) error { + if err := cmd.validate(); err != nil { + return err + } + + if cmd.ContainerID != "" { + return cmd.runWithContainerID(ctx) + } + + params, result, err := cmd.resolveContainer(ctx) + if err != nil { + return err + } + + if err := cmd.runLifecycleHooks(params, result); err != nil { + return err + } + + user := devcconfig.GetRemoteUser(result) + log.Infof("lifecycle commands completed for container %s", params.ContainerID) + _ = devcconfig.WriteResultJSON(os.Stderr, devcconfig.ResultEnvelope{ + ContainerID: params.ContainerID, + RemoteUser: user, + RemoteWorkspaceFolder: params.Workdir, + }) + return nil +} + +func (cmd *RunUserCommandsCmd) registerFlags(runCmd *cobra.Command) { + cmd.registerTargetFlags(runCmd) + cmd.registerConfigFlags(runCmd) + cmd.registerEnvFlags(runCmd) + cmd.registerLifecycleFlags(runCmd) +} + +func (cmd *RunUserCommandsCmd) registerTargetFlags(runCmd *cobra.Command) { cliflags.Add( runCmd, cliflags.String( @@ -74,6 +128,12 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { "", "Path to the docker/podman executable (defaults to 'docker')", ), + ) +} + +func (cmd *RunUserCommandsCmd) registerConfigFlags(runCmd *cobra.Command) { + cliflags.Add( + runCmd, cliflags.String( &cmd.Config, names.Config, @@ -86,6 +146,12 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { "", "Path to an additional devcontainer.json file to override the primary configuration", ), + ) +} + +func (cmd *RunUserCommandsCmd) registerEnvFlags(runCmd *cobra.Command) { + cliflags.Add( + runCmd, cliflags.StringArray( &cmd.RemoteEnv, names.RemoteEnv, @@ -98,6 +164,12 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { []string{}, "Override the default container identification labels (format: key=value, can be specified multiple times)", ), + ) +} + +func (cmd *RunUserCommandsCmd) registerLifecycleFlags(runCmd *cobra.Command) { + cliflags.Add( + runCmd, cliflags.Bool( &cmd.Prebuild, names.Prebuild, @@ -136,49 +208,6 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { "Skip running updateContentCommand", ), ) - - runCmd.MarkFlagsOneRequired(names.WorkspaceFolder, names.ContainerID) - - return runCmd -} - -// NewRunUserCommandsCmdAlias creates the hidden camelCase alias for devcontainer CLI compat. -func NewRunUserCommandsCmdAlias(f *flags.GlobalFlags) *cobra.Command { - primary := NewRunUserCommandsCmd(f) - primary.Use = "runUserCommands" - primary.Hidden = true - return primary -} - -const updateContentCommand = "updateContentCommand" - -// Run executes the run-user-commands logic. -func (cmd *RunUserCommandsCmd) Run(ctx context.Context) error { - if err := cmd.validate(); err != nil { - return err - } - - if cmd.ContainerID != "" { - return cmd.runWithContainerID(ctx) - } - - params, result, err := cmd.resolveContainer(ctx) - if err != nil { - return err - } - - if err := cmd.runLifecycleHooks(params, result); err != nil { - return err - } - - user := devcconfig.GetRemoteUser(result) - log.Infof("lifecycle commands completed for container %s", params.ContainerID) - _ = devcconfig.WriteResultJSON(os.Stderr, devcconfig.ResultEnvelope{ - ContainerID: params.ContainerID, - RemoteUser: user, - RemoteWorkspaceFolder: params.Workdir, - }) - return nil } func (cmd *RunUserCommandsCmd) validate() error { diff --git a/cmd/pro/cluster/add.go b/cmd/pro/cluster/add.go index 85d1ef240..8d758dfd3 100644 --- a/cmd/pro/cluster/add.go +++ b/cmd/pro/cluster/add.go @@ -13,8 +13,6 @@ import ( storagev1 "github.com/devsy-org/api/pkg/apis/storage/v1" proflags "github.com/devsy-org/devsy/cmd/pro/flags" "github.com/devsy-org/devsy/pkg/config" - cliflags "github.com/devsy-org/devsy/pkg/flags" - "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/platform" "github.com/devsy-org/devsy/pkg/platform/client" @@ -61,66 +59,7 @@ func NewAddCmd(globalFlags *proflags.GlobalFlags) *cobra.Command { }, } - cliflags.Add( - c, - cliflags.String( - &cmd.Namespace, - names.Namespace, - "loft", - "The namespace to generate the service account in. The namespace will be created if it does not exist", - ), - cliflags.String( - &cmd.ServiceAccount, - names.ServiceAccount, - "loft-admin", - "The service account name to create", - ), - cliflags.String( - &cmd.DisplayName, - names.DisplayName, - "", - "The display name to show in the UI for this cluster", - ), - cliflags.Bool( - &cmd.Wait, - names.Wait, - false, - "If true, will wait until the cluster is initialized", - ), - cliflags.Bool( - &cmd.Insecure, - names.Insecure, - false, - "If true, deploys the agent in insecure mode", - ), - cliflags.String( - &cmd.HelmChartVersion, - names.HelmChartVersion, - "", - "The agent chart version to deploy", - ), - cliflags.String(&cmd.HelmChartPath, names.HelmChartPath, "", "The agent chart to deploy"), - cliflags.StringArray( - &cmd.HelmSet, - names.HelmSet, - []string{}, - "Extra helm values for the agent chart", - ), - cliflags.StringArray( - &cmd.HelmValues, - names.HelmValues, - []string{}, - "Extra helm values for the agent chart", - ), - cliflags.String( - &cmd.KubeContext, - names.KubeContext, - "", - "The kube context to use for installation", - ), - cliflags.String(&cmd.Host, names.Host, "", "The pro instance to use"), - ) - proflags.BindEnv(c.Flags(), names.Host) + cmd.registerFlags(c) return c } diff --git a/cmd/pro/cluster/add_flags.go b/cmd/pro/cluster/add_flags.go new file mode 100644 index 000000000..11decaadf --- /dev/null +++ b/cmd/pro/cluster/add_flags.go @@ -0,0 +1,96 @@ +package cluster + +import ( + proflags "github.com/devsy-org/devsy/cmd/pro/flags" + cliflags "github.com/devsy-org/devsy/pkg/flags" + "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/spf13/cobra" +) + +func (cmd *ClusterCmd) registerFlags(c *cobra.Command) { + cmd.registerIdentityFlags(c) + cmd.registerBehaviorFlags(c) + cmd.registerHelmFlags(c) + cmd.registerClusterFlags(c) +} + +func (cmd *ClusterCmd) registerIdentityFlags(c *cobra.Command) { + cliflags.Add( + c, + cliflags.String( + &cmd.Namespace, + names.Namespace, + "loft", + "The namespace to generate the service account in. The namespace will be created if it does not exist", + ), + cliflags.String( + &cmd.ServiceAccount, + names.ServiceAccount, + "loft-admin", + "The service account name to create", + ), + cliflags.String( + &cmd.DisplayName, + names.DisplayName, + "", + "The display name to show in the UI for this cluster", + ), + ) +} + +func (cmd *ClusterCmd) registerBehaviorFlags(c *cobra.Command) { + cliflags.Add( + c, + cliflags.Bool( + &cmd.Wait, + names.Wait, + false, + "If true, will wait until the cluster is initialized", + ), + cliflags.Bool( + &cmd.Insecure, + names.Insecure, + false, + "If true, deploys the agent in insecure mode", + ), + ) +} + +func (cmd *ClusterCmd) registerHelmFlags(c *cobra.Command) { + cliflags.Add( + c, + cliflags.String( + &cmd.HelmChartVersion, + names.HelmChartVersion, + "", + "The agent chart version to deploy", + ), + cliflags.String(&cmd.HelmChartPath, names.HelmChartPath, "", "The agent chart to deploy"), + cliflags.StringArray( + &cmd.HelmSet, + names.HelmSet, + []string{}, + "Extra helm values for the agent chart", + ), + cliflags.StringArray( + &cmd.HelmValues, + names.HelmValues, + []string{}, + "Extra helm values for the agent chart", + ), + ) +} + +func (cmd *ClusterCmd) registerClusterFlags(c *cobra.Command) { + cliflags.Add( + c, + cliflags.String( + &cmd.KubeContext, + names.KubeContext, + "", + "The kube context to use for installation", + ), + cliflags.String(&cmd.Host, names.Host, "", "The pro instance to use"), + ) + proflags.BindEnv(c.Flags(), names.Host) +} diff --git a/cmd/pro/start.go b/cmd/pro/start.go index f307cd0cd..e11cfeb76 100644 --- a/cmd/pro/start.go +++ b/cmd/pro/start.go @@ -24,8 +24,6 @@ import ( loftclientset "github.com/devsy-org/api/pkg/clientset/versioned" proflags "github.com/devsy-org/devsy/cmd/pro/flags" "github.com/devsy-org/devsy/pkg/config" - cliflags "github.com/devsy-org/devsy/pkg/flags" - "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/hash" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/machineid" @@ -108,99 +106,7 @@ func NewStartCmd(flags *proflags.GlobalFlags) *cobra.Command { }, } - cliflags.Add( - startCmd, - cliflags.Bool( - &cmd.Docker, - names.Docker, - false, - "If enabled will try to deploy Devsy Pro to the local docker installation.", - ), - cliflags.String(&cmd.DockerImage, names.DockerImage, "", "The docker image to install."), - cliflags.StringArray(&cmd.DockerArgs, names.DockerArg, []string{}, "Extra docker args"), - cliflags.String( - &cmd.Context, - names.Context, - "", - "The kube context to use for installation", - ), - cliflags.String( - &cmd.Namespace, - names.Namespace, - config.ProReleaseName, - "The namespace to install into", - ), - cliflags.String( - &cmd.Host, - names.Host, - "", - "Provide a hostname to enable ingress and configure its hostname", - ), - cliflags.String( - &cmd.Password, - names.Password, - "", - "The password to use for the admin account. (If empty this will be the namespace UID)", - ), - cliflags.String(&cmd.Version, names.Version, "", "The version to install"), - cliflags.String( - &cmd.Values, - names.Values, - "", - "Path to a file for extra helm chart values", - ), - cliflags.Bool( - &cmd.ReuseValues, - names.ReuseValues, - true, - "Reuse previous helm values on upgrade", - ), - cliflags.Bool( - &cmd.Upgrade, - names.Upgrade, - false, - "If true, will try to upgrade the release", - ), - cliflags.String(&cmd.Email, names.Email, "", "The email to use for the installation"), - cliflags.Bool( - &cmd.Reset, - names.Reset, - false, - "If true, an existing instance will be deleted before installing Devsy Pro", - ), - cliflags.Bool( - &cmd.NoWait, - names.NoWait, - false, - "If true, will not wait after installing it", - ), - cliflags.Bool( - &cmd.NoTunnel, - names.NoTunnel, - false, - "If true, will not create a loft.host tunnel for this installation", - ), - cliflags.Bool( - &cmd.NoLogin, - names.NoLogin, - false, - "If true, will not login to a Devsy Pro instance on start", - ), - cliflags.String( - &cmd.ChartPath, - names.ChartPath, - "", - "The local chart path to deploy Devsy Pro", - ), - cliflags.String( - &cmd.ChartRepo, - names.ChartRepo, - "https://charts.devsy.sh/", - "The chart repo to deploy Devsy Pro", - ), - ) - - proflags.BindEnv(startCmd.Flags(), names.Host) + cmd.registerFlags(startCmd) return startCmd } @@ -538,15 +444,35 @@ func (cmd *StartCmd) confirmPortForwardIfUnreachable( } func (cmd *StartCmd) successRemote(ctx context.Context, host string) error { - printSuccess := func() { - url := "https://" + host + ready, err := isHostReachable(ctx, host) + if err != nil { + return err + } else if ready { + cmd.printRemoteSuccessMessage(host) + return nil + } - password := cmd.Password - if password == "" { - password = passwordChangedHint - } + printDNSConfigurationRequired(host) + + if err := waitForHostReachable(ctx, host); err != nil { + return err + } + + log.Info("Devsy Pro is reachable at https://" + host) - fmt.Fprintf(os.Stderr, ` + cmd.printRemoteSuccessMessage(host) + return nil +} + +func (cmd *StartCmd) printRemoteSuccessMessage(host string) { + url := "https://" + host + + password := cmd.Password + if password == "" { + password = passwordChangedHint + } + + fmt.Fprintf(os.Stderr, ` ########################## LOGIN ############################ @@ -566,20 +492,13 @@ Devsy Pro was successfully installed and can now be reached at: %s Thanks for using Devsy Pro! `, - greenBold(url), - greenBold("devsy pro login "+url), - "https://devsy.sh/docs/administration/ssl", - url) - } - ready, err := isHostReachable(ctx, host) - if err != nil { - return err - } else if ready { - printSuccess() - return nil - } + greenBold(url), + greenBold("devsy pro login "+url), + "https://devsy.sh/docs/administration/ssl", + url) +} - // Print DNS Configuration +func printDNSConfigurationRequired(host string) { fmt.Fprint(os.Stderr, ` ################################### DNS CONFIGURATION REQUIRED ################################## @@ -604,7 +523,10 @@ The command will wait until Devsy Pro is reachable under the host. log.Info( "Waiting for you to configure DNS, so Devsy Pro can be reached on https://" + host, ) - err = wait.PollUntilContextTimeout( +} + +func waitForHostReachable(ctx context.Context, host string) error { + return wait.PollUntilContextTimeout( ctx, 5*time.Second, platform.Timeout(), @@ -613,14 +535,6 @@ The command will wait until Devsy Pro is reachable under the host. return isHostReachable(ctx, host) }, ) - if err != nil { - return err - } - - log.Info("Devsy Pro is reachable at https://" + host) - - printSuccess() - return nil } func (cmd *StartCmd) successLocal() error { @@ -1538,6 +1452,26 @@ func uninstall( kubeClient kubernetes.Interface, restConfig *rest.Config, kubeContext, namespace string, +) error { + if err := runHelmUninstall(ctx, kubeClient, kubeContext, namespace); err != nil { + return err + } + + if err := cleanupProResources(ctx, kubeClient, restConfig, namespace); err != nil { + return err + } + + fmt.Fprint(os.Stderr, "\n") + log.Info("uninstalled Devsy Pro") + fmt.Fprint(os.Stderr, "\n") + + return nil +} + +func runHelmUninstall( + ctx context.Context, + kubeClient kubernetes.Interface, + kubeContext, namespace string, ) error { releaseName, err := resolveReleaseName(ctx, kubeClient, namespace) if err != nil { @@ -1559,7 +1493,15 @@ func uninstall( log.Errorf("error during helm command: %s (%v)", string(output), err) } - // we also cleanup the validating webhook configuration and apiservice + return nil +} + +func cleanupProResources( + ctx context.Context, + kubeClient kubernetes.Interface, + restConfig *rest.Config, + namespace string, +) error { apiRegistrationClient, err := clientset.NewForConfig(restConfig) if err != nil { return err @@ -1574,12 +1516,20 @@ func uninstall( return err } - err = deleteUser(ctx, restConfig, "admin") - if err != nil { + if err := deleteUser(ctx, restConfig, "admin"); err != nil { return err } - err = deleteIgnoreNotFound( + return deleteRemainingAgentResources(ctx, kubeClient, apiRegistrationClient, namespace) +} + +func deleteRemainingAgentResources( + ctx context.Context, + kubeClient kubernetes.Interface, + apiRegistrationClient *clientset.Clientset, + namespace string, +) error { + return deleteIgnoreNotFound( func() error { return kubeClient.CoreV1(). Secrets(namespace). @@ -1616,15 +1566,6 @@ func uninstall( Delete(ctx, "loft-applied-defaults", metav1.DeleteOptions{}) }, ) - if err != nil { - return err - } - - fmt.Fprint(os.Stderr, "\n") - log.Info("uninstalled Devsy Pro") - fmt.Fprint(os.Stderr, "\n") - - return nil } func resolveReleaseName( diff --git a/cmd/pro/start_flags.go b/cmd/pro/start_flags.go new file mode 100644 index 000000000..6c0c2b175 --- /dev/null +++ b/cmd/pro/start_flags.go @@ -0,0 +1,137 @@ +package pro + +import ( + proflags "github.com/devsy-org/devsy/cmd/pro/flags" + "github.com/devsy-org/devsy/pkg/config" + cliflags "github.com/devsy-org/devsy/pkg/flags" + "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/spf13/cobra" +) + +func (cmd *StartCmd) registerFlags(startCmd *cobra.Command) { + cmd.registerDockerFlags(startCmd) + cmd.registerClusterFlags(startCmd) + cmd.registerChartFlags(startCmd) + cmd.registerAuthFlags(startCmd) + cmd.registerLifecycleFlags(startCmd) +} + +func (cmd *StartCmd) registerDockerFlags(startCmd *cobra.Command) { + cliflags.Add( + startCmd, + cliflags.Bool( + &cmd.Docker, + names.Docker, + false, + "If enabled will try to deploy Devsy Pro to the local docker installation.", + ), + cliflags.String(&cmd.DockerImage, names.DockerImage, "", "The docker image to install."), + cliflags.StringArray(&cmd.DockerArgs, names.DockerArg, []string{}, "Extra docker args"), + ) +} + +func (cmd *StartCmd) registerClusterFlags(startCmd *cobra.Command) { + cliflags.Add( + startCmd, + cliflags.String( + &cmd.Context, + names.Context, + "", + "The kube context to use for installation", + ), + cliflags.String( + &cmd.Namespace, + names.Namespace, + config.ProReleaseName, + "The namespace to install into", + ), + cliflags.String( + &cmd.Host, + names.Host, + "", + "Provide a hostname to enable ingress and configure its hostname", + ), + ) + + proflags.BindEnv(startCmd.Flags(), names.Host) +} + +func (cmd *StartCmd) registerChartFlags(startCmd *cobra.Command) { + cliflags.Add( + startCmd, + cliflags.String(&cmd.Version, names.Version, "", "The version to install"), + cliflags.String( + &cmd.Values, + names.Values, + "", + "Path to a file for extra helm chart values", + ), + cliflags.Bool( + &cmd.ReuseValues, + names.ReuseValues, + true, + "Reuse previous helm values on upgrade", + ), + cliflags.String( + &cmd.ChartPath, + names.ChartPath, + "", + "The local chart path to deploy Devsy Pro", + ), + cliflags.String( + &cmd.ChartRepo, + names.ChartRepo, + "https://charts.devsy.sh/", + "The chart repo to deploy Devsy Pro", + ), + ) +} + +func (cmd *StartCmd) registerAuthFlags(startCmd *cobra.Command) { + cliflags.Add( + startCmd, + cliflags.String( + &cmd.Password, + names.Password, + "", + "The password to use for the admin account. (If empty this will be the namespace UID)", + ), + cliflags.String(&cmd.Email, names.Email, "", "The email to use for the installation"), + ) +} + +func (cmd *StartCmd) registerLifecycleFlags(startCmd *cobra.Command) { + cliflags.Add( + startCmd, + cliflags.Bool( + &cmd.Upgrade, + names.Upgrade, + false, + "If true, will try to upgrade the release", + ), + cliflags.Bool( + &cmd.Reset, + names.Reset, + false, + "If true, an existing instance will be deleted before installing Devsy Pro", + ), + cliflags.Bool( + &cmd.NoWait, + names.NoWait, + false, + "If true, will not wait after installing it", + ), + cliflags.Bool( + &cmd.NoTunnel, + names.NoTunnel, + false, + "If true, will not create a loft.host tunnel for this installation", + ), + cliflags.Bool( + &cmd.NoLogin, + names.NoLogin, + false, + "If true, will not login to a Devsy Pro instance on start", + ), + ) +} diff --git a/cmd/provider/init.go b/cmd/provider/init.go index 0e75d281f..4e2697124 100644 --- a/cmd/provider/init.go +++ b/cmd/provider/init.go @@ -6,8 +6,6 @@ import ( "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" - cliflags "github.com/devsy-org/devsy/pkg/flags" - "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/status" "github.com/devsy-org/devsy/pkg/workspace" "github.com/spf13/cobra" @@ -75,15 +73,6 @@ func NewInitCmd(f *flags.GlobalFlags) *cobra.Command { ) }, } - cliflags.Add(initCmd, - cliflags.Bool(&cmd.Reset, names.Reset, false, - "Discard previously stored option answers and re-prompt from scratch"), - cliflags.Bool(&cmd.SingleMachine, names.SingleMachine, false, - "Use a single machine for all workspaces"), - cliflags.StringArray(&cmd.Options, names.Option, nil, - "Provider option in the form KEY=VALUE").Shorthand("o"), - cliflags.Bool(&cmd.SkipInit, names.SkipInit, false, - "Skip provider init (testing only)").Hidden(), - ) + cmd.registerFlags(initCmd) return initCmd } diff --git a/cmd/provider/init_flags.go b/cmd/provider/init_flags.go new file mode 100644 index 000000000..a2be5cfcc --- /dev/null +++ b/cmd/provider/init_flags.go @@ -0,0 +1,30 @@ +package provider + +import ( + cliflags "github.com/devsy-org/devsy/pkg/flags" + "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/spf13/cobra" +) + +func (cmd *InitCmd) registerFlags(initCmd *cobra.Command) { + cmd.registerOptionFlags(initCmd) + cmd.registerTestingFlags(initCmd) +} + +func (cmd *InitCmd) registerOptionFlags(initCmd *cobra.Command) { + cliflags.Add(initCmd, + cliflags.Bool(&cmd.Reset, names.Reset, false, + "Discard previously stored option answers and re-prompt from scratch"), + cliflags.Bool(&cmd.SingleMachine, names.SingleMachine, false, + "Use a single machine for all workspaces"), + cliflags.StringArray(&cmd.Options, names.Option, nil, + "Provider option in the form KEY=VALUE").Shorthand("o"), + ) +} + +func (cmd *InitCmd) registerTestingFlags(initCmd *cobra.Command) { + cliflags.Add(initCmd, + cliflags.Bool(&cmd.SkipInit, names.SkipInit, false, + "Skip provider init (testing only)").Hidden(), + ) +} diff --git a/cmd/workspace/logs.go b/cmd/workspace/logs.go index 4b8dac74c..3121d124d 100644 --- a/cmd/workspace/logs.go +++ b/cmd/workspace/logs.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "time" "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" @@ -58,18 +59,9 @@ func (cmd *LogsCmd) Run(ctx context.Context, args []string) error { return err } - baseClient, err := workspace.Get(ctx, workspace.GetOptions{ - DevsyConfig: devsyConfig, - Args: args, - Owner: cmd.Owner, - }) + client, err := cmd.getWorkspaceClient(ctx, devsyConfig, args) if err != nil { - return fmt.Errorf("get workspace for logs: %w", err) - } - - client, ok := baseClient.(clientpkg.WorkspaceClient) - if !ok { - return fmt.Errorf("this command is not supported for proxy providers") + return err } sshServerCmd := fmt.Sprintf("'%s' internal ssh-server --stdio", client.AgentPath()) @@ -87,27 +79,12 @@ func (cmd *LogsCmd) Run(ctx context.Context, args []string) error { return pb.RunPair(ctx, func(ctx context.Context, stdin, stdout *os.File) error { - stderr := log.Writer(log.LevelDebug) - defer func() { _ = stderr.Close() }() - - return agent.InjectAgent(&agent.InjectOptions{ - Ctx: ctx, - Exec: func(ctx context.Context, command string, stdinR io.Reader, stdoutW io.Writer, stderrW io.Writer) error { - return client.Command(ctx, clientpkg.CommandOptions{ - Command: command, - Stdin: stdinR, - Stdout: stdoutW, - Stderr: stderrW, - }) - }, - IsLocal: client.AgentLocal(), - RemoteAgentPath: client.AgentPath(), - DownloadURL: client.AgentURL(), - Command: sshServerCmd, - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - Timeout: timeout, + return injectLogsAgent(ctx, injectLogsAgentParams{ + client: client, + sshServerCmd: sshServerCmd, + timeout: timeout, + stdin: stdin, + stdout: stdout, }) }, func(ctx context.Context, stdout, stdin *os.File) error { @@ -116,6 +93,63 @@ func (cmd *LogsCmd) Run(ctx context.Context, args []string) error { ) } +// getWorkspaceClient resolves args to a WorkspaceClient, rejecting proxy providers +// which don't support log streaming. +func (cmd *LogsCmd) getWorkspaceClient( + ctx context.Context, devsyConfig *config.Config, args []string, +) (clientpkg.WorkspaceClient, error) { + baseClient, err := workspace.Get(ctx, workspace.GetOptions{ + DevsyConfig: devsyConfig, + Args: args, + Owner: cmd.Owner, + }) + if err != nil { + return nil, fmt.Errorf("get workspace for logs: %w", err) + } + + client, ok := baseClient.(clientpkg.WorkspaceClient) + if !ok { + return nil, fmt.Errorf("this command is not supported for proxy providers") + } + + return client, nil +} + +// injectLogsAgent injects the devsy agent binary over stdin/stdout and runs the +// remote ssh-server that runLogsSession then connects to. +func injectLogsAgent(ctx context.Context, params injectLogsAgentParams) error { + stderr := log.Writer(log.LevelDebug) + defer func() { _ = stderr.Close() }() + + return agent.InjectAgent(&agent.InjectOptions{ + Ctx: ctx, + Exec: func(ctx context.Context, command string, stdinR io.Reader, stdoutW io.Writer, stderrW io.Writer) error { + return params.client.Command(ctx, clientpkg.CommandOptions{ + Command: command, + Stdin: stdinR, + Stdout: stdoutW, + Stderr: stderrW, + }) + }, + IsLocal: params.client.AgentLocal(), + RemoteAgentPath: params.client.AgentPath(), + DownloadURL: params.client.AgentURL(), + Command: params.sshServerCmd, + Stdin: params.stdin, + Stdout: params.stdout, + Stderr: stderr, + Timeout: params.timeout, + }) +} + +type injectLogsAgentParams struct { + client clientpkg.WorkspaceClient + sshServerCmd string + timeout time.Duration + stdin *os.File + stdout *os.File +} + func runLogsSession(stdout, stdin *os.File, client clientpkg.WorkspaceClient) error { sshClient, err := ssh.StdioClientWithUser(stdout, stdin, "", false) if err != nil { diff --git a/cmd/workspace/ssh.go b/cmd/workspace/ssh.go index 162374954..9e6c03bb6 100644 --- a/cmd/workspace/ssh.go +++ b/cmd/workspace/ssh.go @@ -84,6 +84,63 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command { }, } + cmd.registerFlags(sshCmd) + + return sshCmd +} + +// Run runs the command logic. +func (cmd *SSHCmd) Run( + ctx context.Context, + devsyConfig *config.Config, + client client2.BaseWorkspaceClient, +) error { + cmd.addPrivateKeysToAgentIfEnabled(ctx, devsyConfig) + + // get user + if cmd.User == "" { + var err error + cmd.User, err = devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + return err + } + } + + // set default context if needed + if cmd.Context == "" { + cmd.Context = devsyConfig.DefaultContext + } + + workspaceClient, ok := client.(client2.WorkspaceClient) + if ok { + return cmd.jumpContainer(ctx, devsyConfig, workspaceClient) + } + proxyClient, ok := client.(client2.ProxyClient) + if ok { + return cmd.startProxyTunnel(ctx, devsyConfig, proxyClient) + } + daemonClient, ok := client.(client2.DaemonClient) + if ok { + return cmd.jumpContainerTailscale(ctx, devsyConfig, daemonClient) + } + + return nil +} + +func (cmd *SSHCmd) registerFlags(sshCmd *cobra.Command) { + cmd.registerPortForwardingFlags(sshCmd) + cmd.registerEnvFlags(sshCmd) + cmd.registerSessionFlags(sshCmd) + cmd.registerAgentForwardingFlags(sshCmd) + cmd.registerServiceFlags(sshCmd) + cmd.registerTerminalFlags(sshCmd) +} + +func (cmd *SSHCmd) registerPortForwardingFlags(sshCmd *cobra.Command) { cliflags.Add( sshCmd, cliflags.StringArray( @@ -102,16 +159,28 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command { "host are to be reverse forwarded to the given host and port, or Unix socket, on the remote side.", ). Shorthand("R"), - cliflags.StringArray(&cmd.SendEnvVars, names.SendEnv, nil, - "Specifies which local env variables shall be sent to the container."), - cliflags.StringArray(&cmd.SetEnvVars, names.SetEnv, nil, - "Specifies env variables to be set in the container."), cliflags.String( &cmd.ForwardPortsTimeout, names.ForwardPortsTimeout, "", "Specifies the timeout after which the command should terminate when the ports are unused.", ), + ) +} + +func (cmd *SSHCmd) registerEnvFlags(sshCmd *cobra.Command) { + cliflags.Add( + sshCmd, + cliflags.StringArray(&cmd.SendEnvVars, names.SendEnv, nil, + "Specifies which local env variables shall be sent to the container."), + cliflags.StringArray(&cmd.SetEnvVars, names.SetEnv, nil, + "Specifies env variables to be set in the container."), + ) +} + +func (cmd *SSHCmd) registerSessionFlags(sshCmd *cobra.Command) { + cliflags.Add( + sshCmd, cliflags.String( &cmd.Command, names.Command, @@ -120,6 +189,18 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command { ), cliflags.String(&cmd.User, names.User, "", "The user of the workspace to use"), cliflags.String(&cmd.WorkDir, names.Workdir, "", "The working directory in the container"), + cliflags.Bool( + &cmd.Stdio, + names.Stdio, + false, + "If true will tunnel connection through stdout and stdin", + ), + ) +} + +func (cmd *SSHCmd) registerAgentForwardingFlags(sshCmd *cobra.Command) { + cliflags.Add( + sshCmd, cliflags.Bool(&cmd.AgentForwarding, names.AgentForwarding, true, "If true forward the local ssh keys to the remote machine"), cliflags.String(&cmd.ReuseSSHAuthSock, names.ReuseSSHAuthSock, "", @@ -128,67 +209,29 @@ func NewSSHCmd(f *flags.GlobalFlags) *cobra.Command { Hidden(), cliflags.Bool(&cmd.GPGAgentForwarding, names.SSHGPGForwarding, false, "Forward the local gpg-agent to the remote machine"), - cliflags.Bool( - &cmd.Stdio, - names.Stdio, - false, - "If true will tunnel connection through stdout and stdin", - ), + cliflags.String(&cmd.GitSSHSigningKey, names.GitSSHSigningKey, "", + "The SSH signing key to use for git commit signing inside the workspace"), + ) +} + +func (cmd *SSHCmd) registerServiceFlags(sshCmd *cobra.Command) { + cliflags.Add( + sshCmd, cliflags.Bool(&cmd.StartServices, names.StartServices, true, "If false will not start any port-forwarding or git / docker credentials helper"), cliflags.Duration(&cmd.SSHKeepAliveInterval, names.SSHKeepAliveInterval, 55*time.Second, "How often should keepalive request be made (55s)"), - cliflags.String(&cmd.GitSSHSigningKey, names.GitSSHSigningKey, "", - "The SSH signing key to use for git commit signing inside the workspace"), + ) +} + +func (cmd *SSHCmd) registerTerminalFlags(sshCmd *cobra.Command) { + cliflags.Add( + sshCmd, cliflags.String(&cmd.TermMode, names.TermMode, machine.TermModeAuto, "PTY TERM selection mode: auto, strict, fallback"), cliflags.Bool(&cmd.InstallTerminfo, names.InstallTerminfo, false, "Install local TERM terminfo on remote before PTY"), ) - - return sshCmd -} - -// Run runs the command logic. -func (cmd *SSHCmd) Run( - ctx context.Context, - devsyConfig *config.Config, - client client2.BaseWorkspaceClient, -) error { - cmd.addPrivateKeysToAgentIfEnabled(ctx, devsyConfig) - - // get user - if cmd.User == "" { - var err error - cmd.User, err = devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - return err - } - } - - // set default context if needed - if cmd.Context == "" { - cmd.Context = devsyConfig.DefaultContext - } - - workspaceClient, ok := client.(client2.WorkspaceClient) - if ok { - return cmd.jumpContainer(ctx, devsyConfig, workspaceClient) - } - proxyClient, ok := client.(client2.ProxyClient) - if ok { - return cmd.startProxyTunnel(ctx, devsyConfig, proxyClient) - } - daemonClient, ok := client.(client2.DaemonClient) - if ok { - return cmd.jumpContainerTailscale(ctx, devsyConfig, daemonClient) - } - - return nil } func (cmd *SSHCmd) addPrivateKeysToAgentIfEnabled(ctx context.Context, devsyConfig *config.Config) { @@ -388,17 +431,9 @@ func (cmd *SSHCmd) startTunnel( } cmd.startTunnelServices(ctx, devsyConfig, containerClient, workspaceClient) - // buildSSHServerCommand runs `devsy internal ssh-server`, which always - // logs structured JSON on stderr; PipeJSONStream re-emits each line at - // its original level instead of double-wrapping it as another log entry. - writer, writerDone := log.PipeJSONStream() - defer func() { - _ = writer.Close() - <-writerDone - }() - gpgTunnel := newGPGTunnel(cmd, devsyConfig) - defer runGPGTunnelInBackground(ctx, gpgTunnel, containerClient)() + writer, cleanup := cmd.setupTunnelWriter(ctx, devsyConfig, containerClient) + defer cleanup() workdir := resolveWorkdir(cmd.WorkDir, workspaceClient) @@ -422,29 +457,75 @@ func (cmd *SSHCmd) startTunnel( }) } + return cmd.runInteractiveTunnelSession(ctx, runInteractiveTunnelSessionParams{ + devsyConfig: devsyConfig, + containerClient: containerClient, + command: command, + envVars: envVars, + writer: writer, + }) +} + +// setupTunnelWriter wires up the JSON log pipe and GPG agent tunnel shared by +// both tunnel modes. buildSSHServerCommand runs `devsy internal ssh-server`, +// which always logs structured JSON on stderr; PipeJSONStream re-emits each +// line at its original level instead of double-wrapping it as another log +// entry. The returned cleanup func stops the GPG tunnel before closing and +// draining the writer, matching the original defer ordering. +func (cmd *SSHCmd) setupTunnelWriter( + ctx context.Context, + devsyConfig *config.Config, + containerClient *ssh.Client, +) (io.Writer, func()) { + writer, writerDone := log.PipeJSONStream() + + gpgTunnel := newGPGTunnel(cmd, devsyConfig) + stopGPGTunnel := runGPGTunnelInBackground(ctx, gpgTunnel, containerClient) + + return writer, func() { + stopGPGTunnel() + _ = writer.Close() + <-writerDone + } +} + +type runInteractiveTunnelSessionParams struct { + devsyConfig *config.Config + containerClient *ssh.Client + command string + envVars map[string]string + writer io.Writer +} + +func (cmd *SSHCmd) runInteractiveTunnelSession( + ctx context.Context, + params runInteractiveTunnelSessionParams, +) error { return machine.StartSSHSession(ctx, machine.StartSSHSessionOptions{ User: cmd.User, Command: cmd.Command, AgentForwarding: cmd.AgentForwarding && - devsyConfig.ContextOption(config.ContextOptionSSHAgentForwarding) == config.BoolTrue, + params.devsyConfig.ContextOption( + config.ContextOptionSSHAgentForwarding, + ) == config.BoolTrue, SessionOptions: machine.SSHSessionOptions{ TermMode: cmd.TermMode, InstallTerminfo: cmd.InstallTerminfo, }, Exec: func(ctx context.Context, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { if cmd.SSHKeepAliveInterval != DisableSSHKeepAlive { - go startSSHKeepAlive(ctx, containerClient, cmd.SSHKeepAliveInterval) + go startSSHKeepAlive(ctx, params.containerClient, cmd.SSHKeepAliveInterval) } return devssh.Run(ctx, devssh.RunOptions{ - Client: containerClient, - Command: command, + Client: params.containerClient, + Command: params.command, Stdin: stdin, Stdout: stdout, Stderr: stderr, - EnvVars: envVars, + EnvVars: params.envVars, }) }, - Stderr: writer, + Stderr: params.writer, }) } diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index d4e5307ed..20a8675ec 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -172,6 +172,12 @@ func (cmd *UpCmd) registerPodmanFlags(upCmd *cobra.Command) { } func (cmd *UpCmd) registerWorkspaceFlags(upCmd *cobra.Command) { + cmd.registerWorkspaceIdentityFlags(upCmd) + cmd.registerWorkspaceSecretsFlags(upCmd) + cmd.registerWorkspaceRuntimeFlags(upCmd) +} + +func (cmd *UpCmd) registerWorkspaceIdentityFlags(upCmd *cobra.Command) { flags.Add( upCmd, flags.String(&cmd.ID, names.ID, "", "ID for the workspace"), @@ -206,6 +212,12 @@ func (cmd *UpCmd) registerWorkspaceFlags(upCmd *cobra.Command) { "report it instead"), flags.StringSlice(&cmd.PrebuildRepositories, names.PrebuildRepo, nil, "Docker repository hosting prebuilds for this workspace"), + ) +} + +func (cmd *UpCmd) registerWorkspaceSecretsFlags(upCmd *cobra.Command) { + flags.Add( + upCmd, flags.StringArray(&cmd.WorkspaceEnv, names.WorkspaceEnv, nil, "Env var for the workspace (KEY=VALUE, repeatable)"), flags.StringSlice(&cmd.WorkspaceEnvFile, names.WorkspaceEnvFile, nil, @@ -230,6 +242,12 @@ func (cmd *UpCmd) registerWorkspaceFlags(upCmd *cobra.Command) { "Env var for workspace initialization (KEY=VALUE, repeatable)"), flags.Bool(&cmd.DisableDaemon, names.DisableDaemon, false, "Do not install the activity-tracking daemon on the target machine"), + ) +} + +func (cmd *UpCmd) registerWorkspaceRuntimeFlags(upCmd *cobra.Command) { + flags.Add( + upCmd, flags.StringArray(&cmd.CacheFrom, names.CacheFrom, nil, "Build cache source (e.g. myregistry.io/cache:latest or type=registry,ref=...); "+ "takes priority over devcontainer.json build.cacheFrom"), diff --git a/pkg/client/clientimplementation/daemonclient/form.go b/pkg/client/clientimplementation/daemonclient/form.go index 57eb40cdc..af823a917 100644 --- a/pkg/client/clientimplementation/daemonclient/form.go +++ b/pkg/client/clientimplementation/daemonclient/form.go @@ -29,19 +29,60 @@ func createInstanceInteractive( formCtx, cancelForm := context.WithCancel(ctx) defer cancelForm() + selection, err := selectProjectClusterTemplate(ctx, formCtx, baseClient, cancelForm) + if err != nil { + return nil, err + } + + renderedParameters, err := resolveNewInstanceParameters( + formCtx, + selection.template, + selection.templateVersion, + ) + if err != nil { + return nil, err + } + + return buildNewInstance(buildNewInstanceParams{ + id: id, + uid: uid, + source: source, + picture: picture, + selectedProject: selection.project, + selectedCluster: selection.cluster, + selectedTemplate: selection.template, + selectedTemplateVersion: selection.templateVersion, + renderedParameters: renderedParameters, + }), nil +} + +type projectClusterTemplateSelection struct { + project *managementv1.Project + cluster *managementv1.Cluster + template *managementv1.DevsyWorkspaceTemplate + templateVersion string +} + +func selectProjectClusterTemplate( + ctx, formCtx context.Context, + baseClient platformclient.Client, + cancelForm CancelFunc, +) (projectClusterTemplateSelection, error) { var selectedCluster *managementv1.Cluster var selectedProject *managementv1.Project var selectedTemplate *managementv1.DevsyWorkspaceTemplate selectedTemplateVersion := "" - projectOptions, err := projectOptions(ctx, baseClient) + + options, err := projectOptions(ctx, baseClient) if err != nil { - return nil, err + return projectClusterTemplateSelection{}, err } + err = huh.NewForm( huh.NewGroup( huh.NewSelect[*managementv1.Project](). Title("Project"). - Options(projectOptions...). + Options(options...). Value(&selectedProject), huh.NewSelect[*managementv1.Cluster](). Title("Cluster"). @@ -66,65 +107,84 @@ func createInstanceInteractive( ), ).RunWithContext(formCtx) if err != nil { - return nil, err + return projectClusterTemplateSelection{}, err } + return projectClusterTemplateSelection{ + project: selectedProject, + cluster: selectedCluster, + template: selectedTemplate, + templateVersion: selectedTemplateVersion, + }, nil +} + +func resolveNewInstanceParameters( + formCtx context.Context, + selectedTemplate *managementv1.DevsyWorkspaceTemplate, + selectedTemplateVersion string, +) (string, error) { parameters := selectedTemplate.Spec.Parameters if len(selectedTemplate.GetVersions()) > 0 { + var err error parameters, err = list.GetTemplateParameters(selectedTemplate, selectedTemplateVersion) if err != nil { - return nil, err + return "", err } } + if len(parameters) == 0 { + return "", nil + } - renderedParameters := "" - if len(parameters) > 0 { - fieldParameters := prepareParameters(parameters) - err = huh.NewForm( - huh.NewGroup(parameterFields(fieldParameters)...), - ).RunWithContext(formCtx) - if err != nil { - return nil, err - } - - renderedParameters, err = renderParameters(fieldParameters) - if err != nil { - return nil, err - } + fieldParameters := prepareParameters(parameters) + if err := huh.NewForm( + huh.NewGroup(parameterFields(fieldParameters)...), + ).RunWithContext(formCtx); err != nil { + return "", err } - instance := &managementv1.DevsyWorkspaceInstance{ + return renderParameters(fieldParameters) +} + +type buildNewInstanceParams struct { + id, uid, source, picture string + selectedProject *managementv1.Project + selectedCluster *managementv1.Cluster + selectedTemplate *managementv1.DevsyWorkspaceTemplate + selectedTemplateVersion string + renderedParameters string +} + +func buildNewInstance(p buildNewInstanceParams) *managementv1.DevsyWorkspaceInstance { + return &managementv1.DevsyWorkspaceInstance{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: encoding.SafeConcatNameMax([]string{id}, 53) + "-", - Namespace: project.ProjectNamespace(selectedProject.GetName()), + GenerateName: encoding.SafeConcatNameMax([]string{p.id}, 53) + "-", + Namespace: project.ProjectNamespace(p.selectedProject.GetName()), Labels: map[string]string{ - storagev1.DevsyWorkspaceIDLabel: id, - storagev1.DevsyWorkspaceUIDLabel: uid, - config.K8sProjectLabel: selectedProject.GetName(), + storagev1.DevsyWorkspaceIDLabel: p.id, + storagev1.DevsyWorkspaceUIDLabel: p.uid, + config.K8sProjectLabel: p.selectedProject.GetName(), }, Annotations: map[string]string{ - storagev1.DevsyWorkspacePictureAnnotation: picture, - storagev1.DevsyWorkspaceSourceAnnotation: source, + storagev1.DevsyWorkspacePictureAnnotation: p.picture, + storagev1.DevsyWorkspaceSourceAnnotation: p.source, }, }, Spec: managementv1.DevsyWorkspaceInstanceSpec{ DevsyWorkspaceInstanceSpec: storagev1.DevsyWorkspaceInstanceSpec{ - DisplayName: id, + DisplayName: p.id, TemplateRef: &storagev1.TemplateRef{ - Name: selectedTemplate.GetName(), - Version: selectedTemplateVersion, + Name: p.selectedTemplate.GetName(), + Version: p.selectedTemplateVersion, }, Target: storagev1.WorkspaceTarget{ Cluster: &storagev1.WorkspaceTargetName{ - Name: selectedCluster.GetName(), + Name: p.selectedCluster.GetName(), }, }, - Parameters: renderedParameters, + Parameters: p.renderedParameters, }, }, } - - return instance, nil } func updateInstanceInteractive( diff --git a/pkg/client/clientimplementation/daemonclient/up.go b/pkg/client/clientimplementation/daemonclient/up.go index 14bd8572a..e3cea9df7 100644 --- a/pkg/client/clientimplementation/daemonclient/up.go +++ b/pkg/client/clientimplementation/daemonclient/up.go @@ -357,7 +357,37 @@ func printLogs( taskID string, reporter status.Reporter, ) (int, error) { - // get logs reader + logsReader, err := openTaskLogsStream(ctx, managementClient, workspace, taskID) + if err != nil { + return -1, err + } + defer func() { _ = logsReader.Close() }() + + scanner := newLogScanner(logsReader) + + streams := newLogOutputStreams(reporter) + defer streams.Close() + + exitCode, done, err := streamLogMessages(scanner, streams.stdout(), streams.stderrStreamer) + if done { + return exitCode, err + } + if err := scanner.Err(); err != nil { + if errors.Is(err, context.Canceled) { + return 0, nil + } + return -1, fmt.Errorf("logs reader error: %w", err) + } + + return 0, nil +} + +func openTaskLogsStream( + ctx context.Context, + managementClient kube.Interface, + workspace *managementv1.DevsyWorkspaceInstance, + taskID string, +) (io.ReadCloser, error) { log.Debugf("printing logs of task: %s", taskID) logsReader, err := managementClient.Loft().ManagementV1().RESTClient().Get(). Namespace(workspace.Namespace). @@ -370,12 +400,13 @@ func printLogs( }, builders.ParameterCodec). Stream(ctx) if err != nil { - return -1, fmt.Errorf("error getting task logs: %w", err) + return nil, fmt.Errorf("error getting task logs: %w", err) } - defer func() { _ = logsReader.Close() }() + return logsReader, nil +} - // create scanner from logs reader - scanner := bufio.NewScanner(logsReader) +func newLogScanner(r io.Reader) *bufio.Scanner { + scanner := bufio.NewScanner(r) // Increase the maximum token size to handle very long lines. // Here, we set a maximum capacity of 1MB. @@ -383,48 +414,73 @@ func printLogs( buf := make([]byte, 1024) // starting buffer size of 1KB scanner.Buffer(buf, maxCapacity) - // create json streamer + return scanner +} + +// logOutputStreams bundles the stdout/stderr JSON streamers used to print +// task logs, plus the status-sniffing writer wrapped around stdout. +type logOutputStreams struct { + stdoutStreamer io.WriteCloser + stdoutDone chan struct{} + stderrStreamer io.WriteCloser + stderrDone chan struct{} + statusWriter *statusSniffingWriter +} + +func newLogOutputStreams(reporter status.Reporter) *logOutputStreams { stdoutStreamer, stdoutDone := log.PipeJSONStream() stderrStreamer, stderrDone := log.PipeJSONStream() - defer func() { - // close the streams - _ = stdoutStreamer.Close() - _ = stderrStreamer.Close() - - // wait for the streams to be closed - <-stdoutDone - <-stderrDone - }() // The remote task runs the same devsy CLI, so its stdout carries the // same NDJSON status lines a local `up` does; sniff them out here. statusWriter := newStatusSniffingWriter(stdoutStreamer, reporter) - defer func() { _ = statusWriter.Close() }() - stdout := io.Writer(statusWriter) - // loop over all lines + return &logOutputStreams{ + stdoutStreamer: stdoutStreamer, + stdoutDone: stdoutDone, + stderrStreamer: stderrStreamer, + stderrDone: stderrDone, + statusWriter: statusWriter, + } +} + +func (s *logOutputStreams) Close() { + _ = s.statusWriter.Close() + _ = s.stdoutStreamer.Close() + _ = s.stderrStreamer.Close() + <-s.stdoutDone + <-s.stderrDone +} + +func (s *logOutputStreams) stdout() io.Writer { + return s.statusWriter +} + +// streamLogMessages reads NDJSON-encoded Message lines from scanner and +// writes their payloads to stdout/stderr until an ExitCode message, a write +// error, or EOF is reached. done reports whether the caller should return +// immediately with (exitCode, err) rather than falling through to +// scanner.Err(). +func streamLogMessages(scanner *bufio.Scanner, stdout, stderr io.Writer) (int, bool, error) { for scanner.Scan() { line := scanner.Text() - // parse message message := &Message{} if err := json.Unmarshal([]byte(line), message); err != nil { - return -1, fmt.Errorf("error parsing JSON from logs reader: %w, line: %s", err, line) + return -1, true, fmt.Errorf( + "error parsing JSON from logs reader: %w, line: %s", + err, + line, + ) } - exitCode, done, err := writeMessage(stdout, stderrStreamer, message) + exitCode, done, err := writeMessage(stdout, stderr, message) if done { - return exitCode, err - } - } - if err := scanner.Err(); err != nil { - if errors.Is(err, context.Canceled) { - return 0, nil + return exitCode, true, err } - return -1, fmt.Errorf("logs reader error: %w", err) } - return 0, nil + return 0, false, nil } func writeMessage(stdout, stderr io.Writer, message *Message) (int, bool, error) { diff --git a/pkg/devcontainer/compose_build.go b/pkg/devcontainer/compose_build.go index 772eb15e9..79af1ddae 100644 --- a/pkg/devcontainer/compose_build.go +++ b/pkg/devcontainer/compose_build.go @@ -125,28 +125,79 @@ func (r *runner) buildAndExtendDockerCompose( if err != nil { return composeExtendResult{}, err } - extendImageBuildInfo := prepared.extendImageBuildInfo + + target, err := r.resolveComposeBuildTarget(prepared, params) + defer target.cleanup() + if err != nil { + return composeExtendResult{buildImageName: target.buildImageName}, err + } + + if err := r.runComposeExtendedBuild(ctx, params, target.dockerComposeFilePath); err != nil { + return composeExtendResult{buildImageName: target.buildImageName}, err + } + + imageMetadata, err := metadata.GetDevContainerMetadata( + params.substitutionContext, + prepared.imageBuildInfo.Metadata, + params.parsedConfig, + prepared.extendImageBuildInfo.Features, + ) + if err != nil { + return composeExtendResult{buildImageName: target.buildImageName}, err + } + + return composeExtendResult{ + buildImageName: target.buildImageName, + composeBuildFilePath: target.dockerComposeFilePath, + imageMetadata: imageMetadata, + metadataLabel: prepared.extendImageBuildInfo.MetadataLabel, + }, nil +} + +// composeBuildTarget holds the resolved image name, override compose file +// path (if any), and cleanup for the feature override compose file. +type composeBuildTarget struct { + buildImageName string + dockerComposeFilePath string + cleanup func() +} + +// resolveComposeBuildTarget determines the image name to build and, when the +// build has features, writes the feature override compose file to build +// against. The returned cleanup func is always non-nil and safe to defer. +func (r *runner) resolveComposeBuildTarget( + prepared preparedComposeBuild, + params *buildAndExtendParams, +) (composeBuildTarget, error) { + target := composeBuildTarget{cleanup: func() {}} buildImageName, err := composeBuildImageName( params.composeHelper, params.project.Name, params.composeService, - hasFeatureBuildInfo(extendImageBuildInfo), + hasFeatureBuildInfo(prepared.extendImageBuildInfo), ) + target.buildImageName = buildImageName if err != nil { - return composeExtendResult{}, err + return target, err } - dockerComposeFilePath, cleanup, err := r.composeFeatureOverride( + target.dockerComposeFilePath, target.cleanup, err = r.composeFeatureOverride( prepared, params.composeService, buildImageName, ) - defer cleanup() - if err != nil { - return composeExtendResult{buildImageName: buildImageName}, err - } + return target, err +} +// runComposeExtendedBuild assembles the compose build arguments and runs +// "docker compose ... build" against the (possibly feature-extended) compose +// override file. +func (r *runner) runComposeExtendedBuild( + ctx context.Context, + params *buildAndExtendParams, + dockerComposeFilePath string, +) error { buildArgs := composeBuildArgs(&composeBuildArgsParams{ projectName: params.project.Name, globalArgs: params.globalArgs, @@ -157,26 +208,7 @@ func (r *runner) buildAndExtendDockerCompose( runServices: params.parsedConfig.Config.RunServices, }) - if err := r.runComposeBuild(ctx, params.composeHelper, buildArgs); err != nil { - return composeExtendResult{buildImageName: buildImageName}, err - } - - imageMetadata, err := metadata.GetDevContainerMetadata( - params.substitutionContext, - prepared.imageBuildInfo.Metadata, - params.parsedConfig, - extendImageBuildInfo.Features, - ) - if err != nil { - return composeExtendResult{buildImageName: buildImageName}, err - } - - return composeExtendResult{ - buildImageName: buildImageName, - composeBuildFilePath: dockerComposeFilePath, - imageMetadata: imageMetadata, - metadataLabel: extendImageBuildInfo.MetadataLabel, - }, nil + return r.runComposeBuild(ctx, params.composeHelper, buildArgs) } // composeFeatureOverride builds the feature override compose file when the build diff --git a/pkg/devcontainer/config/merge.go b/pkg/devcontainer/config/merge.go index 0e4721fe7..415e8c514 100644 --- a/pkg/devcontainer/config/merge.go +++ b/pkg/devcontainer/config/merge.go @@ -38,19 +38,7 @@ func MergeConfiguration( config *DevContainerConfig, imageMetadataEntries []*ImageMetadata, ) (*MergedDevContainerConfig, error) { - // When no image metadata entries are provided, synthesize one from the - // supplied config so that lifecycle hooks, customizations, mounts, etc. - // declared directly in the devcontainer.json still propagate through the - // merge. Callers that build a proper metadata chain (single image, compose, - // etc.) call AddConfigToImageMetadata first and pass a non-empty list, in - // which case the user config is already represented there. - if len(imageMetadataEntries) == 0 && config != nil { - userMetadata := &ImageMetadata{} - userMetadata.DevContainerConfigBase = config.DevContainerConfigBase - userMetadata.DevContainerActions = config.DevContainerActions - userMetadata.NonComposeBase = config.NonComposeBase - imageMetadataEntries = []*ImageMetadata{userMetadata} - } + imageMetadataEntries = ensureImageMetadataEntries(config, imageMetadataEntries) customizations := collectCustomizations(imageMetadataEntries) @@ -59,8 +47,44 @@ func MergeConfiguration( // reverse the order reversed := ReverseSlice(imageMetadataEntries) - // merge config - mergedConfig := &MergedDevContainerConfig{ + mergedConfig := newMergedDevContainerConfig(copiedConfig, customizations) + + // adjust config + mergeRuntimeFields(mergedConfig, reversed) + mergedConfig.Mounts = mergeMounts(reversed) + mergeLifecycleHookFields(mergedConfig, reversed) + mergeUserAndEnvFields(mergedConfig, reversed, copiedConfig) + mergePortsAndShutdownFields(mergedConfig, reversed, copiedConfig) + + return mergedConfig, nil +} + +// ensureImageMetadataEntries synthesizes an ImageMetadata entry from the +// supplied config when no image metadata entries are provided, so that +// lifecycle hooks, customizations, mounts, etc. declared directly in the +// devcontainer.json still propagate through the merge. Callers that build a +// proper metadata chain (single image, compose, etc.) call +// AddConfigToImageMetadata first and pass a non-empty list, in which case the +// user config is already represented there. +func ensureImageMetadataEntries( + config *DevContainerConfig, + imageMetadataEntries []*ImageMetadata, +) []*ImageMetadata { + if len(imageMetadataEntries) != 0 || config == nil { + return imageMetadataEntries + } + userMetadata := &ImageMetadata{} + userMetadata.DevContainerConfigBase = config.DevContainerConfigBase + userMetadata.DevContainerActions = config.DevContainerActions + userMetadata.NonComposeBase = config.NonComposeBase + return []*ImageMetadata{userMetadata} +} + +func newMergedDevContainerConfig( + copiedConfig *DevContainerConfig, + customizations map[string][]any, +) *MergedDevContainerConfig { + return &MergedDevContainerConfig{ UpdatedConfigProperties: UpdatedConfigProperties{ Customizations: customizations, }, @@ -75,8 +99,9 @@ func MergeConfiguration( // devcontainer.json file. Origin: copiedConfig.Origin, } +} - // adjust config +func mergeRuntimeFields(mergedConfig *MergedDevContainerConfig, reversed []*ImageMetadata) { mergedConfig.Init = some(reversed, func(entry *ImageMetadata) *bool { return entry.Init }) mergedConfig.Privileged = some( reversed, @@ -92,7 +117,9 @@ func MergeConfiguration( reversed, func(entry *ImageMetadata) string { return entry.Entrypoint }, ) - mergedConfig.Mounts = mergeMounts(reversed) +} + +func mergeLifecycleHookFields(mergedConfig *MergedDevContainerConfig, reversed []*ImageMetadata) { mergedConfig.OnCreateCommands = mergeLifestyleHooks( reversed, func(entry *ImageMetadata) types.LifecycleHook { return entry.OnCreateCommand }, @@ -113,6 +140,13 @@ func MergeConfiguration( reversed, func(entry *ImageMetadata) types.LifecycleHook { return entry.PostAttachCommand }, ) +} + +func mergeUserAndEnvFields( + mergedConfig *MergedDevContainerConfig, + reversed []*ImageMetadata, + copiedConfig *DevContainerConfig, +) { mergedConfig.WaitFor = firstString( reversed, func(entry *ImageMetadata) string { return entry.WaitFor }, @@ -139,6 +173,13 @@ func MergeConfiguration( reversed, func(entry *ImageMetadata) map[string]string { return entry.ContainerEnv }, ) +} + +func mergePortsAndShutdownFields( + mergedConfig *MergedDevContainerConfig, + reversed []*ImageMetadata, + copiedConfig *DevContainerConfig, +) { mergedConfig.PortsAttributes = mergeMaps( reversed, func(entry *ImageMetadata) map[string]PortAttribute { return entry.PortsAttributes }, @@ -162,8 +203,6 @@ func MergeConfiguration( if mergedConfig.ShutdownAction == "" { mergedConfig.ShutdownAction = defaultShutdownAction(copiedConfig) } - - return mergedConfig, nil } func collectCustomizations(entries []*ImageMetadata) map[string][]any { diff --git a/pkg/devcontainer/config/userenvprobe.go b/pkg/devcontainer/config/userenvprobe.go index 78736a313..0f61c9b92 100644 --- a/pkg/devcontainer/config/userenvprobe.go +++ b/pkg/devcontainer/config/userenvprobe.go @@ -51,12 +51,7 @@ func ProbeUserEnv( probe string, userName string, ) (map[string]string, error) { - userEnvProbe, err := NewUserEnvProbe(probe) - if err != nil { - log.Warnf("Get user env probe: %v", err) - log.Warnf("Falling back to default user env probe: %s", DefaultUserEnvProbe) - userEnvProbe = DefaultUserEnvProbe - } + userEnvProbe := resolveUserEnvProbe(probe) if userEnvProbe == NoneProbe { return map[string]string{}, nil } @@ -66,6 +61,31 @@ func ProbeUserEnv( return nil, fmt.Errorf("find shell for user %s: %w", userName, err) } + probedEnv := probeUserEnvWithFallback(ctx, userEnvProbe, preferredShell, userName) + if probedEnv == nil { + probedEnv = map[string]string{} + } + + return probedEnv, nil +} + +func resolveUserEnvProbe(probe string) UserEnvProbe { + userEnvProbe, err := NewUserEnvProbe(probe) + if err != nil { + log.Warnf("Get user env probe: %v", err) + log.Warnf("Falling back to default user env probe: %s", DefaultUserEnvProbe) + return DefaultUserEnvProbe + } + + return userEnvProbe +} + +func probeUserEnvWithFallback( + ctx context.Context, + userEnvProbe UserEnvProbe, + preferredShell []string, + userName string, +) map[string]string { log.Debugf( "running user env probe with shell %q, probe %q, user %q and command %q", strings.Join(preferredShell, " "), @@ -82,34 +102,32 @@ func ProbeUserEnv( "cat /proc/self/environ", '\x00', ) - if err != nil { - log.Debugf( - "running user env probe with shell %q, probe %q, user %q and command %q", - strings.Join(preferredShell, " "), - string(userEnvProbe), - userName, - "printenv", - ) - - newProbedEnv, newErr := doProbe( - ctx, - userEnvProbe, - preferredShell, - userName, - "printenv", - '\n', - ) - if newErr != nil { - log.Warnf("failed to probe user environment variables: %v, %v", err, newErr) - } else { - probedEnv = newProbedEnv - } + if err == nil { + return probedEnv } - if probedEnv == nil { - probedEnv = map[string]string{} + + log.Debugf( + "running user env probe with shell %q, probe %q, user %q and command %q", + strings.Join(preferredShell, " "), + string(userEnvProbe), + userName, + "printenv", + ) + + newProbedEnv, newErr := doProbe( + ctx, + userEnvProbe, + preferredShell, + userName, + "printenv", + '\n', + ) + if newErr != nil { + log.Warnf("failed to probe user environment variables: %v, %v", err, newErr) + return probedEnv } - return probedEnv, nil + return newProbedEnv } func parseProbeOutput(out []byte, sep byte) map[string]string { diff --git a/pkg/devcontainer/graph/graph_test.go b/pkg/devcontainer/graph/graph_test.go index eaedd0e69..0c2768848 100644 --- a/pkg/devcontainer/graph/graph_test.go +++ b/pkg/devcontainer/graph/graph_test.go @@ -231,69 +231,45 @@ func TestGraphSuite(t *testing.T) { suite.Run(t, new(GraphTestSuite)) } -func (suite *GraphTestSuite) TestEdgeCount() { - testCases := []struct { - name string - setup func() - expectedCount int - }{ - { - name: "empty graph", - setup: func() {}, - expectedCount: 0, - }, - { - name: "nodes without edges", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - }, - expectedCount: 0, - }, - { - name: "single edge", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - }, - expectedCount: 1, - }, - { - name: "multiple edges", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddNode("C", "dataC")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("A", "C")) - suite.Require().NoError(suite.graph.AddEdge("B", "C")) - }, - expectedCount: 3, - }, - { - name: "edges after removal", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddNode("C", "dataC")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("A", "C")) - suite.Require().NoError(suite.graph.RemoveEdge("A", "B")) - }, - expectedCount: 1, - }, - } +func (suite *GraphTestSuite) TestEdgeCount_EmptyGraph() { + suite.Equal(0, suite.graph.EdgeCount()) +} - for _, tc := range testCases { - suite.Run(tc.name, func() { - suite.SetupTest() - tc.setup() +func (suite *GraphTestSuite) TestEdgeCount_NodesWithoutEdges() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - count := suite.graph.EdgeCount() - suite.Equal(tc.expectedCount, count) - }) - } + suite.Equal(0, suite.graph.EdgeCount()) +} + +func (suite *GraphTestSuite) TestEdgeCount_SingleEdge() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + + suite.Equal(1, suite.graph.EdgeCount()) +} + +func (suite *GraphTestSuite) TestEdgeCount_MultipleEdges() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddNode("C", "dataC")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("A", "C")) + suite.Require().NoError(suite.graph.AddEdge("B", "C")) + + suite.Equal(3, suite.graph.EdgeCount()) +} + +func (suite *GraphTestSuite) TestEdgeCount_EdgesAfterRemoval() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddNode("C", "dataC")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("A", "C")) + suite.Require().NoError(suite.graph.RemoveEdge("A", "B")) + + suite.Equal(1, suite.graph.EdgeCount()) } func (suite *GraphTestSuite) TestIsEmpty() { @@ -420,85 +396,58 @@ func (suite *GraphTestSuite) TestGetAllNodes() { suite.Equal("dataB", nodes["B"]) } -func (suite *GraphTestSuite) TestEdgeCases() { - testCases := []struct { - name string - test func() - }{ - { - name: "duplicate edge prevention", - test: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) // duplicate - suite.Equal(1, suite.graph.EdgeCount()) - }, - }, - { - name: "remove non-existing node", - test: func() { - err := suite.graph.RemoveNode("missing") - suite.Error(err) - }, - }, - { - name: "get children of non-existing node", - test: func() { - children := suite.graph.GetChildren("missing") - suite.Empty(children) - }, - }, - { - name: "get parents of non-existing node", - test: func() { - parents := suite.graph.GetParents("missing") - suite.Empty(parents) - }, - }, - { - name: "remove edge to non-existing target", - test: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - err := suite.graph.RemoveEdge("A", "missing") - suite.Error(err) - }, - }, - { - name: "complex circular dependency", - test: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddNode("C", "dataC")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("B", "C")) - suite.Require().NoError(suite.graph.AddEdge("C", "A")) // creates cycle - suite.True(suite.graph.HasCircularDependency()) - }, - }, - { - name: "self loop", - test: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddEdge("A", "A")) - suite.True(suite.graph.HasCircularDependency()) - }, - }, - { - name: "remove subgraph of non-existing node", - test: func() { - err := suite.graph.RemoveSubGraph("missing") - suite.NoError(err) // should not error - }, - }, - } +func (suite *GraphTestSuite) TestEdgeCases_DuplicateEdgePrevention() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) // duplicate - for _, tc := range testCases { - suite.Run(tc.name, func() { - suite.SetupTest() - tc.test() - }) - } + suite.Equal(1, suite.graph.EdgeCount()) +} + +func (suite *GraphTestSuite) TestEdgeCases_RemoveNonExistingNode() { + err := suite.graph.RemoveNode("missing") + suite.Error(err) +} + +func (suite *GraphTestSuite) TestEdgeCases_GetChildrenOfNonExistingNode() { + children := suite.graph.GetChildren("missing") + suite.Empty(children) +} + +func (suite *GraphTestSuite) TestEdgeCases_GetParentsOfNonExistingNode() { + parents := suite.graph.GetParents("missing") + suite.Empty(parents) +} + +func (suite *GraphTestSuite) TestEdgeCases_RemoveEdgeToNonExistingTarget() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + + err := suite.graph.RemoveEdge("A", "missing") + suite.Error(err) +} + +func (suite *GraphTestSuite) TestEdgeCases_ComplexCircularDependency() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddNode("C", "dataC")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("B", "C")) + suite.Require().NoError(suite.graph.AddEdge("C", "A")) // creates cycle + + suite.True(suite.graph.HasCircularDependency()) +} + +func (suite *GraphTestSuite) TestEdgeCases_SelfLoop() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddEdge("A", "A")) + + suite.True(suite.graph.HasCircularDependency()) +} + +func (suite *GraphTestSuite) TestEdgeCases_RemoveSubGraphOfNonExistingNode() { + err := suite.graph.RemoveSubGraph("missing") + suite.NoError(err) // should not error } func (suite *GraphTestSuite) TestIsReachable() { @@ -575,71 +524,40 @@ func (suite *GraphTestSuite) TestSortNodeIDsRoundBasedMultiLevel() { suite.Equal([]string{"A", "C", "B", "D"}, result) } -func (suite *GraphTestSuite) TestTopologicalSortAdvanced() { - testCases := []struct { - name string - setup func() - wantErr bool - verify func([]string) - }{ - { - name: "empty graph", - setup: func() {}, - wantErr: false, - verify: func(result []string) { - suite.Empty(result) - }, - }, - { - name: "diamond dependency", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddNode("C", "dataC")) - suite.Require().NoError(suite.graph.AddNode("D", "dataD")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("A", "C")) - suite.Require().NoError(suite.graph.AddEdge("B", "D")) - suite.Require().NoError(suite.graph.AddEdge("C", "D")) - }, - wantErr: false, - verify: func(result []string) { - suite.Len(result, 4) - suite.Equal("dataA", result[0]) - suite.Equal("dataD", result[3]) - }, - }, - { - name: "three node cycle", - setup: func() { - suite.Require().NoError(suite.graph.AddNode("A", "dataA")) - suite.Require().NoError(suite.graph.AddNode("B", "dataB")) - suite.Require().NoError(suite.graph.AddNode("C", "dataC")) - suite.Require().NoError(suite.graph.AddEdge("A", "B")) - suite.Require().NoError(suite.graph.AddEdge("B", "C")) - suite.Require().NoError(suite.graph.AddEdge("C", "A")) - }, - wantErr: true, - verify: func(result []string) { - suite.Nil(result) - }, - }, - } +func (suite *GraphTestSuite) TestTopologicalSortAdvanced_EmptyGraph() { + result, err := suite.graph.Sort() + suite.NoError(err) + suite.Empty(result) +} - for _, tc := range testCases { - suite.Run(tc.name, func() { - suite.SetupTest() - tc.setup() +func (suite *GraphTestSuite) TestTopologicalSortAdvanced_DiamondDependency() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddNode("C", "dataC")) + suite.Require().NoError(suite.graph.AddNode("D", "dataD")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("A", "C")) + suite.Require().NoError(suite.graph.AddEdge("B", "D")) + suite.Require().NoError(suite.graph.AddEdge("C", "D")) - result, err := suite.graph.Sort() - if tc.wantErr { - suite.Error(err) - } else { - suite.NoError(err) - } - tc.verify(result) - }) - } + result, err := suite.graph.Sort() + suite.NoError(err) + suite.Len(result, 4) + suite.Equal("dataA", result[0]) + suite.Equal("dataD", result[3]) +} + +func (suite *GraphTestSuite) TestTopologicalSortAdvanced_ThreeNodeCycle() { + suite.Require().NoError(suite.graph.AddNode("A", "dataA")) + suite.Require().NoError(suite.graph.AddNode("B", "dataB")) + suite.Require().NoError(suite.graph.AddNode("C", "dataC")) + suite.Require().NoError(suite.graph.AddEdge("A", "B")) + suite.Require().NoError(suite.graph.AddEdge("B", "C")) + suite.Require().NoError(suite.graph.AddEdge("C", "A")) + + result, err := suite.graph.Sort() + suite.Error(err) + suite.Nil(result) } func (suite *GraphTestSuite) TestSortWithPriorityRoundBased() { diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index d332f93f2..5af52c0a0 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -50,6 +50,8 @@ type InjectOptions struct { // Deprecated: Inject is part of the legacy shell injection path. Platform-native AgentDelivery // implementations (LocalDockerDelivery, RemoteDockerDelivery, KubernetesDelivery) are the replacements. +// +//nolint:funlen // legacy shell injection path, frozen pending caller migration to AgentDelivery func Inject(opts InjectOptions) (bool, error) { if err := validateInjectOptions(opts); err != nil { return false, err diff --git a/pkg/options/options_test.go b/pkg/options/options_test.go index b75073283..aaa91ebb9 100644 --- a/pkg/options/options_test.go +++ b/pkg/options/options_test.go @@ -1,7 +1,6 @@ package options import ( - "fmt" "os" "testing" @@ -20,94 +19,105 @@ type assignmentTestCase struct { ExpectedAssignments []string } -func TestInheritFromEnvironment(t *testing.T) { - testCases := []assignmentTestCase{ - { - Name: "assigned, not in the environment", - Names: []string{ - "HOST", - }, - Assignments: []string{ - "HOST=box", - }, - EnvironmentVariablePrefix: "DEVSY_PROVIDER_SSH_", - NotInEnvironment: []string{ - "DEVSY_PROVIDER_SSH_HOST", - }, - Environment: map[string]string{}, - ExpectedAssignments: []string{ - "HOST=box", - }, +const ( + testEnvHostName = "HOST" + testEnvHostAssign = "HOST=box" + testEnvSSHPrefix = "DEVSY_PROVIDER_SSH_" + testEnvSSHHostVar = "DEVSY_PROVIDER_SSH_HOST" +) + +var inheritFromEnvironmentTestCases = []assignmentTestCase{ + { + Name: "assigned, not in the environment", + Names: []string{ + testEnvHostName, }, - { - Name: "not assigned, not in the environment", - Names: []string{ - "HOST", - }, - Assignments: []string{}, - EnvironmentVariablePrefix: "DEVSY_PROVIDER_SSH_", - NotInEnvironment: []string{ - "DEVSY_PROVIDER_SSH_HOST", - }, - Environment: map[string]string{}, - ExpectedAssignments: []string{}, + Assignments: []string{ + testEnvHostAssign, }, - { - Name: "assigned, in the environment", - Names: []string{ - "HOST", - }, - Assignments: []string{ - "HOST=box", - }, - EnvironmentVariablePrefix: "DEVSY_PROVIDER_SSH_", - NotInEnvironment: []string{}, - Environment: map[string]string{ - "DEVSY_PROVIDER_SSH_HOST": "another-box", - }, - ExpectedAssignments: []string{ - "HOST=box", - }, + EnvironmentVariablePrefix: testEnvSSHPrefix, + NotInEnvironment: []string{ + testEnvSSHHostVar, }, - { - Name: "not assigned, in the environment", - Names: []string{ - "HOST", - }, - Assignments: []string{}, - EnvironmentVariablePrefix: "DEVSY_PROVIDER_SSH_", - NotInEnvironment: []string{}, - Environment: map[string]string{ - "DEVSY_PROVIDER_SSH_HOST": "another-box", - }, - ExpectedAssignments: []string{ - "HOST=another-box", - }, + Environment: map[string]string{}, + ExpectedAssignments: []string{ + testEnvHostAssign, }, - } + }, + { + Name: "not assigned, not in the environment", + Names: []string{ + testEnvHostName, + }, + Assignments: []string{}, + EnvironmentVariablePrefix: testEnvSSHPrefix, + NotInEnvironment: []string{ + testEnvSSHHostVar, + }, + Environment: map[string]string{}, + ExpectedAssignments: []string{}, + }, + { + Name: "assigned, in the environment", + Names: []string{ + testEnvHostName, + }, + Assignments: []string{ + testEnvHostAssign, + }, + EnvironmentVariablePrefix: testEnvSSHPrefix, + NotInEnvironment: []string{}, + Environment: map[string]string{ + testEnvSSHHostVar: "another-box", + }, + ExpectedAssignments: []string{ + testEnvHostAssign, + }, + }, + { + Name: "not assigned, in the environment", + Names: []string{ + testEnvHostName, + }, + Assignments: []string{}, + EnvironmentVariablePrefix: testEnvSSHPrefix, + NotInEnvironment: []string{}, + Environment: map[string]string{ + testEnvSSHHostVar: "another-box", + }, + ExpectedAssignments: []string{ + "HOST=another-box", + }, + }, +} - for _, testCase := range testCases { - fmt.Println(testCase.Name) +func TestInheritFromEnvironment(t *testing.T) { + for _, testCase := range inheritFromEnvironmentTestCases { + t.Run(testCase.Name, func(t *testing.T) { + runInheritFromEnvironmentTestCase(t, testCase) + }) + } +} - for _, k := range testCase.NotInEnvironment { - err := os.Unsetenv(k) - if err != nil { - t.Fatalf("unexpected error %v in %s", err, testCase.Name) - } +func runInheritFromEnvironmentTestCase(t *testing.T, testCase assignmentTestCase) { + for _, k := range testCase.NotInEnvironment { + err := os.Unsetenv(k) + if err != nil { + t.Fatalf("unexpected error %v in %s", err, testCase.Name) } - for k, v := range testCase.Environment { - err := os.Setenv(k, v) - if err != nil { - t.Fatalf("unexpected error %v in %s", err, testCase.Name) - } + } + for k, v := range testCase.Environment { + err := os.Setenv(k, v) + if err != nil { + t.Fatalf("unexpected error %v in %s", err, testCase.Name) } + } - result := InheritFromEnvironment( - testCase.Assignments, - testCase.Names, - testCase.EnvironmentVariablePrefix, - ) + result := InheritFromEnvironment( + testCase.Assignments, + testCase.Names, + testCase.EnvironmentVariablePrefix, + ) - assert.DeepEqual(t, result, testCase.ExpectedAssignments) - } + assert.DeepEqual(t, result, testCase.ExpectedAssignments) } diff --git a/pkg/options/resolve.go b/pkg/options/resolve.go index aabef7d68..d10c41d37 100644 --- a/pkg/options/resolve.go +++ b/pkg/options/resolve.go @@ -163,6 +163,10 @@ func ResolveOptions( skipSubOptions bool, singleMachine *bool, ) (*config.Config, error) { + if devConfig == nil { + return nil, nil + } + // get binary paths binaryPaths, err := provider.GetBinaries(devConfig.DefaultContext, providerConfig) if err != nil { @@ -198,28 +202,47 @@ func ResolveOptions( return nil, err } - // save options in dev config - if devConfig != nil { - devConfig = config.CloneConfig(devConfig) - if devConfig.Current().Providers == nil { - devConfig.Current().Providers = map[string]*config.ProviderConfig{} - } - if devConfig.Current().Providers[providerConfig.Name] == nil { - devConfig.Current().Providers[providerConfig.Name] = &config.ProviderConfig{} - } + return applyResolvedProviderOptions(devConfig, applyResolvedProviderOptionsParams{ + providerName: providerConfig.Name, + resolvedOptionValues: resolvedOptionValues, + dynamicOptionDefinitions: dynamicOptionDefinitions, + singleMachine: singleMachine, + }), nil +} + +type applyResolvedProviderOptionsParams struct { + providerName string + resolvedOptionValues map[string]config.OptionValue + dynamicOptionDefinitions config.OptionDefinitions + singleMachine *bool +} + +// applyResolvedProviderOptions clones devConfig and records the resolved +// option values, dynamic option definitions, and single-machine setting for +// the given provider. +func applyResolvedProviderOptions( + devConfig *config.Config, + p applyResolvedProviderOptionsParams, +) *config.Config { + devConfig = config.CloneConfig(devConfig) + if devConfig.Current().Providers == nil { + devConfig.Current().Providers = map[string]*config.ProviderConfig{} + } + if devConfig.Current().Providers[p.providerName] == nil { + devConfig.Current().Providers[p.providerName] = &config.ProviderConfig{} + } - providerCfg := devConfig.Current().Providers[providerConfig.Name] - providerCfg.Options = map[string]config.OptionValue{} - maps.Copy(providerCfg.Options, resolvedOptionValues) + providerCfg := devConfig.Current().Providers[p.providerName] + providerCfg.Options = map[string]config.OptionValue{} + maps.Copy(providerCfg.Options, p.resolvedOptionValues) - providerCfg.DynamicOptions = config.OptionDefinitions{} - maps.Copy(providerCfg.DynamicOptions, dynamicOptionDefinitions) - if singleMachine != nil { - providerCfg.SingleMachine = *singleMachine - } + providerCfg.DynamicOptions = config.OptionDefinitions{} + maps.Copy(providerCfg.DynamicOptions, p.dynamicOptionDefinitions) + if p.singleMachine != nil { + providerCfg.SingleMachine = *p.singleMachine } - return devConfig, nil + return devConfig } // ResolveAgentConfig resolves and returns the complete agent configuration for a provider. diff --git a/pkg/options/resolve_test.go b/pkg/options/resolve_test.go index aeecfa647..680034689 100644 --- a/pkg/options/resolve_test.go +++ b/pkg/options/resolve_test.go @@ -29,642 +29,688 @@ type testCase struct { ExpectErr bool ExpectedOptions map[string]string ExpectedDynamicOptions config.OptionDefinitions + + // FreshFilledKeys get Filled set to time.Now() right before Resolve runs. + FreshFilledKeys []string } -func TestResolveOptions(t *testing.T) { - testCases := []testCase{ - { - Name: "simple", - ExtraValues: map[string]string{ - "WORKSPACE_ID": "test", - }, - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "${WORKSPACE_ID}-test", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test-test", - }, - }, - { - Name: "dependency", - ExtraValues: map[string]string{ - "WORKSPACE_ID": "test", - }, - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "${WORKSPACE_ID}-test-${COMMAND}-$COMMAND", - }, - "COMMAND": { - Command: "echo bar", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test-test-bar-bar", - "COMMAND": "bar", - }, - }, - { - Name: "No extra values", - ProviderOptions: map[string]*types.Option{ - "COMMAND1": { - Command: "echo ${COMMAND2}-test", - }, - "COMMAND2": { - Command: "echo bar", - }, - }, - ExpectedOptions: map[string]string{ - "COMMAND1": "bar-test", - "COMMAND2": "bar", - }, - }, - { - Name: "Cyclic dep", - ProviderOptions: map[string]*types.Option{ - "COMMAND1": { - Command: "echo ${COMMAND2}", - }, - "COMMAND2": { - Command: "echo ${COMMAND1}", - }, - }, - ExpectErr: true, - }, - { - Name: "Override", - ResolvedValues: map[string]config.OptionValue{ - "COMMAND": { - Value: "foo", - UserProvided: true, - }, - }, - ProviderOptions: map[string]*types.Option{ - "COMMAND": { - Command: "echo bar", - }, - }, - ExpectedOptions: map[string]string{ - "COMMAND": "foo", - }, - }, - { - Name: "Override", - ResolvedValues: map[string]config.OptionValue{ - "COMMAND": { - Value: "foo", - UserProvided: true, - }, - }, - ProviderOptions: map[string]*types.Option{ - "COMMAND": { - Command: "echo bar", - }, - "COMMAND1": { - Command: "echo ${COMMAND}-foo-${UNDEFINED}", - }, - "DEFAULT1": { - Default: "${COMMAND}-foo-${UNDEFINED}", - }, - }, - ExpectedOptions: map[string]string{ - "COMMAND": "foo", - "COMMAND1": "foo-foo-", - "DEFAULT1": "foo-foo-${UNDEFINED}", - }, - }, - { - Name: "Expire", - ResolvedValues: map[string]config.OptionValue{ - "EXPIRE": { - Value: "foo", - Filled: &[]types.Time{types.NewTime(time.Time{})}[0], - }, - "NOTEXPIRE": { - Value: "foo", - Filled: &[]types.Time{types.Now()}[0], - }, - }, - ProviderOptions: map[string]*types.Option{ - "EXPIRE": { - Command: "echo bar", - Cache: "10m", - }, - "NOTEXPIRE": { - Command: "echo bar", - Cache: "10m", - }, - }, - ExpectedOptions: map[string]string{ - "EXPIRE": "bar", - "NOTEXPIRE": "foo", - }, - }, - { - Name: "Ignore self", - ProviderOptions: map[string]*types.Option{ - "SELF": { - Command: "SELF=test; echo ${SELF}", - }, - }, - ExpectedOptions: map[string]string{ - "SELF": "test", - }, - }, - { - Name: "Recompute children", - UserValues: map[string]string{ - "PARENT": "foo", - }, - ResolvedValues: map[string]config.OptionValue{ - "PARENT": { - Value: "test", - UserProvided: true, - }, - "CHILD1": { - Value: "test-child1", - }, - "CHILD2": { - Value: "test-child2", - }, - }, - ProviderOptions: map[string]*types.Option{ - "PARENT": {}, - "CHILD1": { - Command: "echo ${PARENT}-child1", - }, - "CHILD2": { - Default: "${PARENT}-child2", - }, - }, - ExpectedOptions: map[string]string{ - "PARENT": "foo", - "CHILD1": "foo-child1", - "CHILD2": "foo-child2", - }, - }, - { - Name: "Error local global", - ProviderOptions: map[string]*types.Option{ - "PARENT": { - Default: "test", - }, - "CHILD1": { - Global: true, - Default: "${PARENT}", - }, - }, - ExpectErr: true, - }, - { - Name: "Error local var", - ProviderOptions: map[string]*types.Option{ - "PARENT": { - Local: true, - Default: "test", - }, - "CHILD1": { - Default: "${PARENT}", - }, - }, - ExpectErr: true, - }, - { - Name: "Don't resolve local", - ProviderOptions: map[string]*types.Option{ - "PARENT": { - Default: "test", - }, - "CHILD1": { - Default: "${PARENT}", - Local: true, - }, - }, - DontResolveLocal: true, - ExpectedOptions: map[string]string{ - "PARENT": "test", - }, - }, - { - Name: "Resolve", - ProviderOptions: map[string]*types.Option{ - "PARENT": { - Default: "test", - }, - "CHILD1": { - Default: "${PARENT}", - }, - }, - DontResolveLocal: true, - ExpectedOptions: map[string]string{ - "PARENT": "test", - "CHILD1": "test", - }, - }, - { - Name: "Skip Required", - ProviderOptions: map[string]*types.Option{ - "PARENT": { - Required: true, - }, - "CHILD1": { - Default: "${PARENT}", - }, - "PARENT2": { - Required: true, - Default: "test", - }, - "CHILD2": { - Default: "${PARENT2}", - }, - }, - SkipRequired: true, - ExpectedOptions: map[string]string{ - "PARENT2": "test", - "CHILD2": "test", - }, - }, - { - Name: "Nested dynamic options", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ExpectedOptions: map[string]string{ - "TEST": "test", - "TEST2": "test2", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - }, - }, - }, - { - Name: "Dynamic options don't update", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ResolvedDynamicDefinitions: map[string]*types.Option{ - "TEST2": { - Default: "test5", - }, - }, - ResolvedValues: map[string]config.OptionValue{ - "TEST": {Value: "test3", Children: []string{"TEST2"}, UserProvided: true}, - "TEST2": {Value: "test4", UserProvided: true}, - }, - ExpectedOptions: map[string]string{ - "TEST": "test3", - "TEST2": "test4", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": { - Default: "test2", - }, - }, - }, - { - Name: "Dynamic options update", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test2", - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - UserValues: map[string]string{ - "TEST": "test1", - }, - ResolvedValues: map[string]config.OptionValue{ - "TEST": {Value: "test3", Children: []string{"TEST2"}}, - "TEST2": {Value: "test4"}, - }, - ResolvedDynamicDefinitions: map[string]*types.Option{ - "TEST2": { - Default: "test5", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST3": "test2", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test2", - }, - }, - }, - { - Name: "Nested dynamic options", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }), - }, - }), - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST2": "test2", - "TEST3": "test3", - "TEST4": "test3-bar-4", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }), - }, - }), - }, - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }), - }, - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }, - }, - { - Name: "Nested dynamic options skip required", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Required: true, - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }), - }, - }), - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - SkipRequired: true, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Required: true, - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST3}-${FOO}-4", - }, - }), - }, - }), - }, - }, - }, - { - Name: "Nested dynamic options use option", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Required: true, - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST2}-${FOO}-4", - }, - }), - }, - }), - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - SkipRequired: true, - UserValues: map[string]string{ - "TEST2": "test2", - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST2": "test2", - "TEST3": "test3", - "TEST4": "test2-bar-4", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Required: true, - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST2}-${FOO}-4", - }, - }), - }, - }), - }, - "TEST3": &types.Option{ - Default: "test3", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST4": &types.Option{ - Default: "${TEST2}-${FOO}-4", - }, - }), - }, - "TEST4": &types.Option{ - Default: "${TEST2}-${FOO}-4", - }, - }, - }, - { - Name: "Nested dynamic options use option", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - }, - }), - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ResolvedValues: map[string]config.OptionValue{ - "TEST5": { - Value: "test5", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST2": "test2", - "TEST3": "test3", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST3": &types.Option{ - Default: "test3", - }, - }), - }, - "TEST3": &types.Option{ - Default: "test3", - }, - }, - }, - { - Name: "Dynamic options unused option", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ResolvedValues: map[string]config.OptionValue{ - "TEST5": { - Value: "test5", - }, - }, - ResolvedDynamicDefinitions: map[string]*types.Option{ - "TEST5": { - Default: "test2", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST2": "test2", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test2", - }, - }, - }, - { - Name: "Dynamic options update default", - ProviderOptions: map[string]*types.Option{ - "TEST": { - Default: "test1", - SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test3", - }, - }), - }, - "FOO": {Command: "echo bar"}, - }, - ResolvedValues: map[string]config.OptionValue{ - "TEST": { - Value: "test1", - }, - "TEST2": { - Value: "test2", - }, - }, - ResolvedDynamicDefinitions: map[string]*types.Option{ - "TEST2": { - Default: "test2", - }, - }, - ExpectedOptions: map[string]string{ - "TEST": "test1", - "TEST2": "test3", - "FOO": "bar", - }, - ExpectedDynamicOptions: config.OptionDefinitions{ - "TEST2": &types.Option{ - Default: "test3", - }, +const ( + testOptTest = "TEST" + testValTest = "test" + testOptCommand = "COMMAND" + testCmdEchoBar = "echo bar" + testValBar = "bar" + testOptCmd1 = "COMMAND1" + testOptCmd2 = "COMMAND2" + testValFoo = "foo" + testOptExpire = "EXPIRE" + testOptNoExp = "NOTEXPIRE" + testOptParent = "PARENT" + testOptChild1 = "CHILD1" + testOptChild2 = "CHILD2" + testRefParent = "${PARENT}" + testOptTest2 = "TEST2" + testValTest2 = "test2" + testOptFoo = "FOO" + testValTest5 = "test5" + testValTest3 = "test3" + testValTest4 = "test4" + testOptTest3 = "TEST3" + testValTest1 = "test1" + testOptTest4 = "TEST4" + testRefChain34 = "${TEST3}-${FOO}-4" + testRefChain24 = "${TEST2}-${FOO}-4" + testOptTest5 = "TEST5" +) + +var resolveOptionsTestCases = []testCase{ + { + Name: "simple", + ExtraValues: map[string]string{ + "WORKSPACE_ID": testValTest, + }, + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: "${WORKSPACE_ID}-test", }, }, - } + ExpectedOptions: map[string]string{ + testOptTest: "test-test", + }, + }, + { + Name: "dependency", + ExtraValues: map[string]string{ + "WORKSPACE_ID": testValTest, + }, + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: "${WORKSPACE_ID}-test-${COMMAND}-$COMMAND", + }, + testOptCommand: { + Command: testCmdEchoBar, + }, + }, + ExpectedOptions: map[string]string{ + testOptTest: "test-test-bar-bar", + testOptCommand: testValBar, + }, + }, + { + Name: "No extra values", + ProviderOptions: map[string]*types.Option{ + testOptCmd1: { + Command: "echo ${COMMAND2}-test", + }, + testOptCmd2: { + Command: testCmdEchoBar, + }, + }, + ExpectedOptions: map[string]string{ + testOptCmd1: "bar-test", + testOptCmd2: testValBar, + }, + }, + { + Name: "Cyclic dep", + ProviderOptions: map[string]*types.Option{ + testOptCmd1: { + Command: "echo ${COMMAND2}", + }, + testOptCmd2: { + Command: "echo ${COMMAND1}", + }, + }, + ExpectErr: true, + }, + { + Name: "Override", + ResolvedValues: map[string]config.OptionValue{ + testOptCommand: { + Value: testValFoo, + UserProvided: true, + }, + }, + ProviderOptions: map[string]*types.Option{ + testOptCommand: { + Command: testCmdEchoBar, + }, + }, + ExpectedOptions: map[string]string{ + testOptCommand: testValFoo, + }, + }, + { + Name: "Override", + ResolvedValues: map[string]config.OptionValue{ + testOptCommand: { + Value: testValFoo, + UserProvided: true, + }, + }, + ProviderOptions: map[string]*types.Option{ + testOptCommand: { + Command: testCmdEchoBar, + }, + testOptCmd1: { + Command: "echo ${COMMAND}-foo-${UNDEFINED}", + }, + "DEFAULT1": { + Default: "${COMMAND}-foo-${UNDEFINED}", + }, + }, + ExpectedOptions: map[string]string{ + testOptCommand: testValFoo, + testOptCmd1: "foo-foo-", + "DEFAULT1": "foo-foo-${UNDEFINED}", + }, + }, + { + Name: "Expire", + ResolvedValues: map[string]config.OptionValue{ + testOptExpire: { + Value: testValFoo, + Filled: &[]types.Time{types.NewTime(time.Time{})}[0], + }, + testOptNoExp: { + Value: testValFoo, + }, + }, + ProviderOptions: map[string]*types.Option{ + testOptExpire: { + Command: testCmdEchoBar, + Cache: "10m", + }, + testOptNoExp: { + Command: testCmdEchoBar, + Cache: "10m", + }, + }, + ExpectedOptions: map[string]string{ + testOptExpire: testValBar, + testOptNoExp: testValFoo, + }, + FreshFilledKeys: []string{testOptNoExp}, + }, + { + Name: "Ignore self", + ProviderOptions: map[string]*types.Option{ + "SELF": { + Command: "SELF=test; echo ${SELF}", + }, + }, + ExpectedOptions: map[string]string{ + "SELF": testValTest, + }, + }, + { + Name: "Recompute children", + UserValues: map[string]string{ + testOptParent: testValFoo, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptParent: { + Value: testValTest, + UserProvided: true, + }, + testOptChild1: { + Value: "test-child1", + }, + testOptChild2: { + Value: "test-child2", + }, + }, + ProviderOptions: map[string]*types.Option{ + testOptParent: {}, + testOptChild1: { + Command: "echo ${PARENT}-child1", + }, + testOptChild2: { + Default: "${PARENT}-child2", + }, + }, + ExpectedOptions: map[string]string{ + testOptParent: testValFoo, + testOptChild1: "foo-child1", + testOptChild2: "foo-child2", + }, + }, + { + Name: "Error local global", + ProviderOptions: map[string]*types.Option{ + testOptParent: { + Default: testValTest, + }, + testOptChild1: { + Global: true, + Default: testRefParent, + }, + }, + ExpectErr: true, + }, + { + Name: "Error local var", + ProviderOptions: map[string]*types.Option{ + testOptParent: { + Local: true, + Default: testValTest, + }, + testOptChild1: { + Default: testRefParent, + }, + }, + ExpectErr: true, + }, + { + Name: "Don't resolve local", + ProviderOptions: map[string]*types.Option{ + testOptParent: { + Default: testValTest, + }, + testOptChild1: { + Default: testRefParent, + Local: true, + }, + }, + DontResolveLocal: true, + ExpectedOptions: map[string]string{ + testOptParent: testValTest, + }, + }, + { + Name: "Resolve", + ProviderOptions: map[string]*types.Option{ + testOptParent: { + Default: testValTest, + }, + testOptChild1: { + Default: testRefParent, + }, + }, + DontResolveLocal: true, + ExpectedOptions: map[string]string{ + testOptParent: testValTest, + testOptChild1: testValTest, + }, + }, + { + Name: "Skip Required", + ProviderOptions: map[string]*types.Option{ + testOptParent: { + Required: true, + }, + testOptChild1: { + Default: testRefParent, + }, + "PARENT2": { + Required: true, + Default: testValTest, + }, + testOptChild2: { + Default: "${PARENT2}", + }, + }, + SkipRequired: true, + ExpectedOptions: map[string]string{ + "PARENT2": testValTest, + testOptChild2: testValTest, + }, + }, + { + Name: "Nested dynamic options", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest, + testOptTest2: testValTest2, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + }, + }, + }, + { + Name: "Dynamic options don't update", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ResolvedDynamicDefinitions: map[string]*types.Option{ + testOptTest2: { + Default: testValTest5, + }, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptTest: { + Value: testValTest3, + Children: []string{testOptTest2}, + UserProvided: true, + }, + testOptTest2: {Value: testValTest4, UserProvided: true}, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest3, + testOptTest2: testValTest4, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: { + Default: testValTest2, + }, + }, + }, + { + Name: "Dynamic options update", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest2, + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + UserValues: map[string]string{ + testOptTest: testValTest1, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptTest: {Value: testValTest3, Children: []string{testOptTest2}}, + testOptTest2: {Value: testValTest4}, + }, + ResolvedDynamicDefinitions: map[string]*types.Option{ + testOptTest2: { + Default: testValTest5, + }, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest3: testValTest2, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest2, + }, + }, + }, + { + Name: "Nested dynamic options", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }), + }, + }), + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest2: testValTest2, + testOptTest3: testValTest3, + testOptTest4: "test3-bar-4", + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }), + }, + }), + }, + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }), + }, + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }, + }, + { + Name: "Nested dynamic options skip required", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Required: true, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }), + }, + }), + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + SkipRequired: true, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Required: true, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain34, + }, + }), + }, + }), + }, + }, + }, + { + Name: "Nested dynamic options use option", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Required: true, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain24, + }, + }), + }, + }), + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + SkipRequired: true, + UserValues: map[string]string{ + testOptTest2: testValTest2, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest2: testValTest2, + testOptTest3: testValTest3, + testOptTest4: "test2-bar-4", + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Required: true, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain24, + }, + }), + }, + }), + }, + testOptTest3: &types.Option{ + Default: testValTest3, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest4: &types.Option{ + Default: testRefChain24, + }, + }), + }, + testOptTest4: &types.Option{ + Default: testRefChain24, + }, + }, + }, + { + Name: "Nested dynamic options use option", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + }, + }), + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptTest5: { + Value: testValTest5, + }, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest2: testValTest2, + testOptTest3: testValTest3, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest3: &types.Option{ + Default: testValTest3, + }, + }), + }, + testOptTest3: &types.Option{ + Default: testValTest3, + }, + }, + }, + { + Name: "Dynamic options unused option", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptTest5: { + Value: testValTest5, + }, + }, + ResolvedDynamicDefinitions: map[string]*types.Option{ + testOptTest5: { + Default: testValTest2, + }, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest2: testValTest2, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest2, + }, + }, + }, + { + Name: "Dynamic options update default", + ProviderOptions: map[string]*types.Option{ + testOptTest: { + Default: testValTest1, + SubOptionsCommand: optionsToSubCommand(config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest3, + }, + }), + }, + testOptFoo: {Command: testCmdEchoBar}, + }, + ResolvedValues: map[string]config.OptionValue{ + testOptTest: { + Value: testValTest1, + }, + testOptTest2: { + Value: testValTest2, + }, + }, + ResolvedDynamicDefinitions: map[string]*types.Option{ + testOptTest2: { + Default: testValTest2, + }, + }, + ExpectedOptions: map[string]string{ + testOptTest: testValTest1, + testOptTest2: testValTest3, + testOptFoo: testValBar, + }, + ExpectedDynamicOptions: config.OptionDefinitions{ + testOptTest2: &types.Option{ + Default: testValTest3, + }, + }, + }, +} - for _, tc := range testCases { +func TestResolveOptions(t *testing.T) { + for _, tc := range resolveOptionsTestCases { t.Run(tc.Name, func(t *testing.T) { runResolveTestCase(t, tc) }) } } +func applyFreshFilledTimestamps(tc testCase) { + for _, key := range tc.FreshFilledKeys { + value := tc.ResolvedValues[key] + now := types.Now() + value.Filled = &now + tc.ResolvedValues[key] = value + } +} + func runResolveTestCase(t *testing.T, tc testCase) { t.Helper() + applyFreshFilledTimestamps(tc) r := resolver.New(tc.UserValues, tc.ExtraValues, buildResolverOpts(tc)...) options, dynamicOptions, err := r.Resolve( context.Background(), diff --git a/pkg/platform/form/form.go b/pkg/platform/form/form.go index 2a6b86633..d9e203bce 100644 --- a/pkg/platform/form/form.go +++ b/pkg/platform/form/form.go @@ -31,13 +31,51 @@ func CreateInstance( formCtx, cancelForm := context.WithCancel(ctx) defer cancelForm() + selection, err := runCreateSelectionForm(ctx, baseClient, formCtx, cancelForm) + if err != nil { + return nil, err + } + + renderedParameters, err := renderedParametersForCreate( + formCtx, + selection.template, + selection.templateVersion, + ) + if err != nil { + return nil, err + } + + return buildCreatedInstance(buildCreatedInstanceParams{ + id: id, + uid: uid, + source: source, + picture: picture, + selectedProject: selection.project, + selectedTemplate: selection.template, + selectedTemplateVersion: selection.templateVersion, + renderedParameters: renderedParameters, + }), nil +} + +type createSelection struct { + project *managementv1.Project + template *managementv1.DevsyWorkspaceTemplate + templateVersion string +} + +func runCreateSelectionForm( + ctx context.Context, + baseClient client.Client, + formCtx context.Context, + cancelForm CancelFunc, +) (createSelection, error) { var selectedCluster *managementv1.Cluster var selectedProject *managementv1.Project var selectedTemplate *managementv1.DevsyWorkspaceTemplate selectedTemplateVersion := "" projectOptions, err := projectOptions(ctx, baseClient) if err != nil { - return nil, err + return createSelection{}, err } err = huh.NewForm( huh.NewGroup( @@ -68,60 +106,93 @@ func CreateInstance( ), ).RunWithContext(formCtx) if err != nil { - return nil, err + return createSelection{}, err } - parameters := selectedTemplate.Spec.Parameters - if len(selectedTemplate.GetVersions()) > 0 { - parameters, err = list.GetTemplateParameters(selectedTemplate, selectedTemplateVersion) - if err != nil { - return nil, err - } + return createSelection{ + project: selectedProject, + template: selectedTemplate, + templateVersion: selectedTemplateVersion, + }, nil +} + +func renderedParametersForCreate( + formCtx context.Context, + selectedTemplate *managementv1.DevsyWorkspaceTemplate, + selectedTemplateVersion string, +) (string, error) { + parameters, err := resolveTemplateParameters(selectedTemplate, selectedTemplateVersion) + if err != nil { + return "", err + } + if len(parameters) == 0 { + return "", nil } - renderedParameters := "" - if len(parameters) > 0 { - fieldParameters := prepareParameters(parameters) - err = huh.NewForm( - huh.NewGroup(parameterFields(fieldParameters)...), - ).RunWithContext(formCtx) - if err != nil { - return nil, err - } + fieldParameters := prepareParameters(parameters) + return runParameterForm(formCtx, fieldParameters) +} - renderedParameters, err = renderParameters(fieldParameters) - if err != nil { - return nil, err - } - } +type buildCreatedInstanceParams struct { + id, uid, source, picture string + selectedProject *managementv1.Project + selectedTemplate *managementv1.DevsyWorkspaceTemplate + selectedTemplateVersion string + renderedParameters string +} - instance := &managementv1.DevsyWorkspaceInstance{ +func buildCreatedInstance(p buildCreatedInstanceParams) *managementv1.DevsyWorkspaceInstance { + return &managementv1.DevsyWorkspaceInstance{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: encoding.SafeConcatNameMax([]string{id}, 53) + "-", - Namespace: project.ProjectNamespace(selectedProject.GetName()), + GenerateName: encoding.SafeConcatNameMax([]string{p.id}, 53) + "-", + Namespace: project.ProjectNamespace(p.selectedProject.GetName()), Labels: map[string]string{ - storagev1.DevsyWorkspaceIDLabel: id, - storagev1.DevsyWorkspaceUIDLabel: uid, - config.K8sProjectLabel: selectedProject.GetName(), + storagev1.DevsyWorkspaceIDLabel: p.id, + storagev1.DevsyWorkspaceUIDLabel: p.uid, + config.K8sProjectLabel: p.selectedProject.GetName(), }, Annotations: map[string]string{ - storagev1.DevsyWorkspacePictureAnnotation: picture, - storagev1.DevsyWorkspaceSourceAnnotation: source, + storagev1.DevsyWorkspacePictureAnnotation: p.picture, + storagev1.DevsyWorkspaceSourceAnnotation: p.source, }, }, Spec: managementv1.DevsyWorkspaceInstanceSpec{ DevsyWorkspaceInstanceSpec: storagev1.DevsyWorkspaceInstanceSpec{ - DisplayName: id, + DisplayName: p.id, TemplateRef: &storagev1.TemplateRef{ - Name: selectedTemplate.GetName(), - Version: selectedTemplateVersion, + Name: p.selectedTemplate.GetName(), + Version: p.selectedTemplateVersion, }, - Parameters: renderedParameters, + Parameters: p.renderedParameters, }, }, } +} + +func resolveTemplateParameters( + selectedTemplate *managementv1.DevsyWorkspaceTemplate, + selectedTemplateVersion string, +) ([]storagev1.AppParameter, error) { + parameters := selectedTemplate.Spec.Parameters + if len(selectedTemplate.GetVersions()) > 0 { + var err error + parameters, err = list.GetTemplateParameters(selectedTemplate, selectedTemplateVersion) + if err != nil { + return nil, err + } + } - return instance, nil + return parameters, nil +} + +func runParameterForm(formCtx context.Context, fieldParameters []*FieldParameter) (string, error) { + if err := huh.NewForm( + huh.NewGroup(parameterFields(fieldParameters)...), + ).RunWithContext(formCtx); err != nil { + return "", err + } + + return renderParameters(fieldParameters) } func UpdateInstance( @@ -132,17 +203,55 @@ func UpdateInstance( formCtx, cancelForm := context.WithCancel(ctx) defer cancelForm() + selectedTemplate, selectedTemplateVersion, err := selectUpdateTemplate( + ctx, + baseClient, + formCtx, + instance, + ) + if err != nil { + return nil, err + } + + renderedParameters, err := renderedParametersForUpdate( + formCtx, + instance, + selectedTemplate, + selectedTemplateVersion, + ) + if err != nil { + return nil, err + } + + newInstance := instance.DeepCopy() + applyInstanceChanges(applyInstanceChangesParams{ + instance: instance, + newInstance: newInstance, + selectedTemplate: selectedTemplate, + selectedTemplateVersion: selectedTemplateVersion, + renderedParameters: renderedParameters, + }) + + return newInstance, nil +} + +func selectUpdateTemplate( + ctx context.Context, + baseClient client.Client, + formCtx context.Context, + instance *managementv1.DevsyWorkspaceInstance, +) (*managementv1.DevsyWorkspaceTemplate, string, error) { projectName := project.ProjectFromNamespace(instance.GetNamespace()) projectTemplates, err := list.Templates(ctx, baseClient, projectName) if err != nil { - return nil, err + return nil, "", err } templateOptions, selectedTemplate := templateOptionsForInstance( projectTemplates.DevsyWorkspaceTemplates, instance, ) if selectedTemplate == nil { - return nil, fmt.Errorf("template not found: %#v", instance.Spec.TemplateRef) + return nil, "", fmt.Errorf("template not found: %#v", instance.Spec.TemplateRef) } var selectedTemplateVersion string @@ -166,29 +275,10 @@ func UpdateInstance( ), ).RunWithContext(formCtx) if err != nil { - return nil, err - } - - renderedParameters, err := renderedParametersForUpdate( - formCtx, - instance, - selectedTemplate, - selectedTemplateVersion, - ) - if err != nil { - return nil, err + return nil, "", err } - newInstance := instance.DeepCopy() - applyInstanceChanges(applyInstanceChangesParams{ - instance: instance, - newInstance: newInstance, - selectedTemplate: selectedTemplate, - selectedTemplateVersion: selectedTemplateVersion, - renderedParameters: renderedParameters, - }) - - return newInstance, nil + return selectedTemplate, selectedTemplateVersion, nil } func templateOptionsForInstance( @@ -219,13 +309,9 @@ func renderedParametersForUpdate( selectedTemplate *managementv1.DevsyWorkspaceTemplate, selectedTemplateVersion string, ) (string, error) { - parameters := selectedTemplate.Spec.Parameters - if len(selectedTemplate.GetVersions()) > 0 { - var err error - parameters, err = list.GetTemplateParameters(selectedTemplate, selectedTemplateVersion) - if err != nil { - return "", err - } + parameters, err := resolveTemplateParameters(selectedTemplate, selectedTemplateVersion) + if err != nil { + return "", err } if len(parameters) == 0 { return "", nil @@ -241,14 +327,7 @@ func renderedParametersForUpdate( return "", err } - err = huh.NewForm( - huh.NewGroup(parameterFields(fieldParameters)...), - ).RunWithContext(formCtx) - if err != nil { - return "", err - } - - return renderParameters(fieldParameters) + return runParameterForm(formCtx, fieldParameters) } func buildFieldParameters( diff --git a/pkg/platform/kubeconfig.go b/pkg/platform/kubeconfig.go index 1c8daf446..574d2a14d 100644 --- a/pkg/platform/kubeconfig.go +++ b/pkg/platform/kubeconfig.go @@ -131,75 +131,126 @@ func kubeConfigForSpaceInstance( // direct cluster access? if hostCluster.GetAnnotations()[annotations.LoftDirectClusterEndpoint] != "" { - tok := &managementv1.DirectClusterEndpointToken{ - Spec: managementv1.DirectClusterEndpointTokenSpec{ - Scope: scope, - TTL: ttl, - }, - } - directClusterEndpointToken, err := managementClient.Loft(). - ManagementV1(). - DirectClusterEndpointTokens(). - Create(ctx, tok, metav1.CreateOptions{}) - if err != nil { - return nil, fmt.Errorf("create direct cluster endpoint token: %w", err) - } + return directClusterEndpointKubeConfigForSpace(directClusterEndpointForSpaceParams{ + ctx: ctx, + managementClient: managementClient, + scope: scope, + ttl: ttl, + hostCluster: hostCluster, + projectName: projectName, + spaceInstance: spaceInstance, + }) + } - directClusterEndpoint := hostCluster.GetAnnotations()[annotations.LoftDirectClusterEndpoint] - host := fmt.Sprintf( - "https://%s/kubernetes/project/%s/space/%s", - directClusterEndpoint, - projectName, + // access through management cluster + access key + return kubeConfigViaAccessKey(accessKeyKubeConfigParams{ + ctx: ctx, + baseClient: baseClient, + managementClient: managementClient, + scope: scope, + ttl: ttl, + displayName: fmt.Sprintf( + "Kube Config for Space %s/%s", + spaceInstance.Namespace, spaceInstance.Name, - ) + ), + resourceType: "space", + projectName: projectName, + resourceName: spaceInstance.Name, + clusterRefNamespace: spaceInstance.Spec.ClusterRef.Namespace, + }) +} - return newKubeConfig( - host, - directClusterEndpointToken.Status.Token, - spaceInstance.Spec.ClusterRef.Namespace, - true, - ), nil +type directClusterEndpointForSpaceParams struct { + ctx context.Context + managementClient kube.Interface + scope *storagev1.AccessKeyScope + ttl int64 + hostCluster managementv1.Cluster + projectName string + spaceInstance *managementv1.SpaceInstance +} + +func directClusterEndpointKubeConfigForSpace( + p directClusterEndpointForSpaceParams, +) (*clientcmdapi.Config, error) { + tok := &managementv1.DirectClusterEndpointToken{ + Spec: managementv1.DirectClusterEndpointTokenSpec{ + Scope: p.scope, + TTL: p.ttl, + }, + } + directClusterEndpointToken, err := p.managementClient.Loft(). + ManagementV1(). + DirectClusterEndpointTokens(). + Create(p.ctx, tok, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("create direct cluster endpoint token: %w", err) } - // access through management cluster + access key + directClusterEndpoint := p.hostCluster.GetAnnotations()[annotations.LoftDirectClusterEndpoint] + host := fmt.Sprintf( + "https://%s/kubernetes/project/%s/space/%s", + directClusterEndpoint, + p.projectName, + p.spaceInstance.Name, + ) + + return newKubeConfig( + host, + directClusterEndpointToken.Status.Token, + p.spaceInstance.Spec.ClusterRef.Namespace, + true, + ), nil +} + +type accessKeyKubeConfigParams struct { + ctx context.Context + baseClient client.Client + managementClient kube.Interface + scope *storagev1.AccessKeyScope + ttl int64 + displayName string + resourceType string + projectName string + resourceName string + clusterRefNamespace string +} + +// kubeConfigViaAccessKey builds a kube config authenticated via a +// management-cluster-scoped OwnedAccessKey, used when no direct cluster +// endpoint is available. +func kubeConfigViaAccessKey(p accessKeyKubeConfigParams) (*clientcmdapi.Config, error) { key := &managementv1.OwnedAccessKey{ Spec: managementv1.OwnedAccessKeySpec{ AccessKeySpec: storagev1.AccessKeySpec{ - User: baseClient.Self().Status.User.Name, - Scope: scope, - TTL: ttl, - DisplayName: fmt.Sprintf( - "Kube Config for Space %s/%s", - spaceInstance.Namespace, - spaceInstance.Name, - ), + User: p.baseClient.Self().Status.User.Name, + Scope: p.scope, + TTL: p.ttl, + DisplayName: p.displayName, }, }, } - ownedAccessKey, err := managementClient.Loft(). + ownedAccessKey, err := p.managementClient.Loft(). ManagementV1(). OwnedAccessKeys(). - Create(ctx, key, metav1.CreateOptions{}) + Create(p.ctx, key, metav1.CreateOptions{}) if err != nil { return nil, fmt.Errorf("create access key: %w", err) } hostName := strings.TrimPrefix( - strings.TrimPrefix(baseClient.Config().Host, "https://"), + strings.TrimPrefix(p.baseClient.Config().Host, "https://"), "https://", ) host := fmt.Sprintf( - "https://%s/kubernetes/project/%s/space/%s", + "https://%s/kubernetes/project/%s/%s/%s", hostName, - projectName, - spaceInstance.Name, + p.projectName, + p.resourceType, + p.resourceName, ) - return newKubeConfig( - host, - ownedAccessKey.Spec.Key, - spaceInstance.Spec.ClusterRef.Namespace, - true, - ), nil + return newKubeConfig(host, ownedAccessKey.Spec.Key, p.clusterRefNamespace, true), nil } func kubeConfigForVirtualClusterInstance( @@ -222,83 +273,113 @@ func kubeConfigForVirtualClusterInstance( return nil, fmt.Errorf("get virtual cluster instance: %w", err) } + req := newVClusterKubeConfigRequest(ctx, newVClusterKubeConfigRequestParams{ + managementClient: managementClient, + namespace: namespace, + projectName: projectName, + virtualClusterInstance: virtualClusterInstance, + }) + + cfg, handled, err := directVirtualClusterKubeConfig(ctx, directVirtualClusterKubeConfigParams{ + baseClient: baseClient, + projectName: projectName, + virtualClusterInstance: virtualClusterInstance, + req: req, + }) + if handled || err != nil { + return cfg, err + } + + // access through management cluster + access key + return kubeConfigViaAccessKey(accessKeyKubeConfigParams{ + ctx: ctx, + baseClient: baseClient, + managementClient: managementClient, + scope: req.scope, + ttl: req.ttl, + displayName: fmt.Sprintf( + "Kube Config for Virtual Cluster %s/%s", + virtualClusterInstance.Namespace, + virtualClusterInstance.Name, + ), + resourceType: "virtualcluster", + projectName: projectName, + resourceName: virtualClusterInstance.Name, + clusterRefNamespace: virtualClusterInstance.Spec.ClusterRef.Namespace, + }) +} + +type newVClusterKubeConfigRequestParams struct { + managementClient kube.Interface + namespace string + projectName string + virtualClusterInstance *managementv1.VirtualClusterInstance +} + +// newVClusterKubeConfigRequest builds the scoped request shared by the +// direct-ingress, direct-cluster-endpoint, and access-key kube config paths. +func newVClusterKubeConfigRequest( + ctx context.Context, + p newVClusterKubeConfigRequestParams, +) vClusterKubeConfigRequest { scope := &storagev1.AccessKeyScope{ VirtualClusters: []storagev1.AccessKeyScopeVirtualCluster{{ - Project: projectName, - VirtualCluster: virtualClusterInstance.Name, + Project: p.projectName, + VirtualCluster: p.virtualClusterInstance.Name, }}, } - req := vClusterKubeConfigRequest{ + + return vClusterKubeConfigRequest{ ctx: ctx, - managementClient: managementClient, - namespace: namespace, - projectName: projectName, + managementClient: p.managementClient, + namespace: p.namespace, + projectName: p.projectName, scope: scope, ttl: int64(configTTL.Seconds()), - instance: virtualClusterInstance, + instance: p.virtualClusterInstance, } +} +type directVirtualClusterKubeConfigParams struct { + baseClient client.Client + projectName string + virtualClusterInstance *managementv1.VirtualClusterInstance + req vClusterKubeConfigRequest +} + +// directVirtualClusterKubeConfig resolves a kube config via direct ingress +// or a direct cluster endpoint, if either is available for this virtual +// cluster. handled reports whether one of those paths applied; if not, the +// caller should fall back to access-key-based config. +func directVirtualClusterKubeConfig( + ctx context.Context, + p directVirtualClusterKubeConfigParams, +) (cfg *clientcmdapi.Config, handled bool, err error) { // direct virtual cluster ingress access? - virtualCluster := virtualClusterInstance.Status.VirtualCluster + virtualCluster := p.virtualClusterInstance.Status.VirtualCluster if virtualCluster != nil && virtualCluster.AccessPoint.Ingress.Enabled { - return directIngressKubeConfig(req) + cfg, err = directIngressKubeConfig(p.req) + return cfg, true, err } // find cluster by clusterRef hostCluster, err := findHostCluster( ctx, - baseClient, - projectName, - virtualClusterInstance.Spec.ClusterRef.ClusterRef, + p.baseClient, + p.projectName, + p.virtualClusterInstance.Spec.ClusterRef.ClusterRef, ) if err != nil { - return nil, fmt.Errorf("find host cluster: %w", err) + return nil, true, fmt.Errorf("find host cluster: %w", err) } // direct cluster access? if hostCluster.GetAnnotations()[annotations.LoftDirectClusterEndpoint] != "" { - return directClusterEndpointKubeConfig(req, hostCluster) - } - - // access through management cluster + access key - key := &managementv1.OwnedAccessKey{ - Spec: managementv1.OwnedAccessKeySpec{ - AccessKeySpec: storagev1.AccessKeySpec{ - User: baseClient.Self().Status.User.Name, - Scope: scope, - TTL: req.ttl, - DisplayName: fmt.Sprintf( - "Kube Config for Virtual Cluster %s/%s", - virtualClusterInstance.Namespace, - virtualClusterInstance.Name, - ), - }, - }, - } - ownedAccessKey, err := managementClient.Loft(). - ManagementV1(). - OwnedAccessKeys(). - Create(ctx, key, metav1.CreateOptions{}) - if err != nil { - return nil, fmt.Errorf("create access key: %w", err) + cfg, err = directClusterEndpointKubeConfig(p.req, hostCluster) + return cfg, true, err } - hostName := strings.TrimPrefix( - strings.TrimPrefix(baseClient.Config().Host, "https://"), - "https://", - ) - host := fmt.Sprintf( - "https://%s/kubernetes/project/%s/virtualcluster/%s", - hostName, - projectName, - virtualClusterInstance.Name, - ) - return newKubeConfig( - host, - ownedAccessKey.Spec.Key, - virtualClusterInstance.Spec.ClusterRef.Namespace, - true, - ), nil + return nil, false, nil } type vClusterKubeConfigRequest struct { diff --git a/pkg/ssh/config_test.go b/pkg/ssh/config_test.go index 03dfe74c0..1c4ac9de9 100644 --- a/pkg/ssh/config_test.go +++ b/pkg/ssh/config_test.go @@ -15,36 +15,35 @@ func TestSSHConfigSuite(t *testing.T) { suite.Run(t, new(SSHConfigTestSuite)) } -func (s *SSHConfigTestSuite) TestAddHostSection() { - tests := []struct { - name string - config string - execPath string - host string - user string - context string - workspace string - workdir string - command string - gpgagent bool - devsyHome string - provider string - expected string - }{ - { - name: "Basic host addition", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "", - provider: "", - expected: `# Devsy Start testhost +var addHostSectionTestCases = []struct { + name string + config string + execPath string + host string + user string + context string + workspace string + workdir string + command string + gpgagent bool + devsyHome string + provider string + expected string +}{ + { + name: "Basic host addition", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "", + provider: "", + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -54,21 +53,21 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace User testuser # Devsy End testhost`, - }, - { - name: "AWS provider with ConnectTimeout", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "", - provider: "aws", - expected: `# Devsy Start testhost + }, + { + name: "AWS provider with ConnectTimeout", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "", + provider: "aws", + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -79,22 +78,22 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace User testuser # Devsy End testhost`, - }, - { - name: "Basic host addition with DEVSY_HOME", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "C:\\\\W S\\d", - provider: "", - //nolint:lll // long ProxyCommand expected output - expected: `# Devsy Start testhost + }, + { + name: "Basic host addition with DEVSY_HOME", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "C:\\\\W S\\d", + provider: "", + //nolint:lll // long ProxyCommand expected output + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -104,22 +103,22 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace --home "C:\\W S\d" User testuser # Devsy End testhost`, - }, - { - name: "Host addition with workdir", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "/path/to/workdir", - command: "", - gpgagent: false, - devsyHome: "", - provider: "", - //nolint:lll // long ProxyCommand expected output - expected: `# Devsy Start testhost + }, + { + name: "Host addition with workdir", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "/path/to/workdir", + command: "", + gpgagent: false, + devsyHome: "", + provider: "", + //nolint:lll // long ProxyCommand expected output + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -129,22 +128,22 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace --workdir "/path/to/workdir" User testuser # Devsy End testhost`, - }, - { - name: "Host addition with gpg agent", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: true, - devsyHome: "", - provider: "", - //nolint:lll // long ProxyCommand expected output - expected: `# Devsy Start testhost + }, + { + name: "Host addition with gpg agent", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: true, + devsyHome: "", + provider: "", + //nolint:lll // long ProxyCommand expected output + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -154,21 +153,21 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace --ssh-gpg-forwarding User testuser # Devsy End testhost`, - }, - { - name: "Host addition with custom command", - config: "", - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "ssh -W %h:%p bastion", - gpgagent: false, - devsyHome: "", - provider: "", - expected: `# Devsy Start testhost + }, + { + name: "Host addition with custom command", + config: "", + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "ssh -W %h:%p bastion", + gpgagent: false, + devsyHome: "", + provider: "", + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -178,22 +177,22 @@ Host testhost ProxyCommand "ssh -W %h:%p bastion" User testuser # Devsy End testhost`, - }, - { - name: "Host addition to existing config", - config: `Host existinghost + }, + { + name: "Host addition to existing config", + config: `Host existinghost User existinguser`, - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "", - provider: "", - expected: `# Devsy Start testhost + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "", + provider: "", + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -205,10 +204,10 @@ Host testhost # Devsy End testhost Host existinghost User existinguser`, - }, - { - name: "Host addition to existing config with Devsy host", - config: `# Devsy Start existingtesthost + }, + { + name: "Host addition to existing config with Devsy host", + config: `# Devsy Start existingtesthost Host existingtesthost ForwardAgent yes LogLevel error @@ -221,17 +220,17 @@ Host existingtesthost Host existinghost User existinguser`, - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "", - provider: "", - expected: `# Devsy Start testhost + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "", + provider: "", + expected: `# Devsy Start testhost Host testhost ForwardAgent yes LogLevel error @@ -254,27 +253,27 @@ Host existingtesthost Host existinghost User existinguser`, - }, - { - name: "Host addition after top level includes", - config: `Include ~/config1 + }, + { + name: "Host addition after top level includes", + config: `Include ~/config1 Include ~/config2 Include ~/config3`, - execPath: "/path/to/exec", - host: "testhost", - user: "testuser", - context: "testcontext", - workspace: "testworkspace", - workdir: "", - command: "", - gpgagent: false, - devsyHome: "", - provider: "", - expected: `Include ~/config1 + execPath: testExecPath, + host: testHostBasic, + user: testUser, + context: testContextAlt, + workspace: testWorkspaceAlt, + workdir: "", + command: "", + gpgagent: false, + devsyHome: "", + provider: "", + expected: `Include ~/config1 Include ~/config2 @@ -291,10 +290,11 @@ Host testhost ProxyCommand "/path/to/exec" workspace ssh --stdio --context testcontext --user testuser testworkspace User testuser # Devsy End testhost`, - }, - } + }, +} - for _, tt := range tests { +func (s *SSHConfigTestSuite) TestAddHostSection() { + for _, tt := range addHostSectionTestCases { s.Run(tt.name, func() { result, err := addHostSection(tt.config, tt.execPath, addHostParams{ path: "", diff --git a/pkg/ssh/server/ssh.go b/pkg/ssh/server/ssh.go index f7b0eab59..ee26906ee 100644 --- a/pkg/ssh/server/ssh.go +++ b/pkg/ssh/server/ssh.go @@ -171,44 +171,12 @@ func NewServer( return nil, err } - forwardHandler := &ssh.ForwardedTCPHandler{} - forwardedUnixHandler := &ssh.ForwardedUnixHandler{} - keepAliveInterval, keepAliveCountMax := keepAliveConfig() server := &server{ shell: sh, workdir: workdir, reuseSock: reuseSock, currentUser: currentUser.Username, - sshServer: ssh.Server{ - Addr: addr, - ClientAliveInterval: keepAliveInterval, - ClientAliveCountMax: keepAliveCountMax, - LocalPortForwardingCallback: func(ctx ssh.Context, dhost string, dport uint32) bool { - log.Debugf("Accepted forward: %s:%d", dhost, dport) - return true - }, - ReversePortForwardingCallback: func(ctx ssh.Context, host string, port uint32) bool { - log.Debugf("attempt to bind %s:%d - %s", host, port, "granted") - return true - }, - ReverseUnixForwardingCallback: reverseUnixForwardingCallback, - ChannelHandlers: map[string]ssh.ChannelHandler{ - "direct-tcpip": ssh.DirectTCPIPHandler, - "direct-streamlocal@openssh.com": ssh.DirectStreamLocalHandler, - "session": ssh.DefaultSessionHandler, - }, - RequestHandlers: map[string]ssh.RequestHandler{ - "tcpip-forward": forwardHandler.HandleSSHRequest, - "streamlocal-forward@openssh.com": forwardedUnixHandler.HandleSSHRequest, - "cancel-streamlocal-forward@openssh.com": forwardedUnixHandler.HandleSSHRequest, - "cancel-tcpip-forward": forwardHandler.HandleSSHRequest, - }, - SubsystemHandlers: map[string]ssh.SubsystemHandler{ - "sftp": func(s ssh.Session) { - sftpHandler(s, currentUser.Username) - }, - }, - }, + sshServer: buildSSHServer(addr, currentUser.Username), } if len(keys) > 0 { @@ -228,6 +196,45 @@ func NewServer( return server, nil } +// buildSSHServer constructs the underlying ssh.Server with its port +// forwarding, channel, request, and SFTP subsystem handlers wired up. +func buildSSHServer(addr, currentUsername string) ssh.Server { + forwardHandler := &ssh.ForwardedTCPHandler{} + forwardedUnixHandler := &ssh.ForwardedUnixHandler{} + keepAliveInterval, keepAliveCountMax := keepAliveConfig() + + return ssh.Server{ + Addr: addr, + ClientAliveInterval: keepAliveInterval, + ClientAliveCountMax: keepAliveCountMax, + LocalPortForwardingCallback: func(ctx ssh.Context, dhost string, dport uint32) bool { + log.Debugf("Accepted forward: %s:%d", dhost, dport) + return true + }, + ReversePortForwardingCallback: func(ctx ssh.Context, host string, port uint32) bool { + log.Debugf("attempt to bind %s:%d - %s", host, port, "granted") + return true + }, + ReverseUnixForwardingCallback: reverseUnixForwardingCallback, + ChannelHandlers: map[string]ssh.ChannelHandler{ + "direct-tcpip": ssh.DirectTCPIPHandler, + "direct-streamlocal@openssh.com": ssh.DirectStreamLocalHandler, + "session": ssh.DefaultSessionHandler, + }, + RequestHandlers: map[string]ssh.RequestHandler{ + "tcpip-forward": forwardHandler.HandleSSHRequest, + "streamlocal-forward@openssh.com": forwardedUnixHandler.HandleSSHRequest, + "cancel-streamlocal-forward@openssh.com": forwardedUnixHandler.HandleSSHRequest, + "cancel-tcpip-forward": forwardHandler.HandleSSHRequest, + }, + SubsystemHandlers: map[string]ssh.SubsystemHandler{ + "sftp": func(s ssh.Session) { + sftpHandler(s, currentUsername) + }, + }, + } +} + func reverseUnixForwardingCallback(_ ssh.Context, socketPath string) bool { log.Debugf("attempt to bind socket %s", socketPath) diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go index dcdbcbbcf..494653f41 100644 --- a/pkg/types/types_test.go +++ b/pkg/types/types_test.go @@ -8,20 +8,22 @@ import ( "gotest.tools/assert" ) -func TestLifecycleHookUnmarshalJSON(t *testing.T) { - type input struct { - Input types.LifecycleHook `json:"input,omitempty"` - } +type lifecycleHookUnmarshalInput struct { + Input types.LifecycleHook `json:"input,omitempty"` +} - testCases := []struct { - Name string - Input string - Expect input - }{ +type lifecycleHookUnmarshalCase struct { + Name string + Input string + Expect lifecycleHookUnmarshalInput +} + +func lifecycleHookUnmarshalTestCases() []lifecycleHookUnmarshalCase { + return []lifecycleHookUnmarshalCase{ { Name: "string", Input: `{"input": "some-string"}`, - Expect: input{ + Expect: lifecycleHookUnmarshalInput{ Input: types.LifecycleHook{ "": []string{"some-string"}, }, @@ -30,7 +32,7 @@ func TestLifecycleHookUnmarshalJSON(t *testing.T) { { Name: "array of strings", Input: `{"input": ["string1", "string2"]}`, - Expect: input{ + Expect: lifecycleHookUnmarshalInput{ Input: types.LifecycleHook{ "": []string{ "string1", @@ -42,7 +44,7 @@ func TestLifecycleHookUnmarshalJSON(t *testing.T) { { Name: "object of strings", Input: `{"input": {"key1": "value1", "key2": "value2"}}`, - Expect: input{ + Expect: lifecycleHookUnmarshalInput{ Input: types.LifecycleHook{ "key1": []string{ "value1", @@ -56,7 +58,7 @@ func TestLifecycleHookUnmarshalJSON(t *testing.T) { { Name: "object of array of strings", Input: `{"input": {"key1": ["value1","value2"], "key2": ["value3","value4"]}}`, - Expect: input{ + Expect: lifecycleHookUnmarshalInput{ Input: types.LifecycleHook{ "key1": []string{ "value1", @@ -70,10 +72,12 @@ func TestLifecycleHookUnmarshalJSON(t *testing.T) { }, }, } +} - for _, testCase := range testCases { +func TestLifecycleHookUnmarshalJSON(t *testing.T) { + for _, testCase := range lifecycleHookUnmarshalTestCases() { t.Run(testCase.Name, func(t *testing.T) { - var data input + var data lifecycleHookUnmarshalInput err := json.Unmarshal([]byte(testCase.Input), &data) assert.NilError(t, err, testCase.Name) diff --git a/pkg/workspace/id_test.go b/pkg/workspace/id_test.go index ff951e482..3fea7f6e7 100644 --- a/pkg/workspace/id_test.go +++ b/pkg/workspace/id_test.go @@ -9,8 +9,12 @@ import ( // Note that these tests document the status quo and not the ideal state. // I've created follow up tickets to adjust the ToID function and update the tests but because // this is a potentially breaking change we'll have to wait for the next major version. -func TestToID(t *testing.T) { - tests := []struct { +func toIDTestCases() []struct { + name string + input string + want string +} { + return []struct { name string input string want string @@ -66,8 +70,10 @@ func TestToID(t *testing.T) { want: "devsyreallylongreponamethatexceeds48charactersto", }, } +} - for _, tt := range tests { +func TestToID(t *testing.T) { + for _, tt := range toIDTestCases() { t.Run(tt.name, func(t *testing.T) { got := ToID(tt.input) if got != tt.want {