Skip to content
This repository was archived by the owner on Aug 25, 2026. It is now read-only.

base/v0_6_exp: add parent directory sugar - #508

Closed
prestist wants to merge 1 commit into
coreos:mainfrom
prestist:directories-sugar
Closed

base/v0_6_exp: add parent directory sugar#508
prestist wants to merge 1 commit into
coreos:mainfrom
prestist:directories-sugar

Conversation

@prestist

@prestist prestist commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator

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.

@prestist

prestist commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator Author

still adding unit tests and cleaning up code.

@prestist
prestist force-pushed the directories-sugar branch 2 times, most recently from d532226 to 9f25bc1 Compare February 9, 2024 21:34
@prestist

prestist commented Feb 9, 2024

Copy link
Copy Markdown
Collaborator Author

Ooof, ok

Looking at other tests they correctly have a storage translation set

you can see this here with this TestTranslateMountUnit but for some reason, this one test is .. not? it has all of the translations minus "$.storage" I will need to look into this.

@prestist
prestist force-pushed the directories-sugar branch 2 times, most recently from 1451196 to 3125dcd Compare February 16, 2024 23:25
@prestist
prestist marked this pull request as ready for review February 16, 2024 23:26
@prestist
prestist force-pushed the directories-sugar branch 3 times, most recently from eb5115d to 07d381f Compare February 20, 2024 20:51
@prestist
prestist requested a review from jlebon February 21, 2024 21:41
@prestist
prestist force-pushed the directories-sugar branch from 07d381f to 0892fbf Compare March 14, 2024 16:54
jlebon
jlebon reviewed Mar 15, 2024
View reviewed changes
Comment thread base/v0_6_exp/schema.go
Append []Resource `yaml:"append"`
Contents Resource `yaml:"contents"`
Mode *int `yaml:"mode"`
Parent Parent `yaml:"parent"`

@jlebon jlebon Mar 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to add this to Directory and Link.

Comment thread base/v0_6_exp/translate.go
return
}

func translateLuks(from Luks, options common.TranslateOptions) (to types.Luks, tm translate.TranslationSet, r report.Report) {

@jlebon jlebon Mar 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels weird to me that we need to rewrite this since IIUC nothing in what we do is related to LUKS (right?).

Comment thread base/v0_6_exp/translate.go
Comment on lines +128 to +133
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)

@jlebon jlebon Mar 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedence for other sugar we have in Butane which ends up having to relist every other section at the level it's operating at?

@travier travier mentioned this pull request May 30, 2024
Closed
@yasminvalim yasminvalim self-assigned this Jun 17, 2024
@yasminvalim
yasminvalim force-pushed the directories-sugar branch 14 times, most recently from d5a0200 to 8998a97 Compare June 24, 2024 12:16
@yasminvalim
yasminvalim force-pushed the directories-sugar branch 6 times, most recently from 63c3086 to da97cc0 Compare June 26, 2024 14:33
@yasminvalim
yasminvalim force-pushed the directories-sugar branch 12 times, most recently from 3ea71ea to cba27e1 Compare July 5, 2024 22:13
@prestist
prestist force-pushed the directories-sugar branch 2 times, most recently from 63ed6c2 to aa476a4 Compare July 18, 2024 14:18
@prestist @yasminvalim
@jmarrero @c4rt0 @lukewarmtemp @ravanelli @HuijingHei
base/v0_6_exp: add parent directory sugar
4c52ca0 
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: coreos#380

Co-authored-by: Yasmin Valim  <[email protected]>
Co-authored-by: Joseph Corchado <[email protected]>
Co-authored-by: Adam Piasecki <[email protected]>
Co-authored-by: Luke Yang <[email protected]>
Co-authored-by: Renata Ravanelli <[email protected]>
Co-authored-by: Huijing Hei <[email protected]>
@prestist
prestist force-pushed the directories-sugar branch from aa476a4 to 4c52ca0 Compare July 19, 2024 17:28
@prestist

prestist commented Jul 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@yasminvalim windows weird ci has been fixed, now we can start addressing issues brought up by @jlebon.

@loeffelpan

loeffelpan commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Any plans to get this done?
Or does coreos/ignition#2265 cover this.

By now creating a file in a third level subdir results in something unhandy like this.

variant: fcos
version: 1.7.0
storage:
  directories:
    - path: /home/builder/.config
      user:
        name: builder
      group:
        name: builder
    - path: /home/builder/.config/systemd
      user:
        name: builder
      group:
        name: builder
    - path: /home/builder/.config/systemd/user
      user:
        name: builder
      group:
        name: builder
    - path: /home/builder/.config/systemd/user/default.target.wants
      user:
        name: builder
      group:
        name: builder

@vic1707

vic1707 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Hi @loeffelpan maybe #644 can be better for you in the mean time ?

@loeffelpan

loeffelpan commented Aug 14, 2026

Copy link
Copy Markdown

Hi @loeffelpan maybe #644 can be better for you in the mean time ?

Thanks.
So you're saying that coreos/ignition#2265 will cover this as well?

I'm using trees with another part of my configution already. However, #644 has merged year before. But latest docs do not list that possibility. Instead they say to use files and directories to manage permission of trees.

Why is that?

@vic1707

vic1707 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Hi @loeffelpan maybe #644 can be better for you in the mean time ?

Thanks. So you're saying that coreos/ignition#2265 will cover this as well?

I'm using trees with another part of my configution already. However, #644 has merged year before. But latest docs do not list that possibility. Instead they say to use files and directories to manage permission of trees.

Why is that?

I'm just a mere contributor, idk if/when coreos/ignition#2265 will happen.
I just thought that #644 could maybe help you get rid of the mess, docs for 1.7 lists the permissions settings 😉

@loeffelpan

loeffelpan commented Aug 14, 2026

Copy link
Copy Markdown

I'm just a mere contributor, idk if/when coreos/ignition#2265 will happen. I just thought that #644 could maybe help you get rid of the mess, docs for 1.7 lists the permissions settings 😉

Sorry, I missed that one in 1.7.
Thanjs for pointing that out. Will make sone things easier in my butane config.

My concern iwas not to know if/when that would be finished. Just asking if you would agree that it will cover this as well.

@prestist or @yasminvalim
Maybe this could be closed in favor of that one?

@yasminvalim

yasminvalim commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

I will be working on this on Ignition repo, since butane is being merged into the Ignition repository, and the Butane repo will be archived soon.

@prestist prestist closed this Aug 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@jlebon jlebon jlebon left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@yasminvalim yasminvalim

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@prestist @loeffelpan @vic1707 @yasminvalim @jlebon