From eaaf273a225d2cdee692adf99c45f5135889f206 Mon Sep 17 00:00:00 2001 From: Adam Hughes Date: Tue, 22 Jun 2021 14:02:34 +0000 Subject: [PATCH 1/4] feat: use functional arguments for CreateContainer Remove usage of deprecated UUID package, and remove CreateInfo struct. --- internal/app/siftool/modif.go | 15 +--- pkg/integrity/testdata/gen_sifs.go | 16 +---- pkg/sif/create.go | 109 +++++++++++++++++++++-------- pkg/sif/create_test.go | 29 ++------ pkg/sif/sif.go | 9 --- 5 files changed, 87 insertions(+), 91 deletions(-) diff --git a/internal/app/siftool/modif.go b/internal/app/siftool/modif.go index 02d38f47..70481106 100644 --- a/internal/app/siftool/modif.go +++ b/internal/app/siftool/modif.go @@ -11,25 +11,12 @@ package siftool import ( "io" - "github.com/google/uuid" "github.com/sylabs/sif/v2/pkg/sif" ) // New creates a new empty SIF file. func (*App) New(path string) error { - id, err := uuid.NewRandom() - if err != nil { - return err - } - - cinfo := sif.CreateInfo{ - Pathname: path, - Launchstr: sif.HdrLaunch, - Sifversion: sif.HdrVersion, - ID: id, - } - - _, err = sif.CreateContainer(cinfo) + _, err := sif.CreateContainer(path) return err } diff --git a/pkg/integrity/testdata/gen_sifs.go b/pkg/integrity/testdata/gen_sifs.go index 28bdc079..d6b642f8 100755 --- a/pkg/integrity/testdata/gen_sifs.go +++ b/pkg/integrity/testdata/gen_sifs.go @@ -11,27 +11,13 @@ import ( "os" "path/filepath" - "github.com/google/uuid" "github.com/sylabs/sif/v2/pkg/integrity" "github.com/sylabs/sif/v2/pkg/sif" "golang.org/x/crypto/openpgp" ) func createImage(path string, dis []sif.DescriptorInput) error { - id, err := uuid.NewV4() - if err != nil { - return err - } - - ci := sif.CreateInfo{ - Pathname: path, - Launchstr: sif.HdrLaunch, - Sifversion: sif.HdrVersion, - ID: id, - InputDescr: dis, - } - - _, err = sif.CreateContainer(ci) + _, err := sif.CreateContainer(path, sif.WithDescriptors(dis...)) return err } diff --git a/pkg/sif/create.go b/pkg/sif/create.go index 5265e0ee..9f181a3d 100644 --- a/pkg/sif/create.go +++ b/pkg/sif/create.go @@ -18,6 +18,8 @@ import ( "path" "strconv" "time" + + "github.com/google/uuid" ) // Find next offset aligned to block size. @@ -216,55 +218,106 @@ func writeHeader(fimg *FileImage) error { return nil } -// CreateContainer is responsible for the creation of a new SIF container -// file. It takes the creation information specification as input -// and produces an output file as specified in the input data. -func CreateContainer(cinfo CreateInfo) (fimg *FileImage, err error) { - fimg = &FileImage{} - fimg.DescrArr = make([]Descriptor, DescrNumEntries) +// createOpts accumulates container creation options. +type createOpts struct { + ID uuid.UUID + InputDescr []DescriptorInput + GetTime func() time.Time +} + +// CreateOpt are used to specify container creation options. +type CreateOpt func(*createOpts) error + +// WithID specifies id as the unique ID. +func WithID(id string) CreateOpt { + return func(co *createOpts) error { + id, err := uuid.Parse(id) + co.ID = id + return err + } +} + +// WithDescriptors appends dis to the list of descriptors. +func WithDescriptors(dis ...DescriptorInput) CreateOpt { + return func(co *createOpts) error { + co.InputDescr = append(co.InputDescr, dis...) + return nil + } +} + +// WithTimeFunc specifies fn as the function to obtain timestamps. +func WithTimeFunc(fn func() time.Time) CreateOpt { + return func(co *createOpts) error { + co.GetTime = fn + return nil + } +} + +// CreateContainer creates a new SIF container file at path, according to opts. +func CreateContainer(path string, opts ...CreateOpt) (*FileImage, error) { + id, err := uuid.NewRandom() + if err != nil { + return nil, err + } + + co := createOpts{ + ID: id, + GetTime: time.Now, + } + + for _, opt := range opts { + if err := opt(&co); err != nil { + return nil, err + } + } + + now := co.GetTime() + + f := &FileImage{} + f.DescrArr = make([]Descriptor, DescrNumEntries) // Prepare a fresh global header - copy(fimg.Header.Launch[:], cinfo.Launchstr) - copy(fimg.Header.Magic[:], HdrMagic) - copy(fimg.Header.Version[:], cinfo.Sifversion) - copy(fimg.Header.Arch[:], HdrArchUnknown) - copy(fimg.Header.ID[:], cinfo.ID[:]) - fimg.Header.Ctime = time.Now().Unix() - fimg.Header.Mtime = time.Now().Unix() - fimg.Header.Dfree = DescrNumEntries - fimg.Header.Dtotal = DescrNumEntries - fimg.Header.Descroff = DescrStartOffset - fimg.Header.Dataoff = DataStartOffset + copy(f.Header.Launch[:], HdrLaunch) + copy(f.Header.Magic[:], HdrMagic) + copy(f.Header.Version[:], HdrVersion) + copy(f.Header.Arch[:], HdrArchUnknown) + f.Header.ID = id + f.Header.Ctime = now.Unix() + f.Header.Mtime = now.Unix() + f.Header.Dfree = DescrNumEntries + f.Header.Dtotal = DescrNumEntries + f.Header.Descroff = DescrStartOffset + f.Header.Dataoff = DataStartOffset // Create container file - fimg.Fp, err = os.OpenFile(cinfo.Pathname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) + f.Fp, err = os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) if err != nil { return nil, fmt.Errorf("container file creation failed: %s", err) } - defer fimg.Fp.Close() + defer f.Fp.Close() // set file pointer to start of data section */ - if _, err = fimg.Fp.Seek(DataStartOffset, 0); err != nil { + if _, err = f.Fp.Seek(DataStartOffset, 0); err != nil { return nil, fmt.Errorf("setting file offset pointer to DataStartOffset: %s", err) } - for _, v := range cinfo.InputDescr { - if err = createDescriptor(fimg, v); err != nil { - return + for _, v := range co.InputDescr { + if err := createDescriptor(f, v); err != nil { + return nil, err } } // Write down the descriptor array - if err = writeDescriptors(fimg); err != nil { - return + if err := writeDescriptors(f); err != nil { + return nil, err } // Write down global header to file - if err = writeHeader(fimg); err != nil { - return + if err := writeHeader(f); err != nil { + return nil, err } - return + return f, nil } func zeroData(fimg *FileImage, descr *Descriptor) error { diff --git a/pkg/sif/create_test.go b/pkg/sif/create_test.go index b1f83aa7..242720d4 100644 --- a/pkg/sif/create_test.go +++ b/pkg/sif/create_test.go @@ -12,8 +12,6 @@ import ( "os" "runtime" "testing" - - "github.com/google/uuid" ) const ( @@ -65,22 +63,9 @@ func TestCreateContainer(t *testing.T) { defer os.Remove(f.Name()) f.Close() - id, err := uuid.NewRandom() - if err != nil { - t.Fatal(err) - } - - // general info for the new SIF file creation - cinfo := CreateInfo{ - Pathname: f.Name(), - Launchstr: HdrLaunch, - Sifversion: HdrVersion, - ID: id, - } - // test container creation without any input descriptors - if _, err := CreateContainer(cinfo); err != nil { - t.Error("CreateContainer(cinfo): should allow empty input descriptor list") + if _, err := CreateContainer(f.Name()); err != nil { + t.Errorf("failed to create container: %v", err) } // data we need to create a definition file descriptor @@ -106,9 +91,6 @@ func TestCreateContainer(t *testing.T) { } definput.Size = fi.Size() - // add this descriptor input element to creation descriptor slice - cinfo.InputDescr = append(cinfo.InputDescr, definput) - // data we need to create a system partition descriptor parinput := DescriptorInput{ Datatype: DataPartition, @@ -137,12 +119,9 @@ func TestCreateContainer(t *testing.T) { t.Errorf("CreateContainer(cinfo): can't set extra info: %s", err) } - // add this descriptor input element to creation descriptor slice - cinfo.InputDescr = append(cinfo.InputDescr, parinput) - // test container creation with two partition input descriptors - if _, err := CreateContainer(cinfo); err != nil { - t.Errorf("CreateContainer(cinfo): CreateContainer(): %s", err) + if _, err := CreateContainer(f.Name(), WithDescriptors(definput, parinput)); err != nil { + t.Errorf("failed to create container: %v", err) } } diff --git a/pkg/sif/sif.go b/pkg/sif/sif.go index aba21f02..f72f63a4 100644 --- a/pkg/sif/sif.go +++ b/pkg/sif/sif.go @@ -453,15 +453,6 @@ func (f *FileImage) GetHeaderIntegrityReader() io.Reader { return f.Header.GetIntegrityReader() } -// CreateInfo wraps all SIF file creation info needed. -type CreateInfo struct { - Pathname string // the end result output filename - Launchstr string // the shell run command - Sifversion string // the SIF specification version used - ID uuid.UUID // image unique identifier - InputDescr []DescriptorInput // slice of input info for descriptor creation -} - // DescriptorInput describes the common info needed to create a data object descriptor. type DescriptorInput struct { Datatype Datatype // datatype being harvested for new descriptor From 77041ccca081a88cb4f02287b440296365e62961 Mon Sep 17 00:00:00 2001 From: Adam Hughes Date: Mon, 5 Jul 2021 14:53:32 +0000 Subject: [PATCH 2/4] refactor: make some const values private --- pkg/sif/create.go | 4 ++-- pkg/sif/load.go | 2 +- pkg/sif/load_test.go | 2 +- pkg/sif/lookup.go | 6 +++--- pkg/sif/lookup_test.go | 2 +- pkg/sif/sif.go | 22 +++++++++++----------- pkg/sif/sif_test.go | 4 ++-- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/pkg/sif/create.go b/pkg/sif/create.go index 9f181a3d..075f257c 100644 --- a/pkg/sif/create.go +++ b/pkg/sif/create.go @@ -277,8 +277,8 @@ func CreateContainer(path string, opts ...CreateOpt) (*FileImage, error) { f.DescrArr = make([]Descriptor, DescrNumEntries) // Prepare a fresh global header - copy(f.Header.Launch[:], HdrLaunch) - copy(f.Header.Magic[:], HdrMagic) + copy(f.Header.Launch[:], hdrLaunch) + copy(f.Header.Magic[:], hdrMagic) copy(f.Header.Version[:], HdrVersion) copy(f.Header.Arch[:], HdrArchUnknown) f.Header.ID = id diff --git a/pkg/sif/load.go b/pkg/sif/load.go index 4c1476a2..7a919948 100644 --- a/pkg/sif/load.go +++ b/pkg/sif/load.go @@ -48,7 +48,7 @@ func readDescriptors(r io.ReaderAt, fimg *FileImage) error { // isValidSif looks at key fields from the global header to assess SIF validity. func isValidSif(f *FileImage) error { - if got, want := trimZeroBytes(f.Header.Magic[:]), HdrMagic; got != want { + if got, want := trimZeroBytes(f.Header.Magic[:]), hdrMagic; got != want { return fmt.Errorf("invalid SIF file: Magic |%v| want |%v|", got, want) } diff --git a/pkg/sif/load_test.go b/pkg/sif/load_test.go index 1e5860eb..51f4ae32 100644 --- a/pkg/sif/load_test.go +++ b/pkg/sif/load_test.go @@ -244,7 +244,7 @@ func TestLoadContainerInvalidMagic(t *testing.T) { // ... and edit the magic to make it invalid. Instead of // exploring all kinds of invalid, simply mess with the last // byte, as this would catch off-by-one errors in the code. - copy(content[HdrLaunchLen:HdrLaunchLen+HdrMagicLen], "SIF_MAGIX") + copy(content[hdrLaunchLen:hdrLaunchLen+hdrMagicLen], "SIF_MAGIX") fp := &mockSifReadWriter{ buf: content, diff --git a/pkg/sif/lookup.go b/pkg/sif/lookup.go index 6c973d0b..6bc6f4b3 100644 --- a/pkg/sif/lookup.go +++ b/pkg/sif/lookup.go @@ -302,15 +302,15 @@ func (d *Descriptor) GetPartType() (Parttype, error) { } // GetArch extracts the Arch field from the Extra field of a Partition Descriptor. -func (d *Descriptor) GetArch() ([HdrArchLen]byte, error) { +func (d *Descriptor) GetArch() ([hdrArchLen]byte, error) { if d.Datatype != DataPartition { - return [HdrArchLen]byte{}, fmt.Errorf("expected DataPartition, got %v", d.Datatype) + return [hdrArchLen]byte{}, fmt.Errorf("expected DataPartition, got %v", d.Datatype) } var pinfo Partition b := bytes.NewReader(d.Extra[:]) if err := binary.Read(b, binary.LittleEndian, &pinfo); err != nil { - return [HdrArchLen]byte{}, fmt.Errorf("while extracting Partition extra info: %s", err) + return [hdrArchLen]byte{}, fmt.Errorf("while extracting Partition extra info: %s", err) } return pinfo.Arch, nil diff --git a/pkg/sif/lookup_test.go b/pkg/sif/lookup_test.go index 88b73c0c..b09d5185 100644 --- a/pkg/sif/lookup_test.go +++ b/pkg/sif/lookup_test.go @@ -449,7 +449,7 @@ func TestGetArch(t *testing.T) { } if trimZeroBytes(arch[:]) != HdrArchAMD64 { - t.Logf("|%s|%s|\n", arch[:HdrArchLen-1], HdrArchAMD64) + t.Logf("|%s|%s|\n", arch[:hdrArchLen-1], HdrArchAMD64) t.Error("part.GetArch() should have returned 'HdrArchAMD64':", err) } diff --git a/pkg/sif/sif.go b/pkg/sif/sif.go index f72f63a4..1a15f383 100644 --- a/pkg/sif/sif.go +++ b/pkg/sif/sif.go @@ -93,8 +93,8 @@ import ( // SIF header constants and quantities. const ( - HdrLaunch = "#!/usr/bin/env run-singularity\n" - HdrMagic = "SIF_MAGIC" // SIF identification + hdrLaunch = "#!/usr/bin/env run-singularity\n" + hdrMagic = "SIF_MAGIC" // SIF identification HdrVersion = "01" // SIF SPEC VERSION HdrArchUnknown = "00" // Undefined/Unsupported arch HdrArch386 = "01" // 386 (i[3-6]86) arch code @@ -109,10 +109,10 @@ const ( HdrArchMIPS64le = "10" // MIPS64 little-endian arch code HdrArchS390x = "11" // IBM s390x arch code - HdrLaunchLen = 32 // len("#!/usr/bin/env... ") - HdrMagicLen = 10 // len("SIF_MAGIC") - HdrVersionLen = 3 // len("99") - HdrArchLen = 3 // len("99") + hdrLaunchLen = 32 // len("#!/usr/bin/env... ") + hdrMagicLen = 10 // len("SIF_MAGIC") + hdrVersionLen = 3 // len("99") + hdrArchLen = 3 // len("99") DescrNumEntries = 48 // the default total number of available descriptors DescrGroupMask = 0xf0000000 // groups start at that offset @@ -329,7 +329,7 @@ type Envvar struct{} type Partition struct { Fstype Fstype Parttype Parttype - Arch [HdrArchLen]byte // arch the image is built for + Arch [hdrArchLen]byte // arch the image is built for } // Signature represents the SIF signature data object descriptor. @@ -352,11 +352,11 @@ type CryptoMessage struct { // Header describes a loaded SIF file. type Header struct { - Launch [HdrLaunchLen]byte // #! shell execution line + Launch [hdrLaunchLen]byte // #! shell execution line - Magic [HdrMagicLen]byte // look for "SIF_MAGIC" - Version [HdrVersionLen]byte // SIF version - Arch [HdrArchLen]byte // arch the primary partition is built for + Magic [hdrMagicLen]byte // look for "SIF_MAGIC" + Version [hdrVersionLen]byte // SIF version + Arch [hdrArchLen]byte // arch the primary partition is built for ID uuid.UUID // image unique identifier Ctime int64 // image creation time diff --git a/pkg/sif/sif_test.go b/pkg/sif/sif_test.go index 5ed11c11..33dfeb1c 100644 --- a/pkg/sif/sif_test.go +++ b/pkg/sif/sif_test.go @@ -19,8 +19,8 @@ func TestHeader_GetIntegrityReader(t *testing.T) { Ctime: 1504657553, Mtime: 1504657653, } - copy(h.Launch[:], HdrLaunch) - copy(h.Magic[:], HdrMagic) + copy(h.Launch[:], hdrLaunch) + copy(h.Magic[:], hdrMagic) copy(h.Version[:], HdrVersion) copy(h.Arch[:], HdrArchAMD64) From 36b1705b7af8fa2b58da65c21abe1ec8e313c265 Mon Sep 17 00:00:00 2001 From: Adam Hughes Date: Mon, 5 Jul 2021 14:57:10 +0000 Subject: [PATCH 3/4] reafactor: define SpecVersion type --- cmd/siftool/siftool.go | 2 +- pkg/sif/create.go | 2 +- pkg/sif/load.go | 2 +- pkg/sif/sif.go | 51 +++++++++++++++++++++++++++++------------- pkg/sif/sif_test.go | 2 +- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/cmd/siftool/siftool.go b/cmd/siftool/siftool.go index f13318b0..439b2d33 100644 --- a/cmd/siftool/siftool.go +++ b/cmd/siftool/siftool.go @@ -27,7 +27,7 @@ func getVersion() *cobra.Command { Args: cobra.ExactArgs(0), Run: func(cmd *cobra.Command, args []string) { cmd.Printf("siftool version %s %s/%s\n", version, runtime.GOOS, runtime.GOARCH) - cmd.Printf("SIF spec versions supported: <= %s\n", sif.HdrVersion) + cmd.Printf("SIF spec versions supported: <= %s\n", sif.CurrentVersion) }, DisableFlagsInUseLine: true, } diff --git a/pkg/sif/create.go b/pkg/sif/create.go index 075f257c..b0e26e1c 100644 --- a/pkg/sif/create.go +++ b/pkg/sif/create.go @@ -279,7 +279,7 @@ func CreateContainer(path string, opts ...CreateOpt) (*FileImage, error) { // Prepare a fresh global header copy(f.Header.Launch[:], hdrLaunch) copy(f.Header.Magic[:], hdrMagic) - copy(f.Header.Version[:], HdrVersion) + copy(f.Header.Version[:], CurrentVersion.bytes()) copy(f.Header.Arch[:], HdrArchUnknown) f.Header.ID = id f.Header.Ctime = now.Unix() diff --git a/pkg/sif/load.go b/pkg/sif/load.go index 7a919948..1ab34e5d 100644 --- a/pkg/sif/load.go +++ b/pkg/sif/load.go @@ -52,7 +52,7 @@ func isValidSif(f *FileImage) error { return fmt.Errorf("invalid SIF file: Magic |%v| want |%v|", got, want) } - if got, want := trimZeroBytes(f.Header.Version[:]), HdrVersion; got > want { + if got, want := trimZeroBytes(f.Header.Version[:]), CurrentVersion.String(); got > want { return fmt.Errorf("invalid SIF file: Version %s want <= %s", got, want) } diff --git a/pkg/sif/sif.go b/pkg/sif/sif.go index 1a15f383..31bc8385 100644 --- a/pkg/sif/sif.go +++ b/pkg/sif/sif.go @@ -84,6 +84,7 @@ package sif import ( "bytes" + "fmt" "io" "os" "time" @@ -93,27 +94,45 @@ import ( // SIF header constants and quantities. const ( - hdrLaunch = "#!/usr/bin/env run-singularity\n" - hdrMagic = "SIF_MAGIC" // SIF identification - HdrVersion = "01" // SIF SPEC VERSION - HdrArchUnknown = "00" // Undefined/Unsupported arch - HdrArch386 = "01" // 386 (i[3-6]86) arch code - HdrArchAMD64 = "02" // AMD64 arch code - HdrArchARM = "03" // ARM arch code - HdrArchARM64 = "04" // AARCH64 arch code - HdrArchPPC64 = "05" // PowerPC 64 arch code - HdrArchPPC64le = "06" // PowerPC 64 little-endian arch code - HdrArchMIPS = "07" // MIPS arch code - HdrArchMIPSle = "08" // MIPS little-endian arch code - HdrArchMIPS64 = "09" // MIPS64 arch code - HdrArchMIPS64le = "10" // MIPS64 little-endian arch code - HdrArchS390x = "11" // IBM s390x arch code - + hdrLaunch = "#!/usr/bin/env run-singularity\n" hdrLaunchLen = 32 // len("#!/usr/bin/env... ") + hdrMagic = "SIF_MAGIC" hdrMagicLen = 10 // len("SIF_MAGIC") hdrVersionLen = 3 // len("99") hdrArchLen = 3 // len("99") +) + +// SpecVersion specifies a SIF specification version. +type SpecVersion uint8 + +func (v SpecVersion) String() string { return fmt.Sprintf("%02d", v) } +func (v SpecVersion) bytes() []byte { return []byte(v.String()) } + +// SIF specification versions. +const ( + version01 SpecVersion = iota + 1 +) + +// CurrentVersion specifies the current SIF specification version. +const CurrentVersion = version01 + +// SIF architecture values. +const ( + HdrArchUnknown = "00" // Undefined/Unsupported arch + HdrArch386 = "01" // 386 (i[3-6]86) arch code + HdrArchAMD64 = "02" // AMD64 arch code + HdrArchARM = "03" // ARM arch code + HdrArchARM64 = "04" // AARCH64 arch code + HdrArchPPC64 = "05" // PowerPC 64 arch code + HdrArchPPC64le = "06" // PowerPC 64 little-endian arch code + HdrArchMIPS = "07" // MIPS arch code + HdrArchMIPSle = "08" // MIPS little-endian arch code + HdrArchMIPS64 = "09" // MIPS64 arch code + HdrArchMIPS64le = "10" // MIPS64 little-endian arch code + HdrArchS390x = "11" // IBM s390x arch code +) +const ( DescrNumEntries = 48 // the default total number of available descriptors DescrGroupMask = 0xf0000000 // groups start at that offset DescrUnusedGroup = DescrGroupMask // descriptor without a group diff --git a/pkg/sif/sif_test.go b/pkg/sif/sif_test.go index 33dfeb1c..ce5f3ddf 100644 --- a/pkg/sif/sif_test.go +++ b/pkg/sif/sif_test.go @@ -21,7 +21,7 @@ func TestHeader_GetIntegrityReader(t *testing.T) { } copy(h.Launch[:], hdrLaunch) copy(h.Magic[:], hdrMagic) - copy(h.Version[:], HdrVersion) + copy(h.Version[:], CurrentVersion.bytes()) copy(h.Arch[:], HdrArchAMD64) tests := []struct { From 907115a71db7fd30e0787c23d154b9aa2e4fc520 Mon Sep 17 00:00:00 2001 From: Adam Hughes Date: Tue, 6 Jul 2021 15:50:39 +0000 Subject: [PATCH 4/4] refactor: use generic variable name for GetTime result --- pkg/sif/create.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/sif/create.go b/pkg/sif/create.go index b0e26e1c..23708f62 100644 --- a/pkg/sif/create.go +++ b/pkg/sif/create.go @@ -271,7 +271,7 @@ func CreateContainer(path string, opts ...CreateOpt) (*FileImage, error) { } } - now := co.GetTime() + t := co.GetTime() f := &FileImage{} f.DescrArr = make([]Descriptor, DescrNumEntries) @@ -282,8 +282,8 @@ func CreateContainer(path string, opts ...CreateOpt) (*FileImage, error) { copy(f.Header.Version[:], CurrentVersion.bytes()) copy(f.Header.Arch[:], HdrArchUnknown) f.Header.ID = id - f.Header.Ctime = now.Unix() - f.Header.Mtime = now.Unix() + f.Header.Ctime = t.Unix() + f.Header.Mtime = t.Unix() f.Header.Dfree = DescrNumEntries f.Header.Dtotal = DescrNumEntries f.Header.Descroff = DescrStartOffset