Instance: Only allow initial.zfs.promote during refresh Update action (#18071)

Only allow `initial.zfs.promote` to be specified during `lxc copy`
operations.
Don't allow `lxc config device set c1 root initial.zfs.promote=true` as
this does not make sense for the purpose of `zfs.promote`.

Author: tomponline

lxd/api_cluster_member.go

@@ -1970,7 +1970,7 @@ func restoreClusterMember(d *Daemon, r *http.Request, mode string) response.Resp
 					ExpiryDate:   inst.ExpiryDate(),
 				}
 
-				err = inst.Update(args, false)
+				err = inst.Update(args, instance.UpdateActionInternal)
 				if err != nil {
 					return fmt.Errorf("Failed updating instance %q: %w", inst.Name(), err)
 				}

lxd/instance.go

@@ -271,7 +271,7 @@ func instanceCreateAsCopy(s *state.State, opts instanceCreateAsCopyOpts, op *ope
 			opts.refresh = false // Instance doesn't exist, so switch to copy mode.
 		} else {
 			// Validate and apply refresh target config before the storage refresh.
-			err = inst.Update(opts.targetInstance, true)
+			err = inst.Update(opts.targetInstance, instance.UpdateActionUserRefresh)
 			if err != nil {
 				return nil, fmt.Errorf("Failed applying refresh target instance config: %w", err)
 			}

lxd/instance/drivers/driver_common.go

@@ -590,15 +590,15 @@ func (d *common) restartCommon(inst instance.Instance, timeout time.Duration) er
 			Snapshot:     inst.IsSnapshot(),
 		}
 
-		err := inst.Update(args, false)
+		err := inst.Update(args, instance.UpdateActionInternal)
 		if err != nil {
 			return err
 		}
 
 		// On function return, set the flag back on
 		defer func() {
 			args.Ephemeral = ephemeral
-			_ = inst.Update(args, false)
+			_ = inst.Update(args, instance.UpdateActionInternal)
 		}()
 	}
 
@@ -1188,7 +1188,7 @@ func (d *common) restoreCommon(inst instance.Instance, source instance.Instance,
 				Snapshot:     d.IsSnapshot(),
 			}
 
