From ccf0ac964b86f82f6abb250dcf444b0bb7f9c01d Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Fri, 9 Feb 2024 12:00:33 -0500 Subject: [PATCH] base/v0_6_exp: add parent directory sugar Add a field called 'Parent' which is used to specify a file's parent directory. When a parent is specified, all directories from the parent to the file will be created, with the 'mode' supplied in the parent directory. resolves: #380 Co-authored-by: Yasmin Valim Co-authored-by: Joseph Corchado Co-authored-by: Adam Piasecki --- base/v0_6_exp/schema.go | 6 + base/v0_6_exp/translate.go | 79 +++++++++++- base/v0_6_exp/translate_test.go | 189 +++++++++++++++++++++++++++++ config/common/errors.go | 1 + docs/config-fcos-v1_6-exp.md | 3 + docs/config-fiot-v1_1-exp.md | 3 + docs/config-flatcar-v1_2-exp.md | 3 + docs/config-openshift-v4_17-exp.md | 3 + docs/config-r4e-v1_2-exp.md | 3 + docs/examples.md | 17 +++ docs/release-notes.md | 3 + internal/doc/butane.yaml | 8 ++ 12 files changed, 313 insertions(+), 5 deletions(-) diff --git a/base/v0_6_exp/schema.go b/base/v0_6_exp/schema.go index 67e7b95c..f341329f 100644 --- a/base/v0_6_exp/schema.go +++ b/base/v0_6_exp/schema.go @@ -67,6 +67,12 @@ type File struct { Append []Resource `yaml:"append"` Contents Resource `yaml:"contents"` Mode *int `yaml:"mode"` + Parent Parent `yaml:"parent"` +} + +type Parent struct { + Path *string `yaml:"path,omitempty"` + Mode *int `yaml:"mode,omitempty"` } type Filesystem struct { diff --git a/base/v0_6_exp/translate.go b/base/v0_6_exp/translate.go index 8cf2395f..ae0b086d 100644 --- a/base/v0_6_exp/translate.go +++ b/base/v0_6_exp/translate.go @@ -83,9 +83,7 @@ func (c Config) ToIgn3_5Unvalidated(options common.TranslateOptions) (types.Conf tr := translate.NewTranslator("yaml", "json", options) tr.AddCustomTranslator(translateIgnition) - tr.AddCustomTranslator(translateFile) - tr.AddCustomTranslator(translateDirectory) - tr.AddCustomTranslator(translateLink) + tr.AddCustomTranslator(translateStorage) tr.AddCustomTranslator(translateResource) tr.AddCustomTranslator(translatePasswdUser) tr.AddCustomTranslator(translateUnit) @@ -99,7 +97,6 @@ func (c Config) ToIgn3_5Unvalidated(options common.TranslateOptions) (types.Conf translate.MergeP(tr, tm, &r, "systemd", &c.Systemd, &ret.Systemd) c.addMountUnits(&ret, &tm) - tm2, r2 := c.processTrees(&ret, options) tm.Merge(tm2) r.Merge(r2) @@ -121,6 +118,62 @@ func translateIgnition(from Ignition, options common.TranslateOptions) (to types return } +func translateStorage(from Storage, options common.TranslateOptions) (to types.Storage, tm translate.TranslationSet, r report.Report) { + tr := translate.NewTranslator("yaml", "json", options) + tr.AddCustomTranslator(translateFile) + tr.AddCustomTranslator(translateDirectory) + tr.AddCustomTranslator(translateLink) + tr.AddCustomTranslator(translateLuks) + tm, r = translate.Prefixed(tr, "directories", &from.Directories, &to.Directories) + translate.MergeP(tr, tm, &r, "disks", &from.Disks, &to.Disks) + translate.MergeP(tr, tm, &r, "files", &from.Files, &to.Files) + translate.MergeP(tr, tm, &r, "filesystems", &from.Filesystems, &to.Filesystems) + translate.MergeP(tr, tm, &r, "links", &from.Links, &to.Links) + translate.MergeP(tr, tm, &r, "luks", &from.Luks, &to.Luks) + translate.MergeP(tr, tm, &r, "raid", &from.Raid, &to.Raid) + for i, file := range from.Files { + if util.NotEmpty(file.Parent.Path) { + yamlPath := path.New("yaml", "files", i, "parent") + + if !strings.Contains(file.Path, *file.Parent.Path) { + r.AddOnError(yamlPath, common.ErrInvalidParent) + continue + } + + dir := filepath.Dir(file.Path) + // make sure to clean the path to avoid consistency issues + + dir = filepath.Clean(dir) + + for dir != "" { + renderedDir := types.Directory{ + Node: types.Node{ + Path: dir, + Group: types.NodeGroup{ID: file.Group.ID, Name: file.Group.Name}, + User: types.NodeUser{ID: file.User.ID, Name: file.User.Name}, + }, + DirectoryEmbedded1: types.DirectoryEmbedded1{ + Mode: file.Parent.Mode, + }, + } + to.Directories = append(to.Directories, renderedDir) + nextDir, _ := filepath.Split(dir) + // make sure to clean the path to avoid consistency issues + nextDir = filepath.Clean(nextDir) + nextDir = filepath.ToSlash(nextDir) + if dir == *file.Parent.Path || nextDir == dir { + // we have reached the parent directory or the end of the path + break + } + dir = nextDir + } + tm.AddFromCommonSource(yamlPath, path.New("json", "directories"), to.Directories) + } + + } + return +} + func translateFile(from File, options common.TranslateOptions) (to types.File, tm translate.TranslationSet, r report.Report) { tr := translate.NewTranslator("yaml", "json", options) tr.AddCustomTranslator(translateResource) @@ -134,6 +187,22 @@ func translateFile(from File, options common.TranslateOptions) (to types.File, t return } +func translateLuks(from Luks, options common.TranslateOptions) (to types.Luks, tm translate.TranslationSet, r report.Report) { + tr := translate.NewTranslator("yaml", "json", options) + tr.AddCustomTranslator(translateResource) + tm, r = translate.Prefixed(tr, "clevis", &from.Clevis, &to.Clevis) + translate.MergeP(tr, tm, &r, "device", &from.Device, &to.Device) + translate.MergeP(tr, tm, &r, "discard", &from.Discard, &to.Discard) + translate.MergeP2(tr, tm, &r, "key_file", &from.KeyFile, "keyFile", &to.KeyFile) + translate.MergeP(tr, tm, &r, "label", &from.Label, &to.Label) + translate.MergeP(tr, tm, &r, "name", &from.Name, &to.Name) + translate.MergeP2(tr, tm, &r, "open_options", &from.OpenOptions, "openOptions", &to.OpenOptions) + translate.MergeP(tr, tm, &r, "options", &from.Options, &to.Options) + translate.MergeP(tr, tm, &r, "uuid", &from.UUID, &to.UUID) + translate.MergeP2(tr, tm, &r, "wipe_volume", &from.WipeVolume, "wipeVolume", &to.WipeVolume) + return +} + func translateResource(from Resource, options common.TranslateOptions) (to types.Resource, tm translate.TranslationSet, r report.Report) { tr := translate.NewTranslator("yaml", "json", options) tm, r = translate.Prefixed(tr, "verification", &from.Verification, &to.Verification) @@ -294,7 +363,7 @@ func (c Config) processTrees(ret *types.Config, options common.TranslateOptions) return ts, r } t := newNodeTracker(ret) - + ts.AddTranslation(path.New("yaml", "storage"), path.New("json", "storage")) for i, tree := range c.Storage.Trees { yamlPath := path.New("yaml", "storage", "trees", i) if options.FilesDir == "" { diff --git a/base/v0_6_exp/translate_test.go b/base/v0_6_exp/translate_test.go index cf718d4a..a698dd88 100644 --- a/base/v0_6_exp/translate_test.go +++ b/base/v0_6_exp/translate_test.go @@ -599,6 +599,195 @@ func TestTranslateFile(t *testing.T) { }) } } +func TestTranslateStorage(t *testing.T) { + tests := []struct { + in Storage + out types.Storage + errPath path.ContextPath + errors error + skip func(t *testing.T) + }{ + // Basic parent file directory + { + in: Storage{ + Files: []File{ + { + Path: "/foo/bar/txt.txt", + Contents: Resource{}, + Mode: util.IntToPtr(420), + Parent: Parent{ + Path: util.StrToPtr("/foo"), + Mode: util.IntToPtr(420), + }, + }, + }, + }, + out: types.Storage{ + Files: []types.File{ + { + Node: types.Node{ + Path: "/foo/bar/txt.txt", + }, + FileEmbedded1: types.FileEmbedded1{ + Mode: util.IntToPtr(420), + Contents: types.Resource{}, + }, + }, + }, + Directories: []types.Directory{ + { + Node: types.Node{ + Path: "/foo/bar", + }, + DirectoryEmbedded1: types.DirectoryEmbedded1{ + Mode: util.IntToPtr(420), + }, + }, + { + Node: types.Node{ + Path: "/foo", + }, + DirectoryEmbedded1: types.DirectoryEmbedded1{ + Mode: util.IntToPtr(420), + }, + }, + }, + }, + errPath: path.ContextPath{}, + errors: nil, + skip: func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + }, + }, + // Empty parent file directory + { + in: Storage{ + Files: []File{ + { + Path: "/foo/bar/txt.txt", + Contents: Resource{}, + Mode: util.IntToPtr(420), + Parent: Parent{ + Path: util.StrToPtr(""), + Mode: util.IntToPtr(420), + }, + }, + }, + }, + out: types.Storage{ + Files: []types.File{ + { + Node: types.Node{ + Path: "/foo/bar/txt.txt", + }, + FileEmbedded1: types.FileEmbedded1{ + Mode: util.IntToPtr(420), + Contents: types.Resource{}, + }, + }, + }, + }, + errPath: path.ContextPath{}, + errors: nil, + skip: func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + }, + }, + // Parent not defined + { + in: Storage{ + Files: []File{ + { + Path: "/foo/bar/txt.txt", + Contents: Resource{}, + Mode: util.IntToPtr(420), + }, + }, + }, + out: types.Storage{ + Files: []types.File{ + { + Node: types.Node{ + Path: "/foo/bar/txt.txt", + }, + FileEmbedded1: types.FileEmbedded1{ + Mode: util.IntToPtr(420), + Contents: types.Resource{}, + }, + }, + }, + }, + errPath: path.ContextPath{}, + errors: nil, + skip: func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + }, + }, + // Parent path is not related to file path + { + in: Storage{ + Files: []File{ + { + Path: "/foo/bar/txt.txt", + Contents: Resource{}, + Mode: util.IntToPtr(420), + Parent: Parent{ + Path: util.StrToPtr("/godzilla"), + Mode: util.IntToPtr(420), + }, + }, + }, + }, + out: types.Storage{ + Files: []types.File{ + { + Node: types.Node{ + Path: "/foo/bar/txt.txt", + }, + FileEmbedded1: types.FileEmbedded1{ + Mode: util.IntToPtr(420), + Contents: types.Resource{}, + }, + }, + }, + }, + errPath: path.New("yaml", "files", 0, "parent"), + errors: common.ErrInvalidParent, + skip: func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + }, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("translate %d", i), func(t *testing.T) { + if test.skip != nil { + // give the test an opportunity to skip + test.skip(t) + } + actual, translations, r := translateStorage(test.in, common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + baseutil.VerifyReport(t, test.in, r) + assert.Equal(t, test.out, actual, "translation mismatch") + assert.NoError(t, translations.DebugVerifyCoverage(actual), "incomplete TranslationSet coverage") + if test.errors != nil { + expected := report.Report{} + expected.AddOnError(test.errPath, test.errors) + assert.Equal(t, expected, r, "bad report for test case %d", i) + } else { + assert.Equal(t, report.Report{}, r, "non-empty report") + } + }) + } +} // TestTranslateDirectory tests translating the ct storage.directories.[i] entries to ignition storage.directories.[i] entires. func TestTranslateDirectory(t *testing.T) { diff --git a/config/common/errors.go b/config/common/errors.go index 922111ab..888c1f52 100644 --- a/config/common/errors.go +++ b/config/common/errors.go @@ -89,6 +89,7 @@ var ( ErrLinkSupport = errors.New("links are not supported in this spec version") ErrLuksSupport = errors.New("luks is not supported in this spec version") ErrRaidSupport = errors.New("raid is not supported in this spec version") + ErrInvalidParent = errors.New("parent must be included in the file path") // Grub ErrGrubUserNameNotSpecified = errors.New("field \"name\" is required") diff --git a/docs/config-fcos-v1_6-exp.md b/docs/config-fcos-v1_6-exp.md index 6f8a4d1c..036b78d6 100644 --- a/docs/config-fcos-v1_6-exp.md +++ b/docs/config-fcos-v1_6-exp.md @@ -117,6 +117,9 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s * **_group_** (object): specifies the file's group. * **_id_** (integer): the group ID of the group. * **_name_** (string): the group name of the group. + * **_parent_** (object): the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + * **_path_** (string): the path of the directory within the file's 'path'. + * **_mode_** (integer): directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. * **_directories_** (list of objects): the list of directories to be created. Every file, directory, and link must have a unique `path`. * **path** (string): the absolute path to the directory. * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. If false and a directory already exists at the path, Ignition will only set its permissions. If false and a non-directory exists at that path, Ignition will fail. Defaults to false. diff --git a/docs/config-fiot-v1_1-exp.md b/docs/config-fiot-v1_1-exp.md index 8749b1e5..a9143d9c 100644 --- a/docs/config-fiot-v1_1-exp.md +++ b/docs/config-fiot-v1_1-exp.md @@ -88,6 +88,9 @@ The Fedora IoT configuration is a YAML document conforming to the following spec * **_group_** (object): specifies the file's group. * **_id_** (integer): the group ID of the group. * **_name_** (string): the group name of the group. + * **_parent_** (object): the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + * **_path_** (string): the path of the directory within the file's 'path'. + * **_mode_** (integer): directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. * **_directories_** (list of objects): the list of directories to be created. Every file, directory, and link must have a unique `path`. * **path** (string): the absolute path to the directory. * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. If false and a directory already exists at the path, Ignition will only set its permissions. If false and a non-directory exists at that path, Ignition will fail. Defaults to false. diff --git a/docs/config-flatcar-v1_2-exp.md b/docs/config-flatcar-v1_2-exp.md index 22a907e6..7ab68603 100644 --- a/docs/config-flatcar-v1_2-exp.md +++ b/docs/config-flatcar-v1_2-exp.md @@ -117,6 +117,9 @@ The Flatcar configuration is a YAML document conforming to the following specifi * **_group_** (object): specifies the file's group. * **_id_** (integer): the group ID of the group. * **_name_** (string): the group name of the group. + * **_parent_** (object): the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + * **_path_** (string): the path of the directory within the file's 'path'. + * **_mode_** (integer): directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. * **_directories_** (list of objects): the list of directories to be created. Every file, directory, and link must have a unique `path`. * **path** (string): the absolute path to the directory. * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. If false and a directory already exists at the path, Ignition will only set its permissions. If false and a non-directory exists at that path, Ignition will fail. Defaults to false. diff --git a/docs/config-openshift-v4_17-exp.md b/docs/config-openshift-v4_17-exp.md index 99a4e3f5..e9cf8bf6 100644 --- a/docs/config-openshift-v4_17-exp.md +++ b/docs/config-openshift-v4_17-exp.md @@ -107,6 +107,9 @@ The OpenShift configuration is a YAML document conforming to the following speci * **_group_** (object): specifies the file's group. * **_id_** (integer): the group ID of the group. * **_name_** (string): the group name of the group. + * **_parent_** (object): the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + * **_path_** (string): the path of the directory within the file's 'path'. + * **_mode_** (integer): directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. * **_luks_** (list of objects): the list of luks devices to be created. Every device must have a unique `name`. * **name** (string): the name of the luks device. * **device** (string): the absolute path to the device. Devices are typically referenced by the `/dev/disk/by-*` symlinks. diff --git a/docs/config-r4e-v1_2-exp.md b/docs/config-r4e-v1_2-exp.md index e6837b12..06051b18 100644 --- a/docs/config-r4e-v1_2-exp.md +++ b/docs/config-r4e-v1_2-exp.md @@ -88,6 +88,9 @@ The RHEL for Edge configuration is a YAML document conforming to the following s * **_group_** (object): specifies the file's group. * **_id_** (integer): the group ID of the group. * **_name_** (string): the group name of the group. + * **_parent_** (object): the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + * **_path_** (string): the path of the directory within the file's 'path'. + * **_mode_** (integer): directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. * **_directories_** (list of objects): the list of directories to be created. Every file, directory, and link must have a unique `path`. * **path** (string): the absolute path to the directory. * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. If false and a directory already exists at the path, Ignition will only set its permissions. If false and a non-directory exists at that path, Ignition will fail. Defaults to false. diff --git a/docs/examples.md b/docs/examples.md index ee62c41c..8a3d51d4 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -149,6 +149,23 @@ storage: mode: 0644 ``` +This example creates a file at `/opt/foo/bar/file.txt` with the contents `Hello, world!`, permissions 0644 (so readable and writable by the owner, and only readable by everyone else). Additionally is specifies a parent directory at `/opt/foo` with permissions 0644, which ensure that if the directories between `/opt/foo` and `/opt/foo/bar/file.txt` (i.e `/opt/foo`, `/opt/foo/bar`) do not exist, they will be created with the specified permissions. Use this to avoid having to create directories separately, with the correct permissions. + + +```yaml +variant: fcos +version: 1.6.0-experimental +storage: + files: + - path: /opt/foo/bar/file.txt + contents: + inline: Hello, world! + mode: 0644 + parent: + path: /opt/foo + mode: 0644 +``` + ### Directory trees Consider a directory tree at `~/conf/tree` on the system running Butane: diff --git a/docs/release-notes.md b/docs/release-notes.md index 5a205b3e..9842b4bd 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -47,6 +47,9 @@ key](https://getfedora.org/security/). - Stabilize OpenShift spec 4.15.0, targeting Ignition spec 3.4.0 - Add OpenShift spec 4.16.0-experimental, targeting Ignition spec 3.5.0-experimental +- Support s390x layouts in `boot_device` section (fcos 1.6.0-exp, openshift 4.15.0-exp) +- Add `parent` field to `files`, to reduce verbosity when configuring a deeply + nested file. _(base 0.6.0-exp)_ ### Bug fixes diff --git a/internal/doc/butane.yaml b/internal/doc/butane.yaml index 8b37492d..72eae7bd 100644 --- a/internal/doc/butane.yaml +++ b/internal/doc/butane.yaml @@ -250,6 +250,14 @@ root: - variant: openshift - name: mode use: mode + - name: parent + after: $ + desc: the parent directory for the specified file, by declaring a parent the directories from the parent to the file's target destination. + children: + - name: path + desc: the path of the directory within the file's 'path'. + - name: mode + desc: directory modes are set to 0755 as a default if not specified and directory does not exist prior to the specified file. - name: directories children: - name: mode