Skip to content

fix(cli): validate shared gateway API and NATS hosts #1485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbaum1994 merged 1 commit into from
Sep 2, 2026
Merged

fix(cli): validate shared gateway API and NATS hosts #1485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/clis/nvcf-cli/cmd/self_hosted_compute_plane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func runSelfHostedComputePlaneRegister(c *cobra.Command, _ []string) error {
NATSURL: selected.Endpoints.NATSURL,
SISHost: validation.Profile.ControlPlane.Hosts.SIS,
ReValHost: validation.Profile.ControlPlane.Hosts.ReVal,
NATSHost: validation.Profile.ControlPlane.Hosts.NATS,
ProbeHTTP: shouldProbeComputeRegisterHTTP(selected.Name),
}); err != nil {
return err
Expand Down
13 changes: 12 additions & 1 deletion src/clis/nvcf-cli/cmd/self_hosted_compute_plane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,9 @@ func TestComputePlaneRegisterDryRunRunsReachabilityCheck(t *testing.T) {
resetComputePlaneFlags(t)

profileFile := writeTestControlPlaneProfile(t, "cp-cluster")
updateTestControlPlaneProfile(t, profileFile, func(profile *controlplaneprofile.ControlPlaneProfile) {
profile.ControlPlane.Endpoints.ComputeReachable.NATSURL = "tls://api.example.test:4222"
})

prevFetcher := fetchClusterIdentity
t.Cleanup(func() { fetchClusterIdentity = prevFetcher })
Expand Down Expand Up @@ -366,9 +369,10 @@ func TestComputePlaneRegisterDryRunRunsReachabilityCheck(t *testing.T) {
assert.Equal(t, "https://api.example.test", got.GatewayHTTPURL)
assert.Equal(t, "https://sis.example.test", got.ICMSURL)
assert.Equal(t, "https://reval.example.test", got.ReValURL)
assert.Equal(t, "tls://nats.example.test:4222", got.NATSURL)
assert.Equal(t, "tls://api.example.test:4222", got.NATSURL)
assert.Equal(t, "sis.example.test", got.SISHost)
assert.Equal(t, "reval.example.test", got.ReValHost)
assert.Equal(t, "nats.example.test", got.NATSHost)
assert.False(t, got.ProbeHTTP)
}

Expand Down Expand Up @@ -442,8 +446,11 @@ func TestComputePlaneRegisterDryRunRejectsMissingSharedGatewayHostsBeforeIdentit
updateTestControlPlaneProfile(t, profileFile, func(profile *controlplaneprofile.ControlPlaneProfile) {
profile.ControlPlane.Endpoints.ComputeReachable.ICMSURL = profile.ControlPlane.Gateway.HTTPURL
profile.ControlPlane.Endpoints.ComputeReachable.ReValURL = profile.ControlPlane.Gateway.HTTPURL
profile.ControlPlane.Endpoints.ComputeReachable.NATSURL = "tls://api.example.test:4222"
profile.ControlPlane.Hosts.API = ""
profile.ControlPlane.Hosts.SIS = ""
profile.ControlPlane.Hosts.ReVal = ""
profile.ControlPlane.Hosts.NATS = ""
})

fetchCalls := 0
Expand All @@ -465,10 +472,14 @@ func TestComputePlaneRegisterDryRunRejectsMissingSharedGatewayHostsBeforeIdentit

err := rootCmd.Execute()
require.Error(t, err)
assert.Contains(t, err.Error(), "controlPlane.gateway.httpURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.api")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.icmsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.sis")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.revalURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.reval")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.natsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.nats")
assert.Equal(t, 0, fetchCalls, "identity discovery must not run after profile validation failure")
}

Expand Down
27 changes: 27 additions & 0 deletions src/clis/nvcf-cli/cmd/self_hosted_control_plane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,11 @@ func TestControlPlaneProfileValidateCommandRejectsMissingSharedGatewayHosts(t *t
doc := strings.ReplaceAll(validControlPlaneProfileYAML(), "https://sis.nvcf-cp.internal", "https://gateway.nvcf-cp.internal")
doc = strings.ReplaceAll(doc, "https://reval.nvcf-cp.internal", "https://gateway.nvcf-cp.internal")
doc = strings.Replace(doc, " httpURL: https://api.nvcf-cp.internal", " httpURL: https://gateway.nvcf-cp.internal", 1)
doc = strings.Replace(doc, " natsURL: tls://nats.nvcf-cp.internal:4222", " natsURL: tls://gateway.nvcf-cp.internal:4222", 1)
doc = removeLine(doc, " api: api.nvcf-cp.internal")
doc = removeLine(doc, " sis: sis.nvcf-cp.internal")
doc = removeLine(doc, " reval: reval.nvcf-cp.internal")
doc = removeLine(doc, " nats: nats.nvcf-cp.internal")
path := writeControlPlaneProfileFixture(t, doc)
resetControlPlaneProfileValidateCommand(t)

Expand All @@ -98,10 +101,34 @@ func TestControlPlaneProfileValidateCommandRejectsMissingSharedGatewayHosts(t *t

err := rootCmd.Execute()
require.Error(t, err)
assert.Contains(t, err.Error(), "controlPlane.gateway.httpURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.api")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.icmsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.sis")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.revalURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.reval")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.natsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.nats")
}

