From 738371b7dac614ad1f8c3ffb01d2c1ae989f146b Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:42:58 -0700 Subject: [PATCH 01/13] internal/migration: Reset ctk status if support changes Signed-off-by: Max Asnaashari --- internal/migration/instance_model.go | 49 ++++++++++++++++------------ 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/internal/migration/instance_model.go b/internal/migration/instance_model.go index fce353bf..0b1824ea 100644 --- a/internal/migration/instance_model.go +++ b/internal/migration/instance_model.go @@ -326,12 +326,6 @@ func (i Instance) ApplyUpdates(srcInst Instance) (Instance, bool) { instanceUpdated = true } - if inst.Properties.BackgroundImport != srcInst.Properties.BackgroundImport { - log.Debug("Instance background import changed", slog.Bool("new", srcInst.Properties.BackgroundImport), slog.Bool("old", inst.Properties.BackgroundImport)) - inst.Properties.BackgroundImport = srcInst.Properties.BackgroundImport - instanceUpdated = true - } - if inst.Properties.Description != srcInst.Properties.Description { log.Debug("Instance description changed", slog.String("new", srcInst.Properties.Description), slog.String("old", inst.Properties.Description)) inst.Properties.Description = srcInst.Properties.Description @@ -364,29 +358,42 @@ func (i Instance) ApplyUpdates(srcInst Instance) (Instance, bool) { } if !slices.Equal(inst.Properties.Disks, srcInst.Properties.Disks) { - oldDisks := map[string]api.InstancePropertiesDisk{} - for _, d := range inst.Properties.Disks { - oldDisks[d.Name] = d - } - - // Preserve background import verification. - newDisks := make([]api.InstancePropertiesDisk, len(srcInst.Properties.Disks)) - for i, newDisk := range srcInst.Properties.Disks { - oldDisk, ok := oldDisks[newDisk.Name] - if ok { - newDisk.BackgroundImportVerified = oldDisk.BackgroundImportVerified + // If background import status has not changed on the source, preserve verification for all known disks. + if inst.Properties.BackgroundImport == srcInst.Properties.BackgroundImport { + oldDisks := map[string]api.InstancePropertiesDisk{} + for _, d := range inst.Properties.Disks { + oldDisks[d.Name] = d } - newDisks[i] = newDisk - } + // Preserve background import verification. + newDisks := make([]api.InstancePropertiesDisk, len(srcInst.Properties.Disks)) + for i, newDisk := range srcInst.Properties.Disks { + oldDisk, ok := oldDisks[newDisk.Name] + if ok { + newDisk.BackgroundImportVerified = oldDisk.BackgroundImportVerified + } + + newDisks[i] = newDisk + } - if !slices.Equal(inst.Properties.Disks, newDisks) { + if !slices.Equal(inst.Properties.Disks, newDisks) { + log.Debug("Instance disks changed") + inst.Properties.Disks = newDisks + instanceUpdated = true + } + } else { log.Debug("Instance disks changed") - inst.Properties.Disks = newDisks + inst.Properties.Disks = srcInst.Properties.Disks instanceUpdated = true } } + if inst.Properties.BackgroundImport != srcInst.Properties.BackgroundImport { + log.Debug("Instance background import changed", slog.Bool("new", srcInst.Properties.BackgroundImport), slog.Bool("old", inst.Properties.BackgroundImport)) + inst.Properties.BackgroundImport = srcInst.Properties.BackgroundImport + instanceUpdated = true + } + if !slices.Equal(inst.Properties.NICs, srcInst.Properties.NICs) { oldNics := map[string]api.InstancePropertiesNIC{} for _, nic := range inst.Properties.NICs { From 83e56e63c1a6609f98dbada3b72b4d7228380262 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:34:06 -0700 Subject: [PATCH 02/13] internal/target: Set migration.stateful=false with vtpm Signed-off-by: Max Asnaashari --- internal/target/incus.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/target/incus.go b/internal/target/incus.go index ea9dc067..c5e378ad 100644 --- a/internal/target/incus.go +++ b/internal/target/incus.go @@ -367,6 +367,7 @@ func (t *InternalIncusTarget) SetPostMigrationVMConfig(ctx context.Context, i mi } if i.Properties.TPM { + apiDef.Config["migration.stateful"] = "false" apiDef.Devices["vtpm"] = map[string]string{ "type": "tpm", "path": "/dev/tpm0", From 21988de99a0f44d75e9a5f76141ab79a03d6d4c1 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:43:20 -0700 Subject: [PATCH 03/13] internal/worker/scripts: Use apt-get autoremove --purge over apt-get autopurge Signed-off-by: Max Asnaashari --- internal/worker/scripts/debian-purge-open-vm-tools.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/worker/scripts/debian-purge-open-vm-tools.sh b/internal/worker/scripts/debian-purge-open-vm-tools.sh index def1be94..00dcb7c2 100644 --- a/internal/worker/scripts/debian-purge-open-vm-tools.sh +++ b/internal/worker/scripts/debian-purge-open-vm-tools.sh @@ -11,4 +11,4 @@ if dpkg -l | grep -q "open-vm-tools" ; then apt-get purge -y open-vm-tools fi -apt-get autopurge -y +apt-get autoremove --purge -y From 1b006338969913ced344c062344b38df009ee01f Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Tue, 17 Mar 2026 15:04:45 -0700 Subject: [PATCH 04/13] internal/util: Use proper windows version casing Signed-off-by: Max Asnaashari --- internal/util/os.go | 4 ++-- internal/util/os_test.go | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/util/os.go b/internal/util/os.go index 94c45738..9bcaaa01 100644 --- a/internal/util/os.go +++ b/internal/util/os.go @@ -12,9 +12,9 @@ var windowsVersions = map[string]string{ "11": "w11", "Server 2003": "2k3", "Server 2008": "2k8", - "Server 2008 R2": "2k8r2", + "Server 2008 R2": "2k8R2", "Server 2012": "2k12", - "Server 2012 R2": "2k12r2", + "Server 2012 R2": "2k12R2", "Server 2016": "2k16", "Server 2019": "2k19", "Server 2022": "2k22", diff --git a/internal/util/os_test.go b/internal/util/os_test.go index 37f4943a..ee5e8d7e 100644 --- a/internal/util/os_test.go +++ b/internal/util/os_test.go @@ -29,7 +29,7 @@ func TestMapWindowsVersionToAbbrevSuccess(t *testing.T) { }, { name: "Windows Server 2008 R2 (64 bit)", - want: "2k8r2", + want: "2k8R2", }, { name: "Windows Server 2012", @@ -37,7 +37,7 @@ func TestMapWindowsVersionToAbbrevSuccess(t *testing.T) { }, { name: "Windows Server 2012 R2", - want: "2k12r2", + want: "2k12R2", }, { name: "Windows Server 2016", @@ -128,9 +128,6 @@ func TestMapWindowsVersionToAbbrevNotSupported(t *testing.T) { { name: "Windows Vista", }, - { - name: "Windows Small Business Server 2003", - }, { name: "Windows XP Home Edition", }, From a4c12e8b6eae8a7b2009f95fd811f8a90addf071 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Tue, 17 Mar 2026 15:11:51 -0700 Subject: [PATCH 05/13] internal/migration: Allow windows artifact versions Signed-off-by: Max Asnaashari --- cmd/migration-manager-worker/internal/worker/worker.go | 3 ++- internal/migration/artifact_model.go | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cmd/migration-manager-worker/internal/worker/worker.go b/cmd/migration-manager-worker/internal/worker/worker.go index b1782d5c..df327fca 100644 --- a/cmd/migration-manager-worker/internal/worker/worker.go +++ b/cmd/migration-manager-worker/internal/worker/worker.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "strings" "time" @@ -660,7 +661,7 @@ func (w *Worker) matchDriverArtifact(artifacts []api.Artifact, cmd api.WorkerCom var artifact *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 { + if match && (len(a.Versions) == 0 || slices.Contains(a.Versions, cmd.DistributionVersion)) { artifact = &a break } diff --git a/internal/migration/artifact_model.go b/internal/migration/artifact_model.go index 64527a50..d33f7207 100644 --- a/internal/migration/artifact_model.go +++ b/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: From d1169b30f9fd0cf991daef32666bb4c639a55511 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:33:01 -0700 Subject: [PATCH 06/13] cmd/migration-manager-worker/internal/worker: Choose versioned drivers over unversioned ones Signed-off-by: Max Asnaashari --- .../internal/worker/worker.go | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/cmd/migration-manager-worker/internal/worker/worker.go b/cmd/migration-manager-worker/internal/worker/worker.go index df327fca..ab07a825 100644 --- a/cmd/migration-manager-worker/internal/worker/worker.go +++ b/cmd/migration-manager-worker/internal/worker/worker.go @@ -658,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 && (len(a.Versions) == 0 || slices.Contains(a.Versions, cmd.DistributionVersion)) { - artifact = &a - break + 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) { From baf75f71923884c2b81c963fba08a7e444b54071 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Tue, 17 Mar 2026 16:19:58 -0700 Subject: [PATCH 07/13] internal/target: Use OS type for image.os Signed-off-by: Max Asnaashari --- internal/target/incus.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/internal/target/incus.go b/internal/target/incus.go index c5e378ad..776825da 100644 --- a/internal/target/incus.go +++ b/internal/target/incus.go @@ -318,15 +318,10 @@ func (t *InternalIncusTarget) SetPostMigrationVMConfig(ctx context.Context, i mi delete(apiDef.Devices, util.WorkerVolume(i.GetArchitecture())) apiDef.Profiles = []string{"default"} - osInfo, err := defs.Get(properties.InstanceOS) - if err != nil { - return err - } - // Handle Windows-specific completion steps. - if apiDef.Config[osInfo.Key] == "win-prepare" { + if apiDef.Config["image.os"] == "win-prepare" { // Fixup the OS name. - apiDef.Config[osInfo.Key] = apiDef.Config["user.migration.os"] + apiDef.Config["image.os"] = apiDef.Config["user.migration.os"] } if !util.InTestingMode() { @@ -405,12 +400,16 @@ func (t *InternalIncusTarget) SetPostMigrationVMConfig(ctx context.Context, i mi return nil } -func (t *InternalIncusTarget) fillInitialProperties(instance incusAPI.InstancesPost, p api.InstanceProperties, storagePool string, defs properties.RawPropertySet[api.TargetType]) (incusAPI.InstancesPost, error) { +func (t *InternalIncusTarget) fillInitialProperties(instance incusAPI.InstancesPost, inst migration.Instance, storagePool string, defs properties.RawPropertySet[api.TargetType]) (incusAPI.InstancesPost, error) { diskDefs, err := defs.GetSubProperties(properties.InstanceDisks) if err != nil { return incusAPI.InstancesPost{}, err } + p := inst.Properties + p.Apply(inst.Overrides.InstancePropertiesConfigurable) + osType := inst.GetOSType() + instance.Config = map[string]string{} for name, info := range defs.GetAll() { switch name { @@ -433,11 +432,11 @@ func (t *InternalIncusTarget) fillInitialProperties(instance incusAPI.InstancesP instance.Config[info.Key] = p.Description instance.Description = p.Description case properties.InstanceOS: - if strings.Contains(strings.ToLower(p.OS), "windows") { + if osType == api.OSTYPE_WINDOWS { instance.Config[info.Key] = "win-prepare" - instance.Config["user.migration.os"] = p.OS + instance.Config["user.migration.os"] = "Windows" } else { - instance.Config[info.Key] = p.OS + instance.Config[info.Key] = string(osType) } case properties.InstanceOSDescription: @@ -514,7 +513,7 @@ func (t *InternalIncusTarget) CreateVMDefinition(instanceDef migration.Instance, } rootDisk := instanceDef.Properties.Disks[0] - ret, err = t.fillInitialProperties(ret, props, q.Placement.StoragePools[rootDisk.Name], defs) + ret, err = t.fillInitialProperties(ret, instanceDef, q.Placement.StoragePools[rootDisk.Name], defs) if err != nil { return incusAPI.InstancesPost{}, err } From fa6bbafffa8b5724a4efafa0b3118d3aab7db769 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:31:48 -0700 Subject: [PATCH 08/13] internal/migration: Don't unset ip addresses on sync Signed-off-by: Max Asnaashari --- internal/migration/instance_model.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/migration/instance_model.go b/internal/migration/instance_model.go index 0b1824ea..8fc74814 100644 --- a/internal/migration/instance_model.go +++ b/internal/migration/instance_model.go @@ -412,6 +412,8 @@ func (i Instance) ApplyUpdates(srcInst Instance) (Instance, bool) { if nic.IPv6Address == "" && oldNIC.IPv6Address != "" { nic.IPv6Address = oldNIC.IPv6Address } + + nic.UUID = oldNIC.UUID } newNics[i] = nic @@ -420,7 +422,7 @@ func (i Instance) ApplyUpdates(srcInst Instance) (Instance, bool) { if !slices.Equal(inst.Properties.NICs, newNics) { log.Debug("Instance nics changed") instanceUpdated = true - inst.Properties.NICs = srcInst.Properties.NICs + inst.Properties.NICs = newNics } } From 38984e7035dd840a064a829117e44f592f3d55ea Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:36:53 -0700 Subject: [PATCH 09/13] internal/worker: Set PATH when running scripts in chroot Signed-off-by: Max Asnaashari --- internal/worker/linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/worker/linux.go b/internal/worker/linux.go index 089c65db..dfba60ab 100644 --- a/internal/worker/linux.go +++ b/internal/worker/linux.go @@ -414,7 +414,7 @@ func runScriptInChroot(scriptName string, args ...string) error { cmd := make([]string, 0, len(args)+2) cmd = append(cmd, chrootMountPath, filepath.Join("/", scriptName)) cmd = append(cmd, args...) - _, err = subprocess.RunCommand("chroot", cmd...) + _, _, err = subprocess.RunCommandSplit(context.TODO(), []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}, nil, "chroot", cmd...) return err } From 343177f1f8f749703a6f4a70eca11603f9e51c64 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:38:16 -0700 Subject: [PATCH 10/13] internal/worker/scripts: Use Type=simple for agent on older SELinux Signed-off-by: Max Asnaashari --- .../worker/scripts/add-incus-agent-override-for-old-systemd.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/worker/scripts/add-incus-agent-override-for-old-systemd.sh b/internal/worker/scripts/add-incus-agent-override-for-old-systemd.sh index 9e0f7d9e..64cea078 100644 --- a/internal/worker/scripts/add-incus-agent-override-for-old-systemd.sh +++ b/internal/worker/scripts/add-incus-agent-override-for-old-systemd.sh @@ -11,13 +11,14 @@ cat < /etc/systemd/system/incus-agent.service.d/override.conf WorkingDirectory= ExecStart= ExecStart=/bin/sh -c "cd /run/incus_agent/; exec ./incus-agent" +Type= +Type=simple EOF # Older SELinux causes errors with Type=notify, so make systemd_notify_t more permissive. # The abstract unix socket is also unlabeled, so we need to allow initrc communication explicitly. if getenforce >/dev/null 2>&1 ; then cat > /tmp/incus_agent.cil << EOF -(allow systemd_notify_t kernel_t (unix_dgram_socket (sendto))) (allow initrc_t unlabeled_t (socket (getopt read write ioctl))) EOF From 2e2bd591d2bc0efd5220261fa969914574527bd3 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:46:13 -0700 Subject: [PATCH 11/13] internal/migration: Fix distro version regex Signed-off-by: Max Asnaashari --- internal/migration/instance_model.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/migration/instance_model.go b/internal/migration/instance_model.go index 8fc74814..db20e9d9 100644 --- a/internal/migration/instance_model.go +++ b/internal/migration/instance_model.go @@ -249,7 +249,7 @@ func (i *Instance) GetDistribution() (api.Distro, string) { case api.OSTYPE_LINUX: // Get the disto's major version, if possible. - versionRegex := regexp.MustCompile(`^[\w /]+?(\d+)(\.\d+)?( \(\w+\))?$`) + versionRegex := regexp.MustCompile(`^[\w /]+?(\d+)(\.\d+)?( \([\w /]+\))?$`) if strings.Contains(strings.ToLower(osVersion), "centos") { distro = api.DISTRO_CENTOS } else if strings.Contains(strings.ToLower(osVersion), "debian") { From e4f3e63d9af184535fe6c5836c51c32e90bb971b Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 16:49:00 -0700 Subject: [PATCH 12/13] internal/worker: Use rhel derivative helper for post-migration tasks Signed-off-by: Max Asnaashari --- internal/worker/linux.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/worker/linux.go b/internal/worker/linux.go index dfba60ab..0e409dd8 100644 --- a/internal/worker/linux.go +++ b/internal/worker/linux.go @@ -237,14 +237,7 @@ func LinuxDoPostMigrationConfig(ctx context.Context, instance api.Instance, dist } } - switch distro { - case api.DISTRO_DEBIAN, api.DISTRO_UBUNTU: - err := runScriptInChroot("debian-purge-open-vm-tools.sh") - if err != nil { - return err - } - - case api.DISTRO_CENTOS, api.DISTRO_ORACLE, api.DISTRO_RHEL: + if distro.IsRHELDerivative() { err := runScriptInChroot("redhat-purge-open-vm-tools.sh") if err != nil { return err @@ -264,6 +257,14 @@ func LinuxDoPostMigrationConfig(ctx context.Context, instance api.Instance, dist } } } + } + + switch distro { + case api.DISTRO_DEBIAN, api.DISTRO_UBUNTU: + err := runScriptInChroot("debian-purge-open-vm-tools.sh") + if err != nil { + return err + } case api.DISTRO_SUSE: err := runScriptInChroot("suse-purge-open-vm-tools.sh") From f2d194f9c31d5ba06c2c27983f0734f76c26d3f8 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Wed, 18 Mar 2026 17:07:23 -0700 Subject: [PATCH 13/13] internal/migration: Ensure artifacts dont collide Signed-off-by: Max Asnaashari --- internal/migration/artifact_model.go | 41 ++++++ internal/migration/artifact_service.go | 25 +++- internal/migration/artifact_service_test.go | 151 ++++++++++++++++++++ 3 files changed, 215 insertions(+), 2 deletions(-) diff --git a/internal/migration/artifact_model.go b/internal/migration/artifact_model.go index d33f7207..070d604e 100644 --- a/internal/migration/artifact_model.go +++ b/internal/migration/artifact_model.go @@ -121,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{ diff --git a/internal/migration/artifact_service.go b/internal/migration/artifact_service.go index 328d7822..6336a693 100644 --- a/internal/migration/artifact_service.go +++ b/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. diff --git a/internal/migration/artifact_service_test.go b/internal/migration/artifact_service_test.go index 28d4e7d8..a022b714 100644 --- a/internal/migration/artifact_service_test.go +++ b/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)) + }) + } +}