From a227f20e834d972a1822b0103a8c9ef9be9d6fd2 Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Fri, 31 Jul 2026 16:19:04 +0200 Subject: [PATCH] fix: authenticate Helm downloads with pull secrets --- README.md | 4 +- docs/reference/cli.md | 4 +- docs/reference/configuration.md | 12 +- docs/user/quick-start.md | 6 + docs/user/troubleshooting.md | 1 + pkg/cmd/discover.go | 2 +- pkg/cmd/generate.go | 2 +- pkg/cmd/root.go | 2 +- pkg/cmd/schema.go | 2 +- pkg/config/default-config.yaml | 5 + pkg/networkoperatorplugin/helm.go | 61 ++++- pkg/networkoperatorplugin/helm_credentials.go | 246 +++++++++++++++++ pkg/networkoperatorplugin/helm_test.go | 258 ++++++++++++++++++ pkg/options/options.go | 2 +- skills/k8s-launch-kit-deploy/SKILL.md | 8 + skills/k8s-launch-kit-discover/SKILL.md | 2 +- skills/k8s-launch-kit-shared/SKILL.md | 2 +- skills/k8s-launch-kit-troubleshoot/SKILL.md | 1 + 18 files changed, 604 insertions(+), 16 deletions(-) create mode 100644 pkg/networkoperatorplugin/helm_credentials.go diff --git a/README.md b/README.md index e92f27b..efb7278 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ or via a profile section in the user-config file. ### Deploy to Cluster Apply the generated deployment files to your Kubernetes cluster by using --deploy. This phase requires --kubeconfig and can be skipped if --deploy is not specified. -The deploy step installs (or upgrades) the `nvidia/network-operator` Helm chart in-process before applying the post-install CRs. The chart version and Helm repository URL are taken from the embedded release catalog and can be selected via `--network-operator-release `. Each profile renders a per-profile `values.yaml` next to the CR manifests; `l8k deploy` reads that file and runs the install. When a release already exists with different values, deploy fails fast — pass `--overwrite-existing` to promote to `helm upgrade --install`. +The deploy step installs (or upgrades) the `nvidia/network-operator` Helm chart in-process before applying the post-install CRs. The chart version and Helm repository URL are taken from the embedded release catalog and can be selected via `--network-operator-release `. Each profile renders a per-profile `values.yaml` next to the CR manifests; `l8k deploy` reads that file and runs the install. When `networkOperator.imagePullSecrets` is configured, l8k reads matching Docker credentials from Secrets already present in the operator namespace and uses them for the chart download (including the `nvcr.io` to `helm.ngc.nvidia.com` NGC credential mapping). Secret data remains in memory and is never logged. When a release already exists with different values, deploy fails fast — pass `--overwrite-existing` to promote to `helm upgrade --install`. Deploy preflight does not treat `SriovNetworkPoolConfig`, `SriovNetworkNodePolicy`, or `OVSNetwork` objects labeled with @@ -243,7 +243,7 @@ Available Commands: Common Flags: --config-dir string Directory containing optional l8k-config.yaml and presets/ overrides --enabled-plugins string Comma-separated list of plugins to enable (default "network-operator") - --image-pull-secrets strings Image pull secret names for Network Operator components (comma-separated) + --image-pull-secrets strings Image pull secret names for Network Operator components and authenticated Helm downloads (comma-separated) --kubeconfig string Path to kubeconfig file for cluster deployment (required when using --deploy; falls back to $KUBECONFIG, then ~/.kube/config) --network-operator-namespace string Override the network operator namespace from the config file --network-operator-release string Network Operator release line to deploy (MAJOR.MINOR). Selects component image tags + repository from a built-in catalog and drives version-gated template sections. Supported: 26.1, 26.4, 26.7 diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 86f27b7..cd0fdf7 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -43,7 +43,7 @@ Run `l8k --help` for the authoritative flag list. Run `l8k schema` for | `--node-selector` | Selector persisted for generated resources. It does not filter discovery scheduling. | | `--keep-namespace` | Keep the temporary `nvidia-k8s-launch-kit` namespace and daemon workload for inspection. | | `--collapse-nic-rails` | Collapse eligible multi-port NICs into one rail. Enabled by default; known dual-port models retain a rail per port. | -| `--image-pull-secrets` | Secret names used to pull the discovery daemon and propagated into generated policies and Network Operator Helm values. | +| `--image-pull-secrets` | Secret names used to pull the discovery daemon, propagated into generated policies and Helm values, and reused for authenticated Helm chart downloads when the registry host matches. | | `--enabled-plugins` | Comma-separated plugins. The supported deployment plugin is `network-operator`. | Discovery also accepts the profile and Spectrum-X flags below. Explicit flags override values from `--user-config` and discovered defaults. @@ -84,7 +84,7 @@ Discovery also accepts the profile and Spectrum-X flags below. Explicit flags ov | `--network-namespaces` | Namespaces that receive secondary-network resources and example workloads. | | `--workload-manifest` | Replace the profile's example workload with a Pod or workload-controller manifest. | | `--enable-doca-driver` | Override `docaDriver.enable` and include the DOCA driver deployment. | -| `--image-pull-secrets` | Secret names propagated into generated Network Operator policies and Helm values. | +| `--image-pull-secrets` | Secret names propagated into generated Network Operator policies and Helm values. Matching credentials already present in the operator namespace authenticate the Helm chart download. | | `--deploy` | Deploy immediately after generation. | | `--kubeconfig` | Kubeconfig used with `--deploy`. | | `--dry-run` | Preview the deploy stage used with `--deploy`. | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 77771d4..194358a 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -60,10 +60,20 @@ The release line fills Network Operator versions, component image tags, DOCA dri | `operatorRepository` | Registry path for the Network Operator controller image. | | `helmRepoURL` | Chart repository used by `l8k deploy`. Empty means Helm phase 0 is skipped. | | `namespace` | Namespace for the Helm release and namespaced Network Operator resources. | -| `imagePullSecrets` | Secret names propagated into the discovery daemon, generated policies, and Helm values for the Network Operator and enabled subcharts. | +| `imagePullSecrets` | Secret names propagated into the discovery daemon, generated policies, and Helm values for the Network Operator and enabled subcharts. During deploy, matching credentials also authenticate the Helm chart download. | When `selectedRelease` is set, catalog values replace explicit version and repository fields so the cohort remains consistent. +For an authenticated Helm repository, each referenced Secret must already +exist in `networkOperator.namespace` before `l8k deploy` starts, and the +kubeconfig must allow `get` on Secrets there. l8k reads +`kubernetes.io/dockerconfigjson` and legacy `kubernetes.io/dockercfg` data in +memory and never logs or persists the credential. Credentials are sent only +when the Docker registry host exactly matches the Helm repository host. The +one explicit cross-host mapping is NGC: `nvcr.io` credentials use the same +`$oauthtoken` and API key required by `helm.ngc.nvidia.com`. Unrelated registry +credentials are never forwarded to the chart server. + ## Network And Workload Namespaces ```yaml diff --git a/docs/user/quick-start.md b/docs/user/quick-start.md index 370e5f7..28d2ea4 100644 --- a/docs/user/quick-start.md +++ b/docs/user/quick-start.md @@ -56,6 +56,12 @@ Deploy runs in phases: 3. Apply per-group `NicNodePolicy` resources and wait for readiness. 4. Apply the remaining CRs and verify each one reaches a terminal state. +If `networkOperator.imagePullSecrets` is configured, create the named Secret +in `networkOperator.namespace` before deploying. Phase 0 reads compatible +Docker credentials through the Kubernetes API to authenticate the chart +download; the kubeconfig therefore needs `get secrets` permission in that +namespace. + Preview the server-side apply without persisting resources: ```bash diff --git a/docs/user/troubleshooting.md b/docs/user/troubleshooting.md index f5dab2b..2c6f185 100644 --- a/docs/user/troubleshooting.md +++ b/docs/user/troubleshooting.md @@ -12,6 +12,7 @@ Validation is the normal deployment acceptance stage. When it does not produce a | Failure area | First checks | | --- | --- | | Discovery | Bootstrap pod status, events, image pulls, and `NicDevice` publication in `nvidia-k8s-launch-kit`. | +| Helm chart download | For an HTTP 401 or image-pull-Secret error, verify each configured Secret exists in `networkOperator.namespace`, the kubeconfig can read it, and its Docker config has credentials for the Helm host or `nvcr.io` for an NGC Helm repository. | | Deploy preflight | Existing Helm values, generated values, stray custom resources, and whether overwrite was explicitly intended. | | Reconciliation | `NicClusterPolicy`, `NicNodePolicy`, and component pod conditions in the Network Operator namespace. | | SR-IOV | `SriovNetworkNodeState` sync status, VF totals, node selectors, and advertised resources. | diff --git a/pkg/cmd/discover.go b/pkg/cmd/discover.go index 9d7fc95..2e66f67 100644 --- a/pkg/cmd/discover.go +++ b/pkg/cmd/discover.go @@ -141,7 +141,7 @@ func init() { fmt.Sprintf("Network Operator release line to deploy (MAJOR.MINOR). Supported: %s", strings.Join(releases.SupportedReleases(), ", "))) discoverCmd.Flags().StringVar(&nodeSelector, "node-selector", "feature.node.kubernetes.io/pci-15b3.present=true", "Node selector written into the saved cluster-config (used at deploy time). Does NOT gate discovery scheduling — the daemon runs on all nodes and NIC nodes are detected via a sysfs PCI-vendor probe") - discoverCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components (comma-separated)") + discoverCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components and authenticated Helm downloads (comma-separated)") discoverCmd.Flags().StringVar(&enabledPlugins, "enabled-plugins", "network-operator", "Comma-separated list of plugins to enable") discoverCmd.Flags().BoolVar(&keepNamespace, "keep-namespace", false, "Skip teardown of the nvidia-k8s-launch-kit namespace (for debugging)") discoverCmd.Flags().BoolVar(&collapseNicRails, "collapse-nic-rails", true, collapseNicRailsFlagHelp) diff --git a/pkg/cmd/generate.go b/pkg/cmd/generate.go index 1cbd89c..9b8a6a3 100644 --- a/pkg/cmd/generate.go +++ b/pkg/cmd/generate.go @@ -220,7 +220,7 @@ func init() { generateCmd.Flags().StringVar(&networkOperatorRelease, "network-operator-release", "", fmt.Sprintf("Network Operator release line to deploy (MAJOR.MINOR). Supported: %s", strings.Join(releases.SupportedReleases(), ", "))) - generateCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components (comma-separated)") + generateCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components and authenticated Helm downloads (comma-separated)") generateCmd.Flags().StringVar(&enabledPlugins, "enabled-plugins", "network-operator", "Comma-separated list of plugins to enable") // Deploy (optional) diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index d0802af..fd742ef 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -278,7 +278,7 @@ func init() { rootCmd.Flags().StringVar(&nodeSelector, "node-selector", "feature.node.kubernetes.io/pci-15b3.present=true", "Node selector written into the saved cluster-config (used at deploy time). Does NOT gate discovery scheduling — the daemon runs on all nodes and NIC nodes are detected via a sysfs PCI-vendor probe") rootCmd.Flags().BoolVar(&collapseNicRails, "collapse-nic-rails", true, collapseNicRailsFlagHelp) rootCmd.Flags().StringVar(&forPreset, "for", "", forFlagHelp()) - rootCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components (comma-separated)") + rootCmd.Flags().StringSliceVar(&imagePullSecrets, "image-pull-secrets", nil, "Image pull secret names for Network Operator components and authenticated Helm downloads (comma-separated)") rootCmd.Flags().StringVar(&saveDeploymentFiles, "save-deployment-files", "./deployment", "Save generated deployment files to the specified directory") rootCmd.Flags().StringSliceVar(&networkNamespaces, "network-namespaces", nil, "Comma-separated namespaces for the secondary-network CRs and example test DaemonSets. One independent copy is rendered per namespace (shared resources like IPPools and NodePolicies are NOT duplicated). Overrides config networkNamespaces; default: 'default'.") rootCmd.Flags().BoolVar(&enableDocaDriver, "enable-doca-driver", false, "Enable DOCA driver deployment (overrides config file docaDriver.enable)") diff --git a/pkg/cmd/schema.go b/pkg/cmd/schema.go index e678526..cf27dc1 100644 --- a/pkg/cmd/schema.go +++ b/pkg/cmd/schema.go @@ -219,7 +219,7 @@ var schemaCmd = &cobra.Command{ }, "--image-pull-secrets": { Type: "[]string", - Description: "Image pull secret names for Network Operator components (comma-separated)", + Description: "Image pull secret names for Network Operator components and authenticated Helm downloads (comma-separated)", }, "--network-operator-release": { Type: "string", diff --git a/pkg/config/default-config.yaml b/pkg/config/default-config.yaml index b822b8c..cc0e05a 100644 --- a/pkg/config/default-config.yaml +++ b/pkg/config/default-config.yaml @@ -25,6 +25,11 @@ networkOperator: # `helm.ngc.nvidia.com/nvidia`; staging under `helm.ngc.nvidia.com/nvstaging/mellanox`. helmRepoURL: https://helm.ngc.nvidia.com/nvidia namespace: nvidia-network-operator + # imagePullSecrets are rendered into every enabled Network Operator + # component. During `l8k deploy`, a matching docker-registry Secret that + # already exists in this namespace is also used to authenticate the Helm + # chart download. For NGC, nvcr.io credentials are valid for + # helm.ngc.nvidia.com. Secret data is read in memory and is never logged. # imagePullSecrets: # - my-registry-secret diff --git a/pkg/networkoperatorplugin/helm.go b/pkg/networkoperatorplugin/helm.go index 46a6c33..0547c9a 100644 --- a/pkg/networkoperatorplugin/helm.go +++ b/pkg/networkoperatorplugin/helm.go @@ -134,12 +134,21 @@ func InstallOrUpgrade( } loadChart := func() (*chart.Chart, error) { - chartPath, cleanup, perr := pullChart(ctx, cfg.HelmRepoURL, networkOperatorChartName, chartVersion) + credentials, cerr := loadHelmRepositoryCredentials(ctx, restConfig, cfg, namespace) + if cerr != nil { + return nil, pkgerrors.NewDeploymentError( + "failed to read configured image pull secrets for Helm repository authentication", + cerr, + fmt.Sprintf("ensure each networkOperator.imagePullSecrets Secret exists in namespace %s and the kubeconfig can get secrets", namespace), + ) + } + chartPath, cleanup, perr := pullChart( + ctx, cfg.HelmRepoURL, networkOperatorChartName, chartVersion, credentials) if perr != nil { return nil, pkgerrors.NewDeploymentError( fmt.Sprintf("failed to fetch network-operator chart %s from %s", chartVersion, cfg.HelmRepoURL), perr, - "verify the helm repository URL and that the chart version exists", + "verify the helm repository URL, chart version, and image pull secret credentials", ) } defer cleanup() @@ -302,7 +311,33 @@ func runUpgrade( // pullChart fetches a chart tarball from repoURL into a temp directory and // returns the local path plus a cleanup func. Uses helm's downloader // directly — no repo cache, no `helm repo add` side effects. -func pullChart(_ context.Context, repoURL, chartName, chartVersion string) (string, func(), error) { +func pullChart( + ctx context.Context, + repoURL, chartName, chartVersion string, + credentials []helmRepositoryCredential, +) (string, func(), error) { + if len(credentials) == 0 { + return pullChartWithCredential(repoURL, chartName, chartVersion, helmRepositoryCredential{}) + } + + var errs []error + for _, credential := range credentials { + if err := ctx.Err(); err != nil { + return "", func() {}, err + } + saved, cleanup, err := pullChartWithCredential(repoURL, chartName, chartVersion, credential) + if err == nil { + return saved, cleanup, nil + } + errs = append(errs, fmt.Errorf("credentials from Secret %q: %w", credential.SourceSecret, err)) + } + return "", func() {}, errors.Join(errs...) +} + +func pullChartWithCredential( + repoURL, chartName, chartVersion string, + credential helmRepositoryCredential, +) (string, func(), error) { tmpDir, err := os.MkdirTemp("", "l8k-helm-chart-*") if err != nil { return "", func() {}, fmt.Errorf("create temp dir for chart pull: %w", err) @@ -316,8 +351,10 @@ func pullChart(_ context.Context, repoURL, chartName, chartVersion string) (stri settings.RepositoryCache = tmpDir getters := getter.All(settings) - chartURL, err := repo.FindChartInRepoURL( + chartURL, err := repo.FindChartInAuthRepoURL( repoURL, + credential.Username, + credential.Password, chartName, chartVersion, "", // certFile @@ -337,6 +374,16 @@ func pullChart(_ context.Context, repoURL, chartName, chartVersion string) (stri RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, } + // An index may point at a chart archive on another host. Do not forward + // the repository credential across that boundary; getter's URL guard is + // relative to chartURL at this stage, not the original repository URL. + if credential.Username != "" && credential.Password != "" && + sameRegistryHost(repoURL, chartURL) { + dl.Options = append(dl.Options, + getter.WithBasicAuth(credential.Username, credential.Password), + getter.WithPassCredentialsAll(false), + ) + } saved, _, err := dl.DownloadTo(chartURL, chartVersion, tmpDir) if err != nil { cleanup() @@ -345,6 +392,12 @@ func pullChart(_ context.Context, repoURL, chartName, chartVersion string) (stri return saved, cleanup, nil } +func sameRegistryHost(left, right string) bool { + leftHost, leftErr := registryHost(left) + rightHost, rightErr := registryHost(right) + return leftErr == nil && rightErr == nil && leftHost == rightHost +} + // isPendingStatus reports whether the helm release is mid-operation // (install/upgrade/rollback). These are the statuses that cause helm's // "another operation in progress" lock — usually a sign that a previous diff --git a/pkg/networkoperatorplugin/helm_credentials.go b/pkg/networkoperatorplugin/helm_credentials.go new file mode 100644 index 0000000..f5d3769 --- /dev/null +++ b/pkg/networkoperatorplugin/helm_credentials.go @@ -0,0 +1,246 @@ +// Copyright 2026 NVIDIA CORPORATION & AFFILIATES. +// +// SPDX-License-Identifier: Apache-2.0 + +package networkoperatorplugin + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "net/url" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/rest" + + "github.com/nvidia/k8s-launch-kit/pkg/config" +) + +const ( + ngcContainerRegistryHost = "nvcr.io" + ngcHelmRepositoryHost = "helm.ngc.nvidia.com" +) + +// helmRepositoryCredential is kept private so repository credentials cannot +// accidentally become part of config serialization or user-facing output. +// sourceSecret is safe to log; Username and Password must never be logged. +type helmRepositoryCredential struct { + Username string + Password string + SourceSecret string +} + +type dockerConfigJSON struct { + Auths map[string]dockerAuthConfig `json:"auths"` +} + +type dockerAuthConfig struct { + Username string `json:"username"` + Password string `json:"password"` + Auth string `json:"auth"` +} + +// loadHelmRepositoryCredentials reads the configured image pull Secrets from +// the Network Operator namespace. Kubernetes imagePullSecrets are not +// automatically consulted by Helm, but an NGC docker-registry Secret carries +// the same $oauthtoken/API-key pair used by helm.ngc.nvidia.com. +func loadHelmRepositoryCredentials( + ctx context.Context, + restConfig *rest.Config, + cfg *config.NetworkOperatorConfig, + namespace string, +) ([]helmRepositoryCredential, error) { + if cfg == nil || len(cfg.ImagePullSecrets) == 0 { + return nil, nil + } + + clientset, err := kubernetes.NewForConfig(restConfig) + if err != nil { + return nil, fmt.Errorf("create Kubernetes client for image pull secrets: %w", err) + } + + return credentialsFromImagePullSecrets( + ctx, + clientset.CoreV1().Secrets(namespace), + cfg.ImagePullSecrets, + cfg.HelmRepoURL, + namespace, + ) +} + +// credentialsFromImagePullSecrets returns credentials in configured Secret +// order. Credentials are accepted only for the exact chart repository host, +// plus nvcr.io when the chart repository is helm.ngc.nvidia.com. The latter is +// an intentional NVIDIA-specific mapping: both endpoints use the same NGC +// $oauthtoken/API-key credential. Never forward credentials for an unrelated +// image registry to the chart host. +func credentialsFromImagePullSecrets( + ctx context.Context, + secrets corev1client.SecretInterface, + secretNames []string, + helmRepoURL string, + namespace string, +) ([]helmRepositoryCredential, error) { + targetHosts, err := helmCredentialHosts(helmRepoURL) + if err != nil { + return nil, err + } + + credentials := make([]helmRepositoryCredential, 0, len(secretNames)) + var errs []error + for _, secretName := range secretNames { + secret, getErr := secrets.Get(ctx, secretName, metav1.GetOptions{}) + if getErr != nil { + errs = append(errs, fmt.Errorf( + "get image pull secret %q in namespace %q: %w", + secretName, namespace, getErr)) + continue + } + + auths, parseErr := dockerAuthsFromSecret(secret) + if parseErr != nil { + errs = append(errs, fmt.Errorf( + "parse image pull secret %q in namespace %q: %w", + secretName, namespace, parseErr)) + continue + } + + credential, found, credentialErr := matchingDockerCredential(auths, targetHosts) + if credentialErr != nil { + errs = append(errs, fmt.Errorf( + "parse credentials in image pull secret %q in namespace %q: %w", + secretName, namespace, credentialErr)) + continue + } + if !found { + // The Secret may be required by a component image from another + // registry. It is valid for values.yaml but must not be sent to + // an unrelated Helm repository. + continue + } + + credential.SourceSecret = secretName + credentials = append(credentials, credential) + } + + // Match kubelet's ordered imagePullSecrets behavior: one usable Secret is + // sufficient even when another configured Secret is absent or malformed. + // If none can authenticate this repository, surface the lookup/parse + // failures instead of letting Helm fail later with an opaque HTTP 401. + if len(credentials) > 0 { + return credentials, nil + } + if len(errs) > 0 { + return nil, errors.Join(errs...) + } + return nil, nil +} + +func helmCredentialHosts(repoURL string) ([]string, error) { + host, err := registryHost(repoURL) + if err != nil { + return nil, fmt.Errorf("parse Helm repository URL %q: %w", repoURL, err) + } + hosts := []string{host} + if host == ngcHelmRepositoryHost { + hosts = append(hosts, ngcContainerRegistryHost) + } + return hosts, nil +} + +func dockerAuthsFromSecret(secret *corev1.Secret) (map[string]dockerAuthConfig, error) { + if secret == nil { + return nil, errors.New("secret is nil") + } + + if raw, ok := secret.Data[corev1.DockerConfigJsonKey]; ok { + var cfg dockerConfigJSON + if err := json.Unmarshal(raw, &cfg); err != nil { + return nil, fmt.Errorf("decode %s: %w", corev1.DockerConfigJsonKey, err) + } + if len(cfg.Auths) == 0 { + return nil, fmt.Errorf("%s contains no auth entries", corev1.DockerConfigJsonKey) + } + return cfg.Auths, nil + } + + if raw, ok := secret.Data[corev1.DockerConfigKey]; ok { + var auths map[string]dockerAuthConfig + if err := json.Unmarshal(raw, &auths); err != nil { + return nil, fmt.Errorf("decode %s: %w", corev1.DockerConfigKey, err) + } + if len(auths) == 0 { + return nil, fmt.Errorf("%s contains no auth entries", corev1.DockerConfigKey) + } + return auths, nil + } + + return nil, fmt.Errorf("secret has neither %s nor %s data", corev1.DockerConfigJsonKey, corev1.DockerConfigKey) +} + +func matchingDockerCredential( + auths map[string]dockerAuthConfig, + targetHosts []string, +) (helmRepositoryCredential, bool, error) { + for _, targetHost := range targetHosts { + for registry, auth := range auths { + host, err := registryHost(registry) + if err != nil || host != targetHost { + continue + } + credential, err := credentialFromDockerAuth(auth) + if err != nil { + return helmRepositoryCredential{}, false, + fmt.Errorf("registry %q: %w", registry, err) + } + return credential, true, nil + } + } + return helmRepositoryCredential{}, false, nil +} + +func credentialFromDockerAuth(auth dockerAuthConfig) (helmRepositoryCredential, error) { + if auth.Username != "" || auth.Password != "" { + if auth.Username == "" || auth.Password == "" { + return helmRepositoryCredential{}, errors.New("username and password must both be set") + } + return helmRepositoryCredential{Username: auth.Username, Password: auth.Password}, nil + } + + if auth.Auth == "" { + return helmRepositoryCredential{}, errors.New("auth entry has no username/password or auth token") + } + decoded, err := base64.StdEncoding.DecodeString(auth.Auth) + if err != nil { + return helmRepositoryCredential{}, fmt.Errorf("decode auth token: %w", err) + } + username, password, ok := strings.Cut(string(decoded), ":") + if !ok || username == "" || password == "" { + return helmRepositoryCredential{}, errors.New("decoded auth token is not username:password") + } + return helmRepositoryCredential{Username: username, Password: password}, nil +} + +func registryHost(raw string) (string, error) { + value := strings.TrimSpace(raw) + if value == "" { + return "", errors.New("registry URL is empty") + } + if !strings.Contains(value, "://") { + value = "https://" + value + } + parsed, err := url.Parse(value) + if err != nil { + return "", err + } + if parsed.Host == "" { + return "", errors.New("registry URL has no host") + } + return strings.ToLower(parsed.Host), nil +} diff --git a/pkg/networkoperatorplugin/helm_test.go b/pkg/networkoperatorplugin/helm_test.go index 0fe3c65..77ca2f8 100644 --- a/pkg/networkoperatorplugin/helm_test.go +++ b/pkg/networkoperatorplugin/helm_test.go @@ -6,8 +6,15 @@ package networkoperatorplugin import ( "context" + "encoding/base64" + "encoding/json" "errors" + "fmt" "io" + "net/http" + "net/http/httptest" + "os" + "sync/atomic" "testing" "time" @@ -16,11 +23,15 @@ import ( "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" + chartloader "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" kubefake "helm.sh/helm/v3/pkg/kube/fake" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage" "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sfake "k8s.io/client-go/kubernetes/fake" "github.com/nvidia/k8s-launch-kit/pkg/networkoperatorplugin/helmclient" ) @@ -326,3 +337,250 @@ func TestInstallOrUpgrade_RejectsEmptyCfg(t *testing.T) { err := InstallOrUpgrade(context.Background(), nil, nil, []byte("nfd: {}\n"), false, 30*time.Second, false) require.Error(t, err) } + +func TestCredentialsFromImagePullSecrets_NGCRegistryCredential(t *testing.T) { + const ( + namespace = "nvidia-network-operator" + secretName = "ngc-image-secret" + username = "$oauthtoken" + password = "test-api-key" + ) + + dockerConfig, err := json.Marshal(dockerConfigJSON{ + Auths: map[string]dockerAuthConfig{ + "nvcr.io": { + Username: username, + Password: password, + }, + }, + }) + require.NoError(t, err) + + clientset := k8sfake.NewSimpleClientset(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace}, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{corev1.DockerConfigJsonKey: dockerConfig}, + }) + + credentials, err := credentialsFromImagePullSecrets( + context.Background(), + clientset.CoreV1().Secrets(namespace), + []string{secretName}, + "https://helm.ngc.nvidia.com/nvstaging/mellanox", + namespace, + ) + require.NoError(t, err) + require.Len(t, credentials, 1) + assert.Equal(t, username, credentials[0].Username) + assert.Equal(t, password, credentials[0].Password) + assert.Equal(t, secretName, credentials[0].SourceSecret) +} + +func TestCredentialsFromImagePullSecrets_ExactHelmHostLegacySecret(t *testing.T) { + const ( + namespace = "operator-system" + secretName = "chart-creds" + ) + + auth := base64.StdEncoding.EncodeToString([]byte("chart-user:chart-password")) + dockerConfig, err := json.Marshal(map[string]dockerAuthConfig{ + "https://charts.example.com/v1/": {Auth: auth}, + }) + require.NoError(t, err) + + clientset := k8sfake.NewSimpleClientset(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace}, + Type: corev1.SecretTypeDockercfg, + Data: map[string][]byte{corev1.DockerConfigKey: dockerConfig}, + }) + + credentials, err := credentialsFromImagePullSecrets( + context.Background(), + clientset.CoreV1().Secrets(namespace), + []string{secretName}, + "https://charts.example.com/networking", + namespace, + ) + require.NoError(t, err) + require.Len(t, credentials, 1) + assert.Equal(t, "chart-user", credentials[0].Username) + assert.Equal(t, "chart-password", credentials[0].Password) +} + +func TestCredentialsFromImagePullSecrets_DoesNotForwardUnrelatedRegistry(t *testing.T) { + const ( + namespace = "nvidia-network-operator" + secretName = "unrelated-registry" + ) + + dockerConfig, err := json.Marshal(dockerConfigJSON{ + Auths: map[string]dockerAuthConfig{ + "registry.internal.example.com": { + Username: "private-user", + Password: "private-password", + }, + }, + }) + require.NoError(t, err) + + clientset := k8sfake.NewSimpleClientset(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace}, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{corev1.DockerConfigJsonKey: dockerConfig}, + }) + + credentials, err := credentialsFromImagePullSecrets( + context.Background(), + clientset.CoreV1().Secrets(namespace), + []string{secretName}, + "https://helm.ngc.nvidia.com/nvidia", + namespace, + ) + require.NoError(t, err) + assert.Empty(t, credentials) +} + +func TestCredentialsFromImagePullSecrets_MissingSecret(t *testing.T) { + const namespace = "nvidia-network-operator" + clientset := k8sfake.NewSimpleClientset() + + credentials, err := credentialsFromImagePullSecrets( + context.Background(), + clientset.CoreV1().Secrets(namespace), + []string{"missing-secret"}, + "https://helm.ngc.nvidia.com/nvidia", + namespace, + ) + require.Error(t, err) + assert.Empty(t, credentials) + assert.Contains(t, err.Error(), "missing-secret") + assert.Contains(t, err.Error(), namespace) +} + +func TestPullChart_UsesImagePullSecretCredentials(t *testing.T) { + const ( + username = "$oauthtoken" + password = "test-api-key" + version = "0.0.0" + ) + + chartPath, err := chartutil.Save(newTestChart(t), t.TempDir()) + require.NoError(t, err) + chartArchive, err := os.ReadFile(chartPath) + require.NoError(t, err) + + var totalRequests atomic.Int32 + var authenticatedRequests atomic.Int32 + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + totalRequests.Add(1) + requestUsername, requestPassword, ok := r.BasicAuth() + if !ok || requestUsername != username || requestPassword != password { + w.Header().Set("WWW-Authenticate", `Basic realm="test"`) + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + authenticatedRequests.Add(1) + + switch r.URL.Path { + case "/index.yaml": + _, _ = fmt.Fprintf(w, `apiVersion: v1 +entries: + network-operator: + - apiVersion: v2 + name: network-operator + version: %s + urls: + - %s/network-operator-%s.tgz +`, version, server.URL, version) + case "/network-operator-" + version + ".tgz": + w.Header().Set("Content-Type", "application/gzip") + _, _ = w.Write(chartArchive) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + pulledPath, cleanup, err := pullChart( + context.Background(), + server.URL, + networkOperatorChartName, + version, + []helmRepositoryCredential{ + { + Username: "expired-user", + Password: "expired-password", + SourceSecret: "expired-secret", + }, + { + Username: username, + Password: password, + SourceSecret: "ngc-image-secret", + }, + }, + ) + require.NoError(t, err) + defer cleanup() + + pulledChart, err := chartloader.Load(pulledPath) + require.NoError(t, err) + assert.Equal(t, networkOperatorChartName, pulledChart.Name()) + assert.EqualValues(t, 2, authenticatedRequests.Load(), "index and chart archive must both use credentials") + assert.EqualValues(t, 3, totalRequests.Load(), "downloader must try configured credentials in order") +} + +func TestPullChart_DoesNotForwardCredentialsToCrossHostChart(t *testing.T) { + const ( + username = "chart-user" + password = "chart-password" + version = "0.0.0" + ) + + chartPath, err := chartutil.Save(newTestChart(t), t.TempDir()) + require.NoError(t, err) + chartArchive, err := os.ReadFile(chartPath) + require.NoError(t, err) + + var archiveReceivedAuth atomic.Bool + archiveServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _, received := r.BasicAuth() + archiveReceivedAuth.Store(received) + w.Header().Set("Content-Type", "application/gzip") + _, _ = w.Write(chartArchive) + })) + defer archiveServer.Close() + + indexServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestUsername, requestPassword, ok := r.BasicAuth() + if !ok || requestUsername != username || requestPassword != password { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + _, _ = fmt.Fprintf(w, `apiVersion: v1 +entries: + network-operator: + - apiVersion: v2 + name: network-operator + version: %s + urls: + - %s/network-operator-%s.tgz +`, version, archiveServer.URL, version) + })) + defer indexServer.Close() + + _, cleanup, err := pullChart( + context.Background(), + indexServer.URL, + networkOperatorChartName, + version, + []helmRepositoryCredential{{ + Username: username, + Password: password, + SourceSecret: "chart-creds", + }}, + ) + require.NoError(t, err) + defer cleanup() + assert.False(t, archiveReceivedAuth.Load(), "repository credentials must not cross chart hosts") +} diff --git a/pkg/options/options.go b/pkg/options/options.go index fb1adfb..215a542 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -49,7 +49,7 @@ type Options struct { // a full semver. Selects component image tags + repository from the // embedded releases catalog and drives version-gated template sections. NetworkOperatorRelease string - ImagePullSecrets []string // Image pull secret names for NicClusterPolicy + ImagePullSecrets []string // Image pull secret names for Network Operator components and Helm repository authentication // Phase 2: Deployment Generation Fabric string // Fabric type to deploy diff --git a/skills/k8s-launch-kit-deploy/SKILL.md b/skills/k8s-launch-kit-deploy/SKILL.md index d6863ad..26f6972 100644 --- a/skills/k8s-launch-kit-deploy/SKILL.md +++ b/skills/k8s-launch-kit-deploy/SKILL.md @@ -25,6 +25,14 @@ l8k deploy [--deployment-files ] [--kubeconfig ] [--dry-run] When the deployment directory contains a `values.yaml` (the `l8k generate` profile renderer emits one per profile), Phase 0 runs first: the Helm Go SDK installs (or upgrades, with `--overwrite-existing`) the `nvidia/network-operator` chart in the namespace from `networkOperator.namespace`. The chart version and Helm repo URL come from the embedded release catalog selected via `--network-operator-release`. Phase 0 is skipped silently when `values.yaml` is absent — backward compatible with users managing the chart out of band. +When `networkOperator.imagePullSecrets` is non-empty, l8k reads the named +Docker Secrets from the operator namespace and uses compatible credentials to +authenticate both the Helm repository index and chart archive requests. The +Secret must exist before Phase 0, and the kubeconfig must allow `get secrets`. +l8k never logs or persists the credential. It sends credentials only to an +exact matching chart host, with one intentional NGC mapping from `nvcr.io` to +`helm.ngc.nvidia.com`; unrelated registry credentials are not forwarded. + Network Operator 26.1+ requestor mode is a Helm-level change: the generated values add Network Operator Deployment environment variables and enable the SR-IOV external drainer where applicable. Applying only the generated CRs diff --git a/skills/k8s-launch-kit-discover/SKILL.md b/skills/k8s-launch-kit-discover/SKILL.md index 09514fb..21ac20d 100644 --- a/skills/k8s-launch-kit-discover/SKILL.md +++ b/skills/k8s-launch-kit-discover/SKILL.md @@ -55,7 +55,7 @@ l8k discover --save-cluster-config [--kubeconfig ] | `--network-operator-namespace` | — | — | **Deprecated for `discover`**: accepted but ignored. The daemon always runs in `nvidia-k8s-launch-kit`. Still used by `l8k generate` / `l8k deploy`. | | `--user-config` | — | — | Base config to merge with discovered hardware | | `--node-selector` | — | `feature.node.kubernetes.io/pci-15b3.present=true` | Value written into the **saved** `cluster-config.yaml` `nodeSelector` (for deploy time). It does **not** gate discovery scheduling or the NicDevice wait set — the daemon is restricted to Ready schedulable nodes and NIC-bearing nodes are detected via a sysfs `0x15b3` probe. | -| `--image-pull-secrets` | — | — | Image pull secret names (comma-separated). Forwarded to the discovery DaemonSet and persisted for generated policies and Network Operator Helm values. | +| `--image-pull-secrets` | — | — | Image pull secret names (comma-separated). Forwarded to the discovery DaemonSet and persisted for generated policies, Network Operator Helm values, and authenticated chart downloads during deploy. | | `--fabric` | — | discovered unanimous link type | Fabric override: `ethernet` or `infiniband`. | | `--deployment-type` | — | `sriov` | Deployment override: `sriov`, `rdma_shared`, or `host_device`. | | `--multirail` | — | `true` | Multirail override. Use `--multirail=false` to opt out; explicit false is persisted. | diff --git a/skills/k8s-launch-kit-shared/SKILL.md b/skills/k8s-launch-kit-shared/SKILL.md index ee92085..838f34f 100644 --- a/skills/k8s-launch-kit-shared/SKILL.md +++ b/skills/k8s-launch-kit-shared/SKILL.md @@ -64,7 +64,7 @@ The root command `l8k --discover-cluster-config ...` still works for backward-co | `--network-operator-namespace ` | Override network operator namespace (default: `nvidia-network-operator`). **No-op for `l8k discover`** — discover always bootstraps into `nvidia-k8s-launch-kit`; the flag still applies to `l8k generate` / `l8k deploy` / `l8k validate`. | | `--network-namespaces ` | Comma-separated namespaces for the secondary-network CRs + example test DaemonSets; one copy rendered per namespace (shared resources like IPPools/NodePolicies are NOT duplicated). Default: `default` | | `--node-selector ` | Restrict to nodes matching labels (comma-separated, ANDed) | -| `--image-pull-secrets ` | Image pull secret names for Network Operator components (comma-separated) | +| `--image-pull-secrets ` | Image pull secret names for Network Operator components and authenticated Helm chart downloads (comma-separated) | `l8k discover` and `l8k generate` both accept the profile flags `--fabric`, `--deployment-type`, `--multirail`, `--spectrum-x`, `--multiplane-mode`, and diff --git a/skills/k8s-launch-kit-troubleshoot/SKILL.md b/skills/k8s-launch-kit-troubleshoot/SKILL.md index f415850..cbebca1 100644 --- a/skills/k8s-launch-kit-troubleshoot/SKILL.md +++ b/skills/k8s-launch-kit-troubleshoot/SKILL.md @@ -53,6 +53,7 @@ kubectl get pods -A -o wide | grep -E 'ContainerCreating|Init' | `CrashLoopBackOff` on mofed pods | Kernel module conflict | Check `thirdPartyRDMAModules`, enable `unloadThirdPartyRDMAModules` | | No VFs on node | SriovNetworkNodePolicy not matching | Verify `nodeSelector` labels match worker nodes | | RDMA not working | Missing RDMA device plugin or wrong resource name | Check `rdma-shared-dp` pods, verify resource annotations | +| Phase 0 Helm chart download returns HTTP 401 or an image-pull-Secret error | The configured Secret is missing from the operator namespace, unreadable by the kubeconfig, or has no compatible Docker auth entry | Verify the Secret in `networkOperator.namespace`; for NGC, its `.dockerconfigjson` must contain `nvcr.io` credentials | | `l8k discover` daemon pods stuck (ImagePullBackOff / Pending) | Bad image tag, missing pull secret, or no `feature.node.kubernetes.io/pci-15b3.present=true` nodes | Re-run with `--keep-namespace` then `kubectl describe pod -n nvidia-k8s-launch-kit`. Fix `networkOperator.componentVersion` / pass `--image-pull-secrets` / verify NFD is running. | | `l8k validate` / `deploy` can't find Network Operator pods | Operator namespace mismatch | Verify `--network-operator-namespace` matches actual namespace (does NOT apply to `l8k discover` — it ignores the flag and uses its own `nvidia-k8s-launch-kit` namespace) | | IPPool not allocating | NV-IPAM subnet exhausted or misconfigured | Check `ippools` CR status, verify CIDR ranges |