func TestControlPlaneProfileValidateCommandAcceptsSharedGatewayAPIAndNATSHosts(t *testing.T) {
doc := strings.Replace(validControlPlaneProfileYAML(), " httpURL: https://api.nvcf-cp.internal", " httpURL: https://gateway.nvcf-cp.internal", 1)
doc = strings.Replace(doc, " natsURL: tls://nats.nvcf-cp.internal:4222", " natsURL: tls://gateway.nvcf-cp.internal:4222", 1)
path := writeControlPlaneProfileFixture(t, doc)
resetControlPlaneProfileValidateCommand(t)

var stdout bytes.Buffer
rootCmd.SetOut(&stdout)
rootCmd.SetErr(&bytes.Buffer{})
rootCmd.SetArgs([]string{
"self-hosted", "control-plane", "profile", "validate",
"--file", path,
"--require", "compute-reachable",
})

err := rootCmd.Execute()
require.NoError(t, err)
assert.Contains(t, stdout.String(), "control-plane profile is valid")
}

func TestControlPlaneProfileValidateCommandHelpShowsAnyRequireMode(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion src/clis/nvcf-cli/internal/selfhosted/controlplaneprofile/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,10 @@ func (v *validator) validateHosts() {

func (v *validator) validateNoDNSHostHeaders() {
cp := v.doc.ControlPlane
requireHostForGatewayURL(v.add, "controlPlane.gateway.httpURL", cp.Gateway.HTTPURL, "", "controlPlane.hosts.api", cp.Hosts.API)
requireHostForGatewayURL(v.add, "controlPlane.gateway.httpURL", cp.Gateway.HTTPURL, cp.Gateway.HTTPURL, "controlPlane.hosts.api", cp.Hosts.API)
requireHostForGatewayURL(v.add, "controlPlane.endpoints.computeReachable.icmsURL", cp.Endpoints.ComputeReachable.ICMSURL, cp.Gateway.HTTPURL, "controlPlane.hosts.sis", cp.Hosts.SIS)
requireHostForGatewayURL(v.add, "controlPlane.endpoints.computeReachable.revalURL", cp.Endpoints.ComputeReachable.ReValURL, cp.Gateway.HTTPURL, "controlPlane.hosts.reval", cp.Hosts.ReVal)
requireHostForGatewayURL(v.add, "controlPlane.endpoints.computeReachable.natsURL", cp.Endpoints.ComputeReachable.NATSURL, cp.Gateway.HTTPURL, "controlPlane.hosts.nats", cp.Hosts.NATS)
}

// validateManagementTLS validates managementTls only when it is provided. An
Expand Down
24 changes: 24 additions & 0 deletions src/clis/nvcf-cli/internal/selfhosted/controlplaneprofile/profile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,33 @@ func TestValidateSharedGatewayHostnameRequiresServiceHostHeaders(t *testing.T) {
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.revalURL")
}

func TestValidateSharedGatewayHostnameRequiresAPIAndNATSHostHeaders(t *testing.T) {
doc := strings.Replace(validControlPlaneProfileYAML(), " httpURL: https://api.nvcf-cp.internal", " httpURL: https://gateway.nvcf-cp.internal", 1)
doc = strings.Replace(doc, " natsURL: tls://nats.nvcf-cp.internal:4222", " natsURL: tls://gateway.nvcf-cp.internal:4222", 1)
doc = strings.Replace(doc, " api: api.nvcf-cp.internal\n", "", 1)
doc = strings.Replace(doc, " nats: nats.nvcf-cp.internal\n", "", 1)

_, err := ParseAndValidate([]byte(doc), ValidateOptions{Require: RequireComputeReachable})
require.Error(t, err)

assert.Contains(t, err.Error(), "controlPlane.gateway.httpURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.api")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.natsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.nats")
}

func TestValidateSharedGatewayHostnameAcceptsAPIAndNATSHostHeaders(t *testing.T) {
doc := strings.Replace(validControlPlaneProfileYAML(), " httpURL: https://api.nvcf-cp.internal", " httpURL: https://gateway.nvcf-cp.internal", 1)
doc = strings.Replace(doc, " natsURL: tls://nats.nvcf-cp.internal:4222", " natsURL: tls://gateway.nvcf-cp.internal:4222", 1)

_, err := ParseAndValidate([]byte(doc), ValidateOptions{Require: RequireComputeReachable})
require.NoError(t, err)
}

func TestValidateDirectServiceHostnamesDoNotRequireHostOverrides(t *testing.T) {
doc := strings.Replace(validControlPlaneProfileYAML(), " sis: sis.nvcf-cp.internal\n", "", 1)
doc = strings.Replace(doc, " reval: reval.nvcf-cp.internal\n", "", 1)
doc = strings.Replace(doc, " nats: nats.nvcf-cp.internal\n", "", 1)

_, err := ParseAndValidate([]byte(doc), ValidateOptions{Require: RequireComputeReachable})
require.NoError(t, err)
Expand Down
8 changes: 6 additions & 2 deletions src/clis/nvcf-cli/internal/selfhosted/reachability/reachability.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type CheckRequest struct {
NATSURL string
SISHost string
ReValHost string
NATSHost string
HTTPClient *http.Client
ProbeHTTP bool
}
Expand Down Expand Up @@ -67,7 +68,7 @@ func Check(ctx context.Context, req CheckRequest) error {
var problems []string
problems = append(problems, validateHTTPService("controlPlane.endpoints.computeReachable.icmsURL", req.ICMSURL, req.GatewayHTTPURL, "controlPlane.hosts.sis", req.SISHost)...)
problems = append(problems, validateHTTPService("controlPlane.endpoints.computeReachable.revalURL", req.ReValURL, req.GatewayHTTPURL, "controlPlane.hosts.reval", req.ReValHost)...)
problems = append(problems, validateNATSServiceShape(req.NATSURL)...)
problems = append(problems, validateNATSServiceShape(req.NATSURL, req.GatewayHTTPURL, req.NATSHost)...)
if len(problems) == 0 && req.ProbeHTTP {
client := req.HTTPClient
if client == nil {
Expand Down Expand Up @@ -108,7 +109,7 @@ func validateHTTPService(urlField, rawURL, gatewayHTTPURL, hostField, hostHeader
return problems
}

func validateNATSServiceShape(rawURL string) []string {
func validateNATSServiceShape(rawURL, gatewayHTTPURL, hostHeader string) []string {
if strings.TrimSpace(rawURL) == "" {
return []string{"controlPlane.endpoints.computeReachable.natsURL: required"}
}
Expand All @@ -119,6 +120,9 @@ func validateNATSServiceShape(rawURL string) []string {
if u.Scheme != "nats" && u.Scheme != "tls" {
return []string{"controlPlane.endpoints.computeReachable.natsURL: scheme must be nats or tls; TCP reachability is not probed"}
}
if isGatewayAddress(u.Hostname(), gatewayHTTPURL) && strings.TrimSpace(hostHeader) == "" {
return []string{"controlPlane.hosts.nats: required when controlPlane.endpoints.computeReachable.natsURL uses a shared gateway address; set controlPlane.hosts.nats to the service Host header"}
}
return nil
}

Expand Down
18 changes: 17 additions & 1 deletion src/clis/nvcf-cli/internal/selfhosted/reachability/reachability_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestCheckRequiresHostHeadersForSharedGatewayHostname(t *testing.T) {
GatewayHTTPURL: "https://gateway.example.test",
ICMSURL: "https://gateway.example.test",
ReValURL: "https://gateway.example.test",
NATSURL: "tls://nats.example.test:4222",
NATSURL: "tls://gateway.example.test:4222",
ProbeHTTP: false,
})

Expand All @@ -59,6 +59,8 @@ func TestCheckRequiresHostHeadersForSharedGatewayHostname(t *testing.T) {
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.icmsURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.reval")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.revalURL")
assert.Contains(t, err.Error(), "controlPlane.hosts.nats")
assert.Contains(t, err.Error(), "controlPlane.endpoints.computeReachable.natsURL")
}

func TestCheckAllowsDirectServiceHostnamesWithoutHostOverrides(t *testing.T) {
Expand All @@ -74,6 +76,20 @@ func TestCheckAllowsDirectServiceHostnamesWithoutHostOverrides(t *testing.T) {
require.NoError(t, err)
}

func TestCheckAllowsSharedGatewayNATSWithHostOverride(t *testing.T) {
err := Check(context.Background(), CheckRequest{
TargetClusterName: "gpu-a",
GatewayHTTPURL: "https://gateway.example.test",
ICMSURL: "https://sis.example.test",
ReValURL: "https://reval.example.test",
NATSURL: "tls://gateway.example.test:4222",
NATSHost: "nats.example.test",
ProbeHTTP: false,
})

require.NoError(t, err)
}

func TestCheckErrorIncludesClusterNameWithoutProblems(t *testing.T) {
err := (&CheckError{TargetClusterName: "gpu-a"}).Error()

Expand Down
Loading