-			err := inst.Update(args, false)
+			err := inst.Update(args, instance.UpdateActionInternal)
 			if err != nil {
 				op.Done(err)
 				return false, nil, err
@@ -1197,7 +1197,7 @@ func (d *common) restoreCommon(inst instance.Instance, source instance.Instance,
 			// On function return, set the flag back on.
 			defer func() {
 				args.Ephemeral = ephemeral
-				err = inst.Update(args, false)
+				err = inst.Update(args, instance.UpdateActionInternal)
 				if err != nil {
 					d.logger.Error("Failed restoring ephemeral flag after restore", logger.Ctx{"err": err})
 				}
@@ -1236,7 +1236,7 @@ func (d *common) restoreCommon(inst instance.Instance, source instance.Instance,
 
 	// Don't pass as user-requested as there's no way to fix a bad config.
 	// This will call d.UpdateBackupFile() to ensure snapshot list is up to date.
-	err = inst.Update(args, false)
+	err = inst.Update(args, instance.UpdateActionInternal)
 	if err != nil {
 		op.Done(err)
 		return false, nil, err
@@ -1400,6 +1400,11 @@ func (d *common) isStartableStatusCode(statusCode api.StatusCode) error {
 	return nil
 }
 
+// isUserRequested returns whether the update action is user-requested.
+func (d *common) isUserRequested(actionType instance.UpdateAction) bool {
+	return actionType == instance.UpdateActionUser || actionType == instance.UpdateActionUserRefresh
+}
+
 // getStartupSnapNameAndExpiry returns the name and expiry for a snapshot to be taken at startup.
 func (d *common) getStartupSnapNameAndExpiry(inst instance.Instance) (string, *time.Time, error) {
 	schedule := strings.ToLower(d.expandedConfig["snapshots.schedule"])
@@ -2235,7 +2240,7 @@ func (d *common) removeDiskDevices() error {
 }
 
 // validateConfig validates the configuration.
-func (d *common) validateConfig(allUpdatedDeviceKeys []string, addDevices deviceConfig.Devices, removeDevices deviceConfig.Devices, oldExpandedDevices deviceConfig.Devices, changedConfigKeys []string, oldExpandedConfig map[string]string, userRequested bool) error {
+func (d *common) validateConfig(allUpdatedDeviceKeys []string, addDevices deviceConfig.Devices, removeDevices deviceConfig.Devices, oldExpandedDevices deviceConfig.Devices, changedConfigKeys []string, oldExpandedConfig map[string]string, actionType instance.UpdateAction) error {
 	if shared.StringPrefixInSlice(deviceConfig.ConfigInitialPrefix, allUpdatedDeviceKeys) {
 		for devName, newDev := range addDevices {
 			for k, newVal := range newDev {
@@ -2248,8 +2253,8 @@ func (d *common) validateConfig(allUpdatedDeviceKeys []string, addDevices device
 					return fmt.Errorf("New device %q with initial configuration %q cannot be added once the instance is created", devName, k)
 				}
 
-				if k == deviceConfig.ConfigInitialPrefix+"zfs.promote" {
-					// Allow zfs.promote to be added as this is needed for volume promotion.
+				if actionType == instance.UpdateActionUserRefresh && k == deviceConfig.ConfigInitialPrefix+"zfs.promote" {
+					// Allow zfs.promote to be added only during refresh as this is needed for volume promotion.
 					continue
 				}
 
@@ -2266,6 +2271,8 @@ func (d *common) validateConfig(allUpdatedDeviceKeys []string, addDevices device
 		}
 	}
 
+	userRequested := d.isUserRequested(actionType)
+
 	if userRequested {
 		// Look for deleted protected keys.
 		protectedKeys := map[string]struct{}{

lxd/instance/drivers/driver_lxc.go

@@ -3742,10 +3742,12 @@ func (d *lxc) CGroupSet(key string, value string) error {
 }
 
 // Update applies updated config.
-func (d *lxc) Update(args db.InstanceArgs, userRequested bool) error {
+func (d *lxc) Update(args db.InstanceArgs, actionType instance.UpdateAction) error {
 	reverter := revert.New()
 	defer reverter.Fail()
 
+	userRequested := d.isUserRequested(actionType)
+
 	unlock, err := d.updateBackupFileLock(context.Background())
 	if err != nil {
 		return err
@@ -3949,7 +3951,7 @@ func (d *lxc) Update(args db.InstanceArgs, userRequested bool) error {
 		return newDevType.UpdatableFields(oldDevType)
 	})
 
-	err = d.validateConfig(allUpdatedDeviceKeys, addDevices, removeDevices, oldExpandedDevices, changedConfig, oldExpandedConfig, userRequested)
+	err = d.validateConfig(allUpdatedDeviceKeys, addDevices, removeDevices, oldExpandedDevices, changedConfig, oldExpandedConfig, actionType)
 	if err != nil {
 		return err
 	}

lxd/instance/drivers/driver_qemu.go

@@ -5759,7 +5759,9 @@ func allowRemoveSecurityProtectionStart(state *state.State, poolName string, vol
 }
 
 // Update the instance config.
-func (d *qemu) Update(args db.InstanceArgs, userRequested bool) error {
+func (d *qemu) Update(args db.InstanceArgs, actionType instance.UpdateAction) error {
+	userRequested := d.isUserRequested(actionType)
+
 	unlock, err := d.updateBackupFileLock(context.Background())
 	if err != nil {
 		return err
@@ -5958,7 +5960,7 @@ func (d *qemu) Update(args db.InstanceArgs, userRequested bool) error {
 		return newDevType.UpdatableFields(oldDevType)
 	})
 
-	err = d.validateConfig(allUpdatedDeviceKeys, addDevices, removeDevices, oldExpandedDevices, changedConfig, oldExpandedConfig, userRequested)
+	err = d.validateConfig(allUpdatedDeviceKeys, addDevices, removeDevices, oldExpandedDevices, changedConfig, oldExpandedConfig, actionType)
 	if err != nil {
 		return err
 	}

lxd/instance/instance_interface.go

@@ -46,6 +46,20 @@ const (
 // TemplateTrigger trigger name.
 type TemplateTrigger string
 
+// UpdateAction defines the trigger source for an instance update.
+type UpdateAction int
+
+const (
+	// UpdateActionUser indicates an update directly requested by users.
+	UpdateActionUser UpdateAction = iota
+
+	// UpdateActionUserRefresh indicates an update requested by user requested refresh workflows.
+	UpdateActionUserRefresh
+
+	// UpdateActionInternal indicates an internal update not directly user requested.
+	UpdateActionInternal
+)
+
 // TemplateTriggerCreate for when an instance is created.
 const TemplateTriggerCreate TemplateTrigger = "create"
 
@@ -100,7 +114,7 @@ type Instance interface {
 
 	// Config handling.
 	Rename(newName string, applyTemplateTrigger bool) error
-	Update(newConfig db.InstanceArgs, userRequested bool) error
+	Update(newConfig db.InstanceArgs, actionType UpdateAction) error
 
 	Delete(force bool, diskVolumesMode string) error
 	Export(w io.Writer, properties map[string]string, expiration time.Time, tracker *ioprogress.ProgressTracker) (api.ImageMetadata, error)

lxd/instance_patch.go

@@ -236,7 +236,7 @@ func instancePatch(d *Daemon, r *http.Request) response.Response {
 		Project:      projectName,
 	}
 
-	err = c.Update(args, true)
+	err = c.Update(args, instance.UpdateActionUser)
 	if err != nil {
 		return response.SmartError(err)
 	}

lxd/instance_put.go

@@ -177,7 +177,7 @@ func instancePut(d *Daemon, r *http.Request) response.Response {
 				Project:      projectName,
 			}
 
-			err = inst.Update(args, true)
+			err = inst.Update(args, instance.UpdateActionUser)
 			if err != nil {
 				return err
 			}

lxd/instance_snapshot.go

@@ -538,7 +538,7 @@ func snapshotPut(s *state.State, r *http.Request, snapInst instance.Instance) re
 				Snapshot:     snapInst.IsSnapshot(),
 			}
 
-			err = snapInst.Update(args, false)
+			err = snapInst.Update(args, instance.UpdateActionInternal)
 			if err != nil {
 				return err
 			}

lxd/instance_test.go

@@ -296,7 +296,7 @@ func (suite *containerTestSuite) TestContainer_AddRoutedNicValidation() {
 			"eth1": eth1,
 		},
 		Name: "testFoo",
-	}, true)
+	}, instance.UpdateActionUser)
 	suite.Req.NoError(err, "Adding multiple routed with gateway mode ['none'] should succeed.")
 
 	eth0["ipv6.gateway"] = "auto"
@@ -310,7 +310,7 @@ func (suite *containerTestSuite) TestContainer_AddRoutedNicValidation() {
 			"eth1": eth1,
 		},
 		Name: "testFoo",
-	}, true)
+	}, instance.UpdateActionUser)
 	suite.Req.Error(err,
 		"Adding multiple routed nic devices with any gateway mode ['auto',''] should throw error.")
 
@@ -323,7 +323,7 @@ func (suite *containerTestSuite) TestContainer_AddRoutedNicValidation() {
 			"eth2": eth2,
 		},
 		Name: "testFoo",
-	}, true)
+	}, instance.UpdateActionUser)
 	suite.Req.NoError(err,
 		"Adding multiple nic devices with unique nictype ['routed'] should throw error.")
 }