8000 feat: Compress and extract slim binaries with zstd by mafredri · Pull Request #2533 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

feat: Compress and extract slim binaries with zstd #2533

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 16 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
10000
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ build: site/out/index.html $(shell find . -not -path './vendor/*' -type f -name
# build slim artifacts and copy them to the site output directory
./scripts/build_go_slim.sh \
--version "$(VERSION)" \
--compress 22 \
--output ./dist/ \
linux:amd64,armv7,arm64 \
windows:amd64,arm64 \
Expand Down
1 change: 1 addition & 0 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ func server() *cobra.Command {
Logger: logger.Named("coderd"),
Database: databasefake.New(),
Pubsub: database.NewPubsubInMemory(),
CacheDir: cacheDir,
Copy link
Member

Choose a reason for hiding this comment

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

cacheDir defaults to /tmp/ on my system. We should probably change that to not use /tmp at all and instead use ~/.cache so we're not dumping 300MB into RAM on startup

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I've hinted at this in #2200 but we should create a separate issue for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

GoogleTokenValidator: googleTokenValidator,
SecureAuthCookie: secureAuthCookie,
SSHKeygenAlgorithm: sshKeygenAlgorithm,
Expand Down
15 changes: 14 additions & 1 deletion coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"net/http"
"net/url"
"path/filepath"
"sync"
"time"

Expand Down Expand Up @@ -42,6 +43,9 @@ type Options struct {
Database database.Store
Pubsub database.Pubsub

// CacheDir is used for caching files served by the API.
CacheDir string

AgentConnectionUpdateFrequency time.Duration
// APIRateLimit is the minutely throughput rate limit per user or ip.
// Setting a rate limit <0 will disable the rate limiter across the entire
Expand Down Expand Up @@ -78,11 +82,20 @@ func New(options *Options) *API {
}
}

siteCacheDir := options.CacheDir
if siteCacheDir != "" {
siteCacheDir = filepath.Join(siteCacheDir, "site")
}
binFS, err := site.ExtractOrReadBinFS(siteCacheDir, site.FS())
if err != nil {
panic(xerrors.Errorf("read site bin failed: %w", err))
}

r := chi.NewRouter()
api := &API{
Options: options,
Handler: r,
siteHandler: site.Handler(site.FS()),
siteHandler: site.Handler(site.FS(), binFS),
}
api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgent, 0)

Expand Down
23 changes: 22 additions & 1 deletion scripts/build_go_slim.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ source "$(dirname "${BASH_SOURCE[0]}")/lib.sh"

version=""
output_path=""
compress=""

args="$(getopt -o "" -l version:,output: -- "$@")"
args="$(getopt -o "" -l version:,output:,compress: -- "$@")"
eval set -- "$args"
while true; do
case "$1" in
Expand All @@ -36,6 +37,10 @@ while true; do
output_path="$2"
shift 2
;;
--compress)
compress="$2"
shift 2
;;
--)
shift
break
Expand All @@ -48,6 +53,13 @@ done

# Check dependencies
dependencies go
if [[ -n $compress ]]; then
dependencies tar zstd

if [[ ! $compress == [0-9]* ]] || ((compress > 22)) || ((compress < 1)); then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ ! $compress == [0-9]* ]] || ((compress > 22)) || ((compress < 1)); then
if [[ "$compress" != [0-9]* ]] || [ "$compress" -gt 22 ] || [ "$compress" -lt 1 ]; then

Copy link
Member Author

Choose a reason for hiding this comment

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

Just curious why we'd want to avoid (( ))? Neither handles non-numbers gracefully (but we've verified that with the glob match).

Copy link
Member

Choose a reason for hiding this comment

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

Consistency, we use lt and gt in other scripts

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, fair enough. I'd like for us to drop the use of [ though. [[ is superior and also supports -gt and -lt. For instance, with [[ there is no need to quote the right-hand variable to avoid errors when it's empty.

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to change it across all scripts 👍

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 won't do it in this PR to keep the noise down, but I can do a follow-up tomorrow.

error "Invalid value for compress, must in in the range of [1, 22]"
fi
fi

# Remove the "v" prefix.
version="${version#v}"
Expand Down Expand Up @@ -92,3 +104,12 @@ for f in ./coder-slim_*; do
dest="$dest_dir/$hyphenated"
cp "$f" "$dest"
done

if [[ -n $compress ]]; then
(
cd "$dest_dir"
tar cf coder.tar coder-*
rm coder-*
zstd --ultra --long -"${compress}" --rm --no-progress coder.tar -o coder.tar.zst
)
fi
158 changes: 150 additions & 8 deletions site/site.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
package site

import (
"archive/tar"
"bytes"
"context"

"errors"
"fmt"
"io"
"io/fs"
"net/http"
"os"
"path"
"path/filepath"
"strings"
"text/template" // html/template escapes some nonces
"time"

"github.com/justinas/nosurf"
"github.com/klauspost/compress/zstd"
"github.com/unrolled/secure"
"golang.org/x/exp/slices"
"golang.org/x/xerrors"
)

Expand All @@ -29,22 +33,25 @@ func WithAPIResponse(ctx context.Context, apiResponse APIResponse) context.Conte
}

