Skip to content

feat: Add "coder projects create" command #246

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Feb 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add create workspace support
  • Loading branch information
kylecarbs committed Feb 11, 2022
commit 8766a33cc1a7fc6da593d1d920a5c09df04d2096
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"nhooyr",
"nolint",
"nosec",
"ntqry",
"oneof",
"parameterscopeid",
"promptui",
Expand Down
76 changes: 75 additions & 1 deletion cli/clitest/clitest.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
package clitest

import (
"archive/tar"
"bufio"
"bytes"
"context"
"errors"
"io"
"os"
"path/filepath"
"regexp"
"testing"

"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
"golang.org/x/xerrors"

"github.com/coder/coder/cli"
"github.com/coder/coder/cli/config"
"github.com/coder/coder/coderd"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/provisioner/echo"
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat 🎉

)

var (
// Used to ensure terminal output doesn't have anything crazy!
stripAnsi = regexp.MustCompile("[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))")
)

// New creates a CLI instance with a configuration pointed to a
// temporary testing directory.
func New(t *testing.T, args ...string) (*cobra.Command, config.Root) {
cmd := cli.Root()
dir := t.TempDir()
Expand All @@ -24,6 +39,8 @@ func New(t *testing.T, args ...string) (*cobra.Command, config.Root) {
return cmd, root
}

// CreateInitialUser creates the initial user and write's the session
// token to the config root provided.
func CreateInitialUser(t *testing.T, client *codersdk.Client, root config.Root) coderd.CreateInitialUserRequest {
user := coderdtest.CreateInitialUser(t, client)
resp, err := client.LoginWithPassword(context.Background(), coderd.LoginWithPasswordRequest{
Expand All @@ -38,6 +55,19 @@ func CreateInitialUser(t *testing.T, client *codersdk.Client, root config.Root)
return user
}

// CreateProjectVersionSource writes the echo provisioner responses into a
// new temporary testing directory.
func CreateProjectVersionSource(t *testing.T, responses *echo.Responses) string {
directory := t.TempDir()
data, err := echo.Tar(responses)
require.NoError(t, err)
err = extractTar(data, directory)
require.NoError(t, err)
return directory
}

// StdoutLogs provides a writer to t.Log that strips
// all ANSI escape codes.
func StdoutLogs(t *testing.T) io.Writer {
reader, writer := io.Pipe()
scanner := bufio.NewScanner(reader)
Expand All @@ -50,8 +80,52 @@ func StdoutLogs(t *testing.T) io.Writer {
if scanner.Err() != nil {
return
}
t.Log(scanner.Text())
t.Log(stripAnsi.ReplaceAllString(scanner.Text(), ""))
}
}()
return writer
}

func extractTar(data []byte, directory string) error {
reader := tar.NewReader(bytes.NewBuffer(data))
for {
header, err := reader.Next()
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return xerrors.Errorf("read project source archive: %w", err)
}
path := filepath.Join(directory, header.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should have a check here to sanitize the header.Name - making sure we can't specify paths like .. that might move up the filesystem. There might be a vulnerability where you could construct a tar that has a header with improper paths (like ../../) - and let you write a file over an existing file.

This would be bad, for example, if a malicious actor could write over a critical config file, a binary (ie, they overwrite our provisionerd with a malicious version that uploads secrets).

Kind of a similar to the example called out here: securego/gosec#439 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and we handle this from provisionerd. But I didn't feel it was necessary for clitest.

We should abstract the tar/untar logic out so it's in one place, then add tests for that specific functionality.

mode := header.FileInfo().Mode()
if mode == 0 {
mode = 0600
}
switch header.Typeflag {
case tar.TypeDir:
err = os.MkdirAll(path, mode)
if err != nil {
return xerrors.Errorf("mkdir: %w", err)
}
case tar.TypeReg:
file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, mode)
if err != nil {
return xerrors.Errorf("create file %q: %w", path, err)
}
// Max file size of 10MB.
_, err = io.CopyN(file, reader, (1<<20)*10)
if errors.Is(err, io.EOF) {
err = nil
}
if err != nil {
_ = file.Close()
return err
}
err = file.Close()
if err != nil {
return err
}
}
}
return nil
}
10 changes: 5 additions & 5 deletions cli/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func login() *cobra.Command {
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s Your Coder deployment hasn't been set up!\n", color.HiBlackString(">"))

_, err := runPrompt(cmd, &promptui.Prompt{
_, err := prompt(cmd, &promptui.Prompt{
Label: "Would you like to create the first user?",
IsConfirm: true,
Default: "y",
Expand All @@ -61,23 +61,23 @@ func login() *cobra.Command {
if err != nil {
return xerrors.Errorf("get current user: %w", err)
}
username, err := runPrompt(cmd, &promptui.Prompt{
username, err := prompt(cmd, &promptui.Prompt{
Label: "What username would you like?",
Default: currentUser.Username,
})
if err != nil {
return xerrors.Errorf("pick username prompt: %w", err)
}

organization, err := runPrompt(cmd, &promptui.Prompt{
organization, err := prompt(cmd, &promptui.Prompt{
Label: "What is the name of your organization?",
Default: "acme-corp",
})
if err != nil {
return xerrors.Errorf("pick organization prompt: %w", err)
}

email, err := runPrompt(cmd, &promptui.Prompt{
email, err := prompt(cmd, &promptui.Prompt{
Label: "What's your email?",
Validate: func(s string) error {
err := validator.New().Var(s, "email")
Expand All @@ -91,7 +91,7 @@ func login() *cobra.Command {
return xerrors.Errorf("specify email prompt: %w", err)
}

password, err := runPrompt(cmd, &promptui.Prompt{
password, err := prompt(cmd, &promptui.Prompt{
Label: "Enter a password:",
Mask: '*',
})
Expand Down
93 changes: 35 additions & 58 deletions cli/projectcreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@ package cli
import (
"archive/tar"
"bytes"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"time"

"github.com/briandowns/spinner"
"github.com/fatih/color"
"github.com/google/uuid"
"github.com/manifoldco/promptui"
"github.com/spf13/cobra"
"github.com/xlab/treeprint"
"golang.org/x/xerrors"

"github.com/coder/coder/coderd"
Expand All @@ -27,7 +26,8 @@ import (

func projectCreate() *cobra.Command {
var (
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd propose structuring these commands as coder <verb> <subject> similar to kubectl. When you demoed this command to me, that was what you had typed the first time, too, suggesting that it feels more "natural" - it also reads better that way, in my opinion.

For example: coder create project

The nice thing about kubectl is that most of the commands follow an identical pattern, so kubectl get pods, kubectl get deployments etc feel natural. Putting the verb after makes it more likely that they will diverge, which is always unexpected (e.g. cases where kubectl project create is a valid command but kubectl workspace create is not)

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a significantly more human approach. I'm trying to think of edge-cases in which that won't work, but I'm yet to find any!

Maybe discoverability would be hurt? eg. adding the root verb for a bespoke action a specific resource has could get really messy. I'm not sure how frequently we'll need to do that though. ssh and plan are the only odd ones I can think of.

Copy link
Contributor

Choose a reason for hiding this comment

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

kubectl has exec and logs and a few other commands that don't follow the pattern, so we could have coder ssh <workspace> or coder plan <workspace> (if we anticipate that there will be other things we'll need to SSH into, we could also do coder ssh workspace <workspace> or similar, of course - we don't have to implement coder ssh project, since that doesn't make much sense)

Thinking about it now, one reason the coder create pattern might be helpful is because of persistent flags. kubectl has a --file flag that applies to all create verbs, so you can implement it as a persistent flag on create. On the other hand, if we have separate create leaf commands, then we'd have to make sure we consistently implement --file for everything individually

Copy link
Contributor

Choose a reason for hiding this comment

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

We can also always make these changes after merging this PR, it's not urgent, but would be nice to do it before we publish things, because then we're on the hook to maintain stability of the interface

Copy link
Member Author

Choose a reason for hiding this comment

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

We lose specificity and customization in output when we standardize using verbs first. eg. it'd be a bit weird for a user to run:

coder describe workspace test

and expect the same visual output as

coder describe user test

Maybe it's good to force us down a consistency route there though.

As you mentioned, we can polish this later. It's definitely a nice standardization forcing-function that provides consistency to our CLI experience!

directory string
directory string
provisioner string
)
cmd := &cobra.Command{
Use: "create",
Expand All @@ -41,16 +41,19 @@ func projectCreate() *cobra.Command {
if err != nil {
return err
}
_, err = runPrompt(cmd, &promptui.Prompt{
_, err = prompt(cmd, &promptui.Prompt{
Copy link
Contributor

Choose a reason for hiding this comment

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

is there an easy way to run this noninteractively, as in a script? for example coder project create abc --org Coder --create-workspace=true

I'd suggest inverting this logic and assuming noninteractive mode, unless the user types -i or --interactive, as the noninteractive mode is usually more common for CLI commands

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to map out the user experience before making it non-interactive. It should work how you mentioned. With flags that allow the prompts to be bypassed.

Is defaulting to interactive bad if we detect a TTY? I'm not sure if it's expected, but it would be cool.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think the best argument against making it non-interactive by default is the principle of least astonishment, that most tools don't do that. But there are examples of tools that do default to doing things interactive, like npm init - so we could just merge this as-is and see what it feels like

One thing I think would be useful, though, is to make sure we're testing a non-interactive use case, ideally as part of our test suite, since that will likely be something people will want (and that people are already doing with v1)

Copy link
Member Author

Choose a reason for hiding this comment

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

Entirely agree!

Default: "y",
IsConfirm: true,
Label: fmt.Sprintf("Set up %s in your organization?", color.New(color.FgHiCyan).Sprintf("%q", directory)),
})
if err != nil {
if errors.Is(err, promptui.ErrAbort) {
return nil
}
return err
}

name, err := runPrompt(cmd, &promptui.Prompt{
name, err := prompt(cmd, &promptui.Prompt{
Default: filepath.Base(directory),
Label: "What's your project's name?",
Validate: func(s string) error {
Expand All @@ -65,7 +68,7 @@ func projectCreate() *cobra.Command {
return err
}

job, err := doProjectLoop(cmd, client, organization, directory, []coderd.CreateParameterValueRequest{})
job, err := validateProjectVersionSource(cmd, client, organization, database.ProvisionerType(provisioner), directory)
if err != nil {
return err
}
Expand All @@ -77,27 +80,44 @@ func projectCreate() *cobra.Command {
return err
}

_, err = prompt(cmd, &promptui.Prompt{
Label: "Create project?",
IsConfirm: true,
Default: "y",
})
if err != nil {
if errors.Is(err, promptui.ErrAbort) {
return nil
}
return err
}

_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s The %s project has been created!\n", color.HiBlackString(">"), color.HiCyanString(project.Name))
_, err = runPrompt(cmd, &promptui.Prompt{
_, err = prompt(cmd, &promptui.Prompt{
Label: "Create a new workspace?",
IsConfirm: true,
Default: "y",
})
if err != nil {
if errors.Is(err, promptui.ErrAbort) {
return nil
}
return err
}

fmt.Printf("Create a new workspace now!\n")
return nil
},
}
currentDirectory, _ := os.Getwd()
cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from")
cmd.Flags().StringVarP(&provisioner, "provisioner", "p", "terraform", "Customize the provisioner backend")
// This is for testing! There's only 1 provisioner type right now.
cmd.Flags().MarkHidden("provisioner")

return cmd
}

func doProjectLoop(cmd *cobra.Command, client *codersdk.Client, organization coderd.Organization, directory string, params []coderd.CreateParameterValueRequest) (*coderd.ProvisionerJob, error) {
func validateProjectVersionSource(cmd *cobra.Command, client *codersdk.Client, organization coderd.Organization, provisioner database.ProvisionerType, directory string, parameters ...coderd.CreateParameterValueRequest) (*coderd.ProvisionerJob, error) {
spin := spinner.New(spinner.CharSets[5], 100*time.Millisecond)
spin.Writer = cmd.OutOrStdout()
spin.Suffix = " Uploading current directory..."
Expand All @@ -118,8 +138,8 @@ func doProjectLoop(cmd *cobra.Command, client *codersdk.Client, organization cod
job, err := client.CreateProjectVersionImportProvisionerJob(cmd.Context(), organization.Name, coderd.CreateProjectImportJobRequest{
StorageMethod: database.ProvisionerStorageMethodFile,
StorageSource: resp.Hash,
Provisioner: database.ProvisionerTypeTerraform,
ParameterValues: params,
Provisioner: provisioner,
ParameterValues: parameters,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -168,20 +188,20 @@ func doProjectLoop(cmd *cobra.Command, client *codersdk.Client, organization cod
if parameterSchema.Name == parameter.CoderWorkspaceTransition {
continue
}
value, err := runPrompt(cmd, &promptui.Prompt{
value, err := prompt(cmd, &promptui.Prompt{
Label: fmt.Sprintf("Enter value for %s:", color.HiCyanString(parameterSchema.Name)),
})
if err != nil {
return nil, err
}
params = append(params, coderd.CreateParameterValueRequest{
parameters = append(parameters, coderd.CreateParameterValueRequest{
Name: parameterSchema.Name,
SourceValue: value,
SourceScheme: database.ParameterSourceSchemeData,
DestinationScheme: parameterSchema.DefaultDestinationScheme,
})
}
return doProjectLoop(cmd, client, organization, directory, params)
return validateProjectVersionSource(cmd, client, organization, provisioner, directory, parameters...)
}

if job.Status != coderd.ProvisionerJobStatusSucceeded {
Expand All @@ -198,50 +218,7 @@ func doProjectLoop(cmd *cobra.Command, client *codersdk.Client, organization cod
if err != nil {
return nil, err
}
return &job, outputProjectInformation(cmd, parameterSchemas, parameterValues, resources)
}

func outputProjectInformation(cmd *cobra.Command, parameterSchemas []coderd.ParameterSchema, parameterValues []coderd.ComputedParameterValue, resources []coderd.ProjectImportJobResource) error {
schemaByID := map[string]coderd.ParameterSchema{}
for _, schema := range parameterSchemas {
schemaByID[schema.ID.String()] = schema
}

_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\n %s\n\n", color.HiBlackString("Parameters"))
for _, value := range parameterValues {
schema, ok := schemaByID[value.SchemaID.String()]
if !ok {
return xerrors.Errorf("schema not found: %s", value.Name)
}
displayValue := value.SourceValue
if !schema.RedisplayValue {
displayValue = "<redacted>"
}
output := fmt.Sprintf("%s %s %s", color.HiCyanString(value.Name), color.HiBlackString("="), displayValue)
if value.DefaultSourceValue {
output += " (default value)"
} else if value.Scope != database.ParameterScopeImportJob {
output += fmt.Sprintf(" (inherited from %s)", value.Scope)
}

root := treeprint.NewWithRoot(output)
if schema.Description != "" {
root.AddBranch(fmt.Sprintf("%s\n%s\n", color.HiBlackString("Description"), schema.Description))
}
if schema.AllowOverrideSource {
root.AddBranch(fmt.Sprintf("%s Users can customize this value!", color.HiYellowString("+")))
}
_, _ = fmt.Fprintln(cmd.OutOrStdout(), " "+strings.Join(strings.Split(root.String(), "\n"), "\n "))
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(), " %s\n\n", color.HiBlackString("Resources"))
for _, resource := range resources {
transition := color.HiGreenString("start")
if resource.Transition == database.WorkspaceTransitionStop {
transition = color.HiRedString("stop")
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(), " %s %s on %s\n\n", color.HiCyanString(resource.Type), color.HiCyanString(resource.Name), transition)
}
return nil
return &job, displayProjectImportInfo(cmd, parameterSchemas, parameterValues, resources)
}

func tarDirectory(directory string) ([]byte, error) {
Expand Down
Loading