From 449614cd017a7df1265cc51eaa110fa7e3d7a46d Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 26 Aug 2026 00:22:03 +0530 Subject: [PATCH 1/2] fix(nvca): fail closed on missing CA in cluster-validator TLS probe --- .../internal/clustervalidator/connectivity.go | 39 ++++--- .../clustervalidator/connectivity_test.go | 100 ++++++++++++++++++ 2 files changed, 123 insertions(+), 16 deletions(-) create mode 100644 src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/connectivity.go b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity.go index 1bc9e4fef..95eb20d51 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/connectivity.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity.go @@ -31,10 +31,12 @@ import ( const defaultConnectTimeout = 10 * time.Second +// inClusterCAPath is the standard mount path for the API server's CA bundle +// inside any pod with automountServiceAccountToken: true. Declared as a var +// (not const) so tests can point it at a fixture file. +var inClusterCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + const ( - // inClusterCAPath is the standard mount path for the API server's CA bundle - // inside any pod with automountServiceAccountToken: true. - inClusterCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" // inClusterAPIURL is the in-cluster ClusterIP DNS name of the Kubernetes // API service. Probing this proves the service-routing layer (kube-proxy, // Cilium eBPF, OVN-Kubernetes, etc.) is working, regardless of which @@ -220,15 +222,18 @@ func probeInClusterDNS(ctx context.Context) bool { // care about the response status; only that the cluster's // kube-proxy / eBPF / OVN-Kubernetes / etc. did its job. // -// Uses the standard in-cluster CA bundle when present; falls back to -// InsecureSkipVerify only when the CA is unreadable (e.g., running outside -// a pod for dev/testing) — TLS verification of a remote API server is -// not the goal of this probe, the routing capability is. +// Requires the standard in-cluster CA bundle to build a verified TLS +// config; fails closed (reports routing as unproven) when the CA is +// unreadable rather than skipping certificate/hostname verification. func probeKubernetesAPIServiceIP(ctx context.Context) bool { + tlsConfig, ok := inClusterTLSConfig() + if !ok { + return false + } client := &http.Client{ Timeout: defaultConnectTimeout, Transport: &http.Transport{ - TLSClientConfig: inClusterTLSConfig(), + TLSClientConfig: tlsConfig, }, } req, err := http.NewRequestWithContext(ctx, http.MethodGet, inClusterAPIURL, nil) @@ -247,14 +252,16 @@ func probeKubernetesAPIServiceIP(ctx context.Context) bool { return false } -// inClusterTLSConfig returns a *tls.Config that trusts the cluster's CA -// when the standard SA-mount is present, or skips verification otherwise. -// Skipping verify is acceptable here because the probe is solely a -// "routing reaches a TLS-speaking endpoint" capability check. -func inClusterTLSConfig() *tls.Config { +// inClusterTLSConfig returns a verified *tls.Config trusting the cluster's +// CA when the standard SA-mount is present. The second return value is +// false when the CA bundle can't be read or parsed, signaling the caller +// to fail closed instead of connecting without certificate/hostname +// verification. +func inClusterTLSConfig() (*tls.Config, bool) { pool := x509.NewCertPool() - if caBytes, err := os.ReadFile(inClusterCAPath); err == nil && pool.AppendCertsFromPEM(caBytes) { - return &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12} + caBytes, err := os.ReadFile(inClusterCAPath) + if err != nil || !pool.AppendCertsFromPEM(caBytes) { + return nil, false } - return &tls.Config{InsecureSkipVerify: true, MinVersion: tls.VersionTLS12} // #nosec G402 — routing-only probe + return &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}, true } diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go new file mode 100644 index 000000000..f22d57373 --- /dev/null +++ b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go @@ -0,0 +1,100 @@ +/* +SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package clustervalidator + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeTestCA generates a self-signed CA certificate PEM file at path and +// returns it. Used to give inClusterTLSConfig a valid CA bundle in tests. +func writeTestCA(t *testing.T, path string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + require.NoError(t, os.WriteFile(path, pemBytes, 0o600)) +} + +func TestInClusterTLSConfig_MissingCA_FailsClosed(t *testing.T) { + orig := inClusterCAPath + t.Cleanup(func() { inClusterCAPath = orig }) + inClusterCAPath = filepath.Join(t.TempDir(), "does-not-exist.crt") + + cfg, ok := inClusterTLSConfig() + assert.False(t, ok) + assert.Nil(t, cfg) +} + +func TestInClusterTLSConfig_InvalidCA_FailsClosed(t *testing.T) { + orig := inClusterCAPath + t.Cleanup(func() { inClusterCAPath = orig }) + path := filepath.Join(t.TempDir(), "invalid.crt") + require.NoError(t, os.WriteFile(path, []byte("not a certificate"), 0o600)) + inClusterCAPath = path + + cfg, ok := inClusterTLSConfig() + assert.False(t, ok) + assert.Nil(t, cfg) +} + +func TestInClusterTLSConfig_ValidCA_VerifiesTLS(t *testing.T) { + orig := inClusterCAPath + t.Cleanup(func() { inClusterCAPath = orig }) + path := filepath.Join(t.TempDir(), "ca.crt") + writeTestCA(t, path) + inClusterCAPath = path + + cfg, ok := inClusterTLSConfig() + require.True(t, ok) + require.NotNil(t, cfg) + assert.False(t, cfg.InsecureSkipVerify) + assert.NotNil(t, cfg.RootCAs) +} + +func TestProbeKubernetesAPIServiceIP_MissingCA_ReturnsFalse(t *testing.T) { + orig := inClusterCAPath + t.Cleanup(func() { inClusterCAPath = orig }) + inClusterCAPath = filepath.Join(t.TempDir(), "does-not-exist.crt") + + assert.False(t, probeKubernetesAPIServiceIP(context.Background())) +} From b8e6801734e09b00e1429d5b7ef35b1d7daca378 Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 26 Aug 2026 00:45:03 +0530 Subject: [PATCH 2/2] test(nvca): table-drive inClusterTLSConfig CA-loader scenarios --- .../clustervalidator/connectivity_test.go | 76 +++++++++++-------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go index f22d57373..8eb4b3007 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/connectivity_test.go @@ -55,40 +55,52 @@ func writeTestCA(t *testing.T, path string) { require.NoError(t, os.WriteFile(path, pemBytes, 0o600)) } -func TestInClusterTLSConfig_MissingCA_FailsClosed(t *testing.T) { - orig := inClusterCAPath - t.Cleanup(func() { inClusterCAPath = orig }) - inClusterCAPath = filepath.Join(t.TempDir(), "does-not-exist.crt") - - cfg, ok := inClusterTLSConfig() - assert.False(t, ok) - assert.Nil(t, cfg) -} - -func TestInClusterTLSConfig_InvalidCA_FailsClosed(t *testing.T) { - orig := inClusterCAPath - t.Cleanup(func() { inClusterCAPath = orig }) - path := filepath.Join(t.TempDir(), "invalid.crt") - require.NoError(t, os.WriteFile(path, []byte("not a certificate"), 0o600)) - inClusterCAPath = path +func TestInClusterTLSConfig(t *testing.T) { + tests := []struct { + name string + setupCA func(t *testing.T, path string) + wantOK bool + }{ + { + name: "missing CA fails closed", + setupCA: func(t *testing.T, path string) {}, + wantOK: false, + }, + { + name: "invalid CA fails closed", + setupCA: func(t *testing.T, path string) { + require.NoError(t, os.WriteFile(path, []byte("not a certificate"), 0o600)) + }, + wantOK: false, + }, + { + name: "valid CA verifies TLS", + setupCA: func(t *testing.T, path string) { + writeTestCA(t, path) + }, + wantOK: true, + }, + } - cfg, ok := inClusterTLSConfig() - assert.False(t, ok) - assert.Nil(t, cfg) -} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + orig := inClusterCAPath + t.Cleanup(func() { inClusterCAPath = orig }) + path := filepath.Join(t.TempDir(), "ca.crt") + tt.setupCA(t, path) + inClusterCAPath = path -func TestInClusterTLSConfig_ValidCA_VerifiesTLS(t *testing.T) { - orig := inClusterCAPath - t.Cleanup(func() { inClusterCAPath = orig }) - path := filepath.Join(t.TempDir(), "ca.crt") - writeTestCA(t, path) - inClusterCAPath = path - - cfg, ok := inClusterTLSConfig() - require.True(t, ok) - require.NotNil(t, cfg) - assert.False(t, cfg.InsecureSkipVerify) - assert.NotNil(t, cfg.RootCAs) + cfg, ok := inClusterTLSConfig() + assert.Equal(t, tt.wantOK, ok) + if !tt.wantOK { + assert.Nil(t, cfg) + return + } + require.NotNil(t, cfg) + assert.False(t, cfg.InsecureSkipVerify) + assert.NotNil(t, cfg.RootCAs) + }) + } } func TestProbeKubernetesAPIServiceIP_MissingCA_ReturnsFalse(t *testing.T) {