More migration fixes

cmd/migration-manager-worker/internal/worker/worker.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -657,20 +658,34 @@ func (w *Worker) matchSourceArtifact(artifacts []api.Artifact, cmd api.WorkerCom
 }
 
 func (w *Worker) matchDriverArtifact(artifacts []api.Artifact, cmd api.WorkerCommand) (*api.Artifact, error) {
-	var artifact *api.Artifact
+	var matchingArtifact *api.Artifact
 	for _, a := range artifacts {
 		match := a.Type == api.ARTIFACTTYPE_DRIVER && a.OS == cmd.OSType && util.MatchArchitecture(a.Architectures, cmd.Architecture) == nil
-		if match {
-			artifact = &a
-			break
+		if match && (len(a.Versions) == 0 || slices.Contains(a.Versions, cmd.DistributionVersion)) {
+			if matchingArtifact == nil {
+				matchingArtifact = &a
+				// Keep looking for a more precise match if no version is specified.
+				if len(matchingArtifact.Versions) > 0 {
+					break
+				} else {
+					continue
+				}
+			}
+
+			// If the another artifact matches more precisely to the version, use that instead.
+			if len(matchingArtifact.Versions) == 0 && slices.Contains(a.Versions, cmd.DistributionVersion) {
+				matchingArtifact = &a
+				break
+			}
 		}
 	}
 
-	if artifact == nil {
+	if matchingArtifact == nil {
 		return nil, fmt.Errorf("Failed to find a matching artifact for %q architecture %q", cmd.OSType, cmd.Architecture)
 	}
 
-	return artifact, nil
+	slog.Info("Using driver artifact", slog.String("uuid", matchingArtifact.UUID.String()), slog.String("ver", cmd.DistributionVersion), slog.Int("len", len(artifacts)))
+	return matchingArtifact, nil
 }
 
 func (w *Worker) matchImageArtifact(artifacts []api.Artifact, cmd api.WorkerCommand, osVersion string) (*api.Artifact, error) {

internal/migration/artifact_model.go

@@ -8,6 +8,7 @@ import (
 	"github.com/lxc/incus/v6/shared/osarch"
 	"golang.org/x/mod/semver"
 
+	"github.com/FuturFusion/migration-manager/internal/util"
 	"github.com/FuturFusion/migration-manager/shared/api"
 )
 
@@ -49,8 +50,11 @@ func (a Artifact) Validate() error {
 
 		switch a.Properties.OS {
 		case api.OSTYPE_WINDOWS:
-			if len(a.Properties.Versions) > 0 {
-				return NewValidationErrf("Artifact for OS %q does not support versions", a.Properties.OS)
+			for _, v := range a.Properties.Versions {
+				err := util.ValidateWindowsVersion(v)
+				if err != nil {
+					return NewValidationErrf("Artifact version is invalid for OS %q: %v", a.Properties.OS, err)
+				}
 			}
 
 		default:
@@ -117,6 +121,47 @@ func (a Artifact) Validate() error {
 	return nil
 }
 
+func (a Artifact) CollidesWith(arts Artifacts) error {
+	archMap := map[string]bool{}
+	verMap := map[string]bool{}
+
+	for _, ver := range a.Properties.Versions {
+		verMap[ver] = true
+	}
+
+	for _, arch := range a.Properties.Architectures {
+		archMap[arch] = true
+	}
+
+	for _, art := range arts {
+		if a.Properties.OS != art.Properties.OS || a.Properties.SourceType != art.Properties.SourceType || a.Type != art.Type {
+			continue
+		}
+
+		archCollides := len(art.Properties.Architectures) == 0 && len(archMap) == 0
+		verCollides := len(art.Properties.Versions) == 0 && len(verMap) == 0
+		for _, arch := range art.Properties.Architectures {
+			if archMap[arch] {
+				archCollides = true
+				break
+			}
+		}
+
+		for _, ver := range art.Properties.Versions {
+			if verMap[ver] {
+				verCollides = true
+				break
+			}
+		}
+
+		if archCollides && verCollides {
+			return fmt.Errorf("Artifact architecture or version collides with another artifact: %q", art.UUID)
+		}
+	}
+
+	return nil
+}
+
 func (a Artifact) ToAPI() api.Artifact {
 	return api.Artifact{
 		ArtifactPost: api.ArtifactPost{

internal/migration/artifact_service.go

@@ -37,7 +37,16 @@ func (a artifactService) Create(ctx context.Context, artifact Artifact) (Artifac
 	}
 
 	err = transaction.Do(ctx, func(ctx context.Context) error {
-		var err error
+		arts, err := a.repo.GetAllByType(ctx, artifact.Type)
+		if err != nil {
+			return err
+		}
+
+		err = artifact.CollidesWith(arts)
+		if err != nil {
+			return err
+		}
+
 		artifact.ID, err = a.repo.Create(ctx, artifact)
 		if err != nil {
 			return fmt.Errorf("Failed to create artifact: %w", err)
@@ -113,7 +122,19 @@ func (a artifactService) Update(ctx context.Context, id uuid.UUID, artifact *Art
 		return err
 	}
 
-	return a.repo.Update(ctx, id, artifact)
+	return transaction.Do(ctx, func(ctx context.Context) error {
+		arts, err := a.repo.GetAllByType(ctx, artifact.Type)
+		if err != nil {
+			return err
+		}
+
+		err = artifact.CollidesWith(arts)
+		if err != nil {
+			return err
+		}
+
+		return a.repo.Update(ctx, id, artifact)
+	})
 }
 
 // WriteFile writes the file into the directory contained in a tarball at ArtifactDir/{uuid}.tar.gz.

internal/migration/artifact_service_test.go

@@ -163,6 +163,9 @@ func TestArtifact_Create(t *testing.T) {
 				CreateFunc: func(ctx context.Context, artifact migration.Artifact) (int64, error) {
 					return -1, nil
 				},
+				GetAllByTypeFunc: func(ctx context.Context, artType api.ArtifactType) (migration.Artifacts, error) {
+					return nil, nil
+				},
 			}
 
 			artifactSvc := migration.NewArtifactService(repo, &sys.OS{})
@@ -368,3 +371,151 @@ func TestArtifact_HasRequiredArtifactsForInstance(t *testing.T) {
 		})
 	}
 }
+
+func TestArtifact_collidesWith(t *testing.T) {
+	art := func(v []string, a []string, t api.ArtifactType, o api.OSType, s api.SourceType) migration.Artifact {
+		return migration.Artifact{
+			UUID: uuid.New(),
+			Type: t,
+			Properties: api.ArtifactPut{
+				OS:            o,
+				Architectures: a,
+				Versions:      v,
+				SourceType:    s,
+			},
+		}
+	}
+	cases := []struct {
+		name      string
+		existing  migration.Artifacts
+		artifact  migration.Artifact
+		assertErr require.ErrorAssertionFunc
+	}{
+		{
+			name:      "success - empty list",
+			existing:  migration.Artifacts{},
+			artifact:  art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.NoError,
+		},
+		{
+			name: "success - existing artifact is specific",
+			existing: migration.Artifacts{
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+
+				art([]string{"v2"}, []string{"a2"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.NoError,
+		},
+		{
+			name: "success - new artifact is specific (arch)",
+			existing: migration.Artifacts{
+				art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_OSIMAGE, api.OSTYPE_FORTIGATE, ""),
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_SDK, "", api.SOURCETYPE_VMWARE),
+			},
+
+			artifact:  art([]string{}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.NoError,
+		},
+		{
+			name: "success - new artifact is specific (version)",
+			existing: migration.Artifacts{
+				art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_OSIMAGE, api.OSTYPE_FORTIGATE, ""),
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_SDK, "", api.SOURCETYPE_VMWARE),
+			},
+
+			artifact:  art([]string{"v1"}, []string{}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.NoError,
+		},
+		{
+			name: "success - new artifact is specific (both)",
+			existing: migration.Artifacts{
+				art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_OSIMAGE, api.OSTYPE_FORTIGATE, ""),
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_SDK, "", api.SOURCETYPE_VMWARE),
+			},
+
+			artifact:  art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.NoError,
+		},
+		//
+		// Errors
+		//
+		{
+			name: "error - new artifact collides (empty)",
+			existing: migration.Artifacts{
+				art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art(nil, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+		{
+			name: "error - new artifact collides (version)",
+			existing: migration.Artifacts{
+				art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art([]string{"v1"}, nil, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+		{
+			name: "error - new artifact collides (arch)",
+			existing: migration.Artifacts{
+				art(nil, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art(nil, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+		{
+			name: "error - new artifact collides",
+			existing: migration.Artifacts{
+				art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+		{
+			name: "error - new artifact collides (existing multi)",
+			existing: migration.Artifacts{
+				art([]string{"v1", "v2"}, []string{"a1", "a2"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+		{
+			name: "error - new artifact collides (new multi)",
+			existing: migration.Artifacts{
+				art([]string{"v1"}, []string{"a1"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			},
+
+			artifact:  art([]string{"v1", "v2"}, []string{"a1", "a2"}, api.ARTIFACTTYPE_DRIVER, api.OSTYPE_WINDOWS, ""),
+			assertErr: require.Error,
+		},
+	}
+
+	for i, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Logf("\n\nTEST %02d: %s\n\n", i, tc.name)
+			tc.assertErr(t, tc.artifact.CollidesWith(tc.existing))
+		})
+	}
+}