Add support for exporting and importing file flags on FreeBSD #1378

dfr · 2022-10-01T14:14:59Z

The implementation could be shared with darwin and other BSDs if needed but initially is only enabled on FreeBSD. This uses the SCHILY.fflags PAX option to encode flags which is supported by libarchive (bsdtar) and star. The flag save/restore entry points are exported, allowing them to be used by buildah/copier.

To support applying diffs to trees with flags, I added logic to reset immutable flags during the UnpackLayer process - this is slightly complicated by the desire to support immutable directories which needs to defer setting fiags on directories to after all modifications to the directory contents. Fortunately, something similar is already in place for setting directory modify times.

pkg/system/lchflags_bsd.go

rhatdan

One question
LGTM

rhatdan · 2022-10-07T11:03:24Z

@giuseppe @vrothberg @nalind @flouthoc @umohnani8 PTAL

dfr · 2022-10-07T12:56:46Z

Rebased

rhatdan · 2022-10-13T09:16:54Z

@nalind @saschagrunert @giuseppe @vrothberg @umohnani8 PTAL

flouthoc · 2022-10-13T12:23:38Z

There are some commits in archive on which and I don't have lot of idea on archive part, but they will affect buildah and podman afaics on *nix as well. I'd defer review for this to @nalind @giuseppe.

flouthoc · 2022-10-13T12:28:01Z

@dfr commit 8bd87e9 adds changes to *nix counterpart as well right ? But commit only talks about FreeBSD. Could we split commit which affects *nix into subcommits ?

Please ignore my concern if i understood this wrong :)

dfr · 2022-10-13T12:53:49Z

@flouthoc that commit could be split into functional parts, e.g.:

Adding fflags_*.go with entry points to encode/decode flags
Adding support for detecting flag changes in changes_unix.go and changes_bsd_test.go
Adding calls to those encode/decode flags in archive.go and diff.go

Would that help to clarify?

flouthoc · 2022-10-13T14:16:28Z

@flouthoc that commit could be split into functional parts, e.g.:

* Adding fflags_*.go with entry points to encode/decode flags

* Adding support for detecting flag changes in changes_unix.go and changes_bsd_test.go

* Adding calls to those encode/decode flags in archive.go and diff.go

Would that help to clarify?

@dfr Yes I think that would be really helpful thanks for considering my comment :) .

dfr · 2022-10-13T14:48:47Z

I split the pkg/archive commit along the lines I suggested - hope that helps.

dfr · 2022-10-13T15:43:39Z

Rebased

pkg/system/rm.go

rhatdan · 2022-10-14T13:41:00Z

@flouthoc PTAL

rhatdan · 2022-10-15T09:55:48Z

@flouthoc @nalind @saschagrunert @giuseppe @vrothberg @umohnani8 PTAL

dfr · 2022-10-19T14:00:45Z

Rebased

rhatdan · 2022-10-19T15:15:03Z

@flouthoc @nalind @saschagrunert @giuseppe @vrothberg @umohnani8 PTAL

flouthoc · 2022-10-19T15:36:19Z

There are some commits in archive on which and I don't have lot of idea on archive part, but they will affect buildah and podman afaics on *nix as well. I'd defer review for this to @nalind @giuseppe.

PR LGTM but I'd like @nalind @giuseppe @mtrmac @vrothberg to take a look for above reason.

mtrmac

LGTM — I’m really not that familiar with pkg/archive, but at least the added tests add quite a bit of confidence.

Just a few optional stylistic nits.

pkg/archive/fflags_bsd.go

rhatdan · 2022-10-19T22:11:59Z

Could you fix the two issues @mtrmac found.

This also adds an integration test for deleting containers with immutable files. Signed-off-by: Doug Rabson <dfr@rabson.org>

Signed-off-by: Doug Rabson <dfr@rabson.org>

Note: these are likely to be identical on other BSDs, including darwin, but I'm keeping this for just FreeBSD to start with. Signed-off-by: Doug Rabson <dfr@rabson.org>

This will also probably work on other BSD derived platforms including darwin. Signed-off-by: Doug Rabson <dfr@rabson.org>

This adds platform-specific entry points to encode and decode file flags. These are exported so that they can be used in buildah/copier. A function which can be used to optionally reset the immutable flag on a file or directory is also added and this will be used when applying layer changes to trees with immutable files or directories. These new entry points are implemented for FreeBSD and are noop stubs on other platforms. The chflags approach could be used on darwin and other BSD-derived platforms if needed. On FreeBSD, file flag information is encoded for import/export using the PAX SCHILY.fflags option. This is compatible with bsdtar (see https://github.com/libarchive/libarchive). Reading the code for libarchive, it also has support for encoding/decoding Linux file flags so this seems like a good reference. Signed-off-by: Doug Rabson <dfr@rabson.org>

Note: the Flags method of system.StatT always returns zero on non-FreeBSD platforms so this should have no functional change for those platforms. Signed-off-by: Doug Rabson <dfr@rabson.org>

This encodes flag information into the tar stream using ReadFileFlagsToTarHeader and decodes with WriteFileFlagsFromTarHeader. To support applying diffs to trees with flags, this adds logic to reset immutable flags during the UnpackLayer process. To support immutable directories, we also need to defer setting flags on directories until after all modifications to the directory contents. Fortunately, something similar is already in place for setting directory modify times. Signed-off-by: Doug Rabson <dfr@rabson.org>

dfr · 2022-10-20T07:58:18Z

Could you fix the two issues @mtrmac found.

Done, thanks!

rhatdan · 2022-10-20T10:21:32Z

LGTM

dfr force-pushed the freebsd-chflags branch 2 times, most recently from 2d00e72 to f820350 Compare October 3, 2022 09:17

rhatdan reviewed Oct 7, 2022

View reviewed changes

pkg/system/lchflags_bsd.go Show resolved Hide resolved

rhatdan approved these changes Oct 7, 2022

View reviewed changes

dfr force-pushed the freebsd-chflags branch from f820350 to df8287e Compare October 7, 2022 12:56

dfr force-pushed the freebsd-chflags branch 2 times, most recently from d3e97d2 to 8bd87e9 Compare October 12, 2022 14:06

dfr force-pushed the freebsd-chflags branch from dabfc82 to d714031 Compare October 13, 2022 14:47

dfr force-pushed the freebsd-chflags branch from d714031 to e0ddfad Compare October 13, 2022 15:43

flouthoc reviewed Oct 14, 2022

View reviewed changes

pkg/system/rm.go Show resolved Hide resolved

dfr force-pushed the freebsd-chflags branch from e0ddfad to 6ebe746 Compare October 14, 2022 09:16

dfr force-pushed the freebsd-chflags branch from 6ebe746 to da09ce2 Compare October 19, 2022 14:00

mtrmac reviewed Oct 19, 2022

View reviewed changes

pkg/archive/fflags_bsd.go Outdated Show resolved Hide resolved

pkg/archive/fflags_bsd.go Outdated Show resolved Hide resolved

dfr force-pushed the freebsd-chflags branch from da09ce2 to b524379 Compare October 20, 2022 07:56

pkg/system: Add Lchflags and use that in resetFileFlags

adb0355

This also adds an integration test for deleting containers with immutable files. Signed-off-by: Doug Rabson <dfr@rabson.org>

dfr added 6 commits October 20, 2022 08:58

pkg/system: Defer the call to resetFileFlags until after RemoveAll fails

a987773

Signed-off-by: Doug Rabson <dfr@rabson.org>

pkg/system: Add file flag names for FreeBSD

3ad57b5

Note: these are likely to be identical on other BSDs, including darwin, but I'm keeping this for just FreeBSD to start with. Signed-off-by: Doug Rabson <dfr@rabson.org>

pkg/system: Add file flags to StatT on FreeBSD

9ad4537

This will also probably work on other BSD derived platforms including darwin. Signed-off-by: Doug Rabson <dfr@rabson.org>

pkg/archive: Add support for detecting flag changes

2baf25d

Note: the Flags method of system.StatT always returns zero on non-FreeBSD platforms so this should have no functional change for those platforms. Signed-off-by: Doug Rabson <dfr@rabson.org>

dfr force-pushed the freebsd-chflags branch from b524379 to 056232e Compare October 20, 2022 07:58

rhatdan merged commit db44035 into containers:main Oct 20, 2022

dfr deleted the freebsd-chflags branch October 20, 2022 16:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for exporting and importing file flags on FreeBSD #1378

Add support for exporting and importing file flags on FreeBSD #1378

dfr commented Oct 1, 2022

rhatdan left a comment

rhatdan commented Oct 7, 2022

dfr commented Oct 7, 2022

rhatdan commented Oct 13, 2022

flouthoc commented Oct 13, 2022 •

edited

flouthoc commented Oct 13, 2022

dfr commented Oct 13, 2022

flouthoc commented Oct 13, 2022

dfr commented Oct 13, 2022

dfr commented Oct 13, 2022

rhatdan commented Oct 14, 2022

rhatdan commented Oct 15, 2022

dfr commented Oct 19, 2022

rhatdan commented Oct 19, 2022

flouthoc commented Oct 19, 2022

mtrmac left a comment

rhatdan commented Oct 19, 2022

dfr commented Oct 20, 2022

rhatdan commented Oct 20, 2022

Add support for exporting and importing file flags on FreeBSD #1378

Add support for exporting and importing file flags on FreeBSD #1378

Conversation

dfr commented Oct 1, 2022

rhatdan left a comment

Choose a reason for hiding this comment

rhatdan commented Oct 7, 2022

dfr commented Oct 7, 2022

rhatdan commented Oct 13, 2022

flouthoc commented Oct 13, 2022 • edited

flouthoc commented Oct 13, 2022

dfr commented Oct 13, 2022

flouthoc commented Oct 13, 2022

dfr commented Oct 13, 2022

dfr commented Oct 13, 2022

rhatdan commented Oct 14, 2022

rhatdan commented Oct 15, 2022

dfr commented Oct 19, 2022

rhatdan commented Oct 19, 2022

flouthoc commented Oct 19, 2022

mtrmac left a comment

Choose a reason for hiding this comment

rhatdan commented Oct 19, 2022

dfr commented Oct 20, 2022

rhatdan commented Oct 20, 2022

flouthoc commented Oct 13, 2022 •

edited