fix(vks): validate network-type and auto-healing constraints found in real API calls#42
Merged
Merged
Conversation
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four constraints the VKS API enforces at create/config time but the CLI never surfaced — each only observable with a real API call, not
--dry-run.Findings & fixes
--auto-healing-configexample showed bothmaxUnhealthyandunhealthyRange, but the API accepts exactly one. Fixed the example and now reject both together (create-cluster andconfig-auto-healing).--k8s-version/--release-channelcombo that the server then rejects with Not found version. Dry-run can't reach the versions endpoint, so it now explicitly states it performs local checks only — version/channel compatibility, VPC/subnet existence, and quota are server-validated.CILIUM_NATIVE_ROUTINGrequires--node-netmask-size— was not marked. Now validated client-side (dry-run and real create) and documented.CILIUM_NATIVE_ROUTINGrequires ≥1--secondary-subnets— was not marked. Same treatment.The spec's
networkTypedescription confirms 3 & 4: "If you choose CILIUM_NATIVE_ROUTING, you need to set the secondarySubnets and nodeNetmaskSize field."Changes
create_cluster.go: newvalidateNetworkRequirements(CIDR for TIGERA/CILIUM_OVERLAY; node-netmask + secondary subnets for CILIUM_NATIVE_ROUTING), run on both dry-run and real create; auto-healing exactly-one guard; dry-run disclaimer; help text on--network-type,--node-netmask-size,--secondary-subnets,--auto-healing-config.config_auto_healing.go: reject--max-unhealthy+--unhealthy-rangetogether.builders_test.go: table test forvalidateNetworkRequirements.create-cluster.md,config-auto-healing.mdupdated (conditional-required markers, mutually-exclusive notes, corrected examples).Verify
go vet ./..., full test suite (-linkmode=external), andCGO_ENABLED=0 go buildall pass. Manually confirmed all four behaviors via--dry-run.