From d0d9c47fe19d86349da534a23921a1d1b072be92 Mon Sep 17 00:00:00 2001 From: Russell Rollins Date: Wed, 24 Feb 2021 14:10:24 -0500 Subject: [PATCH 1/5] Add struct and functional options for packing function. --- slug.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--- slug_test.go | 28 ++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/slug.go b/slug.go index b911cf2..18ff02d 100644 --- a/slug.go +++ b/slug.go @@ -34,6 +34,49 @@ func (e *IllegalSlugError) Error() string { // chain. func (e *IllegalSlugError) Unwrap() error { return e.Err } +// PackerOption is a functional option that can configure non-default Packers. +type PackerOption func(*Packer) error + +// BypassIgnore is a PackerOption that will skip the .terraformignore rules and +// Pack the entire src directory. +func BypassIgnore() PackerOption { + return func(p *Packer) error { + p.bypassIgnore = true + return nil + } +} + +// DereferenceSymlinks is a PackerOption that will allow symlinks that +// reference a target outside of the src directory. +func DereferenceSymlinks() PackerOption { + return func(p *Packer) error { + p.dereference = true + return nil + } +} + +// Packer holds options for the Pack function. +type Packer struct { + dereference bool + bypassIgnore bool +} + +// NewPacker is a constructor for Packer. +func NewPacker(options ...PackerOption) (*Packer, error) { + p := &Packer{ + dereference: false, + bypassIgnore: false, + } + + for _, opt := range options { + if err := opt(p); err != nil { + return nil, fmt.Errorf("option failed: %w", err) + } + } + + return p, nil +} + // Pack creates a slug from a src directory, and writes the new slug // to w. Returns metadata about the slug and any errors. // @@ -41,7 +84,7 @@ func (e *IllegalSlugError) Unwrap() error { return e.Err } // the src directory will be dereferenced. When dereference is set to // false symlinks with a target outside the src directory are omitted // from the slug. -func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { +func (p *Packer) Pack(src string, w io.Writer) (*Meta, error) { // Gzip compress all the output data. gzipW := gzip.NewWriter(w) @@ -50,13 +93,16 @@ func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { // Load the ignore rule configuration, which will use // defaults if no .terraformignore is configured - ignoreRules := parseIgnoreFile(src) + var ignoreRules []rule + if !p.bypassIgnore { + ignoreRules = parseIgnoreFile(src) + } // Track the metadata details as we go. meta := &Meta{} // Walk the tree of files. - err := filepath.Walk(src, packWalkFn(src, src, src, tarW, meta, dereference, ignoreRules)) + err := filepath.Walk(src, packWalkFn(src, src, src, tarW, meta, p.dereference, ignoreRules)) if err != nil { return nil, err } diff --git a/slug_test.go b/slug_test.go index 119ce63..126037b 100644 --- a/slug_test.go +++ b/slug_test.go @@ -17,7 +17,12 @@ import ( func TestPack(t *testing.T) { slug := bytes.NewBuffer(nil) - meta, err := Pack("testdata/archive-dir", slug, true) + p, err := NewPacker(DereferenceSymlinks()) + if err != nil { + t.Fatalf("err: %v", err) + } + + meta, err := p.Pack("testdata/archive-dir", slug) if err != nil { t.Fatalf("err: %v", err) } @@ -141,7 +146,12 @@ func TestPack(t *testing.T) { func TestPackWithDereferencing(t *testing.T) { slug := bytes.NewBuffer(nil) - meta, err := Pack("testdata/archive-dir", slug, true) + p, err := NewPacker(DereferenceSymlinks()) + if err != nil { + t.Fatalf("err: %v", err) + } + + meta, err := p.Pack("testdata/archive-dir", slug) if err != nil { t.Fatalf("err: %v", err) } @@ -191,7 +201,12 @@ func TestPackWithDereferencing(t *testing.T) { func TestPackWithoutDereferencing(t *testing.T) { slug := bytes.NewBuffer(nil) - meta, err := Pack("testdata/archive-dir", slug, false) + p, err := NewPacker() + if err != nil { + t.Fatalf("err: %v", err) + } + + meta, err := p.Pack("testdata/archive-dir", slug) if err != nil { t.Fatalf("err: %v", err) } @@ -244,7 +259,12 @@ func TestUnpack(t *testing.T) { // First create the slug file so we can try to unpack it. slug := bytes.NewBuffer(nil) - if _, err := Pack("testdata/archive-dir", slug, true); err != nil { + p, err := NewPacker(DereferenceSymlinks()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if _, err := p.Pack("testdata/archive-dir", slug); err != nil { t.Fatalf("err: %v", err) } From c00db07d024fab573b2ac245152af5a15aa84c0f Mon Sep 17 00:00:00 2001 From: Russell Rollins Date: Wed, 24 Feb 2021 15:15:02 -0500 Subject: [PATCH 2/5] We shouldn't change the function signature. The package level Pack stays. --- slug.go | 8 ++++++++ slug_test.go | 28 ++++------------------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/slug.go b/slug.go index 18ff02d..a74ebfb 100644 --- a/slug.go +++ b/slug.go @@ -77,6 +77,14 @@ func NewPacker(options ...PackerOption) (*Packer, error) { return p, nil } +// Pack at the package level is used to maintain compatibility with existing +// code that relies on this function signature. New options related to packing +// slugs should be added to the Packer struct instead. +func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { + p := Packer{dereference: dereference} + return p.Pack(src, w) +} + // Pack creates a slug from a src directory, and writes the new slug // to w. Returns metadata about the slug and any errors. // diff --git a/slug_test.go b/slug_test.go index 126037b..119ce63 100644 --- a/slug_test.go +++ b/slug_test.go @@ -17,12 +17,7 @@ import ( func TestPack(t *testing.T) { slug := bytes.NewBuffer(nil) - p, err := NewPacker(DereferenceSymlinks()) - if err != nil { - t.Fatalf("err: %v", err) - } - - meta, err := p.Pack("testdata/archive-dir", slug) + meta, err := Pack("testdata/archive-dir", slug, true) if err != nil { t.Fatalf("err: %v", err) } @@ -146,12 +141,7 @@ func TestPack(t *testing.T) { func TestPackWithDereferencing(t *testing.T) { slug := bytes.NewBuffer(nil) - p, err := NewPacker(DereferenceSymlinks()) - if err != nil { - t.Fatalf("err: %v", err) - } - - meta, err := p.Pack("testdata/archive-dir", slug) + meta, err := Pack("testdata/archive-dir", slug, true) if err != nil { t.Fatalf("err: %v", err) } @@ -201,12 +191,7 @@ func TestPackWithDereferencing(t *testing.T) { func TestPackWithoutDereferencing(t *testing.T) { slug := bytes.NewBuffer(nil) - p, err := NewPacker() - if err != nil { - t.Fatalf("err: %v", err) - } - - meta, err := p.Pack("testdata/archive-dir", slug) + meta, err := Pack("testdata/archive-dir", slug, false) if err != nil { t.Fatalf("err: %v", err) } @@ -259,12 +244,7 @@ func TestUnpack(t *testing.T) { // First create the slug file so we can try to unpack it. slug := bytes.NewBuffer(nil) - p, err := NewPacker(DereferenceSymlinks()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if _, err := p.Pack("testdata/archive-dir", slug); err != nil { + if _, err := Pack("testdata/archive-dir", slug, true); err != nil { t.Fatalf("err: %v", err) } From b3ce76675e5d46a9a80829b9398f8966facf0e21 Mon Sep 17 00:00:00 2001 From: Russell Rollins Date: Wed, 24 Feb 2021 16:12:45 -0500 Subject: [PATCH 3/5] Adds tests of new constructor/options and BypassIgnore. --- slug_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/slug_test.go b/slug_test.go index 119ce63..905af7b 100644 --- a/slug_test.go +++ b/slug_test.go @@ -240,6 +240,77 @@ func TestPackWithoutDereferencing(t *testing.T) { } } +func TestPackWithoutIgnoring(t *testing.T) { + slug := bytes.NewBuffer(nil) + + p, err := NewPacker(BypassIgnore()) + if err != nil { + t.Fatalf("err: %v", err) + } + + meta, err := p.Pack("testdata/archive-dir", slug) + if err != nil { + t.Fatalf("err: %v", err) + } + + gzipR, err := gzip.NewReader(slug) + if err != nil { + t.Fatalf("err: %v", err) + } + + tarR := tar.NewReader(gzipR) + var ( + fileList []string + slugSize int64 + ) + + for { + hdr, err := tarR.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("err: %v", err) + } + + fileList = append(fileList, hdr.Name) + if hdr.Typeflag == tar.TypeReg || hdr.Typeflag == tar.TypeRegA { + slugSize += hdr.Size + } + } + + // baz.txt would normally be ignored, but should not be + var bazFound bool + for _, file := range fileList { + if file == "baz.txt" { + bazFound = true + } + } + if !bazFound { + t.Fatal("expected file baz.txt to be present, but not found") + } + + // .terraform/file.txt would normally be ignored, but should not be + var dotTerraformFileFound bool + for _, file := range fileList { + if file == ".terraform/file.txt" { + dotTerraformFileFound = true + } + } + if !dotTerraformFileFound { + t.Fatal("expected file .terraform/file.txt to be present, but not found") + } + + // Check the metadata + expect := &Meta{ + Files: fileList, + Size: slugSize, + } + if !reflect.DeepEqual(meta, expect) { + t.Fatalf("\nexpect:\n%#v\n\nactual:\n%#v", expect, meta) + } +} + func TestUnpack(t *testing.T) { // First create the slug file so we can try to unpack it. slug := bytes.NewBuffer(nil) @@ -647,6 +718,55 @@ func TestCheckFileMode(t *testing.T) { } } +func TestNewPacker(t *testing.T) { + for _, tc := range []struct { + desc string + options []PackerOption + expect *Packer + }{ + { + desc: "defaults", + expect: &Packer{ + dereference: false, + bypassIgnore: false, + }, + }, + { + desc: "enable dereferencing", + options: []PackerOption{DereferenceSymlinks()}, + expect: &Packer{ + dereference: true, + }, + }, + { + desc: "disable .terraformignore", + options: []PackerOption{BypassIgnore()}, + expect: &Packer{ + bypassIgnore: true, + }, + }, + { + desc: "multiple options", + options: []PackerOption{BypassIgnore(), DereferenceSymlinks()}, + expect: &Packer{ + dereference: true, + bypassIgnore: true, + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + p, err := NewPacker(tc.options...) + if err != nil { + t.Fatalf("err: %v", err) + } + + if !reflect.DeepEqual(p, tc.expect) { + t.Fatalf("\nexpect:\n%#v\n\nactual:\n%#v", p, tc.expect) + } + }) + } +} + func verifyFile(t *testing.T, path string, mode os.FileMode, expect string) { fh, err := os.Open(path) if err != nil { From f73c6bbed0f91484274e7dae9ff339b21f1a4766 Mon Sep 17 00:00:00 2001 From: Russell Rollins Date: Wed, 24 Feb 2021 16:50:13 -0500 Subject: [PATCH 4/5] Flip the bypassIgnore to applyIgnore. --- slug.go | 26 ++++++++++++++++---------- slug_test.go | 20 +++++++++++--------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/slug.go b/slug.go index a74ebfb..a330bd2 100644 --- a/slug.go +++ b/slug.go @@ -37,11 +37,11 @@ func (e *IllegalSlugError) Unwrap() error { return e.Err } // PackerOption is a functional option that can configure non-default Packers. type PackerOption func(*Packer) error -// BypassIgnore is a PackerOption that will skip the .terraformignore rules and -// Pack the entire src directory. -func BypassIgnore() PackerOption { +// ApplyIgnore is a PackerOption that will apply the .terraformignore rules and +// skip packing files it specifies. +func ApplyIgnore() PackerOption { return func(p *Packer) error { - p.bypassIgnore = true + p.applyIgnore = true return nil } } @@ -57,15 +57,15 @@ func DereferenceSymlinks() PackerOption { // Packer holds options for the Pack function. type Packer struct { - dereference bool - bypassIgnore bool + dereference bool + applyIgnore bool } // NewPacker is a constructor for Packer. func NewPacker(options ...PackerOption) (*Packer, error) { p := &Packer{ - dereference: false, - bypassIgnore: false, + dereference: false, + applyIgnore: false, } for _, opt := range options { @@ -81,7 +81,13 @@ func NewPacker(options ...PackerOption) (*Packer, error) { // code that relies on this function signature. New options related to packing // slugs should be added to the Packer struct instead. func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { - p := Packer{dereference: dereference} + p := Packer{ + dereference: dereference, + + // This defaults to false in NewPacker, but is true here. This matches + // the old behavior of Pack, which always used .terraformignore. + applyIgnore: true, + } return p.Pack(src, w) } @@ -102,7 +108,7 @@ func (p *Packer) Pack(src string, w io.Writer) (*Meta, error) { // Load the ignore rule configuration, which will use // defaults if no .terraformignore is configured var ignoreRules []rule - if !p.bypassIgnore { + if p.applyIgnore { ignoreRules = parseIgnoreFile(src) } diff --git a/slug_test.go b/slug_test.go index 905af7b..513a278 100644 --- a/slug_test.go +++ b/slug_test.go @@ -243,7 +243,9 @@ func TestPackWithoutDereferencing(t *testing.T) { func TestPackWithoutIgnoring(t *testing.T) { slug := bytes.NewBuffer(nil) - p, err := NewPacker(BypassIgnore()) + // By default NewPacker() creates a Packer that does not use + // .terraformignore or dereference symlinks. + p, err := NewPacker() if err != nil { t.Fatalf("err: %v", err) } @@ -727,8 +729,8 @@ func TestNewPacker(t *testing.T) { { desc: "defaults", expect: &Packer{ - dereference: false, - bypassIgnore: false, + dereference: false, + applyIgnore: false, }, }, { @@ -739,18 +741,18 @@ func TestNewPacker(t *testing.T) { }, }, { - desc: "disable .terraformignore", - options: []PackerOption{BypassIgnore()}, + desc: "apply .terraformignore", + options: []PackerOption{ApplyIgnore()}, expect: &Packer{ - bypassIgnore: true, + applyIgnore: true, }, }, { desc: "multiple options", - options: []PackerOption{BypassIgnore(), DereferenceSymlinks()}, + options: []PackerOption{ApplyIgnore(), DereferenceSymlinks()}, expect: &Packer{ - dereference: true, - bypassIgnore: true, + dereference: true, + applyIgnore: true, }, }, } { From 3fd0a40d71464f18960515925df0308a06ef29a9 Mon Sep 17 00:00:00 2001 From: Russell Rollins Date: Thu, 25 Feb 2021 09:23:24 -0500 Subject: [PATCH 5/5] Rename "use ignore" boolean to be clearer. --- slug.go | 20 ++++++++++---------- slug_test.go | 14 +++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/slug.go b/slug.go index a330bd2..4f6ca94 100644 --- a/slug.go +++ b/slug.go @@ -37,11 +37,11 @@ func (e *IllegalSlugError) Unwrap() error { return e.Err } // PackerOption is a functional option that can configure non-default Packers. type PackerOption func(*Packer) error -// ApplyIgnore is a PackerOption that will apply the .terraformignore rules and -// skip packing files it specifies. -func ApplyIgnore() PackerOption { +// ApplyTerraformIgnore is a PackerOption that will apply the .terraformignore +// rules and skip packing files it specifies. +func ApplyTerraformIgnore() PackerOption { return func(p *Packer) error { - p.applyIgnore = true + p.applyTerraformIgnore = true return nil } } @@ -57,15 +57,15 @@ func DereferenceSymlinks() PackerOption { // Packer holds options for the Pack function. type Packer struct { - dereference bool - applyIgnore bool + dereference bool + applyTerraformIgnore bool } // NewPacker is a constructor for Packer. func NewPacker(options ...PackerOption) (*Packer, error) { p := &Packer{ - dereference: false, - applyIgnore: false, + dereference: false, + applyTerraformIgnore: false, } for _, opt := range options { @@ -86,7 +86,7 @@ func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { // This defaults to false in NewPacker, but is true here. This matches // the old behavior of Pack, which always used .terraformignore. - applyIgnore: true, + applyTerraformIgnore: true, } return p.Pack(src, w) } @@ -108,7 +108,7 @@ func (p *Packer) Pack(src string, w io.Writer) (*Meta, error) { // Load the ignore rule configuration, which will use // defaults if no .terraformignore is configured var ignoreRules []rule - if p.applyIgnore { + if p.applyTerraformIgnore { ignoreRules = parseIgnoreFile(src) } diff --git a/slug_test.go b/slug_test.go index 513a278..20a611f 100644 --- a/slug_test.go +++ b/slug_test.go @@ -729,8 +729,8 @@ func TestNewPacker(t *testing.T) { { desc: "defaults", expect: &Packer{ - dereference: false, - applyIgnore: false, + dereference: false, + applyTerraformIgnore: false, }, }, { @@ -742,17 +742,17 @@ func TestNewPacker(t *testing.T) { }, { desc: "apply .terraformignore", - options: []PackerOption{ApplyIgnore()}, + options: []PackerOption{ApplyTerraformIgnore()}, expect: &Packer{ - applyIgnore: true, + applyTerraformIgnore: true, }, }, { desc: "multiple options", - options: []PackerOption{ApplyIgnore(), DereferenceSymlinks()}, + options: []PackerOption{ApplyTerraformIgnore(), DereferenceSymlinks()}, expect: &Packer{ - dereference: true, - applyIgnore: true, + dereference: true, + applyTerraformIgnore: true, }, }, } {