diff --git a/.changes/next-release/api-change-vks-e1jsnbim.json b/.changes/next-release/api-change-vks-e1jsnbim.json new file mode 100644 index 0000000..b62d25a --- /dev/null +++ b/.changes/next-release/api-change-vks-e1jsnbim.json @@ -0,0 +1,5 @@ +{ + "type": "api-change", + "category": "vks", + "description": "Rename 'set-auto-upgrade-config' to 'config-auto-upgrade'; the old name still works as a deprecated alias" +} diff --git a/.changes/next-release/bugfix-configure-guu8cc3u.json b/.changes/next-release/bugfix-configure-guu8cc3u.json new file mode 100644 index 0000000..dce052b --- /dev/null +++ b/.changes/next-release/bugfix-configure-guu8cc3u.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "configure", + "description": "Mask credential values in 'configure set' output so client_id/client_secret are no longer echoed in plaintext to stdout (consistent with 'configure list'); non-sensitive values are still shown" +} diff --git a/.changes/next-release/bugfix-vks-tlhtc992.json b/.changes/next-release/bugfix-vks-tlhtc992.json new file mode 100644 index 0000000..096c3cd --- /dev/null +++ b/.changes/next-release/bugfix-vks-tlhtc992.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "vks", + "description": "grn vks wait now aborts immediately on a permanent error (HTTP 403/401/400, or 404 for an active waiter) instead of polling until timeout; transient errors (5xx, network) still retry" +} diff --git a/.changes/next-release/enhancement-core-mk5mzv01.json b/.changes/next-release/enhancement-core-mk5mzv01.json new file mode 100644 index 0000000..fec16d7 --- /dev/null +++ b/.changes/next-release/enhancement-core-mk5mzv01.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "core", + "description": "Add --dry-run to destructive commands that lacked it (vServer server stop/reboot/delete, volume/vpc/subnet/secgroup/secgroup-rule delete, and vks delete-auto-upgrade-config) and unify the preview + confirmation flow across delete commands via shared helpers" +} diff --git a/docs/commands/vks/set-auto-upgrade-config.md b/docs/commands/vks/config-auto-upgrade.md similarity index 81% rename from docs/commands/vks/set-auto-upgrade-config.md rename to docs/commands/vks/config-auto-upgrade.md index 8ea6a2e..70a5155 100644 --- a/docs/commands/vks/set-auto-upgrade-config.md +++ b/docs/commands/vks/config-auto-upgrade.md @@ -1,13 +1,15 @@ -# set-auto-upgrade-config +# config-auto-upgrade ## Description Configure auto-upgrade schedule for a cluster. Sets the days and time when automatic Kubernetes version upgrades will be performed. +> Formerly `set-auto-upgrade-config`. That name still works as a deprecated alias. + ## Synopsis ``` -grn vks set-auto-upgrade-config +grn vks config-auto-upgrade --cluster-id --weekdays --time @@ -29,7 +31,7 @@ grn vks set-auto-upgrade-config Set auto-upgrade to run on weekdays at 3 AM: ```bash -grn vks set-auto-upgrade-config \ +grn vks config-auto-upgrade \ --cluster-id k8s-xxxxx \ --weekdays Mon,Tue,Wed,Thu,Fri \ --time 03:00 @@ -38,7 +40,7 @@ grn vks set-auto-upgrade-config \ Set auto-upgrade to run on weekends at midnight: ```bash -grn vks set-auto-upgrade-config \ +grn vks config-auto-upgrade \ --cluster-id k8s-xxxxx \ --weekdays Sat,Sun \ --time 00:00 diff --git a/docs/commands/vks/index.md b/docs/commands/vks/index.md index a160522..599ad6d 100644 --- a/docs/commands/vks/index.md +++ b/docs/commands/vks/index.md @@ -41,7 +41,7 @@ grn vks [options] | Command | Description | |---------|-------------| -| [set-auto-upgrade-config](set-auto-upgrade-config.md) | Configure auto-upgrade schedule for a cluster | +| [config-auto-upgrade](config-auto-upgrade.md) | Configure auto-upgrade schedule for a cluster (alias: set-auto-upgrade-config) | | [delete-auto-upgrade-config](delete-auto-upgrade-config.md) | Delete auto-upgrade config for a cluster | ### Auto-Healing diff --git a/go/cmd/configure/set.go b/go/cmd/configure/set.go index 2e06f40..f66d9e9 100644 --- a/go/cmd/configure/set.go +++ b/go/cmd/configure/set.go @@ -67,5 +67,17 @@ func runSet(cmd *cobra.Command, args []string) { os.Exit(1) } - fmt.Printf("Set '%s' to '%s' for profile '%s'.\n", key, value, profile) + fmt.Printf("Set '%s' to '%s' for profile '%s'.\n", key, displaySetValue(key, value), profile) +} + +// displaySetValue masks credential values so `configure set` never echoes a +// secret in plaintext (matching how `configure list` masks them). Non-sensitive +// values are shown as-is. +func displaySetValue(key, value string) string { + switch key { + case "client_id", "client_secret": + return config.MaskCredential(value) + default: + return value + } } diff --git a/go/cmd/configure/set_test.go b/go/cmd/configure/set_test.go index b522ee4..3332293 100644 --- a/go/cmd/configure/set_test.go +++ b/go/cmd/configure/set_test.go @@ -9,6 +9,24 @@ import ( "github.com/spf13/cobra" ) +// displaySetValue must mask credentials so `set` never echoes a secret, while +// leaving non-sensitive values readable. +func TestDisplaySetValueMasksSecrets(t *testing.T) { + secret := "super-secret-value-z789" + for _, key := range []string{"client_id", "client_secret"} { + got := displaySetValue(key, secret) + if strings.Contains(got, "super-secret") { + t.Errorf("%s: value not masked, got %q", key, got) + } + if !strings.HasSuffix(got, "z789") { + t.Errorf("%s: expected masked value keeping last 4 chars, got %q", key, got) + } + } + if got := displaySetValue("region", "HCM-3"); got != "HCM-3" { + t.Errorf("region should not be masked, got %q", got) + } +} + // newConfigureTestCmd wires a root command with the persistent `profile` flag // (registered on rootCmd in production) so the configure subcommands resolve it // exactly as they do at runtime. diff --git a/go/cmd/vks/auto_upgrade.go b/go/cmd/vks/auto_upgrade.go index e5597ed..3fca285 100644 --- a/go/cmd/vks/auto_upgrade.go +++ b/go/cmd/vks/auto_upgrade.go @@ -1,19 +1,20 @@ package vks import ( - "bufio" "fmt" "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) var setAutoUpgradeConfigCmd = &cobra.Command{ - Use: "set-auto-upgrade-config", + Use: "config-auto-upgrade", Short: "Configure auto-upgrade schedule for a cluster", - RunE: runSetAutoUpgradeConfig, + // set-auto-upgrade-config is the former name, kept for backward compatibility. + Aliases: []string{"set-auto-upgrade-config"}, + RunE: runSetAutoUpgradeConfig, } var deleteAutoUpgradeConfigCmd = &cobra.Command{ @@ -23,7 +24,7 @@ var deleteAutoUpgradeConfigCmd = &cobra.Command{ } func init() { - // set-auto-upgrade-config flags + // config-auto-upgrade flags f := setAutoUpgradeConfigCmd.Flags() f.String("cluster-id", "", "Cluster ID (required)") f.String("weekdays", "", "Days of the week, e.g. Mon,Wed,Fri (required)") @@ -35,6 +36,7 @@ func init() { // delete-auto-upgrade-config flags g := deleteAutoUpgradeConfigCmd.Flags() g.String("cluster-id", "", "Cluster ID (required)") + g.Bool("dry-run", false, "Preview what will be deleted without executing") g.Bool("force", false, "Skip confirmation prompt") deleteAutoUpgradeConfigCmd.MarkFlagRequired("cluster-id") } @@ -71,20 +73,25 @@ func runSetAutoUpgradeConfig(cmd *cobra.Command, args []string) error { func runDeleteAutoUpgradeConfig(cmd *cobra.Command, args []string) error { clusterID, _ := cmd.Flags().GetString("cluster-id") + dryRun, _ := cmd.Flags().GetBool("dry-run") force, _ := cmd.Flags().GetBool("force") if err := validator.ValidateID(clusterID, "cluster-id"); err != nil { return err } - if !force { - fmt.Print("Are you sure you want to delete the auto-upgrade config? (yes/no): ") - reader := bufio.NewReader(os.Stdin) - response, _ := reader.ReadString('\n') - if strings.TrimSpace(strings.ToLower(response)) != "yes" { - fmt.Println("Delete cancelled.") - return nil - } + fmt.Println("The following will be deleted:") + fmt.Printf(" Auto-upgrade config for cluster: %s\n", clusterID) + fmt.Println("\nThis action is irreversible.") + + if dryRun { + cli.DryRunNotice("delete") + return nil + } + + if !cli.Confirm(force, "Are you sure you want to delete the auto-upgrade config?") { + fmt.Println("Aborted.") + return nil } apiClient, err := createClient(cmd) diff --git a/go/cmd/vks/delete_cluster.go b/go/cmd/vks/delete_cluster.go index d0d36e5..43c9a87 100644 --- a/go/cmd/vks/delete_cluster.go +++ b/go/cmd/vks/delete_cluster.go @@ -1,12 +1,11 @@ package vks import ( - "bufio" "fmt" "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -57,19 +56,13 @@ func runDeleteCluster(cmd *cobra.Command, args []string) error { printClusterPreview(cluster, nodegroups) if dryRun { - fmt.Println("Run without --dry-run to delete.") + cli.DryRunNotice("delete") return nil } - // Confirm unless --force - if !force { - fmt.Print("\nAre you sure you want to delete this cluster? (yes/no): ") - reader := bufio.NewReader(os.Stdin) - response, _ := reader.ReadString('\n') - if strings.TrimSpace(strings.ToLower(response)) != "yes" { - fmt.Println("Delete cancelled.") - return nil - } + if !cli.Confirm(force, "Are you sure you want to delete this cluster?") { + fmt.Println("Aborted.") + return nil } result, err := apiClient.Delete(fmt.Sprintf("/v1/clusters/%s", clusterID), nil) diff --git a/go/cmd/vks/delete_nodegroup.go b/go/cmd/vks/delete_nodegroup.go index 0f49c7d..9b9fe5b 100644 --- a/go/cmd/vks/delete_nodegroup.go +++ b/go/cmd/vks/delete_nodegroup.go @@ -1,12 +1,11 @@ package vks import ( - "bufio" "fmt" "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -66,18 +65,13 @@ func runDeleteNodegroup(cmd *cobra.Command, args []string) error { fmt.Println("This action is irreversible.") if dryRun { - fmt.Println("Run without --dry-run to delete.") + cli.DryRunNotice("delete") return nil } - if !force { - fmt.Print("\nAre you sure you want to delete this node group? (yes/no): ") - reader := bufio.NewReader(os.Stdin) - response, _ := reader.ReadString('\n') - if strings.TrimSpace(strings.ToLower(response)) != "yes" { - fmt.Println("Delete cancelled.") - return nil - } + if !cli.Confirm(force, "Are you sure you want to delete this node group?") { + fmt.Println("Aborted.") + return nil } params := map[string]string{} diff --git a/go/cmd/vks/wait.go b/go/cmd/vks/wait.go index 387f241..af988e4 100644 --- a/go/cmd/vks/wait.go +++ b/go/cmd/vks/wait.go @@ -21,36 +21,64 @@ func statusOf(result interface{}) string { return s } -// evaluateActive decides one poll for an "*-active" waiter. -func evaluateActive(result interface{}, err error) (done bool, failed bool, status string) { +// isPermanentAPIError reports whether err is an HTTP client error that will not +// resolve by polling again (bad request, unauthenticated, forbidden). 404 is +// intentionally excluded here — each waiter interprets it for itself. +func isPermanentAPIError(err error) bool { + var apiErr *client.APIError + if errors.As(err, &apiErr) { + switch apiErr.StatusCode { + case 400, 401, 403: + return true + } + } + return false +} + +// evaluateActive decides one poll for an "*-active" waiter. A fatal error aborts +// the wait immediately (permanent client errors / resource not found); other +// errors are treated as transient and polling continues. +func evaluateActive(result interface{}, err error) (done bool, failed bool, status string, fatal error) { if err != nil { - return false, false, "" + var apiErr *client.APIError + if errors.As(err, &apiErr) && apiErr.StatusCode == 404 { + return false, false, "", err + } + if isPermanentAPIError(err) { + return false, false, "", err + } + return false, false, "", nil } status = statusOf(result) switch status { case "ACTIVE": - return true, false, status + return true, false, status, nil case "ERROR", "FAILED": - return false, true, status + return false, true, status, nil default: - return false, false, status + return false, false, status, nil } } -// evaluateDeleted decides one poll for a "*-deleted" waiter. -func evaluateDeleted(result interface{}, err error) (done bool, failed bool, status string) { +// evaluateDeleted decides one poll for a "*-deleted" waiter. A 404 means the +// resource is gone (success); permanent client errors abort; other errors are +// transient. +func evaluateDeleted(result interface{}, err error) (done bool, failed bool, status string, fatal error) { if err != nil { var apiErr *client.APIError if errors.As(err, &apiErr) && apiErr.StatusCode == 404 { - return true, false, "DELETED" + return true, false, "DELETED", nil + } + if isPermanentAPIError(err) { + return false, false, "", err } - return false, false, "" + return false, false, "", nil } status = statusOf(result) if status == "ACTIVE" { - return false, true, status + return false, true, status, nil } - return false, false, status + return false, false, status, nil } // waitCmd is the parent for all `grn vks wait ` subcommands. @@ -60,8 +88,9 @@ var waitCmd = &cobra.Command{ Long: "Poll until a cluster or node group reaches the requested state (active or deleted).", } -// evaluator decides, for one poll, whether the waiter is done or has failed. -type evaluator func(result interface{}, err error) (done bool, failed bool, status string) +// evaluator decides, for one poll, whether the waiter is done, has failed, or +// hit a fatal error that should abort polling immediately. +type evaluator func(result interface{}, err error) (done bool, failed bool, status string, fatal error) // runWaiter polls describe() every delay seconds up to maxAttempts times, // driving the waiter via eval. Progress goes to stderr; on success it prints @@ -70,7 +99,15 @@ type evaluator func(result interface{}, err error) (done bool, failed bool, stat func runWaiter(label, successMsg string, describe func() (interface{}, error), eval evaluator, delay, maxAttempts int) error { for attempt := 1; attempt <= maxAttempts; attempt++ { result, err := describe() - done, failed, status := eval(result, err) + done, failed, status, fatal := eval(result, err) + + // A fatal error (e.g. 403 Forbidden) will never resolve by polling — + // abort immediately instead of retrying until timeout. + if fatal != nil { + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, "Error waiting for %s: %v\n", label, fatal) + os.Exit(255) + } shown := status if shown == "" { diff --git a/go/cmd/vks/wait_test.go b/go/cmd/vks/wait_test.go index 8a146e4..2429cf5 100644 --- a/go/cmd/vks/wait_test.go +++ b/go/cmd/vks/wait_test.go @@ -8,46 +8,59 @@ import ( ) func TestEvaluateActive(t *testing.T) { + forbidden := &client.APIError{StatusCode: 403, Body: ""} + notFound := &client.APIError{StatusCode: 404, Body: ""} + serverErr := &client.APIError{StatusCode: 500, Body: ""} + cases := []struct { - name string - result interface{} - err error + name string + result interface{} + err error wantDone, wantFail bool + wantFatal bool }{ - {"active", map[string]interface{}{"status": "ACTIVE"}, nil, true, false}, - {"creating", map[string]interface{}{"status": "CREATING"}, nil, false, false}, - {"error", map[string]interface{}{"status": "ERROR"}, nil, false, true}, - {"failed", map[string]interface{}{"status": "FAILED"}, nil, false, true}, - {"transient err", nil, errors.New("boom"), false, false}, + {"active", map[string]interface{}{"status": "ACTIVE"}, nil, true, false, false}, + {"creating", map[string]interface{}{"status": "CREATING"}, nil, false, false, false}, + {"error", map[string]interface{}{"status": "ERROR"}, nil, false, true, false}, + {"failed", map[string]interface{}{"status": "FAILED"}, nil, false, true, false}, + {"transient err", nil, errors.New("boom"), false, false, false}, + {"transient 500", nil, serverErr, false, false, false}, + {"forbidden 403 fatal", nil, forbidden, false, false, true}, + {"not found 404 fatal", nil, notFound, false, false, true}, } for _, tc := range cases { - done, failed, _ := evaluateActive(tc.result, tc.err) - if done != tc.wantDone || failed != tc.wantFail { - t.Errorf("%s: done=%v failed=%v, want done=%v failed=%v", tc.name, done, failed, tc.wantDone, tc.wantFail) + done, failed, _, fatal := evaluateActive(tc.result, tc.err) + if done != tc.wantDone || failed != tc.wantFail || (fatal != nil) != tc.wantFatal { + t.Errorf("%s: done=%v failed=%v fatal=%v, want done=%v failed=%v fatal=%v", + tc.name, done, failed, fatal != nil, tc.wantDone, tc.wantFail, tc.wantFatal) } } } func TestEvaluateDeleted(t *testing.T) { notFound := &client.APIError{StatusCode: 404, Body: ""} - otherAPIErr := &client.APIError{StatusCode: 500, Body: ""} + forbidden := &client.APIError{StatusCode: 403, Body: ""} + serverErr := &client.APIError{StatusCode: 500, Body: ""} cases := []struct { - name string - result interface{} - err error + name string + result interface{} + err error wantDone, wantFail bool + wantFatal bool }{ - {"gone 404", nil, notFound, true, false}, - {"still deleting", map[string]interface{}{"status": "DELETING"}, nil, false, false}, - {"came back active", map[string]interface{}{"status": "ACTIVE"}, nil, false, true}, - {"non-404 err transient", nil, otherAPIErr, false, false}, - {"plain transient err", nil, errors.New("boom"), false, false}, + {"gone 404", nil, notFound, true, false, false}, + {"still deleting", map[string]interface{}{"status": "DELETING"}, nil, false, false, false}, + {"came back active", map[string]interface{}{"status": "ACTIVE"}, nil, false, true, false}, + {"non-404 err transient", nil, serverErr, false, false, false}, + {"plain transient err", nil, errors.New("boom"), false, false, false}, + {"forbidden 403 fatal", nil, forbidden, false, false, true}, } for _, tc := range cases { - done, failed, _ := evaluateDeleted(tc.result, tc.err) - if done != tc.wantDone || failed != tc.wantFail { - t.Errorf("%s: done=%v failed=%v, want done=%v failed=%v", tc.name, done, failed, tc.wantDone, tc.wantFail) + done, failed, _, fatal := evaluateDeleted(tc.result, tc.err) + if done != tc.wantDone || failed != tc.wantFail || (fatal != nil) != tc.wantFatal { + t.Errorf("%s: done=%v failed=%v fatal=%v, want done=%v failed=%v fatal=%v", + tc.name, done, failed, fatal != nil, tc.wantDone, tc.wantFail, tc.wantFatal) } } } diff --git a/go/cmd/vserver/secgroup/delete.go b/go/cmd/vserver/secgroup/delete.go index 2362de2..049208a 100644 --- a/go/cmd/vserver/secgroup/delete.go +++ b/go/cmd/vserver/secgroup/delete.go @@ -1,12 +1,10 @@ package secgroup import ( - "bufio" "fmt" - "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -20,12 +18,14 @@ func init() { f := deleteCmd.Flags() f.String("secgroup-id", "", "Security group ID (required)") f.Bool("force", false, "Skip confirmation prompt") + f.Bool("dry-run", false, "Preview the security group deletion without executing") deleteCmd.MarkFlagRequired("secgroup-id") } func runDelete(cmd *cobra.Command, args []string) error { secgroupID, _ := cmd.Flags().GetString("secgroup-id") force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(secgroupID, "secgroup-id"); err != nil { return err @@ -50,15 +50,13 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } - if !force { - fmt.Print("\nAre you sure you want to delete this security group? [y/N]: ") - reader := bufio.NewReader(os.Stdin) - answer, _ := reader.ReadString('\n') - answer = strings.TrimSpace(strings.ToLower(answer)) - if answer != "y" && answer != "yes" { - fmt.Println("Aborted.") - return nil - } + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this security group?") { + fmt.Println("Aborted.") + return nil } result, err := apiClient.Delete(fmt.Sprintf("/v2/%s/secgroups/%s", projectID, secgroupID), nil) diff --git a/go/cmd/vserver/secgroup/rule/delete.go b/go/cmd/vserver/secgroup/rule/delete.go index d724b0f..6a92a49 100644 --- a/go/cmd/vserver/secgroup/rule/delete.go +++ b/go/cmd/vserver/secgroup/rule/delete.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -14,8 +15,11 @@ var deleteCmd = &cobra.Command{ } func init() { - deleteCmd.Flags().String("secgroup-id", "", "Security group ID (required)") - deleteCmd.Flags().String("rule-id", "", "Security group rule ID (required)") + f := deleteCmd.Flags() + f.String("secgroup-id", "", "Security group ID (required)") + f.String("rule-id", "", "Security group rule ID (required)") + f.Bool("dry-run", false, "Preview the rule deletion without executing") + f.Bool("force", false, "Skip confirmation prompt") deleteCmd.MarkFlagRequired("secgroup-id") deleteCmd.MarkFlagRequired("rule-id") } @@ -23,6 +27,8 @@ func init() { func runDelete(cmd *cobra.Command, args []string) error { secgroupID, _ := cmd.Flags().GetString("secgroup-id") ruleID, _ := cmd.Flags().GetString("rule-id") + force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(secgroupID, "secgroup-id"); err != nil { return err @@ -41,6 +47,22 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } + fmt.Println("The following security group rule will be deleted:") + fmt.Println() + fmt.Printf(" Rule ID: %s\n", ruleID) + fmt.Printf(" Secgroup ID: %s\n", secgroupID) + fmt.Println() + fmt.Println("This action is irreversible.") + + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this rule?") { + fmt.Println("Aborted.") + return nil + } + result, err := apiClient.Delete( fmt.Sprintf("/v2/%s/secgroups/%s/secgroupRules/%s", projectID, secgroupID, ruleID), nil) if err != nil { diff --git a/go/cmd/vserver/server/delete.go b/go/cmd/vserver/server/delete.go index 3f8d611..a0749d0 100644 --- a/go/cmd/vserver/server/delete.go +++ b/go/cmd/vserver/server/delete.go @@ -1,12 +1,10 @@ package server import ( - "bufio" "fmt" - "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -21,6 +19,7 @@ func init() { f.String("server-id", "", "Server ID (required)") f.Bool("delete-all-volumes", false, "Delete all volumes associated with the server") f.Bool("force", false, "Skip confirmation prompt") + f.Bool("dry-run", false, "Preview the server deletion without executing") deleteCmd.MarkFlagRequired("server-id") } @@ -28,6 +27,7 @@ func runDelete(cmd *cobra.Command, args []string) error { serverID, _ := cmd.Flags().GetString("server-id") deleteAllVolumes, _ := cmd.Flags().GetBool("delete-all-volumes") force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(serverID, "server-id"); err != nil { return err @@ -57,15 +57,13 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } - if !force { - fmt.Print("\nAre you sure you want to delete this server? [y/N]: ") - reader := bufio.NewReader(os.Stdin) - answer, _ := reader.ReadString('\n') - answer = strings.TrimSpace(strings.ToLower(answer)) - if answer != "y" && answer != "yes" { - fmt.Println("Aborted.") - return nil - } + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this server?") { + fmt.Println("Aborted.") + return nil } body := map[string]interface{}{ diff --git a/go/cmd/vserver/server/reboot.go b/go/cmd/vserver/server/reboot.go index e10db3e..b7003a5 100644 --- a/go/cmd/vserver/server/reboot.go +++ b/go/cmd/vserver/server/reboot.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -14,7 +15,10 @@ var rebootCmd = &cobra.Command{ } func init() { - rebootCmd.Flags().String("server-id", "", "Server ID (required)") + f := rebootCmd.Flags() + f.String("server-id", "", "Server ID (required)") + f.Bool("dry-run", false, "Preview the reboot action without executing") + f.Bool("force", false, "Skip confirmation prompt") rebootCmd.MarkFlagRequired("server-id") } @@ -34,6 +38,24 @@ func runReboot(cmd *cobra.Command, args []string) error { return err } + force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") + + fmt.Println("The following server will be rebooted:") + fmt.Println() + fmt.Printf(" ID: %s\n", serverID) + fmt.Println() + fmt.Println("This will interrupt the server.") + + if dryRun { + cli.DryRunNotice("reboot") + return nil + } + if !cli.Confirm(force, "Are you sure you want to reboot this server?") { + fmt.Println("Aborted.") + return nil + } + result, err := apiClient.Put(fmt.Sprintf("/v2/%s/servers/%s/reboot", projectID, serverID), nil) if err != nil { return fmt.Errorf("failed to reboot server %s: %w", serverID, err) diff --git a/go/cmd/vserver/server/stop.go b/go/cmd/vserver/server/stop.go index 4c12c76..3fea17f 100644 --- a/go/cmd/vserver/server/stop.go +++ b/go/cmd/vserver/server/stop.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -14,7 +15,10 @@ var stopCmd = &cobra.Command{ } func init() { - stopCmd.Flags().String("server-id", "", "Server ID (required)") + f := stopCmd.Flags() + f.String("server-id", "", "Server ID (required)") + f.Bool("dry-run", false, "Preview the stop action without executing") + f.Bool("force", false, "Skip confirmation prompt") stopCmd.MarkFlagRequired("server-id") } @@ -34,6 +38,24 @@ func runStop(cmd *cobra.Command, args []string) error { return err } + force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") + + fmt.Println("The following server will be stopped:") + fmt.Println() + fmt.Printf(" ID: %s\n", serverID) + fmt.Println() + fmt.Println("This will interrupt the server.") + + if dryRun { + cli.DryRunNotice("stop") + return nil + } + if !cli.Confirm(force, "Are you sure you want to stop this server?") { + fmt.Println("Aborted.") + return nil + } + result, err := apiClient.Put(fmt.Sprintf("/v2/%s/servers/%s/stop", projectID, serverID), nil) if err != nil { return fmt.Errorf("failed to stop server %s: %w", serverID, err) diff --git a/go/cmd/vserver/subnet/delete.go b/go/cmd/vserver/subnet/delete.go index 3307088..b7a7dfc 100644 --- a/go/cmd/vserver/subnet/delete.go +++ b/go/cmd/vserver/subnet/delete.go @@ -1,12 +1,10 @@ package subnet import ( - "bufio" "fmt" - "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -21,6 +19,7 @@ func init() { f.String("subnet-id", "", "Subnet ID (required)") f.String("vpc-id", "", "VPC (network) ID the subnet belongs to (required)") f.Bool("force", false, "Skip confirmation prompt") + f.Bool("dry-run", false, "Preview the subnet deletion without executing") deleteCmd.MarkFlagRequired("subnet-id") deleteCmd.MarkFlagRequired("vpc-id") } @@ -29,6 +28,7 @@ func runDelete(cmd *cobra.Command, args []string) error { subnetID, _ := cmd.Flags().GetString("subnet-id") vpcID, _ := cmd.Flags().GetString("vpc-id") force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(subnetID, "subnet-id"); err != nil { return err @@ -61,15 +61,13 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } - if !force { - fmt.Print("\nAre you sure you want to delete this subnet? [y/N]: ") - reader := bufio.NewReader(os.Stdin) - answer, _ := reader.ReadString('\n') - answer = strings.TrimSpace(strings.ToLower(answer)) - if answer != "y" && answer != "yes" { - fmt.Println("Aborted.") - return nil - } + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this subnet?") { + fmt.Println("Aborted.") + return nil } result, err := apiClient.Delete(fmt.Sprintf("/v2/%s/networks/%s/subnets/%s", projectID, vpcID, subnetID), nil) diff --git a/go/cmd/vserver/volume/delete.go b/go/cmd/vserver/volume/delete.go index 8c2e7aa..5f0a001 100644 --- a/go/cmd/vserver/volume/delete.go +++ b/go/cmd/vserver/volume/delete.go @@ -1,12 +1,10 @@ package volume import ( - "bufio" "fmt" - "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -20,12 +18,14 @@ func init() { f := deleteCmd.Flags() f.String("volume-id", "", "Volume ID (required)") f.Bool("force", false, "Skip confirmation prompt") + f.Bool("dry-run", false, "Preview the volume deletion without executing") deleteCmd.MarkFlagRequired("volume-id") } func runDelete(cmd *cobra.Command, args []string) error { volumeID, _ := cmd.Flags().GetString("volume-id") force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(volumeID, "volume-id"); err != nil { return err @@ -55,15 +55,13 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } - if !force { - fmt.Print("\nAre you sure you want to delete this volume? [y/N]: ") - reader := bufio.NewReader(os.Stdin) - answer, _ := reader.ReadString('\n') - answer = strings.TrimSpace(strings.ToLower(answer)) - if answer != "y" && answer != "yes" { - fmt.Println("Aborted.") - return nil - } + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this volume?") { + fmt.Println("Aborted.") + return nil } result, err := apiClient.Delete(fmt.Sprintf("/v2/%s/volumes/%s", projectID, volumeID), nil) diff --git a/go/cmd/vserver/vpc/delete.go b/go/cmd/vserver/vpc/delete.go index 43f8b31..2d4319a 100644 --- a/go/cmd/vserver/vpc/delete.go +++ b/go/cmd/vserver/vpc/delete.go @@ -1,12 +1,10 @@ package vpc import ( - "bufio" "fmt" - "os" - "strings" "github.com/spf13/cobra" + "github.com/vngcloud/greennode-cli/internal/cli" "github.com/vngcloud/greennode-cli/internal/validator" ) @@ -20,12 +18,14 @@ func init() { f := deleteCmd.Flags() f.String("vpc-id", "", "VPC (network) ID (required)") f.Bool("force", false, "Skip confirmation prompt") + f.Bool("dry-run", false, "Preview the VPC deletion without executing") deleteCmd.MarkFlagRequired("vpc-id") } func runDelete(cmd *cobra.Command, args []string) error { vpcID, _ := cmd.Flags().GetString("vpc-id") force, _ := cmd.Flags().GetBool("force") + dryRun, _ := cmd.Flags().GetBool("dry-run") if err := validator.ValidateID(vpcID, "vpc-id"); err != nil { return err @@ -55,15 +55,13 @@ func runDelete(cmd *cobra.Command, args []string) error { return err } - if !force { - fmt.Print("\nAre you sure you want to delete this VPC? [y/N]: ") - reader := bufio.NewReader(os.Stdin) - answer, _ := reader.ReadString('\n') - answer = strings.TrimSpace(strings.ToLower(answer)) - if answer != "y" && answer != "yes" { - fmt.Println("Aborted.") - return nil - } + if dryRun { + cli.DryRunNotice("delete") + return nil + } + if !cli.Confirm(force, "Are you sure you want to delete this VPC?") { + fmt.Println("Aborted.") + return nil } result, err := apiClient.Delete(fmt.Sprintf("/v2/%s/networks/%s", projectID, vpcID), nil) diff --git a/go/internal/cli/confirm.go b/go/internal/cli/confirm.go new file mode 100644 index 0000000..8820fb6 --- /dev/null +++ b/go/internal/cli/confirm.go @@ -0,0 +1,29 @@ +package cli + +import ( + "bufio" + "fmt" + "os" + "strings" +) + +// DryRunNotice prints the standard footer shown at the end of a --dry-run +// preview. verb is the action that would run, e.g. DryRunNotice("delete") +// prints "Run without --dry-run to delete." +func DryRunNotice(verb string) { + fmt.Printf("\nRun without --dry-run to %s.\n", verb) +} + +// Confirm asks the user to confirm a destructive action and reports whether to +// proceed. It returns true immediately when force is true. The prompt is shown +// as " [y/N]: "; only "y" or "yes" (case-insensitive) proceeds. +func Confirm(force bool, prompt string) bool { + if force { + return true + } + fmt.Printf("\n%s [y/N]: ", prompt) + reader := bufio.NewReader(os.Stdin) + answer, _ := reader.ReadString('\n') + answer = strings.TrimSpace(strings.ToLower(answer)) + return answer == "y" || answer == "yes" +} diff --git a/go/internal/cli/confirm_test.go b/go/internal/cli/confirm_test.go new file mode 100644 index 0000000..6ffe1f8 --- /dev/null +++ b/go/internal/cli/confirm_test.go @@ -0,0 +1,9 @@ +package cli + +import "testing" + +func TestConfirmForceSkipsPrompt(t *testing.T) { + if !Confirm(true, "delete this?") { + t.Error("force=true should proceed without prompting") + } +}