Skip to content

feat: add template checkout command to extract a template to an expanded local directory instead of a tar #2867

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.

Sign up for GitHub

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

Closed
wants to merge 12 commits into from
Closed

feat: add template checkout command to extract a template to an expanded local directory instead of a tar #2867

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
540975b
prefactor `templates pull` to extract common code for the `templates …
ketang Jul 6, 2022
99f447e
add the actual extract command
ketang Jul 6, 2022
3215782
fix outputs
ketang Jul 6, 2022
feb30b4
oh yeah this is called 'export' not 'extract'
ketang Jul 7, 2022
d3efa4e
make export help text slightly more informative
ketang Jul 7, 2022
cf4fba5
remove debugging cruft
ketang Jul 7, 2022
f716c09
allow specifying an alternate destination
ketang Jul 7, 2022
3102599
swap behaviors between `coder templates pull` with `coder templates e…
ketang Jul 7, 2022
a42b278
add a test for `pull` into a directory
ketang Jul 7, 2022
7d55db3
back out pull/export naming change
ketang Jul 8, 2022
2c747db
eliminate unused variable
ketang Jul 10, 2022
3367315
switch to using provisionersdk.Untar, which eliminates need for test
ketang Jul 10, 2022
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
back out pull/export naming change 
old "pull" is "pull" again; new command is "checkout"
  • Loading branch information
@ketang
ketang committed Jul 8, 2022
commit 7d55db35a8ca9a57800b60a9fa144ba4b0374e46
85 changes: 85 additions & 0 deletions cli/templatecheckout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package cli

import (
"archive/tar"
"bytes"
"fmt"
"io"
"os"
"path/filepath"

"github.com/spf13/cobra"
"golang.org/x/xerrors"

"github.com/coder/coder/cli/cliui"
)

func TarBytesToTree(destination string, raw []byte) error {
err := os.Mkdir(destination, 0700)

archiveReader := tar.NewReader(bytes.NewReader(raw))
hdr, err := archiveReader.Next()
for err != io.EOF {
if hdr == nil { // some blog post indicated this could happen sometimes
continue
}
filename := filepath.FromSlash(fmt.Sprintf("%s/%s", destination, hdr.Name))
switch hdr.Typeflag {
case tar.TypeDir:
err = os.Mkdir(filename, 0700)
if err != nil {
return xerrors.Errorf("unable to check out template directory: %w", err)
}
case tar.TypeReg:
f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY, 0600)
if err != nil {
return xerrors.Errorf("unable to create template file: %w", err)
}

_, err = io.Copy(f, archiveReader)
if err != nil {
f.Close() // is this necessary?
return xerrors.Errorf("error writing template file: %w", err)
}
f.Close()
}

hdr, err = archiveReader.Next()
}
return nil
}

func templateCheckout() *cobra.Command {
cmd := &cobra.Command{
Use: "checkout <template name> [destination]",
Copy link
Member

Choose a reason for hiding this comment

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

What is the rationale to make this a separate command from pull? I feel as though it does essentially the same thing, but --extract may be clearer to a caller.

I'm concerned of the checkout name, because of the association with git.

I'd propose a UX like:

$ coder templates pull <name>
Warning: Binary output can mess up your terminal. Use "--extract -" to tell
Warning: coder to output it to your terminal anyway, or consider "--extract
Warning: <FOLDER>" to extract to a directory.

This output is taken verbatim from curl.

Copy link
Contributor Author

@ketang ketang Jul 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Well, one reason to make it a separate command is because I think the more common thing will be to download into an unpacked tree, so I don't want people to always have to type --extract. I'm open to different names, but I chose checkout precisely because of the similarity with version control systems.

Copy link
Member

Choose a reason for hiding this comment

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

checkout gives the appearance that it's checking out a ref if you're familiar with Git. I don't think it'd be synonymous with cloning a repository, which this does.

I think extract is better, but having two commands I think is excessively verbose for what this does. It's not a big deal to type --extract, and I'm fine to swap the default behavior to extract to a directory too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll swap the behaviors and add a --archive flag.

Short: "Download the named template's contents into a subdirectory.",
Long: "Download the named template's contents and extract them into a subdirectory named according to the destination or <template name> if no destination is specified.",
Args: cobra.RangeArgs(1, 2),
RunE: func(cmd *cobra.Command, args []string) error {
templateName := args[0]
var destination string
if len(args) > 1 {
destination = args[1]
} else {
destination = templateName
}

raw, err := fetchTemplateArchiveBytes(cmd, templateName)
if err != nil {
return err
}

// Stat the destination to ensure nothing exists already.
stat, err := os.Stat(destination)
if stat != nil {
return xerrors.Errorf("template file/directory already exists: %s", destination)
}

return TarBytesToTree(destination, raw)
},
}

cliui.AllowSkipPrompt(cmd)

return cmd
}
89 changes: 89 additions & 0 deletions cli/templatecheckout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package cli_test

import (
"archive/tar"
"bufio"
"bytes"
"fmt"
"io/ioutil"
"testing"

"github.com/stretchr/testify/require"

"github.com/coder/coder/cli"
)

func writeTarArchiveFileEntry(tw *tar.Writer, filename string, content []byte) error {
hdr := &tar.Header{
Name: filename,
Mode: 0600,
Size: int64(len(content)),
}

err := tw.WriteHeader(hdr)
if err != nil {
return err
}
_, err = tw.Write([]byte(content))
if err != nil {
return err
}
return nil
}

func TestTemplateCheckoutExtractArchive(t *testing.T) {
t.Parallel()

t.Run("TestTemplateCheckoutExtractArchive", func(t *testing.T) {
subdirName := "subtle"
expectedNames := []string{
"rat-one", "rat-two", fmt.Sprintf("%s/trouble", subdirName),
}
expectedContents := []string{
"{ 'tar' : 'but is it art?' }\n", "{ 'zap' : 'brannigan' }\n", "{ 'with' : 'a T' }\n",
}

t.Parallel()

var bb bytes.Buffer
w := bufio.NewWriter(&bb)
tw := tar.NewWriter(w)

hdr := &tar.Header{
Name: subdirName,
Mode: 0700,
Typeflag: tar.TypeDir,
}
err := tw.WriteHeader(hdr)
if err != nil {
t.Fatalf(err.Error())
}

for i := 0; i < len(expectedNames); i++ {
err = writeTarArchiveFileEntry(tw, expectedNames[i], []byte(expectedContents[i]))
if err != nil {
t.Fatalf(err.Error())
}
}

tw.Close()

dirname, err := ioutil.TempDir("", "template-checkout-test")
if err != nil {
t.Fatalf(err.Error())
}

cli.TarBytesToTree(dirname, bb.Bytes())

for i := 0; i < len(expectedNames); i++ {
filename := fmt.Sprintf("%s/%s", dirname, expectedNames[i])
actualContents, err := ioutil.ReadFile(filename)

if err != nil {
t.Fatalf(err.Error())
}

require.Equal(t, expectedContents[i], string(actualContents))
}
})
}
127 changes: 0 additions & 127 deletions cli/templateexport.go
View file
Open in desktop

This file was deleted.

Loading