Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/archive: avoid user lookups when generating tarball #1837

Conversation

giuseppe
Copy link
Member

This change introduces a new approach to generating tar headers in a way that avoids system-dependent lookups and potential calls to glibc, enhancing portability and security.

The same logic is used by Moby, and the override function is based on code from Moby.

Closes: #1836

Copy link
Contributor

openshift-ci bot commented Feb 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@giuseppe giuseppe force-pushed the archive-ignore-username-groupname branch from 3f13337 to e0bd86a Compare February 19, 2024 11:14
@rhatdan
Copy link
Member

rhatdan commented Feb 19, 2024

LGTM
although CI/CD system is broken right now.

@giuseppe giuseppe force-pushed the archive-ignore-username-groupname branch from e0bd86a to 44056d4 Compare February 20, 2024 15:00
@giuseppe giuseppe marked this pull request as ready for review February 20, 2024 15:00
@giuseppe
Copy link
Member Author

LGTM although CI/CD system is broken right now.

green now

@rhatdan
Copy link
Member

rhatdan commented Feb 21, 2024

LGTM

@rhatdan rhatdan added the 5.0 label Feb 21, 2024
// sysStatOverride, if non-nil, populates hdr from system-dependent fields of fi.
var sysStatOverride func(fi os.FileInfo, hdr *tar.Header) error

func FileInfoHeaderNoLookups(fi os.FileInfo, link string) (*tar.Header, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be exported?

Copy link
Member Author

Choose a reason for hiding this comment

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

unexported and pushed a new version.

This change introduces a new approach to generating tar headers in a way
that avoids system-dependent lookups and potential calls to glibc, enhancing
portability and security.

The same logic is used by Moby, and the override function is based on
code from Moby.

Closes: containers#1836

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the archive-ignore-username-groupname branch from 44056d4 to e024854 Compare February 23, 2024 08:21
@rhatdan
Copy link
Member

rhatdan commented Feb 26, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 26, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit ee9e661 into containers:main Feb 26, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman container export creates tarball with symbolic owner and group
3 participants