Skip to content

Preserve dir mode and ownership in RUN --mount - #6981

Open
simonbrauner wants to merge 1 commit into
podman-container-tools:mainfrom
simonbrauner:issue-6747
Open

Preserve dir mode and ownership in RUN --mount#6981
simonbrauner wants to merge 1 commit into
podman-container-tools:mainfrom
simonbrauner:issue-6747

Conversation

@simonbrauner

@simonbrauner simonbrauner commented Jul 21, 2026

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Fixes: #6747
Fixes: podman-container-tools/podman#27777

Special notes for your reviewer:

Does this PR introduce a user-facing change?

RUN --mount does not reset parent directory mode and ownership with --layers

@simonbrauner
simonbrauner force-pushed the issue-6747 branch 9 times, most recently from bc63624 to 3908788 Compare July 28, 2026 08:52
@simonbrauner

Copy link
Copy Markdown
Author

Initially, the layer pull-ups were always excluded. This led to default directory permissions being used when a child entry survived the filter, because the tar extractor recreated the missing parent directories with root ownership.

I started with this fix simonbrauner@b3ca65c which, as a contrast, included all the layer pull-ups. But that caused this test to fail ff07ebc.

The final solution defers pull-up entries in the tar filter and only emits them when a child entry actually passes through. Parents with no surviving children are discarded at EOF, keeping layers minimal while preserving correct ownership.

@simonbrauner

Copy link
Copy Markdown
Author

PTAL @podman-container-tools/buildah-maintainers @podman-container-tools/buildah-reviewers

@simonbrauner
simonbrauner marked this pull request as ready for review July 28, 2026 11:04
@simonbrauner

Copy link
Copy Markdown
Author

/packit rebuild-failed

@nalind nalind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, nice!

Comment thread image.go
if pullUpSpec.Mode != nil {
hdr.Mode = int64(*pullUpSpec.Mode & os.ModePerm)
}
return tarFilterDefer, false, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return tarFilterDefer, false, nil
// Fall through so timestamp/Windows transforms apply
// before the deferred header is copied for later emit.
action = tarFilterDefer

Comment thread image.go
}
}
return false, false, nil
return tarFilterKeep, false, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return tarFilterKeep, false, nil
return action, false, nil

Comment thread image.go
var initialized bool
wc = newTarFilterer(wc, func(hdr *tar.Header) (skip, replaceContents bool, replacementContents io.Reader) {
wc = newTarFilterer(wc, func(hdr *tar.Header) (action tarFilterAction, replaceContents bool, replacementContents io.Reader) {
modTime := hdr.ModTime

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
modTime := hdr.ModTime
action = tarFilterKeep
modTime := hdr.ModTime

@TomSweeneyRedHat

Copy link
Copy Markdown
Contributor

My AI reviewer suggested handling windows a bit better. I think it looks good, but if @nalind says no, ignore the suggestions.

Otherwise LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants