Skip to content

Commit

Permalink
base/*: fix uncompressed inline child resource with compressed parent
Browse files Browse the repository at this point in the history 
When desugaring inline/local resource contents, the current rules are:

1. If the input config specifies compression: gzip, we pass through the
   compression field unchanged and refuse to compress again.  This is
   useful for local resources that are stored precompressed alongside the
   config.
2. If we decide to compress, we set the compression field to gzip.
3. If we decide not to compress, we do not set the compression field,
   ostensibly so precompressed resources in child configs can inherit the
   compression field from the parent.

But case 3 doesn't really work.  Butane doesn't know that the child
resource is precompressed, so is allowed to compress it again, and only
the compressibility heuristics (which technically are not contractual)
prevent it from doing so and producing invalid output.  (There's no way
to represent nested compression, so the file would be written to the
provisioned system compressed.)  And if the parent resource is not
formally precompressed, but is actually just auto-compressed by Butane,
a change to the parent's contents or to the Butane heuristics could cause
the parent to become uncompressed in the future and break the merged
config.

And case 3 creates a problem for an inline/local child resource overriding
an inline/local parent.  If Butane opts to compress the parent but not
the child, the merged config will have uncompressed contents but inherit
"compression: gzip" from the parent, causing a provisioning failure.  And
if the parent/child contents change over time, this problem may suddenly
appear in the future.

Fix this by changing rule 3.  When desugaring a non-precompressed
inline/local resource, always set the compression field to either "" or
"gzip".  This intuitively makes sense: only we know whether we decided to
compress the resource, so we need to say so.

This involves an API change to MakeDataURL().  We now return a pointer to
the selected compression value, or nil if we're passing through the
compression value from the input (case 1).  In the latter case, the
caller should record a 1:1 translation for the compression field, rather
than attributing its value to the inline/local input field.

Fixes #332.
  • Loading branch information
@bgilbert
bgilbert committed Apr 27, 2022
1 parent efbd60e commit 5996fbe
Show file tree
Hide file tree
Showing 13 changed files with 323 additions and 120 deletions.
23 changes: 19 additions & 4 deletions base/util/url.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,24 @@ import (
"github.com/vincent-petithory/dataurl"
)

func MakeDataURL(contents []byte, currentCompression *string, allowCompression bool) (uri string, gzipped bool, err error) {
func MakeDataURL(contents []byte, currentCompression *string, allowCompression bool) (uri string, compression *string, err error) {
// try three different encodings, and select the smallest one

if util.NilOrEmpty(currentCompression) {
// The config does not specify compression. We need to
// explicitly set the compression field to avoid a child
// config inheriting a compression setting from the parent,
// which may not have used the same compression algorithm.
compression = util.StrToPtr("")
} else {
// The config specifies compression, meaning that the
// contents were compressed by the user, so we can pick a
// data URL encoding but we can't compress again. Return a
// nil compression value so the caller knows not to record a
// translation from input contents to output compression.
compression = nil
}

// URL-escaped, useful for ASCII text
opaque := "," + dataurl.Escape(contents)

Expand All @@ -53,10 +68,10 @@ func MakeDataURL(contents []byte, currentCompression *string, allowCompression b
return
}
gz := ";base64," + base64.StdEncoding.EncodeToString(buf.Bytes())
// Account for space needed by "compression": "gzip".
if len(gz)+25 < len(opaque) {
// Account for space needed by the compression value
if len(gz)+len("gzip") < len(opaque) {
opaque = gz
gzipped = true
compression = util.StrToPtr("gzip")
}
}

Expand Down
18 changes: 9 additions & 9 deletions base/v0_2/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,31 +140,31 @@ func translateResource(from Resource, options common.TranslateOptions) (to types
return
}

src, gzipped, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression)
src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(c, err)
return
}
to.Source = &src
tm.AddTranslation(c, path.New("json", "source"))
if gzipped {
to.Compression = util.StrToPtr("gzip")
if compression != nil {
to.Compression = compression
tm.AddTranslation(c, path.New("json", "compression"))
}
}

if from.Inline != nil {
c := path.New("yaml", "inline")

src, gzipped, err := baseutil.MakeDataURL([]byte(*from.Inline), to.Compression, !options.NoResourceAutoCompression)
src, compression, err := baseutil.MakeDataURL([]byte(*from.Inline), to.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(c, err)
return
}
to.Source = &src
tm.AddTranslation(c, path.New("json", "source"))
if gzipped {
to.Compression = util.StrToPtr("gzip")
if compression != nil {
to.Compression = compression
tm.AddTranslation(c, path.New("json", "compression"))
}
}
Expand Down Expand Up @@ -278,15 +278,15 @@ func walkTree(yamlPath path.ContextPath, ts *translate.TranslationSet, r *report
r.AddOnError(yamlPath, err)
return nil
}
url, gzipped, err := baseutil.MakeDataURL(contents, file.Contents.Compression, !options.NoResourceAutoCompression)
url, compression, err := baseutil.MakeDataURL(contents, file.Contents.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(yamlPath, err)
return nil
}
file.Contents.Source = &url
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents", "source"))
if gzipped {
file.Contents.Compression = util.StrToPtr("gzip")
if compression != nil {
file.Contents.Compression = compression
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents", "compression"))
}
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents"))
Expand Down
82 changes: 64 additions & 18 deletions base/v0_2/translate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ func TestTranslateFile(t *testing.T) {
},
},
{
Source: util.StrToPtr("data:,file%20contents%0A"),
Source: util.StrToPtr("data:,file%20contents%0A"),
Compression: util.StrToPtr(""),
},
},
Contents: types.Resource{
Expand All @@ -223,6 +224,10 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "append", 2, "local"),
To: path.New("json", "append", 2, "source"),
},
{
From: path.New("yaml", "append", 2, "local"),
To: path.New("json", "append", 2, "compression"),
},
},
"",
common.TranslateOptions{
Expand All @@ -244,7 +249,8 @@ func TestTranslateFile(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,xyzzy"),
Source: util.StrToPtr("data:,xyzzy"),
Compression: util.StrToPtr(""),
},
},
},
Expand All @@ -253,6 +259,10 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "contents", "inline"),
To: path.New("json", "contents", "source"),
},
{
From: path.New("yaml", "contents", "inline"),
To: path.New("json", "contents", "compression"),
},
},
"",
common.TranslateOptions{},
Expand All @@ -271,7 +281,8 @@ func TestTranslateFile(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,file%20contents%0A"),
Source: util.StrToPtr("data:,file%20contents%0A"),
Compression: util.StrToPtr(""),
},
},
},
Expand All @@ -280,6 +291,10 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "contents", "local"),
To: path.New("json", "contents", "source"),
},
{
From: path.New("yaml", "contents", "local"),
To: path.New("json", "contents", "compression"),
},
},
"",
common.TranslateOptions{
Expand All @@ -300,7 +315,8 @@ func TestTranslateFile(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,subdir%20file%20contents%0A"),
Source: util.StrToPtr("data:,subdir%20file%20contents%0A"),
Compression: util.StrToPtr(""),
},
},
},
Expand All @@ -309,6 +325,10 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "contents", "local"),
To: path.New("json", "contents", "source"),
},
{
From: path.New("yaml", "contents", "local"),
To: path.New("json", "contents", "compression"),
},
},
"",
common.TranslateOptions{
Expand Down Expand Up @@ -418,10 +438,12 @@ func TestTranslateFile(t *testing.T) {
Compression: util.StrToPtr("gzip"),
},
{
Source: util.StrToPtr(random_b64),
Source: util.StrToPtr(random_b64),
Compression: util.StrToPtr(""),
},
{
Source: util.StrToPtr(random_b64),
Source: util.StrToPtr(random_b64),
Compression: util.StrToPtr(""),
},
{
Source: util.StrToPtr("data:," + zzz),
Expand Down Expand Up @@ -455,10 +477,18 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "append", 1, "inline"),
To: path.New("json", "append", 1, "source"),
},
{
From: path.New("yaml", "append", 1, "inline"),
To: path.New("json", "append", 1, "compression"),
},
{
From: path.New("yaml", "append", 2, "local"),
To: path.New("json", "append", 2, "source"),
},
{
From: path.New("yaml", "append", 2, "local"),
To: path.New("json", "append", 2, "compression"),
},
{
From: path.New("yaml", "append", 3, "inline"),
To: path.New("json", "append", 3, "source"),
Expand Down Expand Up @@ -487,7 +517,8 @@ func TestTranslateFile(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:," + zzz),
Source: util.StrToPtr("data:," + zzz),
Compression: util.StrToPtr(""),
},
},
},
Expand All @@ -496,6 +527,10 @@ func TestTranslateFile(t *testing.T) {
From: path.New("yaml", "contents", "inline"),
To: path.New("json", "contents", "source"),
},
{
From: path.New("yaml", "contents", "inline"),
To: path.New("json", "contents", "compression"),
},
},
"",
common.TranslateOptions{
Expand Down Expand Up @@ -948,7 +983,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Foverridden"),
Source: util.StrToPtr("data:,tree%2Foverridden"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0600),
},
Expand All @@ -962,7 +998,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Foverridden-executable"),
Source: util.StrToPtr("data:,tree%2Foverridden-executable"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0600),
},
Expand All @@ -973,7 +1010,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Fexecutable"),
Source: util.StrToPtr("data:,tree%2Fexecutable"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(func() int {
if runtime.GOOS != "windows" {
Expand All @@ -991,7 +1029,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Ffile"),
Source: util.StrToPtr("data:,tree%2Ffile"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0644),
},
Expand All @@ -1002,7 +1041,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Fsubdir%2Ffile"),
Source: util.StrToPtr("data:,tree%2Fsubdir%2Ffile"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0644),
},
Expand All @@ -1025,7 +1065,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree2%2Ffile"),
Source: util.StrToPtr("data:,tree2%2Ffile"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0644),
},
Expand Down Expand Up @@ -1085,7 +1126,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Ffile"),
Source: util.StrToPtr("data:,tree%2Ffile"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0644),
},
Expand All @@ -1096,7 +1138,8 @@ func TestTranslateTree(t *testing.T) {
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:,tree%2Fsubdir%2Ffile"),
Source: util.StrToPtr("data:,tree%2Fsubdir%2Ffile"),
Compression: util.StrToPtr(""),
},
Mode: util.IntToPtr(0644),
},
Expand Down Expand Up @@ -1425,11 +1468,13 @@ func TestTranslateIgnition(t *testing.T) {
Config: types.IgnitionConfig{
Merge: []types.Resource{
{
Source: util.StrToPtr("data:,xyzzy"),
Source: util.StrToPtr("data:,xyzzy"),
Compression: util.StrToPtr(""),
},
},
Replace: types.Resource{
Source: util.StrToPtr("data:,xyzzy"),
Source: util.StrToPtr("data:,xyzzy"),
Compression: util.StrToPtr(""),
},
},
},
Expand Down Expand Up @@ -1467,7 +1512,8 @@ func TestTranslateIgnition(t *testing.T) {
TLS: types.TLS{
CertificateAuthorities: []types.Resource{
{
Source: util.StrToPtr("data:,xyzzy"),
Source: util.StrToPtr("data:,xyzzy"),
Compression: util.StrToPtr(""),
},
},
},
Expand Down
18 changes: 9 additions & 9 deletions base/v0_3/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,31 +151,31 @@ func translateResource(from Resource, options common.TranslateOptions) (to types
return
}

src, gzipped, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression)
src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(c, err)
return
}
to.Source = &src
tm.AddTranslation(c, path.New("json", "source"))
if gzipped {
to.Compression = util.StrToPtr("gzip")
if compression != nil {
to.Compression = compression
tm.AddTranslation(c, path.New("json", "compression"))
}
}

if from.Inline != nil {
c := path.New("yaml", "inline")

src, gzipped, err := baseutil.MakeDataURL([]byte(*from.Inline), to.Compression, !options.NoResourceAutoCompression)
src, compression, err := baseutil.MakeDataURL([]byte(*from.Inline), to.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(c, err)
return
}
to.Source = &src
tm.AddTranslation(c, path.New("json", "source"))
if gzipped {
to.Compression = util.StrToPtr("gzip")
if compression != nil {
to.Compression = compression
tm.AddTranslation(c, path.New("json", "compression"))
}
}
Expand Down Expand Up @@ -289,15 +289,15 @@ func walkTree(yamlPath path.ContextPath, ts *translate.TranslationSet, r *report
r.AddOnError(yamlPath, err)
return nil
}
url, gzipped, err := baseutil.MakeDataURL(contents, file.Contents.Compression, !options.NoResourceAutoCompression)
url, compression, err := baseutil.MakeDataURL(contents, file.Contents.Compression, !options.NoResourceAutoCompression)
if err != nil {
r.AddOnError(yamlPath, err)
return nil
}
file.Contents.Source = &url
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents", "source"))
if gzipped {
file.Contents.Compression = util.StrToPtr("gzip")
if compression != nil {
file.Contents.Compression = compression
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents", "compression"))
}
ts.AddTranslation(yamlPath, path.New("json", "storage", "files", i, "contents"))
Expand Down

0 comments on commit 5996fbe

Please sign in to comment.