diff --git a/sif/load.go b/sif/load.go index 12cd89b5..2dbeb7da 100644 --- a/sif/load.go +++ b/sif/load.go @@ -1,8 +1,8 @@ -package sifimage +package sif import ( "bufio" - "bytes" + "context" "fmt" "io" "io/ioutil" @@ -11,221 +11,201 @@ import ( "path/filepath" "strings" - "github.com/pkg/errors" + "github.com/sirupsen/logrus" "github.com/sylabs/sif/v2/pkg/sif" - - imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" ) -type loadedSifImage struct { - fimg *sif.FileImage - rootfs sif.Descriptor - deffile *sif.Descriptor - defReader io.Reader - cmdlist []string - runscript *bytes.Buffer - env *sif.Descriptor - envReader io.Reader - envlist []string -} +// injectedScriptTargetPath is the path injectedScript should be written to in the created image. +const injectedScriptTargetPath = "/podman/runscript" -func loadSIFImage(path string) (image loadedSifImage, err error) { - // open up the SIF file and get its header - image.fimg, err = sif.LoadContainerFromPath(path, sif.OptLoadWithFlag(os.O_RDONLY)) - if err != nil { - return - } +// parseDefFile parses a SIF definition file from reader, +// and returns non-trivial contents of the %environment and %runscript sections. +func parseDefFile(reader io.Reader) ([]string, []string, error) { + type parserState int + const ( + parsingOther parserState = iota + parsingEnvironment + parsingRunscript + ) - // check for a system partition and save it - image.rootfs, err = image.fimg.GetDescriptor(sif.WithPartitionType(sif.PartPrimSys)) - if err != nil { - return loadedSifImage{}, errors.Wrap(err, "looking up rootfs from SIF file") - } + environment := []string{} + runscript := []string{} - // look for a definition file object - resultDesc, err := image.fimg.GetDescriptor(sif.WithDataType(sif.DataDeffile)) - if err == nil { - // we assume in practice that typical SIF files don't hold multiple deffiles - image.deffile = &resultDesc - image.defReader = resultDesc.GetReader() - } - if err = image.generateConfig(); err != nil { - return loadedSifImage{}, err - } - - // look for an environment variable set object - resultDesc, err = image.fimg.GetDescriptor(sif.WithDataType(sif.DataEnvVar)) - if err == nil { - // we assume in practice that typical SIF files don't hold multiple EnvVar sets - image.env = &resultDesc - image.envReader = resultDesc.GetReader() - } - - return image, nil -} - -func (image *loadedSifImage) parseEnvironment(scanner *bufio.Scanner) error { + state := parsingOther + scanner := bufio.NewScanner(reader) for scanner.Scan() { s := strings.TrimSpace(scanner.Text()) - if s == "" || strings.HasPrefix(s, "#") { - continue + switch { + case s == `%environment`: + state = parsingEnvironment + case s == `%runscript`: + state = parsingRunscript + case strings.HasPrefix(s, "%"): + state = parsingOther + case state == parsingEnvironment: + if s != "" && !strings.HasPrefix(s, "#") { + environment = append(environment, s) + } + case state == parsingRunscript: + runscript = append(runscript, s) + default: // parsingOther: ignore the line } - if strings.HasPrefix(s, "%") { - return nil - } - image.envlist = append(image.envlist, s) } if err := scanner.Err(); err != nil { - return errors.Wrap(err, "parsing environment from SIF definition file object") + return nil, nil, fmt.Errorf("reading lines from SIF definition file object: %w", err) } - return nil + return environment, runscript, nil } -func (image *loadedSifImage) parseRunscript(scanner *bufio.Scanner) error { - for scanner.Scan() { - s := strings.TrimSpace(scanner.Text()) - if strings.HasPrefix(s, "%") { - return nil - } - image.cmdlist = append(image.cmdlist, s) - } - if err := scanner.Err(); err != nil { - return errors.Wrap(err, "parsing runscript from SIF definition file object") - } - return nil +// generateInjectedScript generates a shell script based on +// SIF definition file %environment and %runscript data, and returns it. +func generateInjectedScript(environment []string, runscript []string) []byte { + script := fmt.Sprintf("#!/bin/bash\n"+ + "%s\n"+ + "%s\n", strings.Join(environment, "\n"), strings.Join(runscript, "\n")) + return []byte(script) } -func (image *loadedSifImage) generateRunscript() error { - base := `#!/bin/bash -` - image.runscript = bytes.NewBufferString(base) - for _, s := range image.envlist { - _, err := image.runscript.WriteString(fmt.Sprintln(s)) +// processDefFile finds sif.DataDeffile in sifImage, if any, +// and returns: +// - the command to run +// - contents of a script to inject as injectedScriptTargetPath, or nil +func processDefFile(sifImage *sif.FileImage) (string, []byte, error) { + var environment, runscript []string + + desc, err := sifImage.GetDescriptor(sif.WithDataType(sif.DataDeffile)) + if err == nil { + environment, runscript, err = parseDefFile(desc.GetReader()) if err != nil { - return errors.Wrap(err, "writing to runscript buffer") + return "", nil, err } } - for _, s := range image.cmdlist { - _, err := image.runscript.WriteString(fmt.Sprintln(s)) - if err != nil { - return errors.Wrap(err, "writing to runscript buffer") - } + + var command string + var injectedScript []byte + if len(environment) == 0 && len(runscript) == 0 { + command = "bash" + injectedScript = nil + } else { + injectedScript = generateInjectedScript(environment, runscript) + command = injectedScriptTargetPath } - return nil + + return command, injectedScript, nil } -func (image *loadedSifImage) generateConfig() error { - if image.deffile == nil { - image.cmdlist = append(image.cmdlist, "bash") +func writeInjectedScript(extractedRootPath string, injectedScript []byte) error { + if injectedScript == nil { return nil } - - // extract %environment/%runscript from definition file - var err error - scanner := bufio.NewScanner(image.defReader) - for scanner.Scan() { - s := strings.TrimSpace(scanner.Text()) - again: - if s == `%environment` { - if err = image.parseEnvironment(scanner); err != nil { - return err - } - } else if s == `%runscript` { - if err = image.parseRunscript(scanner); err != nil { - return err - } - } - s = strings.TrimSpace(scanner.Text()) - if s == `%environment` || s == `%runscript` { - goto again - } + filePath := filepath.Join(extractedRootPath, injectedScriptTargetPath) + parentDirPath := filepath.Dir(filePath) + if err := os.MkdirAll(parentDirPath, 0755); err != nil { + return fmt.Errorf("creating %s: %w", parentDirPath, err) } - if err := scanner.Err(); err != nil { - return errors.Wrap(err, "reading lines from SIF definition file object") + if err := ioutil.WriteFile(filePath, injectedScript, 0755); err != nil { + return fmt.Errorf("writing %s to %s: %w", injectedScriptTargetPath, filePath, err) } - - if len(image.cmdlist) == 0 && len(image.envlist) == 0 { - image.cmdlist = append(image.cmdlist, "bash") - } else { - if err = image.generateRunscript(); err != nil { - return errors.Wrap(err, "generating runscript") - } - image.cmdlist = []string{"/podman/runscript"} - } - return nil } -func (image loadedSifImage) GetConfig(config *imgspecv1.Image) error { - config.Config.Cmd = append(config.Config.Cmd, image.cmdlist...) - return nil -} +// createTarFromSIFInputs creates a tar file at tarPath, using a squashfs image at squashFSPath. +// It can also use extractedRootPath and scriptPath, which are allocated for its exclusive use, +// if necessary. +func createTarFromSIFInputs(ctx context.Context, tarPath, squashFSPath string, injectedScript []byte, extractedRootPath, scriptPath string) error { + // It's safe for the Remove calls to happen even before we create the files, because tempDir is exclusive + // for our use. + defer os.RemoveAll(extractedRootPath) -func (image loadedSifImage) UnloadSIFImage() (err error) { - err = image.fimg.UnloadContainer() - return -} - -func (image loadedSifImage) GetSIFID() string { - return image.fimg.ID() -} - -func (image loadedSifImage) GetSIFArch() string { - return image.fimg.PrimaryArch() -} - -const squashFilename = "rootfs.squashfs" -const tarFilename = "rootfs.tar" - -func runUnSquashFSTar(tempdir string) (err error) { - script := ` -#!/bin/sh -unsquashfs -f ` + squashFilename + ` && tar --acls --xattrs -C ./squashfs-root -cpf ` + tarFilename + ` ./ -` - - if err = ioutil.WriteFile(filepath.Join(tempdir, "script"), []byte(script), 0755); err != nil { + // Almost everything in extractedRootPath comes from squashFSPath. + conversionCommand := fmt.Sprintf("unsquashfs -d %s -f %s && tar --acls --xattrs -C %s -cpf %s ./", + extractedRootPath, squashFSPath, extractedRootPath, tarPath) + script := "#!/bin/sh\n" + conversionCommand + "\n" + if err := ioutil.WriteFile(scriptPath, []byte(script), 0755); err != nil { return err } - cmd := []string{"fakeroot", "--", "./script"} + defer os.Remove(scriptPath) - xcmd := exec.Command(cmd[0], cmd[1:]...) - xcmd.Stderr = os.Stderr - xcmd.Dir = tempdir - err = xcmd.Run() - return -} + // On top of squashFSPath, we only add injectedScript, if necessary. + if err := writeInjectedScript(extractedRootPath, injectedScript); err != nil { + return err + } -func (image *loadedSifImage) writeRunscript(tempdir string) (err error) { - if image.runscript == nil { - return nil - } - rsPath := filepath.Join(tempdir, "squashfs-root", "podman") - if err = os.MkdirAll(rsPath, 0755); err != nil { - return - } - if err = ioutil.WriteFile(filepath.Join(rsPath, "runscript"), image.runscript.Bytes(), 0755); err != nil { - return errors.Wrap(err, "writing /podman/runscript") + logrus.Debugf("Converting squashfs to tar, command: %s ...", conversionCommand) + cmd := exec.CommandContext(ctx, "fakeroot", "--", scriptPath) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("converting image: %w, output: %s", err, string(output)) } + logrus.Debugf("... finished converting squashfs to tar") return nil } -func (image loadedSifImage) SquashFSToTarLayer(tempdir string) (tarpath string, err error) { - f, err := os.Create(filepath.Join(tempdir, squashFilename)) +// convertSIFToElements processes sifImage and creates/returns +// the relevant elements for contructing an OCI-like image: +// - A path to a tar file containing a root filesystem, +// - A command to run. +// The returned tar file path is inside tempDir, which can be assumed to be empty +// at start, and is exclusively used by the current process (i.e. it is safe +// to use hard-coded relative paths within it). +func convertSIFToElements(ctx context.Context, sifImage *sif.FileImage, tempDir string) (string, []string, error) { + // We could allocate unique names for all of these using ioutil.Temp*, but tempDir is exclusive, + // so we can just hard-code a set of unique values here. + // We create and/or manage cleanup of these two paths. + squashFSPath := filepath.Join(tempDir, "rootfs.squashfs") + tarPath := filepath.Join(tempDir, "rootfs.tar") + // We only allocate these paths, the user is responsible for cleaning them up. + extractedRootPath := filepath.Join(tempDir, "rootfs") + scriptPath := filepath.Join(tempDir, "script") + + succeeded := false + // It's safe for the Remove calls to happen even before we create the files, because tempDir is exclusive + // for our use. + // Ideally we would remove squashFSPath immediately after creating extractedRootPath, but we need + // to run both creation and consumption of extractedRootPath in the same fakeroot context. + // So, overall, this process requires at least 2 compressed copies (SIF and squashFSPath) and 2 + // uncompressed copies (extractedRootPath and tarPath) of the data, all using up space at the same time. + // That's rather unsatisfactory, ideally we would be streaming the data directly from a squashfs parser + // reading from the SIF file to a tarball, for 1 compresed and 1 uncompressed copy. + defer os.Remove(squashFSPath) + defer func() { + if !succeeded { + os.Remove(tarPath) + } + }() + + command, injectedScript, err := processDefFile(sifImage) if err != nil { - return + return "", nil, err } - defer f.Close() - if _, err = io.CopyN(f, image.rootfs.GetReader(), image.rootfs.Size()); err != nil { - return + + rootFS, err := sifImage.GetDescriptor(sif.WithPartitionType(sif.PartPrimSys)) + if err != nil { + return "", nil, fmt.Errorf("looking up rootfs from SIF file: %w", err) } - if err = f.Sync(); err != nil { - return + // TODO: We'd prefer not to make a full copy of the file here; unsquashfs ≥ 4.4 + // has an -o option that allows extracting a squashfs from the SIF file directly, + // but that version is not currently available in RHEL 8. + logrus.Debugf("Creating a temporary squashfs image %s ...", squashFSPath) + if err := func() error { // A scope for defer + f, err := os.Create(squashFSPath) + if err != nil { + return err + } + defer f.Close() + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). + if _, err := io.CopyN(f, rootFS.GetReader(), rootFS.Size()); err != nil { + return err + } + return nil + }(); err != nil { + return "", nil, err } - if err = image.writeRunscript(tempdir); err != nil { - return + logrus.Debugf("... finished creating a temporary squashfs image") + + if err := createTarFromSIFInputs(ctx, tarPath, squashFSPath, injectedScript, extractedRootPath, scriptPath); err != nil { + return "", nil, err } - if err = runUnSquashFSTar(tempdir); err != nil { - return - } - return filepath.Join(tempdir, tarFilename), nil + succeeded = true + return tarPath, []string{command}, nil } diff --git a/sif/load_test.go b/sif/load_test.go new file mode 100644 index 00000000..ee9bc726 --- /dev/null +++ b/sif/load_test.go @@ -0,0 +1,58 @@ +package sif + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseDefFile(t *testing.T) { + for _, c := range []struct { + name string + input string + environment []string + runscript []string + }{ + {"Empty input", "", []string{}, []string{}}, + { + name: "Basic smoke test", + input: "Bootstrap: library\n" + + "%environment\n" + + " export FOO=world\n" + + " export BAR=baz\n" + + "%runscript\n" + + ` echo "Hello $FOO"` + "\n" + + " sleep 5\n" + + "%help\n" + + " Abandon all hope.\n", + environment: []string{"export FOO=world", "export BAR=baz"}, + runscript: []string{`echo "Hello $FOO"`, "sleep 5"}, + }, + { + name: "Trailing section marker", + input: "Bootstrap: library\n" + + "%environment\n" + + " export FOO=world\n" + + "%runscript", + environment: []string{"export FOO=world"}, + runscript: []string{}, + }, + } { + env, rs, err := parseDefFile(bytes.NewReader([]byte(c.input))) + require.NoError(t, err, c.name) + assert.Equal(t, c.environment, env, c.name) + assert.Equal(t, c.runscript, rs, c.name) + } +} + +func TestGenerateInjectedScript(t *testing.T) { + res := generateInjectedScript([]string{"export FOO=world", "export BAR=baz"}, + []string{`echo "Hello $FOO"`, "sleep 5"}) + assert.Equal(t, "#!/bin/bash\n"+ + "export FOO=world\n"+ + "export BAR=baz\n"+ + `echo "Hello $FOO"`+"\n"+ + "sleep 5\n", string(res)) +} diff --git a/sif/src.go b/sif/src.go index d7d4251f..ba95a469 100644 --- a/sif/src.go +++ b/sif/src.go @@ -1,217 +1,151 @@ -package sifimage +package sif import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" "os" - "time" "github.com/containers/image/v5/internal/tmpdir" "github.com/containers/image/v5/types" - "github.com/klauspost/pgzip" "github.com/opencontainers/go-digest" - "github.com/pkg/errors" - imgspecs "github.com/opencontainers/image-spec/specs-go" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/sirupsen/logrus" + "github.com/sylabs/sif/v2/pkg/sif" ) type sifImageSource struct { - ref sifReference - sifimg loadedSifImage - workdir string - diffID digest.Digest - diffSize int64 - blobID digest.Digest - blobSize int64 - blobTime time.Time - blobType string - blobFile string - config []byte - configID digest.Digest - configSize int64 - manifest []byte + ref sifReference + workDir string + layerDigest digest.Digest + layerSize int64 + layerFile string + config []byte + configDigest digest.Digest + manifest []byte } -func (s *sifImageSource) getLayerInfo(tarpath string) error { - ftar, err := os.Open(tarpath) +// getBlobInfo returns the digest, and size of the provided file. +func getBlobInfo(path string) (digest.Digest, int64, error) { + f, err := os.Open(path) if err != nil { - return fmt.Errorf("error opening %q for reading: %v", tarpath, err) + return "", -1, fmt.Errorf("opening %q for reading: %w", path, err) } - defer ftar.Close() + defer f.Close() - diffDigester := digest.Canonical.Digester() - s.diffSize, err = io.Copy(diffDigester.Hash(), ftar) + // TODO: Instead of writing the tar file to disk, and reading + // it here again, stream the tar file to a pipe and + // compute the digest while writing it to disk. + logrus.Debugf("Computing a digest of the SIF conversion output...") + digester := digest.Canonical.Digester() + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). + size, err := io.Copy(digester.Hash(), f) if err != nil { - return fmt.Errorf("error reading %q: %v", tarpath, err) + return "", -1, fmt.Errorf("reading %q: %w", path, err) } - s.diffID = diffDigester.Digest() + digest := digester.Digest() + logrus.Debugf("... finished computing the digest of the SIF conversion output") - return nil -} -func (s *sifImageSource) createBlob(tarpath string) error { - s.blobFile = fmt.Sprintf("%s.%s", tarpath, "gz") - fgz, err := os.Create(s.blobFile) - if err != nil { - return errors.Wrapf(err, "creating file for compressed blob") - } - defer fgz.Close() - fileinfo, err := fgz.Stat() - if err != nil { - return fmt.Errorf("error reading modtime of %q: %v", s.blobFile, err) - } - s.blobTime = fileinfo.ModTime() - - ftar, err := os.Open(tarpath) - if err != nil { - return fmt.Errorf("error opening %q for reading: %v", tarpath, err) - } - defer ftar.Close() - - writer := pgzip.NewWriter(fgz) - defer writer.Close() - _, err = io.Copy(writer, ftar) - if err != nil { - return fmt.Errorf("error compressing %q: %v", tarpath, err) - } - - return nil -} - -func (s *sifImageSource) getBlobInfo() error { - fgz, err := os.Open(s.blobFile) - if err != nil { - return fmt.Errorf("error opening %q for reading: %v", s.blobFile, err) - } - defer fgz.Close() - - blobDigester := digest.Canonical.Digester() - s.blobSize, err = io.Copy(blobDigester.Hash(), fgz) - if err != nil { - return fmt.Errorf("error reading %q: %v", s.blobFile, err) - } - s.blobID = blobDigester.Digest() - - return nil + return digest, size, nil } // newImageSource returns an ImageSource for reading from an existing directory. // newImageSource extracts SIF objects and saves them in a temp directory. func newImageSource(ctx context.Context, sys *types.SystemContext, ref sifReference) (types.ImageSource, error) { - var imgSrc sifImageSource - - sifimg, err := loadSIFImage(ref.resolvedFile) + sifImg, err := sif.LoadContainerFromPath(ref.file, sif.OptLoadWithFlag(os.O_RDONLY)) if err != nil { - return nil, errors.Wrap(err, "loading SIF file") + return nil, fmt.Errorf("loading SIF file: %w", err) } + defer func() { + _ = sifImg.UnloadContainer() + }() - workdir, err := ioutil.TempDir(tmpdir.TemporaryDirectoryForBigFiles(sys), "sif") + workDir, err := ioutil.TempDir(tmpdir.TemporaryDirectoryForBigFiles(sys), "sif") if err != nil { - return nil, errors.Wrapf(err, "creating temp directory") + return nil, fmt.Errorf("creating temp directory: %w", err) } + succeeded := false + defer func() { + if !succeeded { + os.RemoveAll(workDir) + } + }() - tarpath, err := sifimg.SquashFSToTarLayer(workdir) + layerPath, commandLine, err := convertSIFToElements(ctx, sifImg, workDir) if err != nil { - return nil, errors.Wrapf(err, "converting rootfs from SquashFS to Tarball") + return nil, fmt.Errorf("converting rootfs from SquashFS to Tarball: %w", err) } - // generate layer info - err = imgSrc.getLayerInfo(tarpath) + layerDigest, layerSize, err := getBlobInfo(layerPath) if err != nil { - return nil, errors.Wrapf(err, "gathering layer diff information") + return nil, fmt.Errorf("gathering blob information: %w", err) } - // prepare compressed blob - err = imgSrc.createBlob(tarpath) - if err != nil { - return nil, errors.Wrapf(err, "creating blob file") - } - - // generate blob info - err = imgSrc.getBlobInfo() - if err != nil { - return nil, errors.Wrapf(err, "gathering blob information") - } - - // populate the rootfs section of the config - rootfs := imgspecv1.RootFS{ - Type: "layers", - DiffIDs: []digest.Digest{imgSrc.diffID}, - } - created := imgSrc.blobTime - history := []imgspecv1.History{ - { - Created: &created, - CreatedBy: fmt.Sprintf("/bin/sh -c #(nop) ADD file:%s in %c", imgSrc.diffID.Hex(), os.PathSeparator), - Comment: "imported from SIF, uuid: " + sifimg.GetSIFID(), + created := sifImg.ModifiedAt() + config := imgspecv1.Image{ + Created: &created, + Architecture: sifImg.PrimaryArch(), + OS: "linux", + Config: imgspecv1.ImageConfig{ + Cmd: commandLine, }, - { - Created: &created, - CreatedBy: "/bin/sh -c #(nop) CMD [\"bash\"]", - EmptyLayer: true, + RootFS: imgspecv1.RootFS{ + Type: "layers", + DiffIDs: []digest.Digest{layerDigest}, + }, + History: []imgspecv1.History{ + { + Created: &created, + CreatedBy: fmt.Sprintf("/bin/sh -c #(nop) ADD file:%s in %c", layerDigest.Hex(), os.PathSeparator), + Comment: "imported from SIF, uuid: " + sifImg.ID(), + }, + { + Created: &created, + CreatedBy: "/bin/sh -c #(nop) CMD [\"bash\"]", + EmptyLayer: true, + }, }, } - - // build an OCI image config - var config imgspecv1.Image - config.Created = &created - config.Architecture = sifimg.GetSIFArch() - config.OS = "linux" - config.RootFS = rootfs - config.History = history - err = sifimg.GetConfig(&config) - if err != nil { - return nil, errors.Wrapf(err, "getting config elements from SIF") - } - - // Encode and digest the image configuration blob. configBytes, err := json.Marshal(&config) if err != nil { - return nil, fmt.Errorf("error generating configuration blob for %q: %v", ref.resolvedFile, err) + return nil, fmt.Errorf("generating configuration blob for %q: %w", ref.resolvedFile, err) } - configID := digest.Canonical.FromBytes(configBytes) - configSize := int64(len(configBytes)) + configDigest := digest.Canonical.FromBytes(configBytes) - // Populate a manifest with the configuration blob and the SquashFS part as the single layer. - layerDescriptor := imgspecv1.Descriptor{ - Digest: imgSrc.blobID, - Size: imgSrc.blobSize, - MediaType: imgspecv1.MediaTypeImageLayerGzip, - } manifest := imgspecv1.Manifest{ - Versioned: imgspecs.Versioned{ - SchemaVersion: 2, - }, + Versioned: imgspecs.Versioned{SchemaVersion: 2}, + MediaType: imgspecv1.MediaTypeImageManifest, Config: imgspecv1.Descriptor{ - Digest: configID, - Size: configSize, + Digest: configDigest, + Size: int64(len(configBytes)), MediaType: imgspecv1.MediaTypeImageConfig, }, - Layers: []imgspecv1.Descriptor{layerDescriptor}, + Layers: []imgspecv1.Descriptor{{ + Digest: layerDigest, + Size: layerSize, + MediaType: imgspecv1.MediaTypeImageLayer, + }}, } manifestBytes, err := json.Marshal(&manifest) if err != nil { - return nil, fmt.Errorf("error generating manifest for %q: %v", ref.resolvedFile, err) + return nil, fmt.Errorf("generating manifest for %q: %w", ref.resolvedFile, err) } + succeeded = true return &sifImageSource{ - ref: ref, - sifimg: sifimg, - workdir: workdir, - diffID: imgSrc.diffID, - diffSize: imgSrc.diffSize, - blobID: imgSrc.blobID, - blobSize: imgSrc.blobSize, - blobType: layerDescriptor.MediaType, - blobFile: imgSrc.blobFile, - config: configBytes, - configID: configID, - configSize: configSize, - manifest: manifestBytes, + ref: ref, + workDir: workDir, + layerDigest: layerDigest, + layerSize: layerSize, + layerFile: layerPath, + config: configBytes, + configDigest: configDigest, + manifest: manifestBytes, }, nil } @@ -222,31 +156,30 @@ func (s *sifImageSource) Reference() types.ImageReference { // Close removes resources associated with an initialized ImageSource, if any. func (s *sifImageSource) Close() error { - os.RemoveAll(s.workdir) - return s.sifimg.UnloadSIFImage() + return os.RemoveAll(s.workDir) } // HasThreadSafeGetBlob indicates whether GetBlob can be executed concurrently. func (s *sifImageSource) HasThreadSafeGetBlob() bool { - return false + return true } // GetBlob returns a stream for the specified blob, and the blob’s size (or -1 if unknown). // The Digest field in BlobInfo is guaranteed to be provided, Size may be -1 and MediaType may be optionally provided. // May update BlobInfoCache, preferably after it knows for certain that a blob truly exists at a specific location. func (s *sifImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache types.BlobInfoCache) (io.ReadCloser, int64, error) { - // We should only be asked about things in the manifest. Maybe the configuration blob. - if info.Digest == s.configID { - return ioutil.NopCloser(bytes.NewBuffer(s.config)), s.configSize, nil - } - if info.Digest == s.blobID { - reader, err := os.Open(s.blobFile) + switch info.Digest { + case s.configDigest: + return ioutil.NopCloser(bytes.NewBuffer(s.config)), int64(len(s.config)), nil + case s.layerDigest: + reader, err := os.Open(s.layerFile) if err != nil { - return nil, -1, fmt.Errorf("error opening %q: %v", s.blobFile, err) + return nil, -1, fmt.Errorf("opening %q: %w", s.layerFile, err) } - return reader, s.blobSize, nil + return reader, s.layerSize, nil + default: + return nil, -1, fmt.Errorf("no blob with digest %q found", info.Digest.String()) } - return nil, -1, fmt.Errorf("no blob with digest %q found", info.Digest.String()) } // GetManifest returns the image's manifest along with its MIME type (which may be empty when it can't be determined but the manifest is available). @@ -255,7 +188,7 @@ func (s *sifImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache // this never happens if the primary manifest is not a manifest list (e.g. if the source never returns manifest lists). func (s *sifImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) { if instanceDigest != nil { - return nil, "", fmt.Errorf("manifest lists are not supported by the sif transport") + return nil, "", errors.New("manifest lists are not supported by the sif transport") } return s.manifest, imgspecv1.MediaTypeImageManifest, nil } @@ -266,7 +199,7 @@ func (s *sifImageSource) GetManifest(ctx context.Context, instanceDigest *digest // (e.g. if the source never returns manifest lists). func (s *sifImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) { if instanceDigest != nil { - return nil, fmt.Errorf("manifest lists are not supported by the sif transport") + return nil, errors.New("manifest lists are not supported by the sif transport") } return nil, nil } diff --git a/sif/transport.go b/sif/transport.go index 3b53f85c..18d894bc 100644 --- a/sif/transport.go +++ b/sif/transport.go @@ -1,8 +1,10 @@ -package sifimage +package sif import ( "context" + "errors" "fmt" + "path/filepath" "strings" "github.com/containers/image/v5/directory/explicitfilepath" @@ -10,7 +12,6 @@ import ( "github.com/containers/image/v5/image" "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" - "github.com/pkg/errors" ) func init() { @@ -22,12 +23,6 @@ var Transport = sifTransport{} type sifTransport struct{} -// sifReference is an ImageReference for SIF images. -type sifReference struct { - file string // As specified by the user. May be relative, contain symlinks, etc. - resolvedFile string // Absolute file path with no symlinks, at least at the time of its creation. Primarily used for policy namespaces. -} - func (t sifTransport) Name() string { return "sif" } @@ -37,21 +32,52 @@ func (t sifTransport) ParseReference(reference string) (types.ImageReference, er return NewReference(reference) } -// NewReference returns an image file reference for a specified path. -func NewReference(file string) (types.ImageReference, error) { - resolved, err := explicitfilepath.ResolvePathToFullyExplicit(file) - if err != nil { - return nil, err - } - return sifReference{file: file, resolvedFile: resolved}, nil -} - // ValidatePolicyConfigurationScope checks that scope is a valid name for a signature.PolicyTransportScopes keys // (i.e. a valid PolicyConfigurationIdentity() or PolicyConfigurationNamespaces() return value). // It is acceptable to allow an invalid value which will never be matched, it can "only" cause user confusion. // scope passed to this function will not be "", that value is always allowed. func (t sifTransport) ValidatePolicyConfigurationScope(scope string) error { - return errors.New(`sif: does not support any scopes except the default "" one`) + if !strings.HasPrefix(scope, "/") { + return fmt.Errorf("Invalid scope %s: Must be an absolute path", scope) + } + // Refuse also "/", otherwise "/" and "" would have the same semantics, + // and "" could be unexpectedly shadowed by the "/" entry. + if scope == "/" { + return errors.New(`Invalid scope "/": Use the generic default scope ""`) + } + cleaned := filepath.Clean(scope) + if cleaned != scope { + return fmt.Errorf(`Invalid scope %s: Uses non-canonical format, perhaps try %s`, scope, cleaned) + } + return nil +} + +// sifReference is an ImageReference for SIF images. +type sifReference struct { + // Note that the interpretation of paths below depends on the underlying filesystem state, which may change under us at any time! + // Either of the paths may point to a different, or no, inode over time. resolvedFile may contain symbolic links, and so on. + + // Generally we follow the intent of the user, and use the "file" member for filesystem operations (e.g. the user can use a relative path to avoid + // being exposed to symlinks and renames in the parent directories to the working directory). + // (But in general, we make no attempt to be completely safe against concurrent hostile filesystem modifications.) + file string // As specified by the user. May be relative, contain symlinks, etc. + resolvedFile string // Absolute file path with no symlinks, at least at the time of its creation. Primarily used for policy namespaces. +} + +// There is no sif.ParseReference because it is rather pointless. +// Callers who need a transport-independent interface will go through +// sifTransport.ParseReference; callers who intentionally deal with SIF files +// can use sif.NewReference. + +// NewReference returns an image file reference for a specified path. +func NewReference(file string) (types.ImageReference, error) { + // We do not expose an API supplying the resolvedFile; we could, but recomputing it + // is generally cheap enough that we prefer being confident about the properties of resolvedFile. + resolved, err := explicitfilepath.ResolvePathToFullyExplicit(file) + if err != nil { + return nil, err + } + return sifReference{file: file, resolvedFile: resolved}, nil } func (ref sifReference) Transport() types.ImageTransport { @@ -60,33 +86,50 @@ func (ref sifReference) Transport() types.ImageTransport { // StringWithinTransport returns a string representation of the reference, which MUST be such that // reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference. +// NOTE: The returned string is not promised to be equal to the original input to ParseReference; +// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa. +// WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix; +// instead, see transports.ImageName(). func (ref sifReference) StringWithinTransport() string { return ref.file } // DockerReference returns a Docker reference associated with this reference +// (fully explicit, i.e. !reference.IsNameOnly, but reflecting user intent, +// not e.g. after redirect or alias processing), or nil if unknown/not applicable. func (ref sifReference) DockerReference() reference.Named { return nil } // PolicyConfigurationIdentity returns a string representation of the reference, suitable for policy lookup. +// This MUST reflect user intent, not e.g. after processing of third-party redirects or aliases; +// The value SHOULD be fully explicit about its semantics, with no hidden defaults, AND canonical +// (i.e. various references with exactly the same semantics should return the same configuration identity) +// It is fine for the return value to be equal to StringWithinTransport(), and it is desirable but +// not required/guaranteed that it will be a valid input to Transport().ParseReference(). +// Returns "" if configuration identities for these references are not supported. func (ref sifReference) PolicyConfigurationIdentity() string { return ref.resolvedFile } // PolicyConfigurationNamespaces returns a list of other policy configuration namespaces to search -// for if explicit configuration for PolicyConfigurationIdentity() is not set +// for if explicit configuration for PolicyConfigurationIdentity() is not set. The list will be processed +// in order, terminating on first match, and an implicit "" is always checked at the end. +// It is STRONGLY recommended for the first element, if any, to be a prefix of PolicyConfigurationIdentity(), +// and each following element to be a prefix of the element preceding it. func (ref sifReference) PolicyConfigurationNamespaces() []string { res := []string{} path := ref.resolvedFile for { lastSlash := strings.LastIndex(path, "/") - if lastSlash == -1 || path == "/" { + if lastSlash == -1 || lastSlash == 0 { break } - res = append(res, path) path = path[:lastSlash] + res = append(res, path) } + // Note that we do not include "/"; it is redundant with the default "" global default, + // and rejected by sifTransport.ValidatePolicyConfigurationScope above. return res } @@ -109,11 +152,13 @@ func (ref sifReference) NewImageSource(ctx context.Context, sys *types.SystemCon return newImageSource(ctx, sys, ref) } +// NewImageDestination returns a types.ImageDestination for this reference. +// The caller must call .Close() on the returned ImageDestination. func (ref sifReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { - return nil, fmt.Errorf(`"sif:" locations can only be read from, not written to`) + return nil, errors.New(`"sif:" locations can only be read from, not written to`) } // DeleteImage deletes the named image from the registry, if supported. func (ref sifReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error { - return errors.Errorf("Deleting images not implemented for sif: images") + return errors.New("Deleting images not implemented for sif: images") } diff --git a/sif/transport_test.go b/sif/transport_test.go new file mode 100644 index 00000000..3ac6b79b --- /dev/null +++ b/sif/transport_test.go @@ -0,0 +1,177 @@ +package sif + +import ( + "context" + "io/ioutil" + "os" + "path/filepath" + "testing" + + _ "github.com/containers/image/v5/internal/testing/explicitfilepath-tmpdir" + "github.com/containers/image/v5/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTransportName(t *testing.T) { + assert.Equal(t, "sif", Transport.Name()) +} + +func TestTransportParseReference(t *testing.T) { + testNewReference(t, Transport.ParseReference) +} + +func TestTransportValidatePolicyConfigurationScope(t *testing.T) { + for _, scope := range []string{ + "/etc/passwd", + "/this/does/not/exist", + } { + err := Transport.ValidatePolicyConfigurationScope(scope) + assert.NoError(t, err, scope) + } + + for _, scope := range []string{ + "relative/path", + "/double//slashes", + "/has/./dot", + "/has/dot/../dot", + "/trailing/slash/", + "/", + } { + err := Transport.ValidatePolicyConfigurationScope(scope) + assert.Error(t, err, scope) + } +} + +func TestNewReference(t *testing.T) { + testNewReference(t, NewReference) +} + +// testNewReference is a test shared for Transport.ParseReference and NewReference. +func testNewReference(t *testing.T, fn func(string) (types.ImageReference, error)) { + tmpDir, err := ioutil.TempDir("", "sif-transport-test") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + tmpFile := filepath.Join(tmpDir, "image.sif") + err = ioutil.WriteFile(tmpFile, nil, 0600) + require.NoError(t, err) + + for _, file := range []string{ + "/dev/null", + tmpFile, + "relativepath", + tmpDir + "/thisdoesnotexist", + } { + ref, err := fn(file) + require.NoError(t, err, file) + sifRef, ok := ref.(sifReference) + require.True(t, ok) + assert.Equal(t, file, sifRef.file, file) + } + + _, err = fn(tmpDir + "/thisparentdoesnotexist/something") + assert.Error(t, err) +} + +// refToTempFile creates a temporary file and returns a reference to it. +// The caller should +// defer os.Remove(tmpFile) +func refToTempFile(t *testing.T) (ref types.ImageReference, tmpDir string) { + f, err := ioutil.TempFile("", "sif-transport-test") + require.NoError(t, err) + tmpFile := f.Name() + err = f.Close() + require.NoError(t, err) + ref, err = NewReference(tmpFile) + require.NoError(t, err) + return ref, tmpFile +} + +func TestReferenceTransport(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + assert.Equal(t, Transport, ref.Transport()) +} + +func TestReferenceStringWithinTransport(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + assert.Equal(t, tmpFile, ref.StringWithinTransport()) +} + +func TestReferenceDockerReference(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + assert.Nil(t, ref.DockerReference()) +} + +func TestReferencePolicyConfigurationIdentity(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + + assert.Equal(t, tmpFile, ref.PolicyConfigurationIdentity()) + // A non-canonical path. Test just one, the various other cases are + // tested in explicitfilepath.ResolvePathToFullyExplicit. + ref, err := NewReference("/./" + tmpFile) + require.NoError(t, err) + assert.Equal(t, tmpFile, ref.PolicyConfigurationIdentity()) +} + +func TestReferencePolicyConfigurationNamespaces(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + // We don't really know enough to make a full equality test here. + ns := ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.NotEmpty(t, ns) + assert.Equal(t, filepath.Dir(tmpFile), ns[0]) + + // Test with a known path where the directory should exist. Test just one non-canonical + // path, the various other cases are tested in explicitfilepath.ResolvePathToFullyExplicit. + for _, path := range []string{"/usr/share/probablydoesnotexist.sif", "/usr/share/././probablydoesnoexist.sif"} { + _, err := os.Lstat(filepath.Dir(path)) + require.NoError(t, err) + ref, err := NewReference(path) + require.NoError(t, err) + ns := ref.PolicyConfigurationNamespaces() + require.NotNil(t, ns) + assert.Equal(t, []string{"/usr/share", "/usr"}, ns) + } + + // "/" as a corner case. + ref, err := NewReference("/") + require.NoError(t, err) + assert.Equal(t, []string{}, ref.PolicyConfigurationNamespaces()) +} + +func TestReferenceNewImage(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + // A pretty pointless smoke test for now; + // we don't want to require every developer of c/image to have fakeroot etc. around. + _, err := ref.NewImage(context.Background(), nil) + assert.Error(t, err) // Empty file is not valid +} + +func TestReferenceNewImageSource(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + // A pretty pointless smoke test for now; + // we don't want to require every developer of c/image to have fakeroot etc. around. + _, err := ref.NewImageSource(context.Background(), nil) + assert.Error(t, err) // Empty file is not valid +} + +func TestReferenceNewImageDestination(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + _, err := ref.NewImageDestination(context.Background(), nil) + assert.Error(t, err) +} + +func TestReferenceDeleteImage(t *testing.T) { + ref, tmpFile := refToTempFile(t) + defer os.Remove(tmpFile) + err := ref.DeleteImage(context.Background(), nil) + assert.Error(t, err) +}