From 509ed7838f71e7d61527fe119a884d10be3dc69b Mon Sep 17 00:00:00 2001 From: tytv2 Date: Tue, 7 Jul 2026 15:46:35 +0700 Subject: [PATCH] fix(vks): validate network-type and auto-healing constraints found in real API calls Four constraints the API enforces but the CLI didn't surface: - CILIUM_NATIVE_ROUTING requires --node-netmask-size and at least one --secondary-subnets. Now validated client-side (dry-run and real create) instead of failing with an opaque server error; help text marks them. - Auto-healing accepts exactly one of maxUnhealthy / unhealthyRange, not both. Enforced in create-cluster (--auto-healing-config) and config-auto-healing; fixed the help example that showed both. - Dry-run now states it does local checks only: version/release-channel compatibility, VPC/subnet existence and quota are server-validated, so a "valid" dry-run no longer implies the create will succeed. --- docs/commands/vks/config-auto-healing.md | 18 ++++++-- docs/commands/vks/create-cluster.md | 32 +++++++------ go/cmd/vks/builders_test.go | 27 +++++++++++ go/cmd/vks/config_auto_healing.go | 5 ++ go/cmd/vks/create_cluster.go | 58 ++++++++++++++++++++---- 5 files changed, 114 insertions(+), 26 deletions(-) diff --git a/docs/commands/vks/config-auto-healing.md b/docs/commands/vks/config-auto-healing.md index 4c58336..af43305 100644 --- a/docs/commands/vks/config-auto-healing.md +++ b/docs/commands/vks/config-auto-healing.md @@ -6,6 +6,8 @@ Configure auto-healing for a VKS cluster. Auto-healing automatically replaces un Only `--enable-auto-healing` is sent unconditionally. The optional threshold flags (`--max-unhealthy`, `--unhealthy-range`, `--timeout-unhealthy`) are only included in the request when explicitly provided on the command line. +`--max-unhealthy` and `--unhealthy-range` are mutually exclusive — set at most one; the API rejects both together. + Use `--dry-run` to preview the configuration that would be sent without executing the request. ## Synopsis @@ -37,14 +39,14 @@ Enable or disable auto-healing. Pass `true` to enable or `false` to disable. **`--max-unhealthy`** (string) -Maximum proportion of unhealthy nodes tolerated before auto-healing is triggered. Accepts a percentage string. +Maximum proportion of unhealthy nodes tolerated before auto-healing is triggered. Accepts a percentage string. Mutually exclusive with `--unhealthy-range`. - Required: No - Constraints: percentage string, e.g. `30%` **`--unhealthy-range`** (string) -Unhealthy node count range. When the number of unhealthy nodes falls within this range, auto-healing is triggered. +Unhealthy node count range. When the number of unhealthy nodes falls within this range, auto-healing is triggered. Mutually exclusive with `--max-unhealthy`. - Required: No - Constraints: bracket-enclosed range string, e.g. `[2-5]` @@ -77,17 +79,25 @@ grn vks config-auto-healing \ --enable-auto-healing true ``` -Enable auto-healing with custom thresholds: +Enable auto-healing with a max-unhealthy threshold: ```bash grn vks config-auto-healing \ --cluster-id cls-abc12345-6789-def0-1234-abcdef012345 \ --enable-auto-healing true \ --max-unhealthy 30% \ - --unhealthy-range '[2-5]' \ --timeout-unhealthy 300 ``` +Enable auto-healing with an unhealthy-range threshold instead (mutually exclusive with `--max-unhealthy`): + +```bash +grn vks config-auto-healing \ + --cluster-id cls-abc12345-6789-def0-1234-abcdef012345 \ + --enable-auto-healing true \ + --unhealthy-range '[2-5]' +``` + Disable auto-healing: ```bash diff --git a/docs/commands/vks/create-cluster.md b/docs/commands/vks/create-cluster.md index ef63318..86a450c 100644 --- a/docs/commands/vks/create-cluster.md +++ b/docs/commands/vks/create-cluster.md @@ -4,9 +4,9 @@ Create a new VKS cluster. By default only the control plane is provisioned; add worker nodes afterwards with [create-nodegroup](create-nodegroup.md). -When `--network-type` is `TIGERA` or `CILIUM_OVERLAY`, `--cidr` is required. The load balancer and block store CSI plugins are enabled by default. +When `--network-type` is `TIGERA` or `CILIUM_OVERLAY`, `--cidr` is required. When it is `CILIUM_NATIVE_ROUTING`, both `--node-netmask-size` and at least one `--secondary-subnets` value are required. The load balancer and block store CSI plugins are enabled by default. -Use `--dry-run` to validate parameters without sending a create request. +Use `--dry-run` to validate parameters without sending a create request. Dry-run performs local checks only — whether the `--k8s-version` is available on the selected `--release-channel`, that the VPC and subnets exist, and quota availability are validated by the server on the actual create. ## Synopsis @@ -137,16 +137,18 @@ Availability-zone strategy for the cluster. **`--secondary-subnets`** (list<string>) -Secondary subnet IDs, comma-separated. +Secondary subnet IDs, comma-separated. Used by `CILIUM_NATIVE_ROUTING`. -- Required: No +- Required: Conditional — at least one value is required when `--network-type` is `CILIUM_NATIVE_ROUTING`. +- Constraints: up to 10 entries. - Syntax: `sub-aaa,sub-bbb` **`--node-netmask-size`** (integer) -Node netmask size. Only sent when explicitly provided. +Node CIDR mask size used in `CILIUM_NATIVE_ROUTING` mode. Only sent when explicitly provided. -- Required: No +- Required: Conditional — required when `--network-type` is `CILIUM_NATIVE_ROUTING`. +- Possible values: `24`, `25`, `26` (default `25`). **`--auto-upgrade-config`** (structure) @@ -171,25 +173,25 @@ JSON syntax: **`--auto-healing-config`** (structure) -Auto-healing configuration. Accepts shorthand or JSON. +Auto-healing configuration. Accepts shorthand or JSON. When `enableAutoHealing` is `true`, set **exactly one** of `maxUnhealthy` or `unhealthyRange` — the API rejects both together. - Required: No - Members: - `enableAutoHealing` (boolean) — enable or disable auto-healing - - `maxUnhealthy` (string) — maximum unhealthy nodes, e.g. `20%` - - `unhealthyRange` (string) — unhealthy node count range, e.g. `[2-5]` - - `timeoutUnhealthy` (integer) — unhealthy timeout in seconds + - `maxUnhealthy` (string) — maximum unhealthy nodes, e.g. `20%` (mutually exclusive with `unhealthyRange`) + - `unhealthyRange` (string) — unhealthy node count range, e.g. `[2-5]` (mutually exclusive with `maxUnhealthy`) + - `timeoutUnhealthy` (integer) — minutes to wait before considering a node unhealthy (5–180) Shorthand syntax: ``` -enableAutoHealing=true,maxUnhealthy=20%,unhealthyRange=[2-5],timeoutUnhealthy=10 +enableAutoHealing=true,maxUnhealthy=20%,timeoutUnhealthy=10 ``` JSON syntax: ```json -{"enableAutoHealing": true, "maxUnhealthy": "20%", "unhealthyRange": "[2-5]", "timeoutUnhealthy": 10} +{"enableAutoHealing": true, "maxUnhealthy": "20%", "timeoutUnhealthy": 10} ``` **`--dry-run`** (boolean) @@ -213,7 +215,9 @@ grn vks create-cluster \ --k8s-version v1.29.13-vks.1740045600 \ --network-type CILIUM_NATIVE_ROUTING \ --vpc-id net-abc12345-0000-0000-0000-000000000001 \ - --subnet-id sub-abc12345-0000-0000-0000-000000000001 + --subnet-id sub-abc12345-0000-0000-0000-000000000001 \ + --node-netmask-size 25 \ + --secondary-subnets sub-abc12345-0000-0000-0000-000000000002 ``` Create a cluster with TIGERA (CIDR required) and auto-healing: @@ -237,5 +241,7 @@ grn vks create-cluster \ --k8s-version v1.29.13-vks.1740045600 \ --network-type CILIUM_NATIVE_ROUTING \ --vpc-id net-abc12345-0000-0000-0000-000000000001 \ + --node-netmask-size 25 \ + --secondary-subnets sub-abc12345-0000-0000-0000-000000000002 \ --dry-run ``` diff --git a/go/cmd/vks/builders_test.go b/go/cmd/vks/builders_test.go index 70a5d22..ee2dcd4 100644 --- a/go/cmd/vks/builders_test.go +++ b/go/cmd/vks/builders_test.go @@ -32,6 +32,33 @@ func TestBuildAutoHealingBodyOnlyChangedOptionalFields(t *testing.T) { } } +func TestValidateNetworkRequirements(t *testing.T) { + cases := []struct { + name string + networkType string + cidr string + nodeNetmaskSet bool + secondary string + wantErrs int + }{ + {"overlay needs cidr", "CILIUM_OVERLAY", "", false, "", 1}, + {"overlay with cidr ok", "CILIUM_OVERLAY", "10.0.0.0/16", false, "", 0}, + {"tigera needs cidr", "TIGERA", "", false, "", 1}, + {"native routing needs both", "CILIUM_NATIVE_ROUTING", "", false, "", 2}, + {"native routing missing netmask", "CILIUM_NATIVE_ROUTING", "", false, "sub-1", 1}, + {"native routing missing secondary", "CILIUM_NATIVE_ROUTING", "", true, "", 1}, + {"native routing complete", "CILIUM_NATIVE_ROUTING", "", true, "sub-1", 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := validateNetworkRequirements(tc.networkType, tc.cidr, tc.nodeNetmaskSet, tc.secondary) + if len(errs) != tc.wantErrs { + t.Errorf("got %d errors %v, want %d", len(errs), errs, tc.wantErrs) + } + }) + } +} + func TestBuildMetadataBodyIncludesOnlyChangedKeys(t *testing.T) { got := buildMetadataBody("env=prod", "", "dedicated=gpu:NoSchedule", map[string]bool{ "labels": true, "tags": false, "taints": true, diff --git a/go/cmd/vks/config_auto_healing.go b/go/cmd/vks/config_auto_healing.go index 51734ef..cde669a 100644 --- a/go/cmd/vks/config_auto_healing.go +++ b/go/cmd/vks/config_auto_healing.go @@ -59,6 +59,11 @@ func runConfigAutoHealing(cmd *cobra.Command, args []string) error { "unhealthy-range": cmd.Flags().Changed("unhealthy-range"), "timeout-unhealthy": cmd.Flags().Changed("timeout-unhealthy"), } + + // The API accepts at most one of maxUnhealthy / unhealthyRange. + if changed["max-unhealthy"] && changed["unhealthy-range"] { + return fmt.Errorf("set exactly one of --max-unhealthy or --unhealthy-range, not both") + } body := buildAutoHealingBody(enable, maxUnhealthy, unhealthyRange, timeoutUnhealthy, changed) if dryRun { diff --git a/go/cmd/vks/create_cluster.go b/go/cmd/vks/create_cluster.go index 3546c9f..b3ba7c9 100644 --- a/go/cmd/vks/create_cluster.go +++ b/go/cmd/vks/create_cluster.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "regexp" + "strings" "github.com/spf13/cobra" "github.com/vngcloud/greennode-cli/internal/cli" @@ -22,7 +23,7 @@ func init() { // Cluster settings (required) f.String("name", "", "Cluster name (required)") f.String("k8s-version", "", "Kubernetes version (required)") - f.String("network-type", "", "Network type: TIGERA, CILIUM_OVERLAY, CILIUM_NATIVE_ROUTING (required)") + f.String("network-type", "", "Network type: TIGERA, CILIUM_OVERLAY, CILIUM_NATIVE_ROUTING (required). TIGERA/CILIUM_OVERLAY need --cidr; CILIUM_NATIVE_ROUTING needs --node-netmask-size and --secondary-subnets") f.String("vpc-id", "", "VPC ID (required)") f.String("subnet-id", "", "Subnet ID") @@ -39,11 +40,11 @@ func init() { f.String("block-store-csi-plugin", "enabled", "Block store CSI plugin (enabled, disabled)") f.String("service-endpoint", "disabled", "Service endpoint (enabled, disabled)") f.String("az-strategy", "SINGLE", "Availability zone strategy") - f.String("secondary-subnets", "", "Secondary subnet IDs (comma-separated)") + f.String("secondary-subnets", "", "Secondary subnet IDs, comma-separated (required for CILIUM_NATIVE_ROUTING, at least one, max 10)") f.String("list-subnet-ids", "", "Subnet IDs for the cluster (comma-separated)") - f.Int("node-netmask-size", 0, "Node netmask size") + f.Int("node-netmask-size", 0, "Node netmask size: 24, 25, or 26 (required for CILIUM_NATIVE_ROUTING)") f.String("auto-upgrade-config", "", "Auto-upgrade config (shorthand time=03:00,weekdays=Mon or JSON; use JSON for multiple weekdays)") - f.String("auto-healing-config", "", "Auto-healing config (shorthand enableAutoHealing=true,maxUnhealthy=20%,unhealthyRange=[2-5],timeoutUnhealthy=10 or JSON)") + f.String("auto-healing-config", "", "Auto-healing config; set exactly one of maxUnhealthy or unhealthyRange (shorthand enableAutoHealing=true,maxUnhealthy=20%,timeoutUnhealthy=10 or JSON)") f.Bool("dry-run", false, "Validate parameters without creating the cluster") } @@ -131,11 +132,25 @@ func runCreateCluster(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("--auto-healing-config: %w", err) } + if enabled, _ := hc["enableAutoHealing"].(bool); enabled { + _, hasMax := hc["maxUnhealthy"] + _, hasRange := hc["unhealthyRange"] + if hasMax == hasRange { + return fmt.Errorf("--auto-healing-config: set exactly one of maxUnhealthy or unhealthyRange") + } + } body["autoHealingConfig"] = hc } + // Network-type-specific requirements (enforced client-side so both dry-run + // and real creates fail fast with a clear message). + netErrs := validateNetworkRequirements(networkType, cidr, cmd.Flags().Changed("node-netmask-size"), secondarySubnets) + if dryRun { - return validateCreateCluster(name, networkType, cidr) + return validateCreateCluster(name, netErrs) + } + if len(netErrs) > 0 { + return fmt.Errorf("%s", strings.Join(netErrs, "; ")) } apiClient, err := createClient(cmd) @@ -152,7 +167,30 @@ func runCreateCluster(cmd *cobra.Command, args []string) error { return outputResult(cmd, result) } -func validateCreateCluster(name, networkType, cidr string) error { +// validateNetworkRequirements checks the fields each network type requires. +// The API mandates --cidr for TIGERA/CILIUM_OVERLAY and both +// --node-netmask-size and at least one --secondary-subnets for +// CILIUM_NATIVE_ROUTING; validating here turns opaque server errors into +// actionable messages. +func validateNetworkRequirements(networkType, cidr string, nodeNetmaskSet bool, secondarySubnets string) []string { + var errs []string + switch networkType { + case "TIGERA", "CILIUM_OVERLAY": + if cidr == "" { + errs = append(errs, fmt.Sprintf("--cidr is required when --network-type is %s", networkType)) + } + case "CILIUM_NATIVE_ROUTING": + if !nodeNetmaskSet { + errs = append(errs, "--node-netmask-size is required when --network-type is CILIUM_NATIVE_ROUTING (allowed: 24, 25, 26)") + } + if secondarySubnets == "" { + errs = append(errs, "--secondary-subnets is required when --network-type is CILIUM_NATIVE_ROUTING (at least one subnet)") + } + } + return errs +} + +func validateCreateCluster(name string, networkErrors []string) error { clusterNameRE := regexp.MustCompile(`^[a-z0-9][a-z0-9\-]{3,18}[a-z0-9]$`) var errors []string @@ -162,9 +200,7 @@ func validateCreateCluster(name, networkType, cidr string) error { "Cluster name '%s' is invalid. Must be 5-20 chars, lowercase alphanumeric and hyphens, start/end with alphanumeric.", name)) } - if (networkType == "TIGERA" || networkType == "CILIUM_OVERLAY") && cidr == "" { - errors = append(errors, fmt.Sprintf("--cidr is required when network-type is %s", networkType)) - } + errors = append(errors, networkErrors...) fmt.Println("=== DRY RUN: Validation results ===") fmt.Println() @@ -177,5 +213,9 @@ func validateCreateCluster(name, networkType, cidr string) error { } fmt.Println("All parameters are valid. Run without --dry-run to create the cluster.") + fmt.Println() + fmt.Println("Note: dry-run performs local checks only. Whether the --k8s-version is") + fmt.Println("available on the selected --release-channel, that the VPC/subnets exist,") + fmt.Println("and quota availability are validated by the server on the actual create.") return nil }