-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Ignore sockets when creating a tar stream of a layer #2222
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
Conversation
The go-tar implementation which is used cannot handle sockets. There's no good reason to preserve a socket, they are basically useless without the process which made them. Signed-off-by: Ian Campbell <[email protected]>
|
If accepted would be good to have this in v1.0.x too I think. |
Codecov Report
@@ Coverage Diff @@
## master #2222 +/- ##
==========================================
+ Coverage 41.06% 41.09% +0.02%
==========================================
Files 66 66
Lines 7748 7751 +3
==========================================
+ Hits 3182 3185 +3
- Misses 4064 4065 +1
+ Partials 502 501 -1
Continue to review full report at Codecov.
|
|
LGTM Would it be possible to create a test? Also, let’s backport this one. |
|
I'll have a look at a test now, I've got about 25 mins left today so it might be tomorrow before it is done. |
I need this in order to test containerd/containerd#2222 Signed-off-by: Ian Campbell <[email protected]>
I need this in order to test containerd/containerd#2222 Signed-off-by: Ian Campbell <[email protected]>
I need this in order to test containerd/containerd#2222 Signed-off-by: Ian Campbell <[email protected]>
|
I have a test now, but it depends on containerd/continuity#110. It looks like: diff --git a/archive/tar_test.go b/archive/tar_test.go
index 4d56d4e2..c94c9d37 100644
--- a/archive/tar_test.go
+++ b/archive/tar_test.go
@@ -1022,6 +1022,22 @@ func TestDiffTar(t *testing.T) {
fstest.CreateDir("/d3/", 0755),
),
},
+ {
+ name: "IgnoreSockets",
+ validators: []tarEntryValidator{
+ fileEntry("f2", []byte("content"), 0644),
+ // There should be _no_ socket here, despite the fstest.CreateSocket below
+ fileEntry("f3", []byte("content"), 0644),
+ },
+ a: fstest.Apply(
+ fstest.CreateFile("/f1", []byte("content"), 0644),
+ ),
+ b: fstest.Apply(
+ fstest.CreateFile("/f2", []byte("content"), 0644),
+ fstest.CreateSocket("/s0", 0644),
+ fstest.CreateFile("/f3", []byte("content"), 0644),
+ ),
+ },
}
for _, at := range tests { |
Signed-off-by: Derek McGowan <[email protected]>
|
LGTM |
|
Awesome, thanks for sorting the test! |
The go-tar implementation which is used cannot handle sockets.
There's no good reason to preserve a socket, they are basically useless without
the process which made them.
Signed-off-by: Ian Campbell [email protected]
I discovered this with https://github.com/dotnet/dotnet-docker-samples/tree/master/aspnetapp and buildkit's dockerfile frontend which gave:
While a
docker buildworks.