-
Notifications
You must be signed in to change notification settings - Fork 245
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
Support storing Ollama [non-]OCI image layers #2075
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -790,6 +790,23 @@ func supportsOverlay(home string, homeMagic graphdriver.FsMagic, rootUID, rootGI | |
return supportsDType, fmt.Errorf("'overlay' not found as a supported filesystem on this host. Please ensure kernel is new enough and has overlay support loaded.: %w", graphdriver.ErrNotSupported) | ||
} | ||
|
||
func cp(r io.Reader, dest string, filename string) error { | ||
if seeker, ok := r.(io.Seeker); ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is something that should have generally been set up by the caller. |
||
if _, err := seeker.Seek(0, io.SeekStart); err != nil { | ||
return err | ||
} | ||
} | ||
|
||
f, err := os.Create(filepath.Join(dest, filename)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Layer creation already has transaction semantics via |
||
if err != nil { | ||
return err | ||
} | ||
defer f.Close() | ||
|
||
_, err = io.Copy(f, r) | ||
return err | ||
} | ||
|
||
func (d *Driver) useNaiveDiff() bool { | ||
if d.usingComposefs { | ||
return true | ||
|
@@ -2329,17 +2346,25 @@ func (d *Driver) ApplyDiff(id, parent string, options graphdriver.ApplyDiffOpts) | |
return 0, err | ||
} | ||
|
||
logrus.Debugf("Applying tar in %s", applyDir) | ||
// Overlay doesn't need the parent id to apply the diff | ||
if err := untar(options.Diff, applyDir, &archive.TarOptions{ | ||
UIDMaps: idMappings.UIDs(), | ||
GIDMaps: idMappings.GIDs(), | ||
IgnoreChownErrors: d.options.ignoreChownErrors, | ||
ForceMask: d.options.forceMask, | ||
WhiteoutFormat: d.getWhiteoutFormat(), | ||
InUserNS: unshare.IsRootless(), | ||
}); err != nil { | ||
return 0, err | ||
if options.LayerFilename != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not a real expert in c/storage but it is a fact that the codebase predates the creation/concept of artifacts, and is very much designed around storing layers. I suspect that this |
||
logrus.Debugf("Applying file in %s", applyDir) | ||
err := cp(options.Diff, applyDir, *options.LayerFilename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ☠️ This is an unrestricted path traversal vulnerability, run as root. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if err != nil { | ||
return 0, err | ||
} | ||
} else { | ||
logrus.Debugf("Applying tar in %s", applyDir) | ||
// Overlay doesn't need the parent id to apply the diff | ||
if err := untar(options.Diff, applyDir, &archive.TarOptions{ | ||
UIDMaps: idMappings.UIDs(), | ||
GIDMaps: idMappings.GIDs(), | ||
IgnoreChownErrors: d.options.ignoreChownErrors, | ||
ForceMask: d.options.forceMask, | ||
WhiteoutFormat: d.getWhiteoutFormat(), | ||
InUserNS: unshare.IsRootless(), | ||
}); err != nil { | ||
return 0, err | ||
} | ||
} | ||
|
||
return directory.Size(applyDir) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2422,14 +2422,21 @@ func (r *layerStore) applyDiffWithOptions(to string, layerOptions *LayerOptions, | |
if uncompressedDigester != nil { | ||
uncompressedWriter = io.MultiWriter(uncompressedWriter, uncompressedDigester.Hash()) | ||
} | ||
payload, err := asm.NewInputTarStream(io.TeeReader(uncompressed, uncompressedWriter), metadata, storage.NewDiscardFilePutter()) | ||
if err != nil { | ||
return -1, err | ||
|
||
var payload io.Reader | ||
if layerOptions != nil && layerOptions.LayerFilename != nil { | ||
payload = diff | ||
} else { | ||
payload, err = asm.NewInputTarStream(io.TeeReader(uncompressed, uncompressedWriter), metadata, storage.NewDiscardFilePutter()) | ||
if err != nil { | ||
return -1, err | ||
} | ||
} | ||
options := drivers.ApplyDiffOpts{ | ||
Diff: payload, | ||
Mappings: r.layerMappings(layer), | ||
MountLabel: layer.MountLabel, | ||
Diff: payload, | ||
Mappings: r.layerMappings(layer), | ||
MountLabel: layer.MountLabel, | ||
LayerFilename: layerOptions.LayerFilename, | ||
} | ||
size, err := r.driver.ApplyDiff(layer.ID, layer.Parent, options) | ||
if err != nil { | ||
|
@@ -2468,6 +2475,12 @@ func (r *layerStore) applyDiffWithOptions(to string, layerOptions *LayerOptions, | |
layer.UncompressedDigest = uncompressedDigest | ||
layer.UncompressedSize = uncompressedCounter.Count | ||
layer.CompressionType = compression | ||
|
||
if layerOptions != nil && layerOptions.LayerFilename != nil { | ||
layer.CompressedSize = size | ||
layer.UncompressedSize = size | ||
Comment on lines
+2480
to
+2481
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of this just doesn’t work; the data was already interpreted as a tar file, and uncompressed. This is at best pretending that didn’t happen. |
||
} | ||
|
||
layer.UIDs = make([]uint32, 0, len(uidLog)) | ||
for uid := range uidLog { | ||
layer.UIDs = append(layer.UIDs, uid) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in all of the other storage drivers which weren’t modified?