From 9182b726b89661d1289c0699fc29f94e2821e5c8 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 03:16:39 -0400 Subject: [PATCH 1/7] Copyediting for SSH/systemd local file embedding changes Small cleanups for https://github.com/coreos/butane/pull/289. --- base/v0_5_exp/translate.go | 7 +++---- base/v0_5_exp/translate_test.go | 24 ++++++++++++------------ base/v0_5_exp/validate.go | 4 ++-- base/v0_5_exp/validate_test.go | 20 ++++++++++---------- config/common/errors.go | 4 +++- docs/config-r4e-v1_1-exp.md | 6 ++++-- docs/release-notes.md | 3 ++- 7 files changed, 36 insertions(+), 32 deletions(-) diff --git a/base/v0_5_exp/translate.go b/base/v0_5_exp/translate.go index 19c0db1e..797c4aec 100644 --- a/base/v0_5_exp/translate.go +++ b/base/v0_5_exp/translate.go @@ -275,7 +275,7 @@ func readSshKeyFile(filesDir string, sshKeyFile string) (string, error) { func translateUnit(from Unit, options common.TranslateOptions) (to types.Unit, tm translate.TranslationSet, r report.Report) { tr := translate.NewTranslator("yaml", "json", options) - tr.AddCustomTranslator(translateUnitDropIn) + tr.AddCustomTranslator(translateDropin) tm, r = translate.Prefixed(tr, "contents", &from.Contents, &to.Contents) translate.MergeP(tr, tm, &r, "dropins", &from.Dropins, &to.Dropins) translate.MergeP(tr, tm, &r, "enabled", &from.Enabled, &to.Enabled) @@ -308,7 +308,7 @@ func translateUnit(from Unit, options common.TranslateOptions) (to types.Unit, t return } -func translateUnitDropIn(from Dropin, options common.TranslateOptions) (to types.Dropin, tm translate.TranslationSet, r report.Report) { +func translateDropin(from Dropin, options common.TranslateOptions) (to types.Dropin, tm translate.TranslationSet, r report.Report) { tr := translate.NewTranslator("yaml", "json", options) tm, r = translate.Prefixed(tr, "contents", &from.Contents, &to.Contents) translate.MergeP(tr, tm, &r, "name", &from.Name, &to.Name) @@ -334,8 +334,7 @@ func translateUnitDropIn(from Dropin, options common.TranslateOptions) (to types r.AddOnError(c, err) return } - stringContents := string(contents) - to.Contents = util.StrToPtr(stringContents) + to.Contents = util.StrToPtr(string(contents)) } return diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index b3b461a1..0cff9ab5 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -1935,7 +1935,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid ssh_keys_inline", + "valid inline keys", PasswdUser{SSHAuthorizedKeys: []SSHAuthorizedKey{SSHAuthorizedKey(sshKeyInline)}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKeyInline)}}, []translate.Translation{ @@ -1952,7 +1952,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid ssh_keys_local", + "valid local keys", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKey1)}}, []translate.Translation{ @@ -1969,7 +1969,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid ssh_keys_local with multiple keys per file", + "valid local keys with multiple keys per file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyMultipleKeysFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ types.SSHAuthorizedKey(sshKey2), @@ -2013,7 +2013,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid ssh_keys_local and ssh_keys", + "valid local and inline keys", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}, SSHAuthorizedKeys: []SSHAuthorizedKey{SSHAuthorizedKey(sshKeyInline)}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKeyInline), types.SSHAuthorizedKey(sshKey1)}}, []translate.Translation{ @@ -2034,7 +2034,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid ssh_keys_local with multiple keys per file and ssh_keys", + "valid local keys with multiple keys per file and inline keys", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyMultipleKeysFileName}, SSHAuthorizedKeys: []SSHAuthorizedKey{SSHAuthorizedKey(sshKeyInline)}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ types.SSHAuthorizedKey(sshKeyInline), @@ -2083,7 +2083,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid empty ssh_keys_local file", + "valid empty local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyEmptyFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey("")}}, []translate.Translation{ @@ -2100,7 +2100,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid blank ssh_keys_local file", + "valid blank local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyBlankFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(""), types.SSHAuthorizedKey("\t")}}, []translate.Translation{ @@ -2121,7 +2121,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "valid Windows style line endings in ssh_keys_local file", + "valid Windows style line endings in local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyWindowsLineEndingsFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ types.SSHAuthorizedKey(sshKey1), @@ -2150,7 +2150,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { sshKeyDir, }, { - "non existing ssh_keys_local file name", + "missing local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyNonExistingFileName}}, types.PasswdUser{}, []translate.Translation{ @@ -2317,7 +2317,7 @@ func TestTranslateUnitLocal(t *testing.T) { "", }, { - "valid dropin_contents_local", + "valid dropin contents_local", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName, Contents: &unitDefinitionFile}}, Name: unitName}, []translate.Translation{ @@ -2330,7 +2330,7 @@ func TestTranslateUnitLocal(t *testing.T) { unitDir, }, { - "non existing dropin_contents_local file name", + "non existing dropin contents_local file name", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitNonExistingFileName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, []translate.Translation{ @@ -2343,7 +2343,7 @@ func TestTranslateUnitLocal(t *testing.T) { unitDir, }, { - "valid empty dropin_contents_local file", + "valid empty dropin contents_local file", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitEmptyFileName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName, Contents: &unitEmptyDefinition}}, Name: unitName}, []translate.Translation{ diff --git a/base/v0_5_exp/validate.go b/base/v0_5_exp/validate.go index 0b8332ac..f24db2ff 100644 --- a/base/v0_5_exp/validate.go +++ b/base/v0_5_exp/validate.go @@ -79,14 +79,14 @@ func (t Tree) Validate(c path.ContextPath) (r report.Report) { func (rs Unit) Validate(c path.ContextPath) (r report.Report) { if rs.ContentsLocal != nil && rs.Contents != nil { - r.AddOnError(c.Append("inline"), common.ErrTooManySystemdSources) + r.AddOnError(c.Append("contents_local"), common.ErrTooManySystemdSources) } return } func (rs Dropin) Validate(c path.ContextPath) (r report.Report) { if rs.ContentsLocal != nil && rs.Contents != nil { - r.AddOnError(c.Append("inline"), common.ErrTooManySystemdSources) + r.AddOnError(c.Append("contents_local"), common.ErrTooManySystemdSources) } return } diff --git a/base/v0_5_exp/validate_test.go b/base/v0_5_exp/validate_test.go index 683516fb..199a95c3 100644 --- a/base/v0_5_exp/validate_test.go +++ b/base/v0_5_exp/validate_test.go @@ -290,7 +290,7 @@ func TestValidateFilesystem(t *testing.T) { } } -// TestValidateUnit tests that multiple sources (i.e. local and inline) are not allowed but zero or one sources are +// TestValidateUnit tests that multiple sources (i.e. contents and contents_local) are not allowed but zero or one sources are func TestValidateUnit(t *testing.T) { tests := []struct { in Unit @@ -298,7 +298,7 @@ func TestValidateUnit(t *testing.T) { errPath path.ContextPath }{ {}, - // inline specified + // contents specified { Unit{ Contents: util.StrToPtr("hello"), @@ -306,7 +306,7 @@ func TestValidateUnit(t *testing.T) { nil, path.New("yaml"), }, - // local specified + // contents_local specified { Unit{ ContentsLocal: util.StrToPtr("hello"), @@ -314,14 +314,14 @@ func TestValidateUnit(t *testing.T) { nil, path.New("yaml"), }, - // inline + local, invalid + // contents + contents_local, invalid { Unit{ Contents: util.StrToPtr("hello"), ContentsLocal: util.StrToPtr("hello, too"), }, common.ErrTooManySystemdSources, - path.New("yaml", "inline"), + path.New("yaml", "contents_local"), }, } @@ -335,7 +335,7 @@ func TestValidateUnit(t *testing.T) { } } -// TestValidateDropin tests that multiple sources (i.e. local and inline) are not allowed but zero or one sources are +// TestValidateDropin tests that multiple sources (i.e. contents and contents_local) are not allowed but zero or one sources are func TestValidateDropin(t *testing.T) { tests := []struct { in Dropin @@ -343,7 +343,7 @@ func TestValidateDropin(t *testing.T) { errPath path.ContextPath }{ {}, - // inline specified + // contents specified { Dropin{ Contents: util.StrToPtr("hello"), @@ -351,7 +351,7 @@ func TestValidateDropin(t *testing.T) { nil, path.New("yaml"), }, - // local specified + // contents_local specified { Dropin{ ContentsLocal: util.StrToPtr("hello"), @@ -359,14 +359,14 @@ func TestValidateDropin(t *testing.T) { nil, path.New("yaml"), }, - // inline + local, invalid + // contents + contents_local, invalid { Dropin{ Contents: util.StrToPtr("hello"), ContentsLocal: util.StrToPtr("hello, too"), }, common.ErrTooManySystemdSources, - path.New("yaml", "inline"), + path.New("yaml", "contents_local"), }, } diff --git a/config/common/errors.go b/config/common/errors.go index 704e7e70..bf1e3c2c 100644 --- a/config/common/errors.go +++ b/config/common/errors.go @@ -32,7 +32,6 @@ var ( // resources and trees ErrTooManyResourceSources = errors.New("only one of the following can be set: inline, local, source") - ErrTooManySystemdSources = errors.New("only one of the following can be set: inline, local") ErrFilesDirEscape = errors.New("local file path traverses outside the files directory") ErrFileType = errors.New("trees may only contain files, directories, and symlinks") ErrNodeExists = errors.New("matching filesystem node has existing contents or different type") @@ -43,6 +42,9 @@ var ( // filesystem nodes ErrDecimalMode = errors.New("unreasonable mode would be reasonable if specified in octal; remember to add a leading zero") + // systemd + ErrTooManySystemdSources = errors.New("only one of the following can be set: contents, contents_local") + // mount units ErrMountUnitNoPath = errors.New("path is required if with_mount_unit is true and format is not swap") ErrMountUnitNoFormat = errors.New("format is required if with_mount_unit is true") diff --git a/docs/config-r4e-v1_1-exp.md b/docs/config-r4e-v1_1-exp.md index fd2d4206..d64e4228 100644 --- a/docs/config-r4e-v1_1-exp.md +++ b/docs/config-r4e-v1_1-exp.md @@ -113,10 +113,12 @@ The RHEL for Edge configuration is a YAML document conforming to the following s * **name** (string): the name of the unit. This must be suffixed with a valid unit type (e.g. "thing.service"). * **_enabled_** (boolean): whether or not the service shall be enabled. When true, the service is enabled. When false, the service is disabled. When omitted, the service is unmodified. In order for this to have any effect, the unit must have an install section. * **_mask_** (boolean): whether or not the service shall be masked. When true, the service is masked by symlinking it to `/dev/null`. - * **_contents_** (string): the contents of the unit. + * **_contents_** (string): the contents of the unit. Mutually exclusive with `contents_local`. + * **_contents_local_** (string): a local path to the contents of the unit, relative to the directory specified by the `--files-dir` command-line argument. Mutually exclusive with `contents`. * **_dropins_** (list of objects): the list of drop-ins for the unit. Every drop-in must have a unique `name`. * **name** (string): the name of the drop-in. This must be suffixed with ".conf". - * **_contents_** (string): the contents of the drop-in. + * **_contents_** (string): the contents of the drop-in. Mutually exclusive with `contents_local`. + * **_contents_local_** (string): a local path to the contents of the drop-in, relative to the directory specified by the `--files-dir` command-line argument. Mutually exclusive with `contents`. * **_passwd_** (object): describes the desired additions to the passwd database. * **_users_** (list of objects): the list of accounts that shall exist. All users must have a unique `name`. * **name** (string): the username for the account. diff --git a/docs/release-notes.md b/docs/release-notes.md index 7fdac43c..675bea10 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -21,7 +21,8 @@ nav_order: 9 flatcar 1.1.0-exp, openshift 4.14.0-exp)_ - Allow specifying user password hash _(openshift 4.13.0+)_ - Support offline Tang provisioning via pre-shared advertisement _(fcos 1.5.0-exp, openshift 4.14.0-exp)_ -- Support local file embedding for SSH keys and systemd units +- Support local file embedding for SSH keys and systemd units _(fcos 1.5.0-exp, + flatcar 1.1.0-exp, openshift 4.14.0-exp, r4e 1.1.0-exp)_ ### Bug fixes From d549d782620efcbb4ea7c3af3f91902ba9c80525 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 03:52:04 -0400 Subject: [PATCH 2/7] base/v0_5_exp: densify translations in SSH/systemd tests No functional changes. --- base/v0_5_exp/translate_test.go | 205 +++++++------------------------- 1 file changed, 41 insertions(+), 164 deletions(-) diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index 0cff9ab5..6b20dfc2 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -1939,14 +1939,8 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeys: []SSHAuthorizedKey{SSHAuthorizedKey(sshKeyInline)}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKeyInline)}}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys", 0), - To: path.New("json", "sshAuthorizedKeys", 0), - }, + {From: path.New("yaml", "ssh_authorized_keys"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -1956,14 +1950,8 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKey1)}}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 0), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -1980,34 +1968,13 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.SSHAuthorizedKey(""), }}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 0), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 1), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 2), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 3), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 4), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 5), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 3)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 4)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 5)}, }, "", sshKeyDir, @@ -2017,18 +1984,9 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}, SSHAuthorizedKeys: []SSHAuthorizedKey{SSHAuthorizedKey(sshKeyInline)}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKeyInline), types.SSHAuthorizedKey(sshKey1)}}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys", 0), - To: path.New("json", "sshAuthorizedKeys", 0), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 1), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, }, "", sshKeyDir, @@ -2046,38 +2004,14 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.SSHAuthorizedKey(""), }}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys", 0), - To: path.New("json", "sshAuthorizedKeys", 0), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 1), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 2), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 3), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 4), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 5), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 6), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 3)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 4)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 5)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 6)}, }, "", sshKeyDir, @@ -2087,14 +2021,8 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyEmptyFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey("")}}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 0), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -2104,18 +2032,9 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyBlankFileName}}, types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(""), types.SSHAuthorizedKey("\t")}}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 0), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 1), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, }, "", sshKeyDir, @@ -2129,22 +2048,10 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.SSHAuthorizedKey(""), }}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 0), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 1), - }, - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys", 2), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, }, "", sshKeyDir, @@ -2154,10 +2061,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyNonExistingFileName}}, types.PasswdUser{}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, osNotFound, sshKeyDir, @@ -2167,10 +2071,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}}, types.PasswdUser{}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, common.ErrNoFilesDir.Error(), "", @@ -2180,10 +2081,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName}}, types.PasswdUser{}, []translate.Translation{ - { - From: path.New("yaml", "ssh_authorized_keys_local"), - To: path.New("json", "sshAuthorizedKeys"), - }, + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, osNotFound, randomDir, @@ -2255,10 +2153,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{ContentsLocal: &unitName, Name: unitName}, types.Unit{Contents: &unitDefinitionFile, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "contents_local"), - To: path.New("json", "contents"), - }, + {From: path.New("yaml", "contents_local"), To: path.New("json", "contents")}, }, "", unitDir, @@ -2276,10 +2171,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{ContentsLocal: &unitEmptyFileName, Name: unitName}, types.Unit{Contents: &unitEmptyDefinition, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "contents_local"), - To: path.New("json", "contents"), - }, + {From: path.New("yaml", "contents_local"), To: path.New("json", "contents")}, }, "", unitDir, @@ -2321,10 +2213,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName, Contents: &unitDefinitionFile}}, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "dropins", 0, "contents_local"), - To: path.New("json", "dropins", 0, "contents"), - }, + {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, "", unitDir, @@ -2334,10 +2223,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitNonExistingFileName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "dropins", 0, "contents_local"), - To: path.New("json", "dropins", 0, "contents"), - }, + {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, osNotFound, unitDir, @@ -2347,10 +2233,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitEmptyFileName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName, Contents: &unitEmptyDefinition}}, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "dropins", 0, "contents_local"), - To: path.New("json", "dropins", 0, "contents"), - }, + {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, "", unitDir, @@ -2360,10 +2243,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "dropins", 0, "contents_local"), - To: path.New("json", "dropins", 0, "contents"), - }, + {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, common.ErrNoFilesDir.Error(), "", @@ -2373,10 +2253,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, []translate.Translation{ - { - From: path.New("yaml", "dropins", 0, "contents_local"), - To: path.New("json", "dropins", 0, "contents"), - }, + {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, osNotFound, randomDir, From 51936734ad037fe0ca129f5f77c58dd7714e536b Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 03:42:55 -0400 Subject: [PATCH 3/7] base/v0_5_exp: record ssh_authorized_keys_local entry in translations Record the specific ssh_authorized_keys_local entry that produced a key, rather than blaming the ssh_authorized_keys_local array as a whole. Add a test for multiple local files. --- base/v0_5_exp/translate.go | 6 +-- base/v0_5_exp/translate_test.go | 65 +++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/base/v0_5_exp/translate.go b/base/v0_5_exp/translate.go index 797c4aec..0b2982cc 100644 --- a/base/v0_5_exp/translate.go +++ b/base/v0_5_exp/translate.go @@ -241,17 +241,17 @@ func translatePasswdUser(from PasswdUser, options common.TranslateOptions) (to t return } - for _, sshKeyFile := range from.SSHAuthorizedKeysLocal { + for keyFileIndex, sshKeyFile := range from.SSHAuthorizedKeysLocal { sshKeys, err := readSshKeyFile(options.FilesDir, sshKeyFile) if err != nil { - r.AddOnError(c, err) + r.AddOnError(c.Append(keyFileIndex), err) continue } // offset for TranslationSets when both ssh_authorized_keys and ssh_authorized_keys_local are available offset := len(to.SSHAuthorizedKeys) for i, line := range regexp.MustCompile("\r?\n").Split(sshKeys, -1) { - tm.AddTranslation(c, path.New("json", "sshAuthorizedKeys", i+offset)) + tm.AddTranslation(c.Append(keyFileIndex), path.New("json", "sshAuthorizedKeys", i+offset)) to.SSHAuthorizedKeys = append(to.SSHAuthorizedKeys, types.SSHAuthorizedKey(line)) } } diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index 6b20dfc2..5a086775 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -1951,7 +1951,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(sshKey1)}}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -1969,12 +1969,37 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 3)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 4)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 5)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 3)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 4)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 5)}, + }, + "", + sshKeyDir, + }, + { + "valid multiple local key files", + PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyFileName, sshKeyMultipleKeysFileName}}, + types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ + types.SSHAuthorizedKey(sshKey1), + types.SSHAuthorizedKey(sshKey2), + types.SSHAuthorizedKey("#comment"), + types.SSHAuthorizedKey(""), + types.SSHAuthorizedKey(""), + types.SSHAuthorizedKey(sshKey3), + types.SSHAuthorizedKey(""), + }}, + []translate.Translation{ + {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 3)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 4)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 5)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 6)}, }, "", sshKeyDir, @@ -1986,7 +2011,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, {From: path.New("yaml", "ssh_authorized_keys", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, }, "", sshKeyDir, @@ -2006,12 +2031,12 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, {From: path.New("yaml", "ssh_authorized_keys", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 3)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 4)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 5)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 6)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 3)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 4)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 5)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 6)}, }, "", sshKeyDir, @@ -2022,7 +2047,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey("")}}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -2033,8 +2058,8 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(""), types.SSHAuthorizedKey("\t")}}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, }, "", sshKeyDir, @@ -2049,9 +2074,9 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 1)}, - {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys", 2)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, + {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, }, "", sshKeyDir, From b357c1ed08d5115e2e1bcf4489bdfd6d2ced5197 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 04:20:17 -0400 Subject: [PATCH 4/7] base/v0_5_exp: check full report in SSH/systemd tests The existing checks wouldn't notice unexpected report entries, since "" is always a suffix of any string. Just hardcode the full report string. --- base/v0_5_exp/translate_test.go | 34 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index 5a086775..97a8c79a 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -1923,7 +1923,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { in PasswdUser out types.PasswdUser translations []translate.Translation - reportSuffix string + report string fileDir string }{ { @@ -2088,7 +2088,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, - osNotFound, + "error at $.ssh_authorized_keys_local.0: open " + filepath.Join(sshKeyDir, sshKeyNonExistingFileName) + ": " + osNotFound + "\n", sshKeyDir, }, { @@ -2098,7 +2098,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, - common.ErrNoFilesDir.Error(), + "error at $.ssh_authorized_keys_local: " + common.ErrNoFilesDir.Error() + "\n", "", }, { @@ -2108,7 +2108,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, }, - osNotFound, + "error at $.ssh_authorized_keys_local.0: open " + filepath.Join(randomDir, sshKeyFileName) + ": " + osNotFound + "\n", randomDir, }, } @@ -2117,11 +2117,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { t.Run(test.name, func(t *testing.T) { actual, translations, r := translatePasswdUser(test.in, common.TranslateOptions{FilesDir: test.fileDir}) assert.Equal(t, test.out, actual, "translation mismatch") - if len(r.Entries) > 0 { - assert.Truef(t, strings.HasSuffix(r.Entries[0].Message, test.reportSuffix), "report mismatch: expected %q but got %q", test.reportSuffix, r.Entries[0].Message) - } else { - assert.True(t, len(test.reportSuffix) == 0, "unexpected report encountered") - } + assert.Equal(t, test.report, r.String(), "bad report") baseutil.VerifyTranslations(t, translations, test.translations) assert.NoError(t, translations.DebugVerifyCoverage(actual), "incomplete TranslationSet coverage") }) @@ -2154,7 +2150,7 @@ func TestTranslateUnitLocal(t *testing.T) { in Unit out types.Unit translations []translate.Translation - reportSuffix string + report string fileDir string }{ { @@ -2188,7 +2184,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{ContentsLocal: &unitNonExistingFileName, Name: unitName}, types.Unit{Name: unitName}, []translate.Translation{}, - osNotFound, + "error at $.contents_local: open " + filepath.Join(unitDir, unitNonExistingFileName) + ": " + osNotFound + "\n", unitDir, }, { @@ -2206,7 +2202,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{ContentsLocal: &unitName, Name: unitName}, types.Unit{Name: unitName}, []translate.Translation{}, - common.ErrNoFilesDir.Error(), + "error at $.contents_local: " + common.ErrNoFilesDir.Error() + "\n", "", }, { @@ -2214,7 +2210,7 @@ func TestTranslateUnitLocal(t *testing.T) { Unit{ContentsLocal: &unitName, Name: unitName}, types.Unit{Name: unitName}, []translate.Translation{}, - osNotFound, + "error at $.contents_local: open " + filepath.Join(randomDir, unitName) + ": " + osNotFound + "\n", randomDir, }, { @@ -2250,7 +2246,7 @@ func TestTranslateUnitLocal(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, - osNotFound, + "error at $.dropins.0.contents_local: open " + filepath.Join(unitDir, unitNonExistingFileName) + ": " + osNotFound + "\n", unitDir, }, { @@ -2270,7 +2266,7 @@ func TestTranslateUnitLocal(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, - common.ErrNoFilesDir.Error(), + "error at $.dropins.0.contents_local: " + common.ErrNoFilesDir.Error() + "\n", "", }, { @@ -2280,7 +2276,7 @@ func TestTranslateUnitLocal(t *testing.T) { []translate.Translation{ {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, }, - osNotFound, + "error at $.dropins.0.contents_local: open " + filepath.Join(randomDir, unitName) + ": " + osNotFound + "\n", randomDir, }, } @@ -2289,11 +2285,7 @@ func TestTranslateUnitLocal(t *testing.T) { t.Run(test.name, func(t *testing.T) { actual, translations, r := translateUnit(test.in, common.TranslateOptions{FilesDir: test.fileDir}) assert.Equal(t, test.out, actual, "translation mismatch") - if len(r.Entries) > 0 { - assert.Truef(t, strings.HasSuffix(r.Entries[0].Message, test.reportSuffix), "report mismatch: expected %q but got %q", test.reportSuffix, r.Entries[0].Message) - } else { - assert.True(t, len(test.reportSuffix) == 0, "unexpected report encountered") - } + assert.Equal(t, test.report, r.String(), "bad report") baseutil.VerifyTranslations(t, translations, test.translations) assert.NoError(t, translations.DebugVerifyCoverage(actual), "incomplete TranslationSet coverage") }) From 1d31d15f29dcdc02d3f9083d86a5844df62f7362 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 05:59:31 -0400 Subject: [PATCH 5/7] base/v0_5_exp: add dropin contents_local translation after reading file Be consistent with the ordering in translateUnit(). --- base/v0_5_exp/translate.go | 3 +-- base/v0_5_exp/translate_test.go | 12 +++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/base/v0_5_exp/translate.go b/base/v0_5_exp/translate.go index 0b2982cc..7fee9cb9 100644 --- a/base/v0_5_exp/translate.go +++ b/base/v0_5_exp/translate.go @@ -315,8 +315,6 @@ func translateDropin(from Dropin, options common.TranslateOptions) (to types.Dro if util.NotEmpty(from.ContentsLocal) { c := path.New("yaml", "contents_local") - tm.AddTranslation(c, path.New("json", "contents")) - if options.FilesDir == "" { r.AddOnError(c, common.ErrNoFilesDir) return @@ -334,6 +332,7 @@ func translateDropin(from Dropin, options common.TranslateOptions) (to types.Dro r.AddOnError(c, err) return } + tm.AddTranslation(c, path.New("json", "contents")) to.Contents = util.StrToPtr(string(contents)) } diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index 97a8c79a..d3adf696 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -2243,9 +2243,7 @@ func TestTranslateUnitLocal(t *testing.T) { "non existing dropin contents_local file name", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitNonExistingFileName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, - []translate.Translation{ - {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, - }, + []translate.Translation{}, "error at $.dropins.0.contents_local: open " + filepath.Join(unitDir, unitNonExistingFileName) + ": " + osNotFound + "\n", unitDir, }, @@ -2263,9 +2261,7 @@ func TestTranslateUnitLocal(t *testing.T) { "missing embed directory for dropin", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, - []translate.Translation{ - {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, - }, + []translate.Translation{}, "error at $.dropins.0.contents_local: " + common.ErrNoFilesDir.Error() + "\n", "", }, @@ -2273,9 +2269,7 @@ func TestTranslateUnitLocal(t *testing.T) { "wrong embed directory for dropin", Unit{Dropins: []Dropin{{Name: dropinName, ContentsLocal: &unitName}}, Name: unitName}, types.Unit{Dropins: []types.Dropin{{Name: dropinName}}, Name: unitName}, - []translate.Translation{ - {From: path.New("yaml", "dropins", 0, "contents_local"), To: path.New("json", "dropins", 0, "contents")}, - }, + []translate.Translation{}, "error at $.dropins.0.contents_local: open " + filepath.Join(randomDir, unitName) + ": " + osNotFound + "\n", randomDir, }, From bb84134ababccdbd2f59f45564dc53c57edcf063 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 05:57:13 -0400 Subject: [PATCH 6/7] base/util: add function to read local file Deduplicate code to validate FilesDir, check for path traversal, and read the file. --- base/util/file.go | 14 +++++++++ base/v0_2/translate.go | 17 +---------- base/v0_3/translate.go | 17 +---------- base/v0_4/translate.go | 17 +---------- base/v0_5_exp/translate.go | 62 +++----------------------------------- 5 files changed, 22 insertions(+), 105 deletions(-) diff --git a/base/util/file.go b/base/util/file.go index 4336ff2c..ecc12406 100644 --- a/base/util/file.go +++ b/base/util/file.go @@ -15,6 +15,7 @@ package util import ( + "os" "path/filepath" "strings" @@ -36,6 +37,19 @@ func EnsurePathWithinFilesDir(path, filesDir string) error { return nil } +func ReadLocalFile(configPath, filesDir string) ([]byte, error) { + if filesDir == "" { + // a files dir isn't configured; refuse to read anything + return nil, common.ErrNoFilesDir + } + // calculate file path within FilesDir and check for path traversal + filePath := filepath.Join(filesDir, filepath.FromSlash(configPath)) + if err := EnsurePathWithinFilesDir(filePath, filesDir); err != nil { + return nil, err + } + return os.ReadFile(filePath) +} + // CheckForDecimalMode fails if the specified mode appears to have been // incorrectly specified in decimal instead of octal. func CheckForDecimalMode(mode int, directory bool) error { diff --git a/base/v0_2/translate.go b/base/v0_2/translate.go index dbe4df5f..ded46dbc 100644 --- a/base/v0_2/translate.go +++ b/base/v0_2/translate.go @@ -118,26 +118,11 @@ func translateResource(from Resource, options common.TranslateOptions) (to types if from.Local != nil { c := path.New("yaml", "local") - - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, filepath.FromSlash(*from.Local)) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir) if err != nil { r.AddOnError(c, err) return } - src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression) if err != nil { r.AddOnError(c, err) diff --git a/base/v0_3/translate.go b/base/v0_3/translate.go index 9d1d5c8e..a539b518 100644 --- a/base/v0_3/translate.go +++ b/base/v0_3/translate.go @@ -125,26 +125,11 @@ func translateResource(from Resource, options common.TranslateOptions) (to types if from.Local != nil { c := path.New("yaml", "local") - - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, filepath.FromSlash(*from.Local)) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir) if err != nil { r.AddOnError(c, err) return } - src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression) if err != nil { r.AddOnError(c, err) diff --git a/base/v0_4/translate.go b/base/v0_4/translate.go index a8c8b0b6..74c0c187 100644 --- a/base/v0_4/translate.go +++ b/base/v0_4/translate.go @@ -140,26 +140,11 @@ func translateResource(from Resource, options common.TranslateOptions) (to types if from.Local != nil { c := path.New("yaml", "local") - - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, filepath.FromSlash(*from.Local)) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir) if err != nil { r.AddOnError(c, err) return } - src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression) if err != nil { r.AddOnError(c, err) diff --git a/base/v0_5_exp/translate.go b/base/v0_5_exp/translate.go index 7fee9cb9..6a62bd96 100644 --- a/base/v0_5_exp/translate.go +++ b/base/v0_5_exp/translate.go @@ -143,26 +143,11 @@ func translateResource(from Resource, options common.TranslateOptions) (to types if from.Local != nil { c := path.New("yaml", "local") - - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, filepath.FromSlash(*from.Local)) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir) if err != nil { r.AddOnError(c, err) return } - src, compression, err := baseutil.MakeDataURL(contents, to.Compression, !options.NoResourceAutoCompression) if err != nil { r.AddOnError(c, err) @@ -242,7 +227,7 @@ func translatePasswdUser(from PasswdUser, options common.TranslateOptions) (to t } for keyFileIndex, sshKeyFile := range from.SSHAuthorizedKeysLocal { - sshKeys, err := readSshKeyFile(options.FilesDir, sshKeyFile) + sshKeys, err := baseutil.ReadLocalFile(sshKeyFile, options.FilesDir) if err != nil { r.AddOnError(c.Append(keyFileIndex), err) continue @@ -250,7 +235,7 @@ func translatePasswdUser(from PasswdUser, options common.TranslateOptions) (to t // offset for TranslationSets when both ssh_authorized_keys and ssh_authorized_keys_local are available offset := len(to.SSHAuthorizedKeys) - for i, line := range regexp.MustCompile("\r?\n").Split(sshKeys, -1) { + for i, line := range regexp.MustCompile("\r?\n").Split(string(sshKeys), -1) { tm.AddTranslation(c.Append(keyFileIndex), path.New("json", "sshAuthorizedKeys", i+offset)) to.SSHAuthorizedKeys = append(to.SSHAuthorizedKeys, types.SSHAuthorizedKey(line)) } @@ -260,19 +245,6 @@ func translatePasswdUser(from PasswdUser, options common.TranslateOptions) (to t return } -func readSshKeyFile(filesDir string, sshKeyFile string) (string, error) { - // calculate file path within FilesDir and check for path traversal - filePath := filepath.Join(filesDir, sshKeyFile) - if err := baseutil.EnsurePathWithinFilesDir(filePath, filesDir); err != nil { - return "", err - } - contents, err := os.ReadFile(filePath) - if err != nil { - return "", err - } - return string(contents), nil -} - func translateUnit(from Unit, options common.TranslateOptions) (to types.Unit, tm translate.TranslationSet, r report.Report) { tr := translate.NewTranslator("yaml", "json", options) tr.AddCustomTranslator(translateDropin) @@ -284,19 +256,7 @@ func translateUnit(from Unit, options common.TranslateOptions) (to types.Unit, t if util.NotEmpty(from.ContentsLocal) { c := path.New("yaml", "contents_local") - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, *from.ContentsLocal) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.ContentsLocal, options.FilesDir) if err != nil { r.AddOnError(c, err) return @@ -315,19 +275,7 @@ func translateDropin(from Dropin, options common.TranslateOptions) (to types.Dro if util.NotEmpty(from.ContentsLocal) { c := path.New("yaml", "contents_local") - if options.FilesDir == "" { - r.AddOnError(c, common.ErrNoFilesDir) - return - } - - // calculate file path within FilesDir and check for - // path traversal - filePath := filepath.Join(options.FilesDir, *from.ContentsLocal) - if err := baseutil.EnsurePathWithinFilesDir(filePath, options.FilesDir); err != nil { - r.AddOnError(c, err) - return - } - contents, err := os.ReadFile(filePath) + contents, err := baseutil.ReadLocalFile(*from.ContentsLocal, options.FilesDir) if err != nil { r.AddOnError(c, err) return From 6f88d0c78279080c1bff31a1d90e3251601a117f Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Tue, 14 Mar 2023 06:31:42 -0400 Subject: [PATCH 7/7] base/v0_5_exp: ignore blank lines in SSH key files There's no benefit to including them in the output sshAuthorizedKeys array, and if we had more than one, the config would fail validation. --- base/v0_5_exp/translate.go | 10 +++++----- base/v0_5_exp/translate_test.go | 26 ++------------------------ 2 files changed, 7 insertions(+), 29 deletions(-) diff --git a/base/v0_5_exp/translate.go b/base/v0_5_exp/translate.go index 6a62bd96..7aa06877 100644 --- a/base/v0_5_exp/translate.go +++ b/base/v0_5_exp/translate.go @@ -232,11 +232,11 @@ func translatePasswdUser(from PasswdUser, options common.TranslateOptions) (to t r.AddOnError(c.Append(keyFileIndex), err) continue } - - // offset for TranslationSets when both ssh_authorized_keys and ssh_authorized_keys_local are available - offset := len(to.SSHAuthorizedKeys) - for i, line := range regexp.MustCompile("\r?\n").Split(string(sshKeys), -1) { - tm.AddTranslation(c.Append(keyFileIndex), path.New("json", "sshAuthorizedKeys", i+offset)) + for _, line := range regexp.MustCompile("\r?\n").Split(string(sshKeys), -1) { + if line == "" { + continue + } + tm.AddTranslation(c.Append(keyFileIndex), path.New("json", "sshAuthorizedKeys", len(to.SSHAuthorizedKeys))) to.SSHAuthorizedKeys = append(to.SSHAuthorizedKeys, types.SSHAuthorizedKey(line)) } } diff --git a/base/v0_5_exp/translate_test.go b/base/v0_5_exp/translate_test.go index d3adf696..e43c713e 100644 --- a/base/v0_5_exp/translate_test.go +++ b/base/v0_5_exp/translate_test.go @@ -1962,19 +1962,13 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ types.SSHAuthorizedKey(sshKey2), types.SSHAuthorizedKey("#comment"), - types.SSHAuthorizedKey(""), - types.SSHAuthorizedKey(""), types.SSHAuthorizedKey(sshKey3), - types.SSHAuthorizedKey(""), }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 3)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 4)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 5)}, }, "", sshKeyDir, @@ -1986,10 +1980,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.SSHAuthorizedKey(sshKey1), types.SSHAuthorizedKey(sshKey2), types.SSHAuthorizedKey("#comment"), - types.SSHAuthorizedKey(""), - types.SSHAuthorizedKey(""), types.SSHAuthorizedKey(sshKey3), - types.SSHAuthorizedKey(""), }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, @@ -1997,9 +1988,6 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 1)}, {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 2)}, {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 3)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 4)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 5)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 1), To: path.New("json", "sshAuthorizedKeys", 6)}, }, "", sshKeyDir, @@ -2023,10 +2011,7 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.SSHAuthorizedKey(sshKeyInline), types.SSHAuthorizedKey(sshKey2), types.SSHAuthorizedKey("#comment"), - types.SSHAuthorizedKey(""), - types.SSHAuthorizedKey(""), types.SSHAuthorizedKey(sshKey3), - types.SSHAuthorizedKey(""), }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, @@ -2034,9 +2019,6 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 3)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 4)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 5)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 6)}, }, "", sshKeyDir, @@ -2044,10 +2026,9 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { { "valid empty local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyEmptyFileName}}, - types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey("")}}, + types.PasswdUser{}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, }, "", sshKeyDir, @@ -2055,11 +2036,10 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { { "valid blank local file", PasswdUser{SSHAuthorizedKeysLocal: []string{sshKeyBlankFileName}}, - types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey(""), types.SSHAuthorizedKey("\t")}}, + types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{types.SSHAuthorizedKey("\t")}}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, }, "", sshKeyDir, @@ -2070,13 +2050,11 @@ func TestTranslateSSHAuthorizedKey(t *testing.T) { types.PasswdUser{SSHAuthorizedKeys: []types.SSHAuthorizedKey{ types.SSHAuthorizedKey(sshKey1), types.SSHAuthorizedKey("#comment"), - types.SSHAuthorizedKey(""), }}, []translate.Translation{ {From: path.New("yaml", "ssh_authorized_keys_local"), To: path.New("json", "sshAuthorizedKeys")}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 0)}, {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 1)}, - {From: path.New("yaml", "ssh_authorized_keys_local", 0), To: path.New("json", "sshAuthorizedKeys", 2)}, }, "", sshKeyDir,