Fix windows tests and renable in CI

.github/workflows/tests.yml

@@ -26,11 +26,14 @@ jobs:
     strategy:
       fail-fast: false # Keep running if one leg fails.
       matrix:
-        os: [ubuntu-latest] # , macos-latest, windows-latest] Enable later so we don't waste github actions resources
+        os: [ubuntu-latest, windows-latest] # , macos-latest, windows-latest] Enable later so we don't waste github actions resources
         go-version: ${{ fromJSON(needs.get-go-versions.outputs.matrix) }}
     runs-on: ${{ matrix.os }}
     needs: get-go-versions
     steps:
+      - name: Set git to use LF
+        run: git config --global core.autocrlf false
+
       - name: Checkout code
         uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744
 

examples/cli/tuf-client/cmd/get.go

@@ -151,7 +151,7 @@ func verifyEnv() (*localConfig, error) {
 		return nil, fmt.Errorf("no local download folder: %w", err)
 	}
 	// verify there's a local root.json available for bootstrapping trust
-	_, err = os.Stat(fmt.Sprintf("%s/%s.json", env.MetadataDir, metadata.ROOT))
+	_, err = os.Stat(filepath.Join(env.MetadataDir, fmt.Sprintf("%s.json", metadata.ROOT)))
 	if err != nil {
 		return nil, fmt.Errorf("no local download folder: %w", err)
 	}

examples/cli/tuf-client/cmd/init.go

@@ -69,7 +69,7 @@ func InitializeCmd() error {
 		if err != nil {
 			return err
 		}
-		rootPath = fmt.Sprintf("%s/%s.json", rootPath, metadata.ROOT)
+		rootPath = filepath.Join(rootPath, fmt.Sprintf("%s.json", metadata.ROOT))
 		// no need to copy root.json to the metadata folder as we already download it in the expected location
 		copyTrusted = false
 	}

metadata/metadata_test.go

