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

Ignore error when xattr data is not available #457

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

rchincha
Copy link
Contributor

When the underlying filesystem is overlayfs, it is possible that xattrs are present but without the data. So check for this case specifically and ignore it.

Signed-off-by: Ramkumar Chinchani [email protected]

@rchincha
Copy link
Contributor Author

Is this CI failure related to this PR?

Run # TODO: Move this to 'make fetch-deps'.
  # TODO: Move this to 'make fetch-deps'.
  GO111MODULE=off go get -u github.com/cpuguy83/go-md2man
  GO111MODULE=off go get -u golang.org/x/lint/golint
  GO111MODULE=off go get -u github.com/securego/gosec/cmd/gosec
  GO111MODULE=off go get -u github.com/client9/misspell/cmd/misspell
  shell: /usr/bin/bash -e {0}
# github.com/securego/gosec/rules
Error: ../../../go/src/github.com/securego/gosec/rules/readfile.go:29:17: undefined: any
Error: ../../../go/src/github.com/securego/gosec/rules/readfile.go:63:11: assignment mismatch: 2 variables but 1 value
Error: ../../../go/src/github.com/securego/gosec/rules/readfile.go:135:19: undefined: any
Error: Process completed with exit code 2.


// in case of overlayfs, we may not get the xattr data itself,
// so ignore if this particular error code
if err == syscall.ENODATA {
Copy link
Member

Choose a reason for hiding this comment

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

if errors.Is(err, unix.ENODATA) would be better.

@cyphar
Copy link
Member

cyphar commented Apr 17, 2023

No, it's because we haven't bumped the Go version for linting recently. I'll fix it now.

@cyphar cyphar added this to the 0.4.8 milestone Apr 21, 2023
@cyphar
Copy link
Member

cyphar commented Apr 29, 2023

@rchincha Can you rebase this? CI has been fixed.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2023

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 73.40%. Comparing base (7b9470b) to head (89a7ea1).
Report is 39 commits behind head on main.

❗ Current head 89a7ea1 differs from pull request most recent head 4d17e64. Consider uploading reports for the commit 4d17e64 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
- Coverage   73.45%   73.40%   -0.05%     
==========================================
  Files          60       57       -3     
  Lines        4886     4885       -1     
==========================================
- Hits         3589     3586       -3     
  Misses        937      937              
- Partials      360      362       +2     
Files Coverage Δ
pkg/system/copy.go 48.27% <100.00%> (+10.34%) ⬆️
mutate/compress.go 44.92% <50.00%> (+1.59%) ⬆️

... and 4 files with indirect coverage changes

rchincha and others added 9 commits October 25, 2023 23:32
Goals of this PR:
1. Allow passing in options to individual compressor
2. Do not change default behavior

Signed-off-by: Ramkumar Chinchani <[email protected]>
…ix-empty-lgetxattr

2024 01 22/backport fix empty lgetxattr
generatelayer closes the tarwriter, but generateinsertlayer forgets to.

Closing the tarwriter writes the required footer of 1k of zeros.

This results in tar files that are complete but invalid, and different
reading tools will behave differently:

- bsdtar doesn't complain and exits 0
- gnu tar (and security scanning tools that use it) will exit 2 with an
unexpected EOF message
- python's tarfile library will raise an Unexpected EOF error
- golang's archive/tar library can raise an unexpected EOF error, but
  for some files created by generateinsertlayer, it just raises a
  plain EOF error, which means golang based tools generally ignore this
  and work fine, this includes umoci.

Signed-off-by: Michael McCracken <[email protected]>
(cherry picked from commit ad29ed3)
This warning is a little confusing when err is nil,
and doesn't add any info in that case. Let's clean that up.

Signed-off-by: Michael McCracken <[email protected]>
(cherry picked from commit 653952b)
add a check to image-verify to ensure that all generated tar blobs are
valid and do not cause gnu tar to exit nonzero

add an insert test that adds a very small file to trigger unexpected EOF
in the case where GenerateInsertLayer forgets to close the TarWriter.

Signed-off-by: Michael McCracken <[email protected]>
(cherry picked from commit 7bb2940)
…ckerfork-main/close-tarwriter

2024.02.22/stackerfork main/close tarwriter
@rchincha rchincha force-pushed the stacker branch 4 times, most recently from fc29e2e to e224054 Compare March 25, 2024 18:47
@tych0
Copy link
Member

tych0 commented Mar 25, 2024

Can you rebase with only the commits for this branch and drop all the merge noise?

@rchincha rchincha force-pushed the stacker branch 8 times, most recently from 24d357b to 3c7bd3c Compare March 28, 2024 21:07
@rchincha rchincha force-pushed the stacker branch 5 times, most recently from 8c0bc21 to d67d94b Compare March 28, 2024 22:08
Current behavior determines if a path is a whiteout if a overlay char
dev is present.

Additionally, also check the extended attrs.

Signed-off-by: Ramkumar Chinchani <[email protected]>
rchincha and others added 2 commits March 29, 2024 08:44
fix: handle overlay xattr opaque bit
CI failures indicate that this could be another error code that needs
to be handled.

Signed-off-by: Ramkumar Chinchani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants