diff --git a/config/config_test.go b/config/config_test.go index 26524ac8a..2656937e8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1096,12 +1096,12 @@ func TestParse(t *testing.T) { { "template_uid", `template { - uid = 1000 + user = "1000" }`, &Config{ Templates: &TemplateConfigs{ &TemplateConfig{ - Uid: Int(1000), + User: String("1000"), }, }, }, @@ -1110,12 +1110,12 @@ func TestParse(t *testing.T) { { "template_gid", `template { - gid = 1000 + group = "1000" }`, &Config{ Templates: &TemplateConfigs{ &TemplateConfig{ - Gid: Int(1000), + Group: String("1000"), }, }, }, @@ -1128,8 +1128,8 @@ func TestParse(t *testing.T) { &Config{ Templates: &TemplateConfigs{ &TemplateConfig{ - Uid: nil, - Gid: nil, + User: nil, + Group: nil, }, }, }, diff --git a/config/template.go b/config/template.go index 3dd34776d..1696dec98 100644 --- a/config/template.go +++ b/config/template.go @@ -64,19 +64,19 @@ type TemplateConfig struct { // secrets from Vault. Perms *os.FileMode `mapstructure:"perms"` - // Uid is the numeric uid that will be set when creating the file on disk. + // User is the username or uid that will be set when creating the file on disk. // Useful when simply setting Perms is not enough. // // Platform dependent: this doesn't work on Windows but it fails gracefully // with a warning - Uid *int `mapstructure:"uid"` + User *string `mapstructure:"user"` - // Gid is the numeric gid that will be set when creating the file on disk. + // Group is the group name or gid that will be set when creating the file on disk. // Useful when simply setting Perms is not enough. // // Platform dependent: this doesn't work on Windows but it fails gracefully // with a warning - Gid *int `mapstructure:"gid"` + Group *string `mapstructure:"group"` // Source is the path on disk to the template contents to evaluate. Either // this or Contents should be specified, but not both. diff --git a/manager/runner.go b/manager/runner.go index 951f43da1..a41857090 100644 --- a/manager/runner.go +++ b/manager/runner.go @@ -780,8 +780,8 @@ func (r *Runner) runTemplate(tmpl *template.Template, runCtx *templateRunCtx) (* DryStream: r.outStream, Path: config.StringVal(templateConfig.Destination), Perms: config.FileModeVal(templateConfig.Perms), - Uid: templateConfig.Uid, - Gid: templateConfig.Gid, + User: config.StringVal(templateConfig.User), + Group: config.StringVal(templateConfig.Group), }) if err != nil { return nil, errors.Wrap(err, "error rendering "+templateConfig.Display()) diff --git a/renderer/file_ownership.go b/renderer/file_ownership.go index f4879573c..ff82d79f3 100644 --- a/renderer/file_ownership.go +++ b/renderer/file_ownership.go @@ -5,45 +5,68 @@ package renderer import ( "os" + "os/user" + "strconv" "syscall" ) -func getFileOwnership(path string) (*int, *int) { - file_info, err := os.Stat(path) +func getFileOwnership(path string) (int, int, error) { + fileInfo, err := os.Stat(path) if err != nil { - return nil, nil + return 0, 0, err } - file_sys := file_info.Sys() - st := file_sys.(*syscall.Stat_t) - return intPtr(int(st.Uid)), intPtr(int(st.Gid)) + fileSys := fileInfo.Sys() + st := fileSys.(*syscall.Stat_t) + return int(st.Uid), int(st.Gid), nil } -func setFileOwnership(path string, uid, gid *int) error { - wantedUid := sanitizeUidGid(uid) - wantedGid := sanitizeUidGid(gid) - if wantedUid == -1 && wantedGid == -1 { +func setFileOwnership(path string, uid, gid int) error { + if uid == -1 && gid == -1 { return nil //noop } - return os.Chown(path, wantedUid, wantedGid) + return os.Chown(path, uid, gid) } -func isChownNeeded(path string, uid, gid *int) bool { - wantedUid := sanitizeUidGid(uid) - wantedGid := sanitizeUidGid(gid) - if wantedUid == -1 && wantedGid == -1 { - return false +func isChownNeeded(path string, uid, gid int) (bool, error) { + if uid == -1 && gid == -1 { + return false, nil } - currUid, currGid := getFileOwnership(path) - return wantedUid != *currUid || wantedGid != *currGid + currUid, currGid, err := getFileOwnership(path) + if err != nil { + return false, err + } + return uid != currUid || gid != currGid, nil +} + +// parseUidGid parses the uid/gid so that it can be input to os.Chown +func parseUidGid(s string) (int, error) { + if s == "" { + return -1, nil + } + return strconv.Atoi(s) +} + +func lookupUser(s string) (int, error) { + if id, err := parseUidGid(s); err == nil { + return id, nil + } + u, err := user.Lookup(s) + if err != nil { + return 0, err + } + return strconv.Atoi(u.Uid) } -// sanitizeUidGid sanitizes the uid/gid so that can be an input for os.Chown -func sanitizeUidGid(id *int) int { - if id == nil { - return -1 +func lookupGroup(s string) (int, error) { + if id, err := parseUidGid(s); err == nil { + return id, nil + } + u, err := user.LookupGroup(s) + if err != nil { + return 0, err } - return *id + return strconv.Atoi(u.Gid) } diff --git a/renderer/file_ownership_windows.go b/renderer/file_ownership_windows.go index 79397e6fc..a88db2559 100644 --- a/renderer/file_ownership_windows.go +++ b/renderer/file_ownership_windows.go @@ -4,16 +4,35 @@ package renderer import ( - "log" + "fmt" ) -func setFileOwnership(path string, uid, gid *int) error { - if uid != nil || gid != nil { - log.Printf("[WARN] (runner) cannot set uid/gid for rendered files on Windows") +var notSupportedError error = fmt.Errorf("managing file ownership is not supported on Windows") + +func setFileOwnership(path string, uid, gid int) error { + if uid == -1 && gid == -1 { + return nil + } + return notSupportedError +} + +func isChownNeeded(path string, wantedUid, wantedGid int) (bool, error) { + if wantedUid == -1 && wantedGid == -1 { + return false, nil } - return nil + return false, notSupportedError } -func isChownNeeded(path string, wantedUid, wantedGid *int) bool { - return false +func lookupUser(user string) (int, error) { + if user == "" { + return -1, nil + } + return 0, notSupportedError +} + +func lookupGroup(group string) (int, error) { + if group == "" { + return -1, nil + } + return 0, notSupportedError } diff --git a/renderer/renderer.go b/renderer/renderer.go index 6190381a1..7808ed24e 100644 --- a/renderer/renderer.go +++ b/renderer/renderer.go @@ -36,7 +36,7 @@ type RenderInput struct { DryStream io.Writer Path string Perms os.FileMode - Uid, Gid *int + User, Group string } // RenderResult is returned and stored. It contains the status of the render @@ -66,10 +66,23 @@ func Render(i *RenderInput) (*RenderResult, error) { return nil, errors.Wrap(err, "failed reading file") } + uid, err := lookupUser(i.User) + if err != nil { + return nil, errors.Wrap(err, "failed looking up user") + } + gid, err := lookupGroup(i.Group) + if err != nil { + return nil, errors.Wrap(err, "failed looking up group") + } + var chownNeeded bool if fileExists { - chownNeeded = isChownNeeded(i.Path, i.Uid, i.Gid) + chownNeeded, err = isChownNeeded(i.Path, uid, gid) + if err != nil { + log.Printf("[WARN] (runner) could not determine existing output file's permissions") + chownNeeded = true + } } if bytes.Equal(existing, i.Contents) && fileExists && !chownNeeded { @@ -87,7 +100,7 @@ func Render(i *RenderInput) (*RenderResult, error) { return nil, errors.Wrap(err, "failed writing file") } - if err = setFileOwnership(i.Path, i.Uid, i.Gid); err != nil { + if err = setFileOwnership(i.Path, uid, gid); err != nil { return nil, errors.Wrap(err, "failed setting file ownership") } } diff --git a/renderer/renderer_test.go b/renderer/renderer_test.go index f3540f65c..f674fa1a8 100644 --- a/renderer/renderer_test.go +++ b/renderer/renderer_test.go @@ -6,6 +6,7 @@ import ( "os" "path" "path/filepath" + "strconv" "testing" ) @@ -320,8 +321,6 @@ func TestRender_Chown(t *testing.T) { // Can't change uid unless root, but can try // changing the group id. - // setting Uid to -1 means no change - wantedUid := -1 // we enumerate the groups the current user (running the tests) belongs to callerGroups, err := os.Getgroups() @@ -365,8 +364,7 @@ func TestRender_Chown(t *testing.T) { rr, err := Render(&RenderInput{ Path: path, Contents: contents, - Uid: intPtr(wantedUid), - Gid: intPtr(wantedGid), + Group: strconv.Itoa(wantedGid), }) if err != nil { t.Fatal(err) @@ -378,10 +376,13 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotGid != wantedGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotGid != wantedGid { t.Fatalf("Bad render results; gotUid: %v, wantedGid: %v, gotGid: %v", - *gotUid, wantedGid, *gotGid) + gotUid, wantedGid, gotGid) } }) @@ -410,8 +411,7 @@ func TestRender_Chown(t *testing.T) { rr, err := Render(&RenderInput{ Path: path, Contents: diff_contents, - Uid: intPtr(wantedUid), - Gid: intPtr(wantedGid), + Group: strconv.Itoa(wantedGid), }) if err != nil { t.Fatal(err) @@ -423,10 +423,13 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotGid != wantedGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotGid != wantedGid { t.Fatalf("Bad render results; gotUid: %v, wantedGid: %v, gotGid: %v", - *gotUid, wantedGid, *gotGid) + gotUid, wantedGid, gotGid) } }) @@ -443,8 +446,7 @@ func TestRender_Chown(t *testing.T) { rr, err := Render(&RenderInput{ Path: path, Contents: contents, - Uid: intPtr(wantedUid), - Gid: intPtr(wantedGid), + Group: strconv.Itoa(wantedGid), }) if err != nil { t.Fatal(err) @@ -456,10 +458,13 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotGid != wantedGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotGid != wantedGid { t.Fatalf("Bad render results; gotUid: %v, wantedGid: %v, gotGid: %v", - *gotUid, wantedGid, *gotGid) + gotUid, wantedGid, gotGid) } }) @@ -476,8 +481,7 @@ func TestRender_Chown(t *testing.T) { rr, err := Render(&RenderInput{ Path: path, Contents: contents, - Uid: intPtr(wantedUid), - Gid: intPtr(wantedGid), + Group: strconv.Itoa(wantedGid), }) if err != nil { t.Fatal(err) @@ -489,14 +493,17 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotGid != wantedGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotGid != wantedGid { t.Fatalf("Bad render results; gotUid: %v, wantedGid: %v, gotGid: %v", - *gotUid, wantedGid, *gotGid) + gotUid, wantedGid, gotGid) } }) - t.Run("should-be-noop-when-missing-uid", func(t *testing.T) { + t.Run("should-be-noop-when-missing-user", func(t *testing.T) { outDir, err := ioutil.TempDir("", "") if err != nil { @@ -517,13 +524,15 @@ func TestRender_Chown(t *testing.T) { } // getting file uid:gid for the default behaviour - wantUid, wantGid := getFileOwnership(path) - + wantUid, wantGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } diff_contents := []byte("not-first") rr, err := Render(&RenderInput{ Path: path, Contents: diff_contents, - Gid: intPtr(-1), + Group: "", }) if err != nil { t.Fatal(err) @@ -535,14 +544,17 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotUid != *wantUid || *gotGid != *wantGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotUid != wantUid || gotGid != wantGid { t.Fatalf("Bad render results; we shouldn't have altered uid/gid. wantUid: %v, wantGid: %v, gotUid: %v, gotGid: %v ", - *wantUid, *wantGid, *gotUid, *gotGid) + wantUid, wantGid, gotUid, gotGid) } }) - t.Run("should-be-noop-when-missing-gid", func(t *testing.T) { + t.Run("should-be-noop-when-missing-group", func(t *testing.T) { outDir, err := ioutil.TempDir("", "") if err != nil { @@ -563,13 +575,16 @@ func TestRender_Chown(t *testing.T) { } // getting file uid:gid for the default behaviour - wantUid, wantGid := getFileOwnership(path) + wantUid, wantGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } diff_contents := []byte("not-first") rr, err := Render(&RenderInput{ Path: path, Contents: diff_contents, - Uid: intPtr(-1), + User: "", }) if err != nil { t.Fatal(err) @@ -581,14 +596,17 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotUid != *wantUid || *gotGid != *wantGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotUid != wantUid || gotGid != wantGid { t.Fatalf("Bad render results; we shouldn't have altered uid/gid. wantUid: %v, wantGid: %v, gotUid: %v, gotGid: %v ", - *wantUid, *wantGid, *gotUid, *gotGid) + wantUid, wantGid, gotUid, gotGid) } }) - t.Run("should-be-noop-when-uid-and-gid-are-both-set-to-minus1", func(t *testing.T) { + t.Run("should-be-noop-when-user-and-group-are-both-empty", func(t *testing.T) { outDir, err := ioutil.TempDir("", "") if err != nil { @@ -609,14 +627,17 @@ func TestRender_Chown(t *testing.T) { } // getting file uid:gid for the default behaviour - wantUid, wantGid := getFileOwnership(path) + wantUid, wantGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } diff_contents := []byte("not-first") rr, err := Render(&RenderInput{ Path: path, Contents: diff_contents, - Uid: intPtr(-1), - Gid: intPtr(-1), + User: "", + Group: "", }) if err != nil { t.Fatal(err) @@ -628,10 +649,13 @@ func TestRender_Chown(t *testing.T) { rr.WouldRender, rr.DidRender) } - gotUid, gotGid := getFileOwnership(path) - if *gotUid != *wantUid || *gotGid != *wantGid { + gotUid, gotGid, err := getFileOwnership(path) + if err != nil { + t.Fatalf("getFileOwnership: %s", err) + } + if gotUid != wantUid || gotGid != wantGid { t.Fatalf("Bad render results; we shouldn't have altered uid/gid. wantUid: %v, wantGid: %v, gotUid: %v, gotGid: %v ", - *wantUid, *wantGid, *gotUid, *gotGid) + wantUid, wantGid, gotUid, gotGid) } }) }