From 15e2505c2d9583b67e735b6f93cb976038fdba64 Mon Sep 17 00:00:00 2001 From: Seven Cheng Date: Fri, 17 Jul 2026 18:03:00 +0800 Subject: [PATCH 1/2] fix(nvca): render OTel auth extension by mode Signed-off-by: Seven Cheng --- .../manifests/otel_collector_config.yaml | 9 +- .../operator/reconcile/nvcaagent_reconcile.go | 6 +- .../pkg/operator/reconcile/otel_reconcile.go | 26 ++++- .../operator/reconcile/otel_reconcile_test.go | 101 +++++++++++++----- 4 files changed, 104 insertions(+), 38 deletions(-) diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/manifests/otel_collector_config.yaml b/src/compute-plane-services/nvca/pkg/operator/reconcile/manifests/otel_collector_config.yaml index a4e75895e..23ff04281 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/manifests/otel_collector_config.yaml +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/manifests/otel_collector_config.yaml @@ -98,15 +98,16 @@ processors: - log.attributes["event_name"] == "" extensions: - bearertokenauth: - filename: "${env:NGC_SERVICE_API_KEY_FILE}" - +{{ if .UseOAuth2 }} oauth2client: client_id: ${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_ID} client_secret_file: ${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_SECRET_FILE} token_url: "${env:NVCA_OTEL_COLLECTOR_OAUTH_TOKEN_URL}" scopes: ["write"] - +{{ else }} + bearertokenauth: + filename: "${env:NGC_SERVICE_API_KEY_FILE}" +{{ end }} health_check: endpoint: ":${env:NVCA_OTEL_COLLECTOR_HEALTH_CHECK_PORT}" diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go index 846f3dd9e..17975d2e7 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go @@ -2638,6 +2638,10 @@ func (bc *BackendK8sCache) setupOTelCollectorConfigMap(ctx context.Context, nb * } log.Info("setting up OTel collector ConfigMap") + configData, err := bc.getOTelCollectorConfigData(nb) + if err != nil { + return fmt.Errorf("render OTel collector ConfigMap data: %w", err) + } cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -2646,7 +2650,7 @@ func (bc *BackendK8sCache) setupOTelCollectorConfigMap(ctx context.Context, nb * Annotations: getNBAnnotations(nb), Labels: getAppLabels(), }, - Data: bc.getOTelCollectorConfigData(), + Data: configData, } return bc.createOrUpdateConfigMap(ctx, cm) diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go index 82ef03f9f..c319da2e6 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go @@ -18,8 +18,11 @@ limitations under the License. package operator import ( + "bytes" _ "embed" + "fmt" "strings" + "text/template" nvidiaiov1 "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/apis/nvcf/v1" ) @@ -42,13 +45,26 @@ type otelCollectorAuthConfig struct { authenticator string } +// otelCollectorConfigTemplateData contains values used to render the OTel collector configuration template. +type otelCollectorConfigTemplateData struct { + UseOAuth2 bool +} + // getOTelCollectorConfigData returns the OTel collector configuration data. -// Values will be substituted by OTel Collector at runtime -// from environment variables set in the container spec. -func (bc *BackendK8sCache) getOTelCollectorConfigData() map[string]string { - return map[string]string{ - "config.yaml": otelCollectorConfigTpl, +// The authentication extension is selected when the ConfigMap is rendered. Remaining +// values are substituted by the OTel Collector at runtime from the container environment. +func (bc *BackendK8sCache) getOTelCollectorConfigData(nb *nvidiaiov1.NVCFBackend) (map[string]string, error) { + tmpl, err := template.New("otel_collector_config.yaml").Parse(otelCollectorConfigTpl) + if err != nil { + return nil, fmt.Errorf("parse OTel collector config template: %w", err) } + + var config bytes.Buffer + if err := tmpl.Execute(&config, otelCollectorConfigTemplateData{UseOAuth2: useOTelCollectorOAuth2(nb)}); err != nil { + return nil, fmt.Errorf("render OTel collector config template: %w", err) + } + + return map[string]string{"config.yaml": config.String()}, nil } func useOTelCollectorOAuth2(nb *nvidiaiov1.NVCFBackend) bool { diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go index 6752bda35..5102438ef 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" nvidiaiov1 "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/apis/nvcf/v1" ) @@ -90,34 +91,78 @@ func TestGetFNDSEndpoint(t *testing.T) { } func TestGetOTelCollectorConfigData(t *testing.T) { - bc := &BackendK8sCache{} - configData := bc.getOTelCollectorConfigData() - - require.Contains(t, configData, "config.yaml") - config := configData["config.yaml"] - - // Verify environment variable placeholders are present in config (NVCA_OTEL_COLLECTOR_* naming) - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_REQUESTS_NAMESPACE}", "should contain requests namespace env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_MEMORY_LIMIT_PERCENTAGE}", "should contain memory limit percentage env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_SPIKE_LIMIT_PERCENTAGE}", "should contain spike limit percentage env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_HEALTH_CHECK_PORT}", "should contain health check port env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_FNDS_ENDPOINT}", "should contain FNDS endpoint env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_METRICS_PORT}", "should contain metrics port env var placeholder") - - // Verify OAuth-related env var placeholders are present - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_ID}", "should contain OAuth client ID env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_SECRET_FILE}", "should contain OAuth client secret file env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_OAUTH_TOKEN_URL}", "should contain OAuth token URL env var placeholder") - assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_AUTHENTICATOR}", "should contain authenticator env var placeholder") - assert.Contains(t, config, "${env:NGC_SERVICE_API_KEY_FILE}", "should contain NGC service API key file env var placeholder") - - // Verify key config sections are present - assert.Contains(t, config, "memory_limiter", "should contain memory_limiter processor") - assert.Contains(t, config, "k8s_events", "should contain k8s_events receiver") - assert.Contains(t, config, "k8sattributes", "should contain k8sattributes processor") - assert.Contains(t, config, "otlphttp", "should contain otlphttp exporter") - assert.Contains(t, config, NVCAOTelCollectorAuthenticatorBearerTokenAuth, "should contain bearertokenauth extension") - assert.Contains(t, config, NVCAOTelCollectorAuthenticatorOAuth2Client, "should contain oauth2client extension") + tests := []struct { + name string + nb *nvidiaiov1.NVCFBackend + expectedAuthenticator string + unexpectedExtension string + expectedPlaceholders []string + unexpectedPlaceholders []string + }{ + { + name: "service API key authentication", + nb: &nvidiaiov1.NVCFBackend{}, + expectedAuthenticator: NVCAOTelCollectorAuthenticatorBearerTokenAuth, + unexpectedExtension: NVCAOTelCollectorAuthenticatorOAuth2Client, + expectedPlaceholders: []string{"${env:NGC_SERVICE_API_KEY_FILE}"}, + unexpectedPlaceholders: []string{ + "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_ID}", + "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_SECRET_FILE}", + "${env:NVCA_OTEL_COLLECTOR_OAUTH_TOKEN_URL}", + }, + }, + { + name: "OAuth authentication", + nb: &nvidiaiov1.NVCFBackend{Spec: nvidiaiov1.NVCFBackendSpec{ + NVCFBackendSpecT: otelAuthSpec(true, "client-id", "", "", "", ""), + }}, + expectedAuthenticator: NVCAOTelCollectorAuthenticatorOAuth2Client, + unexpectedExtension: NVCAOTelCollectorAuthenticatorBearerTokenAuth, + expectedPlaceholders: []string{ + "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_ID}", + "${env:NVCA_OTEL_COLLECTOR_OAUTH_CLIENT_SECRET_FILE}", + "${env:NVCA_OTEL_COLLECTOR_OAUTH_TOKEN_URL}", + }, + unexpectedPlaceholders: []string{"${env:NGC_SERVICE_API_KEY_FILE}"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bc := &BackendK8sCache{} + configData, err := bc.getOTelCollectorConfigData(tt.nb) + require.NoError(t, err) + require.Contains(t, configData, "config.yaml") + config := configData["config.yaml"] + + var parsed struct { + Extensions map[string]any `json:"extensions"` + } + require.NoError(t, yaml.Unmarshal([]byte(config), &parsed)) + assert.Contains(t, parsed.Extensions, tt.expectedAuthenticator) + assert.NotContains(t, parsed.Extensions, tt.unexpectedExtension) + assert.Contains(t, parsed.Extensions, "health_check") + + for _, placeholder := range tt.expectedPlaceholders { + assert.Contains(t, config, placeholder) + } + for _, placeholder := range tt.unexpectedPlaceholders { + assert.NotContains(t, config, placeholder) + } + + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_REQUESTS_NAMESPACE}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_MEMORY_LIMIT_PERCENTAGE}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_SPIKE_LIMIT_PERCENTAGE}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_HEALTH_CHECK_PORT}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_FNDS_ENDPOINT}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_METRICS_PORT}") + assert.Contains(t, config, "${env:NVCA_OTEL_COLLECTOR_AUTHENTICATOR}") + assert.Contains(t, config, "memory_limiter") + assert.Contains(t, config, "k8s_events") + assert.Contains(t, config, "k8sattributes") + assert.Contains(t, config, "otlphttp") + }) + } } // otelAuthSpec builds minimal NVCFBackendSpecT for getOTelCollectorAuthConfig tests. From caadbc241a080079291a47775b25bf2291ee3bf9 Mon Sep 17 00:00:00 2001 From: Seven Cheng Date: Sat, 18 Jul 2026 09:15:19 +0800 Subject: [PATCH 2/2] fix(nvca): remove OTel client ID placeholder Signed-off-by: Seven Cheng --- .../operator/reconcile/nvcaagent_reconcile_test.go | 4 ++-- .../nvca/pkg/operator/reconcile/otel_reconcile.go | 8 +++----- .../pkg/operator/reconcile/otel_reconcile_test.go | 12 ++++++------ 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go index 504d97df2..94d90693b 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go @@ -4626,7 +4626,7 @@ func TestGetOTelCollectorContainerCommandArgsAndEnv_OAuthAuth(t *testing.T) { expectedAuthenticator: NVCAOTelCollectorAuthenticatorOAuth2Client, }, { - name: "Vault disabled - service API key bearer token authentication with placeholder OAuth env vars", + name: "Vault disabled - service API key bearer token authentication with empty OAuth client ID", nb: &nvidiaiov1.NVCFBackend{ Spec: nvidiaiov1.NVCFBackendSpec{ NVCFBackendSpecT: nvidiaiov1.NVCFBackendSpecT{ @@ -4642,7 +4642,7 @@ func TestGetOTelCollectorContainerCommandArgsAndEnv_OAuthAuth(t *testing.T) { }, }, envType: nvidiaiov1.EnvTypeProd, - expectedOAuthClientID: NVCAOTelCollectorOAuthPlaceholderClientID, + expectedOAuthClientID: "", expectedOAuthSecretFile: "/home/nvca/vault-agent/secrets/oauth-client-secrets.env", expectedOAuthTokenURL: "", expectedAuthenticator: NVCAOTelCollectorAuthenticatorBearerTokenAuth, diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go index c319da2e6..5b91a328a 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile.go @@ -30,8 +30,6 @@ import ( const ( NVCAOTelCollectorAuthenticatorOAuth2Client = "oauth2client" NVCAOTelCollectorAuthenticatorBearerTokenAuth = "bearertokenauth" - - NVCAOTelCollectorOAuthPlaceholderClientID = "nvca-otel-collector-client-id" ) //go:embed manifests/otel_collector_config.yaml @@ -71,10 +69,10 @@ func useOTelCollectorOAuth2(nb *nvidiaiov1.NVCFBackend) bool { return nb.Spec.VaultConfig.Enabled && getOAuthConfig(nb).ClientID != "" } -// getOTelCollectorAuthConfig determines the authentication configuration for the OTel collector. -// Falls back to NVCAOTelCollectorAuthenticatorBearerTokenAuth when Vault is disabled or client ID is empty. +// getOTelCollectorAuthConfig selects OAuth2 authentication when Vault is enabled +// and a client ID is configured; otherwise, it selects bearer-token authentication. func (bc *BackendK8sCache) getOTelCollectorAuthConfig(nb *nvidiaiov1.NVCFBackend) otelCollectorAuthConfig { - clientID := NVCAOTelCollectorOAuthPlaceholderClientID + clientID := "" vaultSecretFilePath := DefaultVaultSecretFilePath authenticator := NVCAOTelCollectorAuthenticatorBearerTokenAuth diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go index 5102438ef..da6d2217d 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/otel_reconcile_test.go @@ -239,28 +239,28 @@ func TestGetOTelCollectorAuthConfig(t *testing.T) { expectedAuthenticator: NVCAOTelCollectorAuthenticatorOAuth2Client, }, { - name: "Vault disabled → bearer, placeholder", + name: "Vault disabled → bearer, empty OAuth client ID", nb: &nvidiaiov1.NVCFBackend{Spec: nvidiaiov1.NVCFBackendSpec{NVCFBackendSpecT: otelAuthSpec(false, "", otelCollectorTokenURLProd, otelCollectorTokenURLStage, "", "")}}, envType: nvidiaiov1.EnvTypeProd, - expectedClientID: NVCAOTelCollectorOAuthPlaceholderClientID, + expectedClientID: "", expectedSecretFile: "/home/nvca/vault-agent/secrets/oauth-client-secrets.env", expectedTokenURL: "", expectedAuthenticator: NVCAOTelCollectorAuthenticatorBearerTokenAuth, }, { - name: "Vault absent → bearer, placeholder, stage URL", + name: "Vault absent → bearer, empty OAuth client ID, stage URL", nb: &nvidiaiov1.NVCFBackend{Spec: nvidiaiov1.NVCFBackendSpec{NVCFBackendSpecT: nvidiaiov1.NVCFBackendSpecT{}}}, envType: nvidiaiov1.EnvTypeStage, - expectedClientID: NVCAOTelCollectorOAuthPlaceholderClientID, + expectedClientID: "", expectedSecretFile: "/home/nvca/vault-agent/secrets/oauth-client-secrets.env", expectedTokenURL: "", expectedAuthenticator: NVCAOTelCollectorAuthenticatorBearerTokenAuth, }, { - name: "Vault enabled, empty ClientID → fallback to bearer auth and placeholder", + name: "Vault enabled, empty ClientID → fallback to bearer auth with empty OAuth client ID", nb: &nvidiaiov1.NVCFBackend{Spec: nvidiaiov1.NVCFBackendSpec{NVCFBackendSpecT: otelAuthSpec(true, "", otelCollectorTokenURLProd, otelCollectorTokenURLStage, "2.53.0", "")}}, envType: nvidiaiov1.EnvTypeProd, - expectedClientID: NVCAOTelCollectorOAuthPlaceholderClientID, + expectedClientID: "", expectedSecretFile: "/home/nvca/vault-agent/secrets/oauth-client-secrets.env", expectedTokenURL: "", expectedAuthenticator: NVCAOTelCollectorAuthenticatorBearerTokenAuth,