From 34ccdcd3aafbc33d21c12420a0af14ecb2f45de1 Mon Sep 17 00:00:00 2001 From: Marques Johansson Date: Mon, 26 Aug 2024 13:57:35 -0400 Subject: [PATCH 1/5] fix: "device update --lock" should also unlock Signed-off-by: Marques Johansson --- docs/metal_device_update.md | 2 +- internal/devices/update.go | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/metal_device_update.md b/docs/metal_device_update.md index abb7ba3d..0d231f78 100644 --- a/docs/metal_device_update.md +++ b/docs/metal_device_update.md @@ -27,7 +27,7 @@ metal device update -i [-H ] [-d ] [--locked -H, --hostname string The new hostname of the device. -i, --id string The UUID of the device. -s, --ipxe-script-url string Add or update the URL of the iPXE script. - -l, --locked Locks or unlocks the device for future changes (). + -l, --locked bools Locks or unlocks the device for future changes (). (default []) -t, --tags strings Adds or updates the tags for the device --tags="tag1,tag2". -u, --userdata string Adds or updates the userdata for the device. --userdata-file string Path to a userdata file for device initialization. Can not be used with --userdata. diff --git a/internal/devices/update.go b/internal/devices/update.go index b53187c0..23331dd1 100644 --- a/internal/devices/update.go +++ b/internal/devices/update.go @@ -34,7 +34,6 @@ import ( func (c *Client) Update() *cobra.Command { var ( description string - locked bool userdata string userdataFile string hostname string @@ -80,8 +79,15 @@ func (c *Client) Update() *cobra.Command { deviceUpdate.Userdata = &userdata } - if locked { - deviceUpdate.Locked = &locked + if cmd.Flag("locked").Changed { + locked, err := cmd.Flags().GetBoolSlice("locked") + if err != nil { + return fmt.Errorf("could not parse locked value: %w", err) + } + if len(locked) > 1 { + return fmt.Errorf("parameter locked may only be set once") + } + deviceUpdate.Locked = &locked[0] } if len(tags) > 0 { @@ -123,12 +129,11 @@ func (c *Client) Update() *cobra.Command { updateDeviceCmd.Flags().StringVarP(&description, "description", "d", "", "Adds or updates the description for the device.") updateDeviceCmd.Flags().StringVarP(&userdata, "userdata", "u", "", "Adds or updates the userdata for the device.") updateDeviceCmd.Flags().StringVarP(&userdataFile, "userdata-file", "", "", "Path to a userdata file for device initialization. Can not be used with --userdata.") - updateDeviceCmd.Flags().BoolVarP(&locked, "locked", "l", false, "Locks or unlocks the device for future changes ().") + updateDeviceCmd.Flags().BoolSliceP("locked", "l", []bool{}, "Locks or unlocks the device for future changes ().") updateDeviceCmd.Flags().StringSliceVarP(&tags, "tags", "t", []string{}, `Adds or updates the tags for the device --tags="tag1,tag2".`) updateDeviceCmd.Flags().BoolVarP(&alwaysPXE, "always-pxe", "a", false, "Updates the always_pxe toggle for the device ().") updateDeviceCmd.Flags().StringVarP(&ipxescripturl, "ipxe-script-url", "s", "", "Add or update the URL of the iPXE script.") updateDeviceCmd.Flags().StringVarP(&customdata, "customdata", "c", "", "Adds or updates custom data to be included with your device's metadata.") _ = updateDeviceCmd.MarkFlagRequired("id") - return updateDeviceCmd } From bef38730286cb1a51cd5176863c11f0dc97074cd Mon Sep 17 00:00:00 2001 From: Marques Johansson Date: Mon, 26 Aug 2024 14:09:40 -0400 Subject: [PATCH 2/5] chore(device update): use Cobra to enforce exclusive fields and no addition args Signed-off-by: Marques Johansson --- internal/devices/update.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/devices/update.go b/internal/devices/update.go index 23331dd1..a99bf83f 100644 --- a/internal/devices/update.go +++ b/internal/devices/update.go @@ -63,10 +63,6 @@ func (c *Client) Update() *cobra.Command { deviceUpdate.Description = &description } - if userdata != "" && userdataFile != "" { - return fmt.Errorf("either userdata or userdata-file should be set") - } - if userdataFile != "" { userdataRaw, readErr := os.ReadFile(userdataFile) if readErr != nil { @@ -135,5 +131,7 @@ func (c *Client) Update() *cobra.Command { updateDeviceCmd.Flags().StringVarP(&ipxescripturl, "ipxe-script-url", "s", "", "Add or update the URL of the iPXE script.") updateDeviceCmd.Flags().StringVarP(&customdata, "customdata", "c", "", "Adds or updates custom data to be included with your device's metadata.") _ = updateDeviceCmd.MarkFlagRequired("id") + updateDeviceCmd.MarkFlagsMutuallyExclusive("userdata", "userdata-file") + updateDeviceCmd.Args = cobra.NoArgs return updateDeviceCmd } From 67dbcd03437dd16721863556ac7fdc419b0a8a42 Mon Sep 17 00:00:00 2001 From: Marques Johansson Date: Mon, 26 Aug 2024 17:03:49 -0400 Subject: [PATCH 3/5] docs: revise device update --locked arg Signed-off-by: Marques Johansson --- docs/metal_device_update.md | 2 +- internal/devices/update.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/metal_device_update.md b/docs/metal_device_update.md index 0d231f78..2a289799 100644 --- a/docs/metal_device_update.md +++ b/docs/metal_device_update.md @@ -7,7 +7,7 @@ Updates a device. Updates the hostname of a device. Updates or adds a description, tags, userdata, custom data, and iPXE settings for an already provisioned device. Can also lock or unlock future changes to the device. ``` -metal device update -i [-H ] [-d ] [--locked ] [-t ] [-u | --userdata-file ] [-c ] [-s ] [--always-pxe=] [flags] +metal device update -i [-H ] [-d ] [--locked=] [-t ] [-u | --userdata-file ] [-c ] [-s ] [--always-pxe=] [flags] ``` ### Examples diff --git a/internal/devices/update.go b/internal/devices/update.go index a99bf83f..47be9555 100644 --- a/internal/devices/update.go +++ b/internal/devices/update.go @@ -45,7 +45,7 @@ func (c *Client) Update() *cobra.Command { ) // updateDeviceCmd represents the updateDevice command updateDeviceCmd := &cobra.Command{ - Use: `update -i [-H ] [-d ] [--locked ] [-t ] [-u | --userdata-file ] [-c ] [-s ] [--always-pxe=]`, + Use: `update -i [-H ] [-d ] [--locked=] [-t ] [-u | --userdata-file ] [-c ] [-s ] [--always-pxe=]`, Short: "Updates a device.", Long: "Updates the hostname of a device. Updates or adds a description, tags, userdata, custom data, and iPXE settings for an already provisioned device. Can also lock or unlock future changes to the device.", Example: ` # Updates the hostname of a device: From f304a91aae31ebfdfa72a6255179e5cb7d8783c7 Mon Sep 17 00:00:00 2001 From: Marques Johansson Date: Mon, 26 Aug 2024 17:09:18 -0400 Subject: [PATCH 4/5] chore: refactor device update locked to simple bool Signed-off-by: Marques Johansson --- docs/metal_device_update.md | 2 +- internal/devices/update.go | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/metal_device_update.md b/docs/metal_device_update.md index 2a289799..58e034d4 100644 --- a/docs/metal_device_update.md +++ b/docs/metal_device_update.md @@ -27,7 +27,7 @@ metal device update -i [-H ] [-d ] [--locked= -H, --hostname string The new hostname of the device. -i, --id string The UUID of the device. -s, --ipxe-script-url string Add or update the URL of the iPXE script. - -l, --locked bools Locks or unlocks the device for future changes (). (default []) + -l, --locked Locks or unlocks the device for future changes (). -t, --tags strings Adds or updates the tags for the device --tags="tag1,tag2". -u, --userdata string Adds or updates the userdata for the device. --userdata-file string Path to a userdata file for device initialization. Can not be used with --userdata. diff --git a/internal/devices/update.go b/internal/devices/update.go index 47be9555..9125d832 100644 --- a/internal/devices/update.go +++ b/internal/devices/update.go @@ -76,14 +76,11 @@ func (c *Client) Update() *cobra.Command { } if cmd.Flag("locked").Changed { - locked, err := cmd.Flags().GetBoolSlice("locked") + locked, err := cmd.Flags().GetBool("locked") if err != nil { return fmt.Errorf("could not parse locked value: %w", err) } - if len(locked) > 1 { - return fmt.Errorf("parameter locked may only be set once") - } - deviceUpdate.Locked = &locked[0] + deviceUpdate.Locked = &locked } if len(tags) > 0 { @@ -125,7 +122,7 @@ func (c *Client) Update() *cobra.Command { updateDeviceCmd.Flags().StringVarP(&description, "description", "d", "", "Adds or updates the description for the device.") updateDeviceCmd.Flags().StringVarP(&userdata, "userdata", "u", "", "Adds or updates the userdata for the device.") updateDeviceCmd.Flags().StringVarP(&userdataFile, "userdata-file", "", "", "Path to a userdata file for device initialization. Can not be used with --userdata.") - updateDeviceCmd.Flags().BoolSliceP("locked", "l", []bool{}, "Locks or unlocks the device for future changes ().") + updateDeviceCmd.Flags().BoolP("locked", "l", false, "Locks or unlocks the device for future changes ().") updateDeviceCmd.Flags().StringSliceVarP(&tags, "tags", "t", []string{}, `Adds or updates the tags for the device --tags="tag1,tag2".`) updateDeviceCmd.Flags().BoolVarP(&alwaysPXE, "always-pxe", "a", false, "Updates the always_pxe toggle for the device ().") updateDeviceCmd.Flags().StringVarP(&ipxescripturl, "ipxe-script-url", "s", "", "Add or update the URL of the iPXE script.") From c6562a20d37b122be18c4c5c7f4a10ff1d9310e8 Mon Sep 17 00:00:00 2001 From: Marques Johansson Date: Fri, 6 Sep 2024 12:02:07 -0400 Subject: [PATCH 5/5] test: add e2e test for device update --locked=true|false Signed-off-by: Marques Johansson --- .../deviceupdatetest/device_update_test.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/e2e/devices/deviceupdatetest/device_update_test.go b/test/e2e/devices/deviceupdatetest/device_update_test.go index 12d2b5d2..f6172d17 100644 --- a/test/e2e/devices/deviceupdatetest/device_update_test.go +++ b/test/e2e/devices/deviceupdatetest/device_update_test.go @@ -42,7 +42,7 @@ func TestCli_Devices_Update(t *testing.T) { t.Fatal(err) } if status == true { - root.SetArgs([]string{subCommand, "update", "-i", device.GetId(), "-H", "metal-cli-update-dev-test", "-d", "This device used for testing"}) + root.SetArgs([]string{subCommand, "update", "-i", device.GetId(), "-H", "metal-cli-update-dev-test", "-d", "This device used for testing", "--locked=true"}) out := helper.ExecuteAndCaptureOutput(t, root) @@ -50,6 +50,23 @@ func TestCli_Devices_Update(t *testing.T) { t.Error("expected output should include metal-cli-update-dev-test in the out string ") } } + + root.SetArgs([]string{subCommand, "delete", "-i", device.GetId()}) + + out := helper.ExecuteAndCaptureOutput(t, root) + + if !strings.Contains(string(out[:]), "not delete") { + t.Error("expected output should include 'not delete' in the out string due to device locking") + } + + root.SetArgs([]string{subCommand, "update", "-i", device.GetId(), "--locked=false"}) + + out = helper.ExecuteAndCaptureOutput(t, root) + + if !strings.Contains(string(out[:]), "metal-cli-update-dev-test") { + t.Error("expected output should include metal-cli-update-dev-test in the out string ") + } + }, }, }