From 5c80ccacc4deb0be5341e40896a0b2ab1cc0c164 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Thu, 20 Nov 2014 10:26:36 -0800 Subject: [PATCH 1/2] config: fix parsing of file permissions The file permissions can be specified (unfortunately) as a string or an octal integer. During the normalization step, every field is unmarshalled into an interface{}. String types are kept in tact but integers are converted to decimal integers. If the raw config represented the permissions as an octal, it would be converted to decimal _before_ it was saved to RawFilePermissions. Permissions() would then try to convert it again, assuming it was an octal. The new behavior doesn't assume the radix of the number, allowing decimal and octal input. --- config/config_test.go | 34 ++++++++++++++++++++++++++++++++++ system/file.go | 7 +++---- system/file_test.go | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index dbcbc12..7d46003 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -276,6 +276,40 @@ Address=10.209.171.177/19 if cfg.Coreos.Update.RebootStrategy != "reboot" { t.Errorf("Failed to parse locksmith strategy") } + + contents = ` +coreos: +write_files: + - path: /home/me/notes + permissions: 0744 +` + cfg, err = NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error :%v", err) + } + + if len(cfg.WriteFiles) != 1 { + t.Error("Failed to parse correct number of write_files") + } else { + wf := cfg.WriteFiles[0] + if wf.Content != "" { + t.Errorf("WriteFile has incorrect contents '%s'", wf.Content) + } + if wf.Encoding != "" { + t.Errorf("WriteFile has incorrect encoding %s", wf.Encoding) + } + // Verify that the normalization of the config converted 0744 to its decimal + // representation, 484. + if wf.RawFilePermissions != "484" { + t.Errorf("WriteFile has incorrect permissions %s", wf.RawFilePermissions) + } + if wf.Path != "/home/me/notes" { + t.Errorf("WriteFile has incorrect path %s", wf.Path) + } + if wf.Owner != "" { + t.Errorf("WriteFile has incorrect owner %s", wf.Owner) + } + } } // Assert that our interface conversion doesn't panic diff --git a/system/file.go b/system/file.go index bfe2b15..f56ab10 100644 --- a/system/file.go +++ b/system/file.go @@ -17,7 +17,6 @@ package system import ( - "errors" "fmt" "io/ioutil" "os" @@ -39,10 +38,10 @@ func (f *File) Permissions() (os.FileMode, error) { return os.FileMode(0644), nil } - // Parse string representation of file mode as octal - perm, err := strconv.ParseInt(f.RawFilePermissions, 8, 32) + // Parse string representation of file mode as integer + perm, err := strconv.ParseInt(f.RawFilePermissions, 0, 32) if err != nil { - return 0, errors.New("Unable to parse file permissions as octal integer") + return 0, fmt.Errorf("Unable to parse file permissions %q as integer", f.RawFilePermissions) } return os.FileMode(perm), nil } diff --git a/system/file_test.go b/system/file_test.go index 707aba9..f520649 100644 --- a/system/file_test.go +++ b/system/file_test.go @@ -85,6 +85,38 @@ func TestWriteFileInvalidPermission(t *testing.T) { } } +func TestDecimalFilePermissions(t *testing.T) { + dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") + if err != nil { + t.Fatalf("Unable to create tempdir: %v", err) + } + defer os.RemoveAll(dir) + + fn := "foo" + fullPath := path.Join(dir, fn) + + wf := File{config.File{ + Path: fn, + RawFilePermissions: "484", // Decimal representation of 0744 + }} + + path, err := WriteFile(&wf, dir) + if err != nil { + t.Fatalf("Processing of WriteFile failed: %v", err) + } else if path != fullPath { + t.Fatalf("WriteFile returned bad path: want %s, got %s", fullPath, path) + } + + fi, err := os.Stat(fullPath) + if err != nil { + t.Fatalf("Unable to stat file: %v", err) + } + + if fi.Mode() != os.FileMode(0744) { + t.Errorf("File has incorrect mode: %v", fi.Mode()) + } +} + func TestWriteFilePermissions(t *testing.T) { dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") if err != nil { From 6f91b76d796b87fd5ae221eb0a05987186595f68 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Thu, 20 Nov 2014 10:43:32 -0800 Subject: [PATCH 2/2] docs: correct type of permissions --- Documentation/cloud-config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/cloud-config.md b/Documentation/cloud-config.md index fa2d969..e36778c 100644 --- a/Documentation/cloud-config.md +++ b/Documentation/cloud-config.md @@ -326,7 +326,7 @@ Each item in the list may have the following keys: - **path**: Absolute location on disk where contents should be written - **content**: Data to write at the provided `path` -- **permissions**: String representing file permissions in octal notation (i.e. '0644') +- **permissions**: Integer representing file permissions, typically in octal notation (i.e. 0644) - **owner**: User and group that should own the file written to disk. This is equivalent to the `:` argument to `chown : `. Explicitly not implemented is the **encoding** attribute.