@@ -15,8 +15,8 @@ import (
 	"crypto/ed25519"
 	"crypto/sha256"
 	"encoding/json"
-	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -500,7 +500,8 @@ func TestToByte(t *testing.T) {
 
 func TestFromFile(t *testing.T) {
 	root := Root(fixedExpire)
-	_, err := root.FromFile(fmt.Sprintf("%s/1.root.json", TEST_REPOSITORY_DATA))
+	filePath := filepath.Join(TEST_REPOSITORY_DATA, "1.root.json")
+	_, err := root.FromFile(filePath)
 	assert.NoError(t, err)
 
 	assert.Equal(t, fixedExpire, root.Signed.Expires)
@@ -548,7 +549,7 @@ func TestToFile(t *testing.T) {
 	tmpDir, err := os.MkdirTemp(tmp, "0750")
 	assert.NoError(t, err)
 
-	fileName := fmt.Sprintf("%s/1.root.json", tmpDir)
+	fileName := filepath.Join(tmpDir, "1.root.json")
 	assert.NoFileExists(t, fileName)
 	root, err := Root().FromBytes(testRootBytes)
 	assert.NoError(t, err)

metadata/trustedmetadata/trustedmetadata_test.go

@@ -13,8 +13,8 @@ package trustedmetadata
 
 import (
 	"crypto"
-	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -32,39 +32,50 @@ func setAllRolesBytes(path string) {
 	log := metadata.GetLogger()
 
 	allRoles = make(map[string][]byte)
-	root, err := os.ReadFile(fmt.Sprintf("%s/root.json", path))
+	rootPath := filepath.Join(path, "root.json")
+	root, err := os.ReadFile(rootPath)
 	if err != nil {
-		log.Error(err, "failed to root bytes")
+		log.Error(err, "failed to read root bytes")
 		os.Exit(1)
 	}
 	allRoles[metadata.ROOT] = root
-	targets, err := os.ReadFile(fmt.Sprintf("%s/targets.json", path))
+
+	targetsPath := filepath.Join(path, "targets.json")
+	targets, err := os.ReadFile(targetsPath)
 	if err != nil {
-		log.Error(err, "failed to targets bytes")
+		log.Error(err, "failed to read targets bytes")
 		os.Exit(1)
 	}
 	allRoles[metadata.TARGETS] = targets
-	snapshot, err := os.ReadFile(fmt.Sprintf("%s/snapshot.json", path))
+
+	snapshotPath := filepath.Join(path, "snapshot.json")
+	snapshot, err := os.ReadFile(snapshotPath)
 	if err != nil {
-		log.Error(err, "failed to snapshot bytes")
+		log.Error(err, "failed to read snapshot bytes")
 		os.Exit(1)
 	}
 	allRoles[metadata.SNAPSHOT] = snapshot
-	timestamp, err := os.ReadFile(fmt.Sprintf("%s/timestamp.json", path))
+
+	timestampPath := filepath.Join(path, "timestamp.json")
+	timestamp, err := os.ReadFile(timestampPath)
 	if err != nil {
-		log.Error(err, "failed to timestamp bytes")
+		log.Error(err, "failed to read timestamp bytes")
 		os.Exit(1)
 	}
 	allRoles[metadata.TIMESTAMP] = timestamp
-	role1, err := os.ReadFile(fmt.Sprintf("%s/role1.json", path))
+
+	role1Path := filepath.Join(path, "role1.json")
+	role1, err := os.ReadFile(role1Path)
 	if err != nil {
-		log.Error(err, "failed to role1 bytes")
+		log.Error(err, "failed to read role1 bytes")
 		os.Exit(1)
 	}
 	allRoles["role1"] = role1
-	role2, err := os.ReadFile(fmt.Sprintf("%s/role2.json", path))
+
+	role2Path := filepath.Join(path, "role2.json")
+	role2, err := os.ReadFile(role2Path)
 	if err != nil {
-		log.Error(err, "failed to role2 bytes")
+		log.Error(err, "failed to read role2 bytes")
 		os.Exit(1)
 	}
 	allRoles["role2"] = role2
@@ -98,7 +109,7 @@ func modifyRootMetadata(fn modifyRoot) ([]byte, error) {
 	}
 	fn(root)
 
-	signer, err := signature.LoadSignerFromPEMFile(testutils.KeystoreDir+"/root_key", crypto.SHA256, cryptoutils.SkipPassword)
+	signer, err := signature.LoadSignerFromPEMFile(filepath.Join(testutils.KeystoreDir, "root_key"), crypto.SHA256, cryptoutils.SkipPassword)
 	if err != nil {
 		log.Error(err, "failed to load signer from pem file")
 	}
@@ -121,7 +132,7 @@ func modifyTimestamptMetadata(fn modifyTimestamp) ([]byte, error) {
 	}
 	fn(timestamp)
 
-	signer, err := signature.LoadSignerFromPEMFile(testutils.KeystoreDir+"/timestamp_key", crypto.SHA256, cryptoutils.SkipPassword)
+	signer, err := signature.LoadSignerFromPEMFile(filepath.Join(testutils.KeystoreDir, "timestamp_key"), crypto.SHA256, cryptoutils.SkipPassword)
 	if err != nil {
 		log.Error(err, "failed to load signer from pem file")
 	}
@@ -144,7 +155,7 @@ func modifySnapshotMetadata(fn modifySnapshot) ([]byte, error) {
 	}
 	fn(snapshot)
 
-	signer, err := signature.LoadSignerFromPEMFile(testutils.KeystoreDir+"/snapshot_key", crypto.SHA256, cryptoutils.SkipPassword)
+	signer, err := signature.LoadSignerFromPEMFile(filepath.Join(testutils.KeystoreDir, "snapshot_key"), crypto.SHA256, cryptoutils.SkipPassword)
 	if err != nil {
 		log.Error(err, "failed to load signer from pem file")
 	}
@@ -167,7 +178,7 @@ func modifyTargetsMetadata(fn modifyTargets) ([]byte, error) {
 	}
 	fn(targets)
 
-	signer, err := signature.LoadSignerFromPEMFile(testutils.KeystoreDir+"/targets_key", crypto.SHA256, cryptoutils.SkipPassword)
+	signer, err := signature.LoadSignerFromPEMFile(filepath.Join(testutils.KeystoreDir, "targets_key"), crypto.SHA256, cryptoutils.SkipPassword)
 	if err != nil {
 		log.Error(err, "failed to load signer from pem file")
 	}

metadata/updater/updater.go

@@ -20,6 +20,8 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"regexp"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -231,7 +233,7 @@ func (update *Updater) DownloadTarget(targetFile *metadata.TargetFiles, filePath
 			targetFilePath = fmt.Sprintf("%s.%s", hashes, dirName)
 		} else {
 			// <dir-prefix>/<hash>.<target-name>
-			targetFilePath = fmt.Sprintf("%s/%s.%s", dirName, hashes, baseName)
+			targetFilePath = filepath.Join(dirName, fmt.Sprintf("%s.%s", hashes, baseName))
 		}
 	}
 	fullURL := fmt.Sprintf("%s%s", targetBaseURL, targetFilePath)
@@ -569,6 +571,40 @@ func (update *Updater) preOrderDepthFirstWalk(targetFilePath string) (*metadata.
 	return nil, fmt.Errorf("target %s not found", targetFilePath)
 }
 
+// on windows, you can't rename a file across drives, so let's move instead
+func MoveFile(source, destination string) (err error) {
+	if runtime.GOOS == "windows" {
+		inputFile, err := os.Open(source)
+		if err != nil {
+			return fmt.Errorf("Couldn't open source file: %s", err)
+		}
+		defer inputFile.Close()
+		outputFile, err := os.Create(destination)
+		if err != nil {
+			inputFile.Close()
+			return fmt.Errorf("Couldn't open dest file: %s", err)
+		}
+		defer outputFile.Close()
+		c, err := io.Copy(outputFile, inputFile)
+		if err != nil {
+			return fmt.Errorf("Writing to output file failed: %s", err)
+		}
+		if c <= 0 {
+			return fmt.Errorf("Nothing copied to output file")
+		}
+		inputFile.Close()
+		// The copy was successful, so now delete the original file
+		err = os.Remove(source)
+		if err != nil {
+			return fmt.Errorf("Failed removing original file: %s", err)
+		}
+		return nil
+	} else {
+		return os.Rename(source, destination)
+	}
+
+}
+
 // persistMetadata writes metadata to disk atomically to avoid data loss
 func (update *Updater) persistMetadata(roleName string, data []byte) error {
 	log := metadata.GetLogger()
@@ -587,6 +623,7 @@ func (update *Updater) persistMetadata(roleName string, data []byte) error {
 	if err != nil {
 		return err
 	}
+	defer file.Close()
 	// write the data content to the temporary file
 	err = os.WriteFile(file.Name(), data, 0644)
 	if err != nil {
@@ -597,11 +634,21 @@ func (update *Updater) persistMetadata(roleName string, data []byte) error {
 		}
 		return err
 	}
+
+	// can't move/rename an open file on windows, so close it first
+	file.Close()
 	// if all okay, rename the temporary file to the desired one
-	err = os.Rename(file.Name(), fileName)
+	err = MoveFile(file.Name(), fileName)
 	if err != nil {
 		return err
 	}
+	read, err := os.ReadFile(fileName)
+	if err != nil {
+		return err
+	}
+	if string(read) != string(data) {
+		return fmt.Errorf("failed to persist metadata: %w", err)
+	}
 	return nil
 }
 
@@ -642,12 +689,20 @@ func (update *Updater) GetTrustedMetadataSet() trustedmetadata.TrustedMetadata {
 	return *update.trusted
 }
 
+func IsWindowsPath(path string) bool {
+	match, _ := regexp.MatchString(`^[a-zA-Z]:\\`, path)
+	return match
+}
+
 // ensureTrailingSlash ensures url ends with a slash
-func ensureTrailingSlash(url string) string {
-	if strings.HasSuffix(url, "/") {
-		return url
+func ensureTrailingSlash(urlStr string) string {
+	if IsWindowsPath(urlStr) {
+		return urlStr + string(filepath.Separator)
+	}
+	if strings.HasSuffix(urlStr, "/") {
+		return urlStr
 	}
-	return url + "/"
+	return urlStr + "/"
 }
 
 // reverseSlice reverses the elements in a generic type of slice

metadata/updater/updater_consistent_snapshot_test.go

@@ -12,6 +12,7 @@
 package updater
 
 import (
+	"runtime/debug"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -33,10 +34,12 @@ func TestTopLevelRolesUpdateWithConsistentSnapshotDisabled(t *testing.T) {
 	updaterConfig, err := loadUpdaterConfig()
 	assert.NoError(t, err)
 	updater := initUpdater(updaterConfig)
-
 	// cleanup fetch tracker metadata
 	simulator.Sim.FetchTracker.Metadata = []simulator.FTMetadata{}
 	err = updater.Refresh()
+	if err != nil {
+		t.Log(string(debug.Stack()))
+	}
 	assert.NoError(t, err)
 
 	// metadata files are fetched with the expected version (or None)
@@ -65,7 +68,9 @@ func TestTopLevelRolesUpdateWithConsistentSnapshotEnabled(t *testing.T) {
 	updaterConfig, err := loadUpdaterConfig()
 	assert.NoError(t, err)
 	updater := initUpdater(updaterConfig)
-
+	if updater == nil {
+		t.Fatal("updater is nil")
+	}
 	// cleanup fetch tracker metadata
 	simulator.Sim.FetchTracker.Metadata = []simulator.FTMetadata{}
 	err = updater.Refresh()
@@ -127,7 +132,9 @@ func TestDelegatesRolesUpdateWithConsistentSnapshotDisabled(t *testing.T) {
 	updaterConfig, err := loadUpdaterConfig()
 	assert.NoError(t, err)
 	updater := initUpdater(updaterConfig)
-
+	if updater == nil {
+		t.Fatal("updater is nil")
+	}
 	err = updater.Refresh()
 	assert.NoError(t, err)
 
@@ -191,7 +198,9 @@ func TestDelegatesRolesUpdateWithConsistentSnapshotEnabled(t *testing.T) {
 	updaterConfig, err := loadUpdaterConfig()
 	assert.NoError(t, err)
 	updater := initUpdater(updaterConfig)
-
+	if updater == nil {
+		t.Fatal("updater is nil")
+	}
 	err = updater.Refresh()
 	assert.NoError(t, err)