Merge pull request #159 from tri-adam/nits

Small Improvements

pkg/sif/create.go

@@ -194,6 +194,8 @@ func createContainer(rw ReadWriter, co createOpts) (*FileImage, error) {
 
 	h := header{
 		LaunchScript:      co.launchScript,
+		Magic:             hdrMagic,
+		Version:           CurrentVersion.bytes(),
 		Arch:              hdrArchUnknown,
 		ID:                co.id,
 		CreatedAt:         co.t.Unix(),
@@ -204,8 +206,6 @@ func createContainer(rw ReadWriter, co createOpts) (*FileImage, error) {
 		DescriptorsSize:   rdsSize,
 		DataOffset:        co.descriptorsOffset + rdsSize,
 	}
-	copy(h.Magic[:], hdrMagic)
-	copy(h.Version[:], CurrentVersion.bytes())
 
 	f := &FileImage{
 		rw:     rw,

pkg/sif/descriptor.go

@@ -98,7 +98,7 @@ func (d *rawDescriptor) setExtra(v interface{}) error {
 // getPartitionMetadata gets metadata for a partition data object.
 func (d rawDescriptor) getPartitionMetadata() (fs FSType, pt PartType, arch string, err error) {
 	if got, want := d.DataType, DataPartition; got != want {
-		return 0, 0, "", &unexpectedDataTypeError{got, want}
+		return 0, 0, "", &unexpectedDataTypeError{got, []DataType{want}}
 	}
 
 	var p partition
@@ -188,7 +188,7 @@ func getHashType(ht hashType) (crypto.Hash, error) {
 // SignatureMetadata gets metadata for a signature data object.
 func (d Descriptor) SignatureMetadata() (ht crypto.Hash, fp []byte, err error) {
 	if got, want := d.raw.DataType, DataSignature; got != want {
-		return ht, fp, &unexpectedDataTypeError{got, want}
+		return ht, fp, &unexpectedDataTypeError{got, []DataType{want}}
 	}
 
 	var s signature
@@ -211,7 +211,7 @@ func (d Descriptor) SignatureMetadata() (ht crypto.Hash, fp []byte, err error) {
 // CryptoMessageMetadata gets metadata for a crypto message data object.
 func (d Descriptor) CryptoMessageMetadata() (FormatType, MessageType, error) {
 	if got, want := d.raw.DataType, DataCryptoMessage; got != want {
-		return 0, 0, &unexpectedDataTypeError{got, want}
+		return 0, 0, &unexpectedDataTypeError{got, []DataType{want}}
 	}
 
 	var m cryptoMessage

pkg/sif/descriptor_input.go

@@ -95,20 +95,49 @@ func OptObjectTime(t time.Time) DescriptorInputOpt {
 
 type unexpectedDataTypeError struct {
 	got  DataType
-	want DataType
+	want []DataType
 }
 
 func (e *unexpectedDataTypeError) Error() string {
-	return fmt.Sprintf("unexpected data type %v, expected %v", e.got, e.want)
+	return fmt.Sprintf("unexpected data type %v, expected one of: %v", e.got, e.want)
 }
 
 func (e *unexpectedDataTypeError) Is(target error) bool {
 	t, ok := target.(*unexpectedDataTypeError)
 	if !ok {
 		return false
 	}
-	return (e.got == t.got || t.got == 0) &&
-		(e.want == t.want || t.want == 0)
+
+	if len(t.want) > 0 {
+		// Use a map to check that the "want" errors in e and t contain the same values, ignoring
+		// any ordering differences.
+		acc := make(map[DataType]int, len(t.want))
+
+		// Increment counter for each data type in e.
+		for _, dt := range e.want {
+			if _, ok := acc[dt]; !ok {
+				acc[dt] = 0
+			}
+			acc[dt]++
+		}
+
+		// Decrement counter for each data type in e.
+		for _, dt := range t.want {
+			if _, ok := acc[dt]; !ok {
+				return false
+			}
+			acc[dt]--
+		}
+
+		// If the "want" errors in e and t are equivalent, all counters should be zero.
+		for _, n := range acc {
+			if n != 0 {
+				return false
+			}
+		}
+	}
+
+	return (e.got == t.got || t.got == 0)
 }
 
 // OptCryptoMessageMetadata sets metadata for a crypto message data object. The format type is set
@@ -118,7 +147,7 @@ func (e *unexpectedDataTypeError) Is(target error) bool {
 func OptCryptoMessageMetadata(ft FormatType, mt MessageType) DescriptorInputOpt {
 	return func(t DataType, opts *descriptorOpts) error {
 		if got, want := t, DataCryptoMessage; got != want {
-			return &unexpectedDataTypeError{got, want}
+			return &unexpectedDataTypeError{got, []DataType{want}}
 		}
 
 		m := cryptoMessage{
@@ -141,7 +170,7 @@ var errUnknownArchitcture = errors.New("unknown architecture")
 func OptPartitionMetadata(fs FSType, pt PartType, arch string) DescriptorInputOpt {
 	return func(t DataType, opts *descriptorOpts) error {
 		if got, want := t, DataPartition; got != want {
-			return &unexpectedDataTypeError{got, want}
+			return &unexpectedDataTypeError{got, []DataType{want}}
 		}
 
 		sifarch := getSIFArch(arch)
@@ -184,7 +213,7 @@ func sifHashType(h crypto.Hash) hashType {
 func OptSignatureMetadata(ht crypto.Hash, fp []byte) DescriptorInputOpt {
 	return func(t DataType, opts *descriptorOpts) error {
 		if got, want := t, DataSignature; got != want {
-			return &unexpectedDataTypeError{got, want}
+			return &unexpectedDataTypeError{got, []DataType{want}}
 		}
 
 		s := signature{

pkg/sif/descriptor_input_test.go

@@ -131,7 +131,7 @@ func TestNewDescriptorInput(t *testing.T) {
 			opts: []DescriptorInputOpt{
 				OptCryptoMessageMetadata(FormatOpenPGP, MessageClearSignature),
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataCryptoMessage},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataCryptoMessage}},
 		},
 		{
 			name: "OptCryptoMessageMetadata",
@@ -147,7 +147,7 @@ func TestNewDescriptorInput(t *testing.T) {
 			opts: []DescriptorInputOpt{
 				OptPartitionMetadata(FsSquash, PartPrimSys, "386"),
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataPartition},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataPartition}},
 		},
 		{
 			name: "OptPartitionMetadata",
@@ -163,7 +163,7 @@ func TestNewDescriptorInput(t *testing.T) {
 			opts: []DescriptorInputOpt{
 				OptSignatureMetadata(crypto.SHA256, fp),
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataSignature},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataSignature}},
 		},
 		{
 			name: "OptSignatureMetadata",

pkg/sif/descriptor_test.go

@@ -77,7 +77,7 @@ func TestDescriptor_GetReader(t *testing.T) {
 	}
 }
 
-func TestDescriptor_GetName(t *testing.T) {
+func TestDescriptor_Name(t *testing.T) {
 	// load the test container
 	f, err := LoadContainerFromPath(
 		filepath.Join(corpus, "one-group.sif"),
@@ -105,7 +105,7 @@ func TestDescriptor_GetName(t *testing.T) {
 	}
 }
 
-func TestDescriptor_GetPartitionMetadata(t *testing.T) {
+func TestDescriptor_PartitionMetadata(t *testing.T) {
 	p := partition{
 		Fstype:   FsSquash,
 		Parttype: PartPrimSys,
@@ -133,7 +133,7 @@ func TestDescriptor_GetPartitionMetadata(t *testing.T) {
 			rd: rawDescriptor{
 				DataType: DataGeneric,
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataPartition},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataPartition}},
 		},
 		{
 			name:     "PartPrimSys",
@@ -170,7 +170,7 @@ func TestDescriptor_GetPartitionMetadata(t *testing.T) {
 	}
 }
 
-func TestDescriptor_GetSignatureMetadata(t *testing.T) {
+func TestDescriptor_SignatureMetadata(t *testing.T) {
 	s := signature{
 		Hashtype: hashSHA384,
 	}
@@ -198,7 +198,7 @@ func TestDescriptor_GetSignatureMetadata(t *testing.T) {
 			rd: rawDescriptor{
 				DataType: DataGeneric,
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataSignature},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataSignature}},
 		},
 		{
 			name:   "OK",
@@ -233,7 +233,7 @@ func TestDescriptor_GetSignatureMetadata(t *testing.T) {
 	}
 }
 
-func TestDescriptor_GetCryptoMessageMetadata(t *testing.T) {
+func TestDescriptor_CryptoMessageMetadata(t *testing.T) {
 	m := cryptoMessage{
 		Formattype:  FormatOpenPGP,
 		Messagetype: MessageClearSignature,
@@ -258,7 +258,7 @@ func TestDescriptor_GetCryptoMessageMetadata(t *testing.T) {
 			rd: rawDescriptor{
 				DataType: DataGeneric,
 			},
-			wantErr: &unexpectedDataTypeError{DataGeneric, DataCryptoMessage},
+			wantErr: &unexpectedDataTypeError{DataGeneric, []DataType{DataCryptoMessage}},
 		},
 		{
 			name:   "OK",

pkg/sif/load.go

@@ -8,7 +8,6 @@
 package sif
 
 import (
-	"bytes"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -23,11 +22,11 @@ var (
 
 // isValidSif looks at key fields from the global header to assess SIF validity.
 func isValidSif(f *FileImage) error {
-	if got, want := trimZeroBytes(f.h.Magic[:]), hdrMagic; got != want {
+	if f.h.Magic != hdrMagic {
 		return errInvalidMagic
 	}
 
-	if got, want := trimZeroBytes(f.h.Version[:]), CurrentVersion.String(); got > want {
+	if f.h.Version != CurrentVersion.bytes() {
 		return errIncompatibleVersion
 	}
 
@@ -173,7 +172,3 @@ func (f *FileImage) UnloadContainer() error {
 	}
 	return nil
 }
-
-func trimZeroBytes(str []byte) string {
-	return string(bytes.TrimRight(str, "\x00"))
-}

pkg/sif/load_test.go

@@ -136,47 +136,3 @@ func TestLoadContainerInvalidMagic(t *testing.T) {
 		t.Errorf(`LoadContainerFp(fp, true) did not report an error for a container with invalid magic.`)
 	}
 }
-
-func TestTrimZeroBytes(t *testing.T) {
-	tt := []struct {
-		name   string
-		in     []byte
-		expect string
-	}{
-		{
-			name:   "no zero",
-			in:     []byte("hello!"),
-			expect: "hello!",
-		},
-		{
-			name:   "c string x00",
-			in:     []byte("hello!\x00"),
-			expect: "hello!",
-		},
-		{
-			name:   "c string 000",
-			in:     []byte("hello!\000"),
-			expect: "hello!",
-		},
-		{
-			name:   "many zeroes x00",
-			in:     []byte("hello!\x00\x00\x00\x00\x00\x00\x00"),
-			expect: "hello!",
-		},
-		{
-			name:   "many zeroes 000",
-			in:     []byte("hello!\000\000\000\000\000\000\000"),
-			expect: "hello!",
-		},
-	}
-
-	for _, tc := range tt {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			actual := trimZeroBytes(tc.in)
-			if tc.expect != actual {
-				t.Fatalf("Expected %q, but got %q", tc.expect, actual)
-			}
-		})
-	}
-}

pkg/sif/sif.go

@@ -87,16 +87,23 @@ import (
 // SIF header constants and quantities.
 const (
 	hdrLaunchLen  = 32 // len("#!/usr/bin/env... ")
-	hdrMagic      = "SIF_MAGIC"
 	hdrMagicLen   = 10 // len("SIF_MAGIC")
 	hdrVersionLen = 3  // len("99")
 )
 
+var hdrMagic = [...]byte{'S', 'I', 'F', '_', 'M', 'A', 'G', 'I', 'C', '\x00'}
+
 // 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()) }
+
+// bytes returns the value of b, formatted for direct inclusion in a SIF header.
+func (v SpecVersion) bytes() [hdrVersionLen]byte {
+	var b [3]byte
+	copy(b[:], fmt.Sprintf("%02d", v))
+	return b
+}
 
 // SIF specification versions.
 const (
@@ -310,10 +317,14 @@ type FileImage struct {
 }
 
 // LaunchScript returns the image launch script.
-func (f *FileImage) LaunchScript() string { return trimZeroBytes(f.h.LaunchScript[:]) }
+func (f *FileImage) LaunchScript() string {
+	return string(bytes.TrimRight(f.h.LaunchScript[:], "\x00"))
+}
 
 // Version returns the SIF specification version of the image.
-func (f *FileImage) Version() string { return trimZeroBytes(f.h.Version[:]) }
+func (f *FileImage) Version() string {
+	return string(bytes.TrimRight(f.h.Version[:], "\x00"))
+}
 
 // PrimaryArch returns the primary CPU architecture of the image.
 func (f *FileImage) PrimaryArch() string { return f.h.Arch.GoArch() }

pkg/sif/sif_test.go

@@ -18,14 +18,14 @@ var corpus = filepath.Join("..", "..", "test", "images")
 
 func TestHeader_GetIntegrityReader(t *testing.T) {
 	h := header{
+		Magic:      hdrMagic,
+		Version:    CurrentVersion.bytes(),
 		Arch:       hdrArchAMD64,
 		ID:         uuid.UUID{0xb2, 0x65, 0x9d, 0x4e, 0xbd, 0x50, 0x4e, 0xa5, 0xbd, 0x17, 0xee, 0xc5, 0xe5, 0x4f, 0x91, 0x8e},
 		CreatedAt:  1504657553,
 		ModifiedAt: 1504657653,
 	}
 	copy(h.LaunchScript[:], "#!/usr/bin/env run-singularity\n")
-	copy(h.Magic[:], hdrMagic)
-	copy(h.Version[:], CurrentVersion.bytes())
 
 	tests := []struct {
 		name    string