// Handler returns an HTTP handler for serving the static site.
func Handler(fileSystem fs.FS) http.Handler {
func Handler(siteFS fs.FS, binFS http.FileSystem) http.Handler {
// html files are handled by a text/template. Non-html files
// are served by the default file server.
//
// REMARK: text/template is needed to inject values on each request like
// CSRF.
files, err := htmlFiles(fileSystem)

files, err := htmlFiles(siteFS)
if err != nil {
panic(xerrors.Errorf("Failed to return handler for static files. Html files failed to load: %w", err))
}

mux := http.NewServeMux()
mux.Handle("/bin/", http.StripPrefix("/bin", http.FileServer(binFS)))
mux.Handle("/", http.FileServer(http.FS(siteFS))) // All other non-html static files.

return secureHeaders(&handler{
fs: fileSystem,
fs: siteFS,
htmlFiles: files,
h: http.FileServer(http.FS(fileSystem)), // All other non-html static files
h: mux,
})
}

Expand Down Expand Up @@ -146,8 +153,13 @@ func (h *handler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
return
}

switch {
// If requesting binaries, serve straight up.
case reqFile == "bin" || strings.HasPrefix(reqFile, "bin/"):
h.h.ServeHTTP(resp, req)
return
// If the original file path exists we serve it.
if h.exists(reqFile) {
case h.exists(reqFile):
if ShouldCacheFile(reqFile) {
resp.Header().Add("Cache-Control", "public, max-age=31536000, immutable")
}
Expand Down Expand Up @@ -357,7 +369,6 @@ func htmlFiles(files fs.FS) (*htmlTemplates, error) {

return nil
})

if err != nil {
return nil, err
}
Expand All @@ -366,3 +377,134 @@ func htmlFiles(files fs.FS) (*htmlTemplates, error) {
tpls: root,
}, nil
}

// ExtractOrReadBinFS checks the provided fs for compressed coder
// binaries and extracts them into dest/bin if found. As a fallback,
// the provided FS is checked for a /bin directory, if it is non-empty
// it is returned. Finally dest/bin is returned as a fallback allowing
// binaries to be manually placed in dest (usually
// ${CODER_CACHE_DIRECTORY}/site/bin).
func ExtractOrReadBinFS(dest string, siteFS fs.FS) (http.FileSystem, error) {
if dest == "" {
// No destionation on fs, embedd fs is the only option.
binFS, err := fs.Sub(siteFS, "bin")
if err != nil {
return nil, xerrors.Errorf("cache path is empty and embedd fs does not have /bin: %w", err)
}
return http.FS(binFS), nil
}

dest = filepath.Join(dest, "bin")
mkdest := func() (http.FileSystem, error) {
err := os.MkdirAll(dest, 0o700)
if err != nil {
return nil, xerrors.Errorf("mkdir failed: %w", err)
}
return http.Dir(dest), nil
}

archive, err := siteFS.Open("bin/coder.tar.zst")
if err != nil {
if xerrors.Is(err, fs.ErrNotExist) {
files, err := fs.ReadDir(siteFS, "bin")
if err != nil {
if xerrors.Is(err, fs.ErrNotExist) {
// Given fs does not have a bin directory,
// serve from cache directory.
return mkdest()
}
return nil, xerrors.Errorf("site fs read dir failed: %w", err)
}

if len(filterFiles(files, "GITKEEP")) > 0 {
// If there are other files than bin/GITKEEP,
// serve the files.
binFS, err := fs.Sub(siteFS, "bin")
if err != nil {
return nil, xerrors.Errorf("site fs sub dir failed: %w", err)
}
return http.FS(binFS), nil
}

// Nothing we can do, serve the cache directory,
// thus allowing binaries to be places there.
return mkdest()
}
return nil, xerrors.Errorf("open coder binary archive failed: %w", err)
}
defer archive.Close()

dir, err := mkdest()
if err != nil {
return nil, err
}

n, err := extractBin(dest, archive)
if err != nil {
return nil, xerrors.Errorf("extract coder binaries failed: %w", err)
}
if n == 0 {
return nil, xerrors.New("no files were extracted from coder binaries archive")
}

return dir, nil
}

func filterFiles(files []fs.DirEntry, names ...string) []fs.DirEntry {
var filtered []fs.DirEntry
for _, f := range files {
if slices.Contains(names, f.Name()) {
continue
}
filtered = append(filtered, f)
}
return filtered
}

func extractBin(dest string, r io.Reader) (n int, err error) {
opts := []zstd.DOption{
// Concurrency doesn't help us when decoding the tar and
// can actually slow us down.
zstd.WithDecoderConcurrency(1),
// Ignoring checksums give us a slight performance
// increase, we assume no corruption due to embedding.
zstd.IgnoreChecksum(true),
// Allow the decoder to use more memory giving us a 2-3x
// performance boost.
zstd.WithDecoderLowmem(false),
}
zr, err := zstd.NewReader(r, opts...)
if err != nil {
return 0, xerrors.Errorf("open zstd archive failed: %w", err)
}
defer zr.Close()

tr := tar.NewReader(zr)
for {
h, err := tr.Next()
if err != nil {
if errors.Is(err, io.EOF) {
return n, nil
}
return n, xerrors.Errorf("read tar archive failed: %w", err)
}

name := filepath.Join(dest, filepath.Base(h.Name))
f, err := os.Create(name)
if err != nil {
return n, xerrors.Errorf("create file failed: %w", err)
}
//#nosec // We created this tar, no risk of decompression bomb.
_, err = io.Copy(f, tr)
if err != nil {
_ = f.Close()
return n, xerrors.Errorf("write file contents failed: %w", err)
}
err = f.Close()
if err != nil {
return n, xerrors.Errorf("close file failed: %w", err)
}

n++
}
}
Loading
0