diff --git a/cmd/generate-config/config/config-openapi-spec.json b/cmd/generate-config/config/config-openapi-spec.json index e677bbe94b..31ee73a012 100755 --- a/cmd/generate-config/config/config-openapi-spec.json +++ b/cmd/generate-config/config/config-openapi-spec.json @@ -1009,7 +1009,7 @@ } }, "kubelet": { - "description": "Settings specified in this section are transferred as-is into the Kubelet config." + "description": "Settings specified in this section are transferred as-is into the Kubelet config,\nexcept imageCredentialProviderConfigPath and imageCredentialProviderBinDir, which\nenable the kubelet image credential provider and are applied as kubelet startup\nflags. Both must be set together, be absolute paths, and be owned by root and not\nwritable by group or others, including parent directories and contents." }, "manifests": { "type": "object", diff --git a/docs/user/howto_config.md b/docs/user/howto_config.md index 2ae85bb83e..5c14618d3a 100644 --- a/docs/user/howto_config.md +++ b/docs/user/howto_config.md @@ -551,6 +551,54 @@ those volumes must then be manually deleted by the user. Once the MicroShift con supported values, the user may restart MicroShift. They should see that MicroShift does not redeploy the disabled components after restart. +## Kubelet Image Credential Provider + +The `kubelet` section is normally passed through as-is into the kubelet +configuration. Two keys are the exception: `imageCredentialProviderConfigPath` +and `imageCredentialProviderBinDir` are consumed by MicroShift and applied as +kubelet startup flags. They enable the kubelet +[image credential provider](https://kubernetes.io/docs/tasks/administer-cluster/kubelet-credential-provider/), +which lets kubelet obtain registry credentials from an external provider +binary at image pull time instead of relying on static credentials in CRI-O. +This is intended for token-based registries such as Amazon ECR, whose +credentials expire after a short time. + +```yaml +kubelet: + imageCredentialProviderConfigPath: /etc/microshift/credential-providers.yaml + imageCredentialProviderBinDir: /usr/libexec/microshift/credential-providers +``` + +`imageCredentialProviderConfigPath` is the path to a kubelet +`CredentialProviderConfig` file, or to a directory of such files. +`imageCredentialProviderBinDir` is the directory containing the provider +binaries named by that configuration. MicroShift does not ship any provider +binary; obtain the one for your registry (for example `ecr-credential-provider` +from the upstream `kubernetes/cloud-provider-aws` project) and install it +yourself. On image-based systems the binary must be included in every OS image +build, since `/usr` is replaced on each update. + +Place the bin directory under `/usr/libexec` or `/usr/local/bin`, which carry +the `bin_t` SELinux label that the confined kubelet (`kubelet_t`) is permitted +to execute. A bin directory under `/etc/microshift` (labeled +`kubernetes_file_t`) or `/opt` (labeled `usr_t`) passes MicroShift's path +validation but is denied execution under SELinux enforcing: the provider never +runs, the image pull fails, and the only trace is an AVC denial in the audit +log (`ausearch -m AVC -ts recent`). MicroShift does not validate SELinux +labels, so this is a placement rule you must follow. + +Both keys must be set together and must be absolute paths. Because the +provider binary runs with kubelet's privileges, MicroShift refuses to start +unless both paths, all of their parent directories, and every file inside a +directory are owned by root and not writable by group or others. Symbolic +links are resolved and the resolved path is checked and passed to kubelet. +When the keys are omitted, kubelet starts without a credential provider, as +before. + +Changing either key or the provider configuration file requires a MicroShift +restart. On startup with a valid configuration, the journal contains +`Kubelet image credential provider configured` with the paths in use. + ## Drop-in configuration directory In addition to the existing `/etc/microshift/config.yaml` configuration file there is a `/etc/microshift/config.d` configuration directory where you can place fragments of configuration. diff --git a/packaging/microshift/config.yaml b/packaging/microshift/config.yaml index a6171e6c76..5bfc80a30b 100644 --- a/packaging/microshift/config.yaml +++ b/packaging/microshift/config.yaml @@ -664,7 +664,11 @@ ingress: # If unset, the default timeout is 1h tunnelTimeout: 1h -# Settings specified in this section are transferred as-is into the Kubelet config. +# Settings specified in this section are transferred as-is into the Kubelet config, +# except imageCredentialProviderConfigPath and imageCredentialProviderBinDir, which +# enable the kubelet image credential provider and are applied as kubelet startup +# flags. Both must be set together, be absolute paths, and be owned by root and not +# writable by group or others, including parent directories and contents. kubelet: manifests: # The locations on the filesystem to scan for kustomization diff --git a/pkg/config/config.go b/pkg/config/config.go index 35fafe3a26..d6b865e34d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -56,7 +56,11 @@ type Config struct { Ingress IngressConfig `json:"ingress"` Storage Storage `json:"storage"` Telemetry Telemetry `json:"telemetry"` - // Settings specified in this section are transferred as-is into the Kubelet config. + // Settings specified in this section are transferred as-is into the Kubelet config, + // except imageCredentialProviderConfigPath and imageCredentialProviderBinDir, which + // enable the kubelet image credential provider and are applied as kubelet startup + // flags. Both must be set together, be absolute paths, and be owned by root and not + // writable by group or others, including parent directories and contents. // +kubebuilder:validation:Schemaless Kubelet map[string]any `json:"kubelet"` @@ -67,6 +71,19 @@ type Config struct { // Internal-only fields userSettings *Config `json:"-"` // the values read from the config file + // Image credential provider paths. These are kubelet flags, not + // KubeletConfiguration fields. The exported fields hold the canonical + // (symlink-resolved) paths computed by validateKubeletCredentialProvider(); + // the kubelet component reads them through + // KubeletImageCredentialProviderPaths(). The raw fields hold the values read + // from the Kubelet map during updateComputedValues() and are the input to + // validation. Keeping the two separate means a later updateComputedValues() + // cannot revert a validated path back to the unresolved user value. + KubeletImageCredentialProviderConfigPath string `json:"-"` + KubeletImageCredentialProviderBinDir string `json:"-"` + kubeletImageCredentialProviderConfigPathRaw string `json:"-"` + kubeletImageCredentialProviderBinDirRaw string `json:"-"` + MultiNode MultiNodeConfig `json:"-"` // the value read from commond line Warnings []string `json:"-"` // Warnings that should not prevent the service from starting. @@ -587,6 +604,10 @@ func (c *Config) updateComputedValues() error { c.C2CC.stripEmptyRemoteClusters() c.C2CC.resolveRoutingDefaults() + if err := c.readKubeletCredentialProviderKeys(); err != nil { + return err + } + return nil } @@ -745,6 +766,9 @@ func (c *Config) validate() error { return fmt.Errorf("error validating clusterToCluster: %w", err) } } + if err := c.validateKubeletCredentialProvider(); err != nil { + return err + } return nil } diff --git a/pkg/config/kubelet.go b/pkg/config/kubelet.go new file mode 100644 index 0000000000..b07e3b8a70 --- /dev/null +++ b/pkg/config/kubelet.go @@ -0,0 +1,534 @@ +package config + +import ( + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "syscall" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" + kubeletconfigv1 "k8s.io/kubernetes/pkg/kubelet/apis/config/v1" + kubeletconfigv1alpha1 "k8s.io/kubernetes/pkg/kubelet/apis/config/v1alpha1" + kubeletconfigv1beta1 "k8s.io/kubernetes/pkg/kubelet/apis/config/v1beta1" +) + +// credentialProviderCodec returns the same strict decoder the vendored kubelet +// uses to read the credential provider configuration: see the scheme setup in +// vendor/k8s.io/kubernetes/pkg/credentialprovider/plugin/plugin.go and decode() +// in the sibling config.go. Strict decoding rejects unknown fields, and all +// three API versions kubelet accepts (v1alpha1, v1beta1, v1 of +// kubelet.config.k8s.io) are registered together with the internal type and its +// conversions. Building the decoder from the same vendored packages keeps this +// structural check from diverging from the kubelet compiled into the same +// binary; a lenient decoder would let a typo'd field through to the os.Exit at +// registration. +// +// The scheme is built lazily on first use so that importers of pkg/config that +// never touch the feature (generate-config, show-config without it) do not pay +// for four AddToScheme calls at package init. +var credentialProviderCodec = sync.OnceValue(func() runtime.Decoder { + s := runtime.NewScheme() + utilruntime.Must(kubeletconfig.AddToScheme(s)) + utilruntime.Must(kubeletconfigv1alpha1.AddToScheme(s)) + utilruntime.Must(kubeletconfigv1beta1.AddToScheme(s)) + utilruntime.Must(kubeletconfigv1.AddToScheme(s)) + return serializer.NewCodecFactory(s, serializer.EnableStrict).UniversalDecoder() +}) + +// isNotExistErr reports whether err means the path cannot exist. It covers both +// a plain "no such file or directory" and ENOTDIR, which EvalSymlinks returns +// when a non-directory appears mid-path (e.g. "/etc/cp.yaml/extra" where +// cp.yaml is a regular file). +func isNotExistErr(err error) bool { + return os.IsNotExist(err) || errors.Is(err, syscall.ENOTDIR) +} + +const ( + // These are configuration key names, not credentials. + kubeletImageCredentialProviderConfigPathKey = "imageCredentialProviderConfigPath" //nolint:gosec // G101: not a credential + kubeletImageCredentialProviderBinDirKey = "imageCredentialProviderBinDir" //nolint:gosec // G101: not a credential +) + +// kubeletReservedKeys lists the keys under the kubelet: section that MicroShift +// consumes itself (as kubelet startup flags) instead of passing through into the +// generated KubeletConfiguration. +var kubeletReservedKeys = []string{ + kubeletImageCredentialProviderConfigPathKey, + kubeletImageCredentialProviderBinDirKey, +} + +// ownershipFn reports the owning uid and mode of an already symlink-resolved +// path. Production passes lstatOwnership; tests inject a fake so the +// trusted-path ownership rules can be exercised without running as root. +type ownershipFn func(path string) (uid uint32, mode os.FileMode, err error) + +// lstatOwnership is the production ownershipFn. +func lstatOwnership(path string) (uint32, os.FileMode, error) { + fi, err := os.Lstat(path) + if err != nil { + return 0, 0, err + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0, fmt.Errorf("unable to determine ownership of %q", path) + } + return st.Uid, fi.Mode(), nil +} + +// trustChecker applies the trusted-path rule to path chains, memoizing the +// components it has already verified so a shared ancestor (notably every entry +// under one bin directory) is stat'd once per validation. +type trustChecker struct { + ownership ownershipFn + verified map[string]struct{} +} + +func newTrustChecker(ownership ownershipFn) *trustChecker { + return &trustChecker{ownership: ownership, verified: make(map[string]struct{})} +} + +// checkChain walks every component of the canonical (already symlink-resolved) +// path from / to the final object and requires each to be owned by root and not +// writable by group or others. Components verified earlier in the same +// validation are skipped. +func (tc *trustChecker) checkChain(canonical string) error { + for _, component := range trustedPathComponents(canonical) { + if _, ok := tc.verified[component]; ok { + continue + } + uid, mode, err := tc.ownership(component) + if err != nil { + return err + } + if uid != 0 || mode&0o022 != 0 { + return fmt.Errorf("%q must be owned by root and not writable by group or others", component) + } + tc.verified[component] = struct{}{} + } + return nil +} + +// readKubeletCredentialProviderKeys copies the two credential-provider keys from +// the schemaless kubelet map into the raw (unexported) Config fields. It runs on +// every updateComputedValues(); it never touches the canonical exported fields, +// which are owned exclusively by validateKubeletCredentialProvider(), so +// re-running computed-value processing cannot revert a validated path back to the +// unresolved user value. c.Kubelet is left untouched. +func (c *Config) readKubeletCredentialProviderKeys() error { + configPath, err := kubeletStringValue(c.Kubelet, kubeletImageCredentialProviderConfigPathKey) + if err != nil { + return err + } + binDir, err := kubeletStringValue(c.Kubelet, kubeletImageCredentialProviderBinDirKey) + if err != nil { + return err + } + c.kubeletImageCredentialProviderConfigPathRaw = configPath + c.kubeletImageCredentialProviderBinDirRaw = binDir + return nil +} + +// KubeletImageCredentialProviderPaths returns the canonical (symlink-resolved) +// credential-provider paths computed by validateKubeletCredentialProvider(), and +// whether the feature is active. The kubelet component reads the paths through +// this accessor rather than the fields, so the canonical values it hands to +// kubelet cannot be reverted by a later updateComputedValues(). +func (c *Config) KubeletImageCredentialProviderPaths() (configPath, binDir string, enabled bool) { + if c.KubeletImageCredentialProviderConfigPath == "" { + return "", "", false + } + return c.KubeletImageCredentialProviderConfigPath, c.KubeletImageCredentialProviderBinDir, true +} + +// kubeletStringValue reads key from the kubelet map. A missing key, an explicit +// null, or an empty string all mean "unset"; a non-string value is an error. +func kubeletStringValue(m map[string]any, key string) (string, error) { + if m == nil { + return "", nil + } + raw, ok := m[key] + if !ok || raw == nil { + return "", nil + } + s, ok := raw.(string) + if !ok { + return "", fmt.Errorf("kubelet.%s must be a string, got %T", key, raw) + } + return s, nil +} + +// KubeletPassthrough returns a copy of the kubelet map with the MicroShift-owned +// keys removed, so only genuine KubeletConfiguration settings are written to the +// generated kubelet config file. A nil map returns nil. +func (c *Config) KubeletPassthrough() map[string]any { + if c.Kubelet == nil { + return nil + } + out := maps.Clone(c.Kubelet) + for _, k := range kubeletReservedKeys { + delete(out, k) + } + return out +} + +// credentialPathKind selects which object types validateCredentialProviderPath +// accepts for the final object. +type credentialPathKind int + +const ( + // credentialProviderConfigKind accepts a regular file or a directory. + credentialProviderConfigKind credentialPathKind = iota + // credentialProviderBinDirKind accepts only a directory. + credentialProviderBinDirKind +) + +// validateKubeletCredentialProvider validates the two credential-provider keys +// using the real filesystem for ownership checks. +func (c *Config) validateKubeletCredentialProvider() error { + return c.validateKubeletCredentialProviderWith(lstatOwnership) +} + +// validateKubeletCredentialProviderWith validates the two credential-provider +// keys, resolving ownership through the supplied hook. The rules are applied in +// order and the first failure wins. On success the canonical (symlink-resolved) +// paths are stored in the exported fields the kubelet component reads through +// KubeletImageCredentialProviderPaths(); the raw fields and c.Kubelet are left +// untouched. The exported fields are recomputed from the raw values on every +// call, so validation is idempotent. +func (c *Config) validateKubeletCredentialProviderWith(ownership ownershipFn) error { + configPath := c.kubeletImageCredentialProviderConfigPathRaw + binDir := c.kubeletImageCredentialProviderBinDirRaw + + // Clear any previously computed canonical values so a failed (or now-inactive) + // configuration cannot leave stale paths behind. + c.KubeletImageCredentialProviderConfigPath = "" + c.KubeletImageCredentialProviderBinDir = "" + + // Neither set: the feature is inactive. + if configPath == "" && binDir == "" { + return nil + } + + // Both keys must be provided together. + if configPath == "" || binDir == "" { + return fmt.Errorf("kubelet.%s and kubelet.%s must be set together", + kubeletImageCredentialProviderConfigPathKey, kubeletImageCredentialProviderBinDirKey) + } + + paths := []struct { + key string + value string + kind credentialPathKind + }{ + {kubeletImageCredentialProviderConfigPathKey, configPath, credentialProviderConfigKind}, + {kubeletImageCredentialProviderBinDirKey, binDir, credentialProviderBinDirKind}, + } + + // A single checker verifies both paths (and, for the bin dir, its entries), + // so shared ancestors are stat'd once. Absolute check and canonicalization + // happen in one pass; a relative second path is therefore reported only after + // the first path has resolved. + checker := newTrustChecker(ownership) + canonical := make([]string, len(paths)) + for i, p := range paths { + if !filepath.IsAbs(p.value) { + return fmt.Errorf("kubelet.%s (%q) must be an absolute path", p.key, p.value) + } + resolved, err := validateCredentialProviderPath(p.value, p.kind, checker) + if err != nil { + return fmt.Errorf("error validating kubelet.%s (%q): %w", p.key, p.value, err) + } + canonical[i] = resolved + } + + // The two keys must not resolve to the same path. Kubelet would then read the + // bin dir as the config directory (and vice versa) and fail at registration, + // so reject it here with a clear message instead. + if canonical[0] == canonical[1] { + return fmt.Errorf("kubelet.%s and kubelet.%s must not resolve to the same path (%q)", + kubeletImageCredentialProviderConfigPathKey, kubeletImageCredentialProviderBinDirKey, canonical[0]) + } + + // Structural pre-validation of the provider configuration, on the canonical + // paths. Upstream kubelet calls os.Exit(1) when provider registration fails, + // which in MicroShift terminates the whole process after other components are + // up. These checks turn the structural conditions that reach that exit into + // ordinary fail-fast configuration errors. The configured values are used only + // for the error prefixes; the filesystem work uses the canonical paths. + if err := validateCredentialProviderStructure(configPath, binDir, canonical[0], canonical[1]); err != nil { + return err + } + + // Store canonical paths only once both keys have passed validation. + c.KubeletImageCredentialProviderConfigPath = canonical[0] + c.KubeletImageCredentialProviderBinDir = canonical[1] + return nil +} + +// validateCredentialProviderPath resolves path, checks that the final object is +// of an acceptable kind, and applies the trusted-path rule. It returns the +// canonical path. The object type is checked against the real filesystem (so a +// FIFO, socket or device is rejected), while ownership is checked through the +// checker's hook. +func validateCredentialProviderPath(path string, kind credentialPathKind, checker *trustChecker) (string, error) { + canonical, err := filepath.EvalSymlinks(path) + if err != nil { + if isNotExistErr(err) { + return "", fmt.Errorf("file or directory does not exist") + } + return "", err + } + + fi, err := os.Lstat(canonical) + if err != nil { + return "", err + } + isDir := fi.IsDir() + + switch kind { + case credentialProviderConfigKind: + if !isDir && !fi.Mode().IsRegular() { + return "", fmt.Errorf("%q must be a regular file or a directory", canonical) + } + case credentialProviderBinDirKind: + if !isDir { + return "", fmt.Errorf("%q must be a directory", canonical) + } + } + + if err := checker.checkChain(canonical); err != nil { + return "", err + } + + // If the final object is a directory, every entry it contains must also + // satisfy the trusted-path rule. + if isDir { + if err := validateDirEntries(canonical, checker); err != nil { + return "", err + } + } + + return canonical, nil +} + +// validateDirEntries applies the trusted-path rule to every entry in dir. +// Symlinked entries are resolved and the full rule, including the target's +// ancestors, is applied to the target. +func validateDirEntries(dir string, checker *trustChecker) error { + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + for _, entry := range entries { + entryPath := filepath.Join(dir, entry.Name()) + resolved, err := filepath.EvalSymlinks(entryPath) + if err != nil { + if isNotExistErr(err) { + return fmt.Errorf("%q does not exist", entryPath) + } + return err + } + if err := checker.checkChain(resolved); err != nil { + return err + } + } + return nil +} + +// validateCredentialProviderStructure verifies the structural conditions that +// would otherwise make kubelet call os.Exit(1) at provider registration: a +// configuration directory with no configuration files, a file that does not +// decode as a CredentialProviderConfig, a file that declares no providers, a +// provider name declared more than once, and a provider name that does not +// resolve to an executable in the bin directory. It does not replicate kubelet's +// semantic validation. configKey and binDirKey are the configured values, used +// only in messages; the checks operate on the symlink-resolved paths. +func validateCredentialProviderStructure(configKey, binDirKey, canonicalConfigPath, canonicalBinDir string) error { + configPrefix := func(err error) error { + return fmt.Errorf("error validating kubelet.%s (%q): %w", + kubeletImageCredentialProviderConfigPathKey, configKey, err) + } + binDirPrefix := func(err error) error { + return fmt.Errorf("error validating kubelet.%s (%q): %w", + kubeletImageCredentialProviderBinDirKey, binDirKey, err) + } + + files, err := collectCredentialProviderConfigFiles(canonicalConfigPath) + if err != nil { + return configPrefix(err) + } + + // Record the file each provider name was first declared in, both to reject + // duplicates across all files (kubelet rejects these in its semantic + // validation, which exits) and to name the source file in the missing-binary + // error. Duplicate detection is pure string comparison, so it cannot drift + // from kubelet. + declaredIn := make(map[string]string) + for _, file := range files { + names, err := decodeCredentialProviderNames(file) + if err != nil { + return configPrefix(err) + } + for _, name := range names { + // Kubelet joins the bin dir and the provider name directly; a name + // containing a separator would escape the bin dir, so reject it. + if strings.Contains(name, "/") { + return configPrefix(fmt.Errorf("provider name %q must not contain \"/\"", name)) + } + if first, ok := declaredIn[name]; ok { + return configPrefix(fmt.Errorf("provider %q is declared more than once (in %q and %q)", name, first, file)) + } + declaredIn[name] = file + } + } + + // Check the provider binaries in a deterministic order for a stable error. + names := make([]string, 0, len(declaredIn)) + for name := range declaredIn { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + // Report the joined path. filepath.Join(binDir, name) is exactly what + // kubelet passes to exec.LookPath at registration; a missing or + // non-executable binary is what MicroShift is standing in for here, so the + // error is attributed to the bin dir, whose contents need fixing. + joined := filepath.Join(canonicalBinDir, name) + info, err := os.Stat(joined) + if err != nil || !info.Mode().IsRegular() || info.Mode()&0o111 == 0 { + return binDirPrefix(fmt.Errorf("provider %q (declared in %q) has no executable at %q", name, declaredIn[name], joined)) + } + } + return nil +} + +// collectCredentialProviderConfigFiles returns the configuration files kubelet +// would read for canonicalConfigPath. A regular file yields itself; a directory +// yields its entries whose extension is .json, .yaml or .yml, sorted +// lexicographically. A real directory entry is skipped (kubelet checks +// DirEntry.IsDir(), which is false for a symlink). Kubelet does not resolve +// symlinks and reads whatever remains with os.ReadFile, so a dangling symlink, a +// symlink to a directory, or any other non-regular entry with a matching +// extension is an error here rather than a skip: kubelet would fail on (or block +// on, for a FIFO) it and reach os.Exit. An empty directory is an error. +func collectCredentialProviderConfigFiles(canonicalConfigPath string) ([]string, error) { + fi, err := os.Stat(canonicalConfigPath) + if err != nil { + return nil, err + } + if !fi.IsDir() { + return []string{canonicalConfigPath}, nil + } + + entries, err := os.ReadDir(canonicalConfigPath) + if err != nil { + return nil, err + } + + var files []string + for _, entry := range entries { + switch filepath.Ext(entry.Name()) { + case ".json", ".yaml", ".yml": + default: + continue + } + // Skip only a real directory, matching kubelet's DirEntry.IsDir() check. + // DirEntry.IsDir() is false for a symlink, so a symlink named foo.yaml + // pointing at a directory is NOT skipped here; it is caught below as a + // non-regular file, the way kubelet would fail os.ReadFile on it. + if entry.IsDir() { + continue + } + entryPath := filepath.Join(canonicalConfigPath, entry.Name()) + resolved, err := filepath.EvalSymlinks(entryPath) + if err != nil { + // Kubelet does not resolve symlinks: it includes any matching entry + // in configFiles and later os.ReadFile fails, reaching os.Exit. A + // dangling symlink with a matching extension is therefore an error + // here, not a skip. + if isNotExistErr(err) { + return nil, fmt.Errorf("configuration file %q does not exist (dangling symlink)", entryPath) + } + return nil, err + } + info, err := os.Lstat(resolved) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + // A symlink to a directory, a FIFO named x.yaml, etc. Kubelet would + // os.ReadFile it and block or fail at registration; reject it here. + return nil, fmt.Errorf("configuration file %q is not a regular file", resolved) + } + files = append(files, resolved) + } + slices.Sort(files) + + if len(files) == 0 { + return nil, fmt.Errorf("directory contains no .json, .yaml, or .yml configuration files") + } + return files, nil +} + +// decodeCredentialProviderNames decodes a single configuration file the same way +// kubelet does (strict, via credentialProviderCodec) and returns the declared +// provider names. Decoding with the vendored kubelet packages keeps the check +// aligned with the kubelet in the same build: unknown fields are rejected and all +// three accepted API versions convert to the internal type. It does not replicate +// kubelet's semantic validation beyond kind, group, and the presence of at least +// one provider. +func decodeCredentialProviderNames(file string) ([]string, error) { + data, err := os.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("unable to read file %q: %w", file, err) + } + + obj, gvk, err := credentialProviderCodec().Decode(data, nil, nil) + if err != nil { + return nil, fmt.Errorf("file %q is not a valid CredentialProviderConfig: %w", file, err) + } + if gvk.Kind != "CredentialProviderConfig" { + return nil, fmt.Errorf("file %q is not a valid CredentialProviderConfig: unexpected kind %q", file, gvk.Kind) + } + if gvk.Group != kubeletconfig.GroupName { + return nil, fmt.Errorf("file %q is not a valid CredentialProviderConfig: unexpected group %q", file, gvk.Group) + } + cfg, ok := obj.(*kubeletconfig.CredentialProviderConfig) + if !ok { + return nil, fmt.Errorf("file %q is not a valid CredentialProviderConfig: unexpected type %T", file, obj) + } + if len(cfg.Providers) == 0 { + return nil, fmt.Errorf("file %q declares no providers", file) + } + + names := make([]string, 0, len(cfg.Providers)) + for _, p := range cfg.Providers { + names = append(names, p.Name) + } + return names, nil +} + +// trustedPathComponents returns every path component of abs, ordered from the +// root "/" down to abs itself. +func trustedPathComponents(abs string) []string { + abs = filepath.Clean(abs) + var components []string + for { + components = append(components, abs) + parent := filepath.Dir(abs) + if parent == abs { + break + } + abs = parent + } + slices.Reverse(components) + return components +} diff --git a/pkg/config/kubelet_test.go b/pkg/config/kubelet_test.go new file mode 100644 index 0000000000..12957fc048 --- /dev/null +++ b/pkg/config/kubelet_test.go @@ -0,0 +1,982 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeStat overrides the ownership/mode reported for a specific path. +type fakeStat struct { + uid uint32 + mode os.FileMode +} + +// newOwnership returns an ownershipFn that reports paths as root-owned and +// non-group/other-writable by default, using the real filesystem only to learn +// whether a path is a directory. Entries in overrides let individual components +// report a different uid/mode so ownership failures can be exercised without +// running as root. The returned map records how many times each path was +// queried, so tests can assert the trusted-path walk is not repeated. +func newOwnership(overrides map[string]fakeStat) (ownershipFn, map[string]int) { + calls := map[string]int{} + fn := func(path string) (uint32, os.FileMode, error) { + calls[path]++ + if o, ok := overrides[path]; ok { + return o.uid, o.mode, nil + } + // Default: root-owned, non-group/other-writable, with a mode that + // matches whether the real path is a directory. + fi, err := os.Lstat(path) + if err != nil { + return 0, 0, err + } + mode := os.FileMode(0o644) + if fi.IsDir() { + mode = 0o755 + } + return 0, mode, nil + } + return fn, calls +} + +func TestReadKubeletCredentialProviderKeys(t *testing.T) { + ttests := []struct { + name string + kubelet map[string]any + wantConfig string + wantBinDir string + expectError string + }{ + { + name: "nil map", + kubelet: nil, + }, + { + name: "keys absent", + kubelet: map[string]any{"cpuManagerPolicy": "static"}, + }, + { + name: "both present", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "/etc/microshift/cp.yaml", + "imageCredentialProviderBinDir": "/usr/libexec/cp", + }, + wantConfig: "/etc/microshift/cp.yaml", + wantBinDir: "/usr/libexec/cp", + }, + { + name: "only config present", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "/etc/microshift/cp.yaml", + }, + wantConfig: "/etc/microshift/cp.yaml", + }, + { + name: "empty string is unset", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "", + "imageCredentialProviderBinDir": "", + }, + }, + { + name: "explicit null is unset", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": nil, + "imageCredentialProviderBinDir": nil, + }, + }, + { + name: "int type is rejected", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": 42, + }, + expectError: "kubelet.imageCredentialProviderConfigPath must be a string, got int", + }, + { + name: "bool type is rejected", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "/etc/microshift/cp.yaml", + "imageCredentialProviderBinDir": true, + }, + expectError: "kubelet.imageCredentialProviderBinDir must be a string, got bool", + }, + { + name: "map type is rejected", + kubelet: map[string]any{ + "imageCredentialProviderConfigPath": map[string]any{"a": "b"}, + }, + expectError: "kubelet.imageCredentialProviderConfigPath must be a string, got map[string]interface {}", + }, + } + + for _, tt := range ttests { + t.Run(tt.name, func(t *testing.T) { + c := &Config{Kubelet: tt.kubelet} + err := c.readKubeletCredentialProviderKeys() + if tt.expectError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectError) + return + } + require.NoError(t, err) + // Reading populates the raw fields, not the canonical exported ones. + assert.Equal(t, tt.wantConfig, c.kubeletImageCredentialProviderConfigPathRaw) + assert.Equal(t, tt.wantBinDir, c.kubeletImageCredentialProviderBinDirRaw) + assert.Empty(t, c.KubeletImageCredentialProviderConfigPath) + assert.Empty(t, c.KubeletImageCredentialProviderBinDir) + // c.Kubelet must never be modified during reading. + assert.Equal(t, tt.kubelet, c.Kubelet) + }) + } +} + +func TestKubeletPassthrough(t *testing.T) { + t.Run("nil map returns nil", func(t *testing.T) { + c := &Config{Kubelet: nil} + assert.Nil(t, c.KubeletPassthrough()) + }) + + t.Run("empty map returns empty, non-nil map", func(t *testing.T) { + c := &Config{Kubelet: map[string]any{}} + got := c.KubeletPassthrough() + assert.NotNil(t, got) + assert.Empty(t, got) + }) + + t.Run("map with only reserved keys returns empty map", func(t *testing.T) { + c := &Config{Kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "/etc/microshift/cp.yaml", + "imageCredentialProviderBinDir": "/usr/libexec/cp", + }} + assert.Empty(t, c.KubeletPassthrough()) + }) + + t.Run("drops exactly the reserved keys and preserves the rest", func(t *testing.T) { + c := &Config{Kubelet: map[string]any{ + "imageCredentialProviderConfigPath": "/etc/microshift/cp.yaml", + "imageCredentialProviderBinDir": "/usr/libexec/cp", + "cpuManagerPolicy": "static", + "kubeReserved": map[string]any{"memory": "500Mi"}, + }} + got := c.KubeletPassthrough() + assert.Equal(t, map[string]any{ + "cpuManagerPolicy": "static", + "kubeReserved": map[string]any{"memory": "500Mi"}, + }, got) + // The original map is untouched. + assert.Contains(t, c.Kubelet, "imageCredentialProviderConfigPath") + assert.Contains(t, c.Kubelet, "imageCredentialProviderBinDir") + }) +} + +// mkFile / mkDir create real filesystem objects for path/type checks; ownership +// is asserted through the injected ownership hook, not the real files. +func mkFile(t *testing.T, dir, name string, mode os.FileMode) string { + t.Helper() + p := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(p, []byte("x"), mode)) + return p +} + +func mkDir(t *testing.T, dir, name string) string { + t.Helper() + p := filepath.Join(dir, name) + require.NoError(t, os.Mkdir(p, 0o755)) + return p +} + +// credentialProviderConfigYAML returns a structurally valid +// CredentialProviderConfig (kubelet.config.k8s.io/v1) declaring one provider per +// name. +func credentialProviderConfigYAML(names ...string) string { + return credentialProviderConfigYAMLAt("kubelet.config.k8s.io/v1", names...) +} + +// credentialProviderConfigYAMLAt is credentialProviderConfigYAML with an explicit +// config apiVersion, so the v1beta1 and v1alpha1 code paths can be exercised. +func credentialProviderConfigYAMLAt(apiVersion string, names ...string) string { + var b strings.Builder + fmt.Fprintf(&b, "apiVersion: %s\n", apiVersion) + b.WriteString("kind: CredentialProviderConfig\n") + b.WriteString("providers:\n") + for _, n := range names { + fmt.Fprintf(&b, "- name: %s\n", n) + b.WriteString(" matchImages: [\"*.dkr.ecr.*.amazonaws.com\"]\n") + b.WriteString(" defaultCacheDuration: \"12h\"\n") + b.WriteString(" apiVersion: credentialprovider.kubelet.k8s.io/v1\n") + } + return b.String() +} + +// mkConfigFile writes a structurally valid config file naming the given +// providers and returns its path. +func mkConfigFile(t *testing.T, dir, name string, providers ...string) string { + t.Helper() + p := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(p, []byte(credentialProviderConfigYAML(providers...)), 0o644)) + return p +} + +// mkExecProvider writes an executable file (0o755) in binDir named name, so the +// provider-binary check resolves it. +func mkExecProvider(t *testing.T, binDir, name string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(binDir, name), []byte("#!/bin/sh\n"), 0o755)) +} + +func TestValidateKubeletCredentialProvider(t *testing.T) { + t.Run("neither set is OK", func(t *testing.T) { + own, _ := newOwnership(nil) + c := &Config{} + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("only config set", func(t *testing.T) { + own, _ := newOwnership(nil) + c := &Config{kubeletImageCredentialProviderConfigPathRaw: "/etc/cp.yaml"} + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be set together") + }) + + t.Run("only bin dir set", func(t *testing.T) { + own, _ := newOwnership(nil) + c := &Config{kubeletImageCredentialProviderBinDirRaw: "/usr/libexec/cp"} + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be set together") + }) + + t.Run("relative config path", func(t *testing.T) { + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: "relative/cp.yaml", + kubeletImageCredentialProviderBinDirRaw: "/usr/libexec/cp", + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "kubelet.imageCredentialProviderConfigPath") + assert.Contains(t, err.Error(), "must be an absolute path") + }) + + t.Run("relative bin dir", func(t *testing.T) { + // The config path is validated first (merged loop), so it must be a valid + // absolute path for the relative bin-dir error to be the one reported. + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: "relative/cp", + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "kubelet.imageCredentialProviderBinDir") + assert.Contains(t, err.Error(), "must be an absolute path") + }) + + t.Run("missing config path", func(t *testing.T) { + dir := t.TempDir() + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: filepath.Join(dir, "does-not-exist.yaml"), + kubeletImageCredentialProviderBinDirRaw: dir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "file or directory does not exist") + }) + + t.Run("missing bin dir", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkFile(t, dir, "cp.yaml", 0o644) + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: filepath.Join(dir, "missing"), + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "file or directory does not exist") + }) + + t.Run("config path descends through a non-directory", func(t *testing.T) { + dir := t.TempDir() + file := mkFile(t, dir, "cp.yaml", 0o644) + own, _ := newOwnership(nil) + c := &Config{ + // cp.yaml is a regular file, so treating it as a directory is + // an ENOTDIR mid-path, reported as "does not exist". + kubeletImageCredentialProviderConfigPathRaw: filepath.Join(file, "extra"), + kubeletImageCredentialProviderBinDirRaw: dir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "file or directory does not exist") + }) + + t.Run("config path is a FIFO", func(t *testing.T) { + dir := t.TempDir() + fifo := filepath.Join(dir, "cp.fifo") + require.NoError(t, syscall.Mkfifo(fifo, 0o644)) + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: fifo, + kubeletImageCredentialProviderBinDirRaw: dir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be a regular file or a directory") + }) + + t.Run("config path is a regular file", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("config path is a directory", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + mkConfigFile(t, cfgDir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("config path and bin dir resolve to the same directory", func(t *testing.T) { + dir := t.TempDir() + shared := mkDir(t, dir, "shared") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: shared, + kubeletImageCredentialProviderBinDirRaw: shared, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must not resolve to the same path") + }) + + t.Run("bin dir is a file", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkFile(t, dir, "cp.yaml", 0o644) + binFile := mkFile(t, dir, "notadir", 0o644) + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binFile, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be a directory") + }) + + t.Run("valid config canonicalizes and stores canonical paths", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + require.NoError(t, c.validateKubeletCredentialProviderWith(own)) + wantCfg, _ := filepath.EvalSymlinks(cfgFile) + wantBin, _ := filepath.EvalSymlinks(binDir) + gotCfg, gotBin, enabled := c.KubeletImageCredentialProviderPaths() + assert.True(t, enabled) + assert.Equal(t, wantCfg, gotCfg) + assert.Equal(t, wantBin, gotBin) + }) + + t.Run("canonical paths survive a later updateComputedValues", func(t *testing.T) { + dir := t.TempDir() + realDir := mkDir(t, dir, "real-bin") + mkExecProvider(t, realDir, "ecr-credential-provider") + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + // A symlinked bin dir so canonical differs from the configured value. + link := filepath.Join(dir, "link-bin") + require.NoError(t, os.Symlink(realDir, link)) + + c := NewDefault() + c.Kubelet = map[string]any{ + "imageCredentialProviderConfigPath": cfgFile, + "imageCredentialProviderBinDir": link, + } + require.NoError(t, c.readKubeletCredentialProviderKeys()) + + own, _ := newOwnership(nil) + require.NoError(t, c.validateKubeletCredentialProviderWith(own)) + wantBin, _ := filepath.EvalSymlinks(realDir) + _, gotBin, enabled := c.KubeletImageCredentialProviderPaths() + require.True(t, enabled) + require.Equal(t, wantBin, gotBin) + + // updateComputedValues() re-reads the raw values from c.Kubelet; it must + // not revert the canonical paths the accessor returns. + require.NoError(t, c.updateComputedValues()) + _, gotBin, enabled = c.KubeletImageCredentialProviderPaths() + assert.True(t, enabled) + assert.Equal(t, wantBin, gotBin, "accessor reverted to the unresolved path after updateComputedValues") + }) +} + +func TestValidateKubeletCredentialProviderTrustedPath(t *testing.T) { + // newValidPair returns a config file and bin dir that both pass validation + // when the default (all-root) ownership hook is used. + newValidPair := func(t *testing.T) (string, string) { + dir := t.TempDir() + return mkFile(t, dir, "cp.yaml", 0o644), mkDir(t, dir, "bin") + } + + t.Run("non-root owner on final object", func(t *testing.T) { + cfgFile, binDir := newValidPair(t) + canonical, _ := filepath.EvalSymlinks(binDir) + own, _ := newOwnership(map[string]fakeStat{canonical: {uid: 1000, mode: 0o755}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), canonical) + assert.Contains(t, err.Error(), "must be owned by root and not writable by group or others") + }) + + t.Run("group-writable ancestor", func(t *testing.T) { + cfgFile, binDir := newValidPair(t) + canonical, _ := filepath.EvalSymlinks(binDir) + ancestor := filepath.Dir(canonical) + own, _ := newOwnership(map[string]fakeStat{ancestor: {uid: 0, mode: 0o775}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), ancestor) + assert.Contains(t, err.Error(), "must be owned by root") + }) + + t.Run("world-writable final object", func(t *testing.T) { + cfgFile, binDir := newValidPair(t) + canonical, _ := filepath.EvalSymlinks(binDir) + own, _ := newOwnership(map[string]fakeStat{canonical: {uid: 0, mode: 0o757}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be owned by root") + }) + + t.Run("world-writable contained entry", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkFile(t, dir, "cp.yaml", 0o644) + binDir := mkDir(t, dir, "bin") + plugin := mkFile(t, binDir, "ecr-credential-provider", 0o755) + canonicalPlugin, _ := filepath.EvalSymlinks(plugin) + own, _ := newOwnership(map[string]fakeStat{canonicalPlugin: {uid: 0, mode: 0o757}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), canonicalPlugin) + assert.Contains(t, err.Error(), "must be owned by root") + }) + + t.Run("compliant root-owned dir and file is OK", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("symlink to compliant target resolves to canonical", func(t *testing.T) { + dir := t.TempDir() + realDir := mkDir(t, dir, "real-bin") + mkExecProvider(t, realDir, "ecr-credential-provider") + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + link := filepath.Join(dir, "link-bin") + require.NoError(t, os.Symlink(realDir, link)) + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: link, + } + require.NoError(t, c.validateKubeletCredentialProviderWith(own)) + wantBin, _ := filepath.EvalSymlinks(realDir) + _, gotBin, _ := c.KubeletImageCredentialProviderPaths() + assert.Equal(t, wantBin, gotBin) + }) + + t.Run("symlinked bin-dir entry is checked at its target including ancestors", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkFile(t, dir, "cp.yaml", 0o644) + binDir := mkDir(t, dir, "bin") + // The real plugin lives outside binDir, under an unsafe ancestor. + unsafeParent := mkDir(t, dir, "unsafe") + realPlugin := mkFile(t, unsafeParent, "plugin", 0o755) + require.NoError(t, os.Symlink(realPlugin, filepath.Join(binDir, "plugin"))) + canonicalParent, _ := filepath.EvalSymlinks(unsafeParent) + own, _ := newOwnership(map[string]fakeStat{canonicalParent: {uid: 0, mode: 0o777}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), canonicalParent) + assert.Contains(t, err.Error(), "must be owned by root") + }) + + t.Run("symlink to unsafe target is rejected", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkFile(t, dir, "cp.yaml", 0o644) + realDir := mkDir(t, dir, "real-bin") + link := filepath.Join(dir, "link-bin") + require.NoError(t, os.Symlink(realDir, link)) + canonical, _ := filepath.EvalSymlinks(realDir) + own, _ := newOwnership(map[string]fakeStat{canonical: {uid: 1000, mode: 0o755}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: link, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be owned by root") + }) + + t.Run("each ancestor is stat'd only once across the whole validation", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "a", "b", "c") + binDir := mkDir(t, dir, "bin") + // Three entries under one bin dir: their shared ancestors must not be + // re-walked once memoized. + mkExecProvider(t, binDir, "a") + mkExecProvider(t, binDir, "b") + mkExecProvider(t, binDir, "c") + own, calls := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + require.NoError(t, c.validateKubeletCredentialProviderWith(own)) + require.NotEmpty(t, calls) + for path, n := range calls { + assert.Equalf(t, 1, n, "path %q was stat'd %d times, expected exactly once", path, n) + } + }) +} + +func TestValidateKubeletCredentialProviderStructure(t *testing.T) { + t.Run("directory with no matching files names the directory", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgDir) + assert.Contains(t, err.Error(), "contains no .json, .yaml, or .yml") + }) + + t.Run("directory with only a .txt names the directory", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + require.NoError(t, os.WriteFile(filepath.Join(cfgDir, "readme.txt"), []byte("x"), 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgDir) + assert.Contains(t, err.Error(), "contains no .json, .yaml, or .yml") + }) + + t.Run("wrong kind names the file", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, []byte( + "apiVersion: kubelet.config.k8s.io/v1\nkind: NotThatKind\nproviders: []\n"), 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgFile) + assert.Contains(t, err.Error(), "is not a valid CredentialProviderConfig") + }) + + t.Run("wrong apiVersion names the file", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, []byte( + "apiVersion: example.com/v1\nkind: CredentialProviderConfig\nproviders: []\n"), 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgFile) + assert.Contains(t, err.Error(), "is not a valid CredentialProviderConfig") + }) + + t.Run("malformed YAML names the file and includes the decode error", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, []byte("providers: [ this is : not : yaml\n"), 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgFile) + assert.Contains(t, err.Error(), "is not a valid CredentialProviderConfig") + }) + + t.Run("empty providers reports declares no providers", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, []byte( + "apiVersion: kubelet.config.k8s.io/v1\nkind: CredentialProviderConfig\nproviders: []\n"), 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgFile) + assert.Contains(t, err.Error(), "declares no providers") + }) + + t.Run("provider with no file in bin dir names provider, source file and joined path", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + canonicalBin, _ := filepath.EvalSymlinks(binDir) + canonicalCfg, _ := filepath.EvalSymlinks(cfgFile) + // The error is attributed to the bin dir, whose contents need fixing. + assert.Contains(t, err.Error(), "kubelet.imageCredentialProviderBinDir") + assert.Contains(t, err.Error(), `provider "ecr-credential-provider"`) + assert.Contains(t, err.Error(), fmt.Sprintf("declared in %q", canonicalCfg)) + assert.Contains(t, err.Error(), "has no executable at") + assert.Contains(t, err.Error(), filepath.Join(canonicalBin, "ecr-credential-provider")) + }) + + t.Run("provider file present but not executable is rejected", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + // 0o644: present but not executable, so the execute-bit check rejects it. + require.NoError(t, os.WriteFile(filepath.Join(binDir, "ecr-credential-provider"), []byte("x"), 0o644)) + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), `provider "ecr-credential-provider"`) + assert.Contains(t, err.Error(), "has no executable at") + }) + + t.Run("provider name containing a slash is rejected", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "sub/provider") + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), `provider name "sub/provider" must not contain "/"`) + }) + + t.Run("duplicate provider across two files is rejected", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + fileA := mkConfigFile(t, cfgDir, "a.yaml", "dup") + fileB := mkConfigFile(t, cfgDir, "b.yaml", "dup") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "dup") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), `provider "dup" is declared more than once`) + canonicalA, _ := filepath.EvalSymlinks(fileA) + canonicalB, _ := filepath.EvalSymlinks(fileB) + assert.Contains(t, err.Error(), canonicalA) + assert.Contains(t, err.Error(), canonicalB) + }) + + t.Run("duplicate provider within one file is rejected", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "dup", "dup") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "dup") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), `provider "dup" is declared more than once`) + }) + + t.Run("valid single file and executable provider passes", func(t *testing.T) { + dir := t.TempDir() + cfgFile := mkConfigFile(t, dir, "cp.yaml", "ecr-credential-provider") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("valid directory with two files and both providers present passes", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + mkConfigFile(t, cfgDir, "a.yaml", "provider-a") + mkConfigFile(t, cfgDir, "b.json", "provider-b") + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "provider-a") + mkExecProvider(t, binDir, "provider-b") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("v1beta1 config decodes and passes", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, + []byte(credentialProviderConfigYAMLAt("kubelet.config.k8s.io/v1beta1", "ecr-credential-provider")), 0o644)) + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("v1alpha1 config decodes and passes", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + require.NoError(t, os.WriteFile(cfgFile, + []byte(credentialProviderConfigYAMLAt("kubelet.config.k8s.io/v1alpha1", "ecr-credential-provider")), 0o644)) + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + assert.NoError(t, c.validateKubeletCredentialProviderWith(own)) + }) + + t.Run("unknown field is rejected by strict decoding", func(t *testing.T) { + dir := t.TempDir() + cfgFile := filepath.Join(dir, "cp.yaml") + // matchImage (singular) is not a field of CredentialProvider; strict + // decoding rejects it, the way kubelet does at registration. + require.NoError(t, os.WriteFile(cfgFile, []byte( + "apiVersion: kubelet.config.k8s.io/v1\n"+ + "kind: CredentialProviderConfig\n"+ + "providers:\n"+ + "- name: ecr-credential-provider\n"+ + " matchImage: [\"*.example.com\"]\n"), 0o644)) + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "ecr-credential-provider") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), cfgFile) + assert.Contains(t, err.Error(), "is not a valid CredentialProviderConfig") + }) + + t.Run("world-writable bin dir wins over unresolvable provider", func(t *testing.T) { + dir := t.TempDir() + // Config names a provider that does not exist, but the bin dir is + // world-writable; the trusted-path check runs first, so the permission + // error is reported, not the provider error. + cfgFile := mkConfigFile(t, dir, "cp.yaml", "no-such-provider") + binDir := mkDir(t, dir, "bin") + canonicalBin, _ := filepath.EvalSymlinks(binDir) + own, _ := newOwnership(map[string]fakeStat{canonicalBin: {uid: 0, mode: 0o757}}) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgFile, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), "must be owned by root and not writable by group or others") + assert.NotContains(t, err.Error(), "has no executable") + }) + + t.Run("dangling .yaml symlink alongside a valid file is an error naming the link", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + mkConfigFile(t, cfgDir, "a.yaml", "provider-a") + // A symlink with a matching extension whose target does not exist: + // kubelet would include it and fail at os.ReadFile, reaching os.Exit. + // The per-entry trusted-path walk (validateDirEntries) resolves every + // directory entry and rejects the broken link before the structural + // stage runs, so the observable message is "does not exist"; the + // collectCredentialProviderConfigFiles branch (asserted directly below) + // is the same defense at the structural stage. + dangling := filepath.Join(cfgDir, "b.yaml") + require.NoError(t, os.Symlink(filepath.Join(dir, "nonexistent-target.yaml"), dangling)) + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "provider-a") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), dangling) + assert.Contains(t, err.Error(), "does not exist") + }) + + t.Run("collectCredentialProviderConfigFiles rejects a dangling symlink", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + mkConfigFile(t, cfgDir, "a.yaml", "provider-a") + dangling := filepath.Join(cfgDir, "b.yaml") + require.NoError(t, os.Symlink(filepath.Join(dir, "nonexistent-target.yaml"), dangling)) + _, err := collectCredentialProviderConfigFiles(cfgDir) + require.Error(t, err) + assert.Contains(t, err.Error(), dangling) + assert.Contains(t, err.Error(), "dangling symlink") + }) + + t.Run("symlink to a directory named x.yaml is rejected as not a regular file", func(t *testing.T) { + // DirEntry.IsDir() is false for a symlink, so kubelet does not skip a + // symlink-to-directory: it os.ReadFile's it and fails, reaching os.Exit. + // collectCredentialProviderConfigFiles must treat it as an error, not a + // skip. + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + mkConfigFile(t, cfgDir, "a.yaml", "provider-a") + targetDir := mkDir(t, dir, "target-dir") + link := filepath.Join(cfgDir, "b.yaml") + require.NoError(t, os.Symlink(targetDir, link)) + binDir := mkDir(t, dir, "bin") + mkExecProvider(t, binDir, "provider-a") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + canonicalTarget, _ := filepath.EvalSymlinks(targetDir) + assert.Contains(t, err.Error(), canonicalTarget) + assert.Contains(t, err.Error(), "is not a regular file") + }) + + t.Run("collectCredentialProviderConfigFiles skips a real directory named x.yaml", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + validFile := mkConfigFile(t, cfgDir, "a.yaml", "provider-a") + // A real subdirectory named b.yaml: kubelet checks DirEntry.IsDir() and + // skips it; so must we. + require.NoError(t, os.Mkdir(filepath.Join(cfgDir, "b.yaml"), 0o755)) + files, err := collectCredentialProviderConfigFiles(cfgDir) + require.NoError(t, err) + canonicalValid, _ := filepath.EvalSymlinks(validFile) + assert.Equal(t, []string{canonicalValid}, files) + }) + + t.Run("FIFO named x.yaml is rejected as not a regular file", func(t *testing.T) { + dir := t.TempDir() + cfgDir := mkDir(t, dir, "cp.d") + fifo := filepath.Join(cfgDir, "x.yaml") + require.NoError(t, syscall.Mkfifo(fifo, 0o644)) + binDir := mkDir(t, dir, "bin") + own, _ := newOwnership(nil) + c := &Config{ + kubeletImageCredentialProviderConfigPathRaw: cfgDir, + kubeletImageCredentialProviderBinDirRaw: binDir, + } + err := c.validateKubeletCredentialProviderWith(own) + require.Error(t, err) + assert.Contains(t, err.Error(), fifo) + assert.Contains(t, err.Error(), "is not a regular file") + }) +} diff --git a/pkg/node/kubelet.go b/pkg/node/kubelet.go index 6a0c8ce6d8..20639d6337 100644 --- a/pkg/node/kubelet.go +++ b/pkg/node/kubelet.go @@ -90,6 +90,8 @@ func (s *KubeletServer) configure(cfg *config.Config) { kubeletFlags.NodeLabels["node.openshift.io/os_id"] = osID kubeletFlags.NodeLabels["node.kubernetes.io/instance-type"] = "rhde" + setImageCredentialProviderFlags(kubeletFlags, cfg) + kubeletConfig, err := loadConfigFile(filepath.Join(config.DataDir, "/resources/kubelet/config/config.yaml")) if err != nil { @@ -100,6 +102,26 @@ func (s *KubeletServer) configure(cfg *config.Config) { s.kubeletflags = kubeletFlags } +// setImageCredentialProviderFlags copies the (already validated and +// canonicalized) image credential provider paths onto the kubelet flags. When +// the feature is not configured the flags are left at their defaults. +func setImageCredentialProviderFlags(kubeletFlags *kubeletoptions.KubeletFlags, cfg *config.Config) { + configPath, binDir, enabled := cfg.KubeletImageCredentialProviderPaths() + if !enabled { + return + } + + kubeletFlags.ImageCredentialProviderConfigPath = configPath + kubeletFlags.ImageCredentialProviderBinDir = binDir + + // The paths logged here are the canonical (symlink-resolved) ones handed to + // kubelet. The values the user configured are available from + // `microshift show-config`, so they are not duplicated in the journal. + klog.InfoS("Kubelet image credential provider configured", + "configPath", configPath, + "binDir", binDir) +} + func (s *KubeletServer) writeConfig(cfg *config.Config) error { data, err := s.generateConfig(cfg) if err != nil { @@ -136,8 +158,11 @@ func (s *KubeletServer) generateConfig(cfg *config.Config) ([]byte, error) { } userProvidedConfig := "" - if cfg.Kubelet != nil { - b, err := yaml.Marshal(cfg.Kubelet) + // The MicroShift-owned keys (image credential provider paths) are applied as + // kubelet startup flags, not KubeletConfiguration fields, so they must be + // filtered out of the generated config here. + if passthrough := cfg.KubeletPassthrough(); len(passthrough) > 0 { + b, err := yaml.Marshal(passthrough) if err != nil { return nil, fmt.Errorf("failed to re-marshal user provided kubelet config: %w", err) } diff --git a/pkg/node/kubelet_test.go b/pkg/node/kubelet_test.go index a98a071c0f..bd8ffd6b64 100644 --- a/pkg/node/kubelet_test.go +++ b/pkg/node/kubelet_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/openshift/microshift/pkg/config" + kubeletoptions "k8s.io/kubernetes/cmd/kubelet/app/options" + "github.com/stretchr/testify/assert" ) @@ -11,6 +13,10 @@ func Test_GenerateConfig(t *testing.T) { cfg := config.NewDefault() cfg.Kubelet = map[string]any{ "cpuManagerPolicy": "static", + // Reserved keys are MicroShift-owned kubelet flags and must never + // appear in the generated KubeletConfiguration. + "imageCredentialProviderConfigPath": "/etc/microshift/credential-providers.yaml", + "imageCredentialProviderBinDir": "/usr/libexec/microshift/credential-providers", "reservedMemory": []any{ map[string]any{ "limits": map[string]any{ @@ -47,4 +53,48 @@ reservedMemory: data, err := kubelet.generateConfig(cfg) assert.NoError(t, err) assert.Contains(t, string(data), expectedConfigPart) + // The reserved keys are stripped from the passthrough config. + assert.NotContains(t, string(data), "imageCredentialProviderConfigPath") + assert.NotContains(t, string(data), "imageCredentialProviderBinDir") +} + +func Test_GenerateConfig_EmptyKubelet(t *testing.T) { + // An empty (or reserved-keys-only) kubelet map must not inject anything into + // the generated KubeletConfiguration: the output must match the nil-map case, + // with no stray "{}" appended. + kubelet := &KubeletServer{} + + nilCfg := config.NewDefault() + nilCfg.Kubelet = nil + nilData, err := kubelet.generateConfig(nilCfg) + assert.NoError(t, err) + + emptyCfg := config.NewDefault() + emptyCfg.Kubelet = map[string]any{} + emptyData, err := kubelet.generateConfig(emptyCfg) + assert.NoError(t, err) + + assert.Equal(t, string(nilData), string(emptyData)) + assert.NotContains(t, string(emptyData), "{}") +} + +func Test_setImageCredentialProviderFlags(t *testing.T) { + t.Run("sets both flags to the canonical values when configured", func(t *testing.T) { + cfg := &config.Config{ + KubeletImageCredentialProviderConfigPath: "/etc/microshift/credential-providers.yaml", + KubeletImageCredentialProviderBinDir: "/usr/libexec/microshift/credential-providers", + } + flags := kubeletoptions.NewKubeletFlags() + setImageCredentialProviderFlags(flags, cfg) + assert.Equal(t, "/etc/microshift/credential-providers.yaml", flags.ImageCredentialProviderConfigPath) + assert.Equal(t, "/usr/libexec/microshift/credential-providers", flags.ImageCredentialProviderBinDir) + }) + + t.Run("leaves flags empty when not configured", func(t *testing.T) { + cfg := &config.Config{} + flags := kubeletoptions.NewKubeletFlags() + setImageCredentialProviderFlags(flags, cfg) + assert.Empty(t, flags.ImageCredentialProviderConfigPath) + assert.Empty(t, flags.ImageCredentialProviderBinDir) + }) } diff --git a/test/suites/standard2/kubelet-credential-provider.robot b/test/suites/standard2/kubelet-credential-provider.robot new file mode 100644 index 0000000000..7f97d796f1 --- /dev/null +++ b/test/suites/standard2/kubelet-credential-provider.robot @@ -0,0 +1,188 @@ +*** Settings *** +Documentation Kubelet image credential provider configuration tests + +Resource ../../resources/common.resource +Resource ../../resources/microshift-config.resource +Resource ../../resources/microshift-process.resource +Library ../../resources/journalctl.py + +Suite Setup Setup +Suite Teardown Teardown + +Test Tags restart slow + + +*** Variables *** +${CURSOR} ${EMPTY} +${CP_DROPIN} 10-credential-provider +${CP_CONFIGURED_LOG} Kubelet image credential provider configured +${CP_BIN_DIR} /usr/libexec/microshift/credential-providers +${CP_MOCK_PROVIDER} ${CP_BIN_DIR}/mock-credential-provider +${CP_CONFIG_FILE} /etc/microshift/credential-providers.yaml +${CP_CONFIG_DIR} /etc/microshift/credential-providers.d +${KUBELET_GENERATED_CONFIG} /var/lib/microshift/resources/kubelet/config/config.yaml +${CP_VALID} SEPARATOR=\n +... --- +... kubelet: +... \ \ imageCredentialProviderConfigPath: ${CP_CONFIG_FILE} +... \ \ imageCredentialProviderBinDir: ${CP_BIN_DIR} +${CP_MISSING_BIN_DIR} SEPARATOR=\n +... --- +... kubelet: +... \ \ imageCredentialProviderConfigPath: ${CP_CONFIG_FILE} +... \ \ imageCredentialProviderBinDir: /usr/libexec/microshift/no-such-dir +${CP_ONLY_CONFIG_PATH} SEPARATOR=\n +... --- +... kubelet: +... \ \ imageCredentialProviderConfigPath: ${CP_CONFIG_FILE} +${CP_PROVIDER_CONFIG} SEPARATOR=\n +... apiVersion: kubelet.config.k8s.io/v1 +... kind: CredentialProviderConfig +... providers: +... \ \ - name: mock-credential-provider +... \ \ \ \ matchImages: +... \ \ \ \ \ \ - "registry.example.invalid" +... \ \ \ \ defaultCacheDuration: "1m" +... \ \ \ \ apiVersion: credentialprovider.kubelet.k8s.io/v1 +${CP_BAD_PROVIDER_CONFIG} SEPARATOR=\n +... apiVersion: kubelet.config.k8s.io/v1 +... kind: CredentialProviderConfig +... providers: +... \ \ - name: no-such-provider +... \ \ \ \ matchImages: +... \ \ \ \ \ \ - "registry.example.invalid" +... \ \ \ \ defaultCacheDuration: "1m" +... \ \ \ \ apiVersion: credentialprovider.kubelet.k8s.io/v1 +${CP_EMPTY_DIR_CONFIG} SEPARATOR=\n +... --- +... kubelet: +... \ \ imageCredentialProviderConfigPath: ${CP_CONFIG_DIR} +... \ \ imageCredentialProviderBinDir: ${CP_BIN_DIR} +${CP_MOCK_SCRIPT} SEPARATOR=\n +... \#!/bin/bash +... \# Mock kubelet image credential provider: returns static credentials. +... cat >/dev/null +... cat <<'JSON' +... {"kind":"CredentialProviderResponse", +... "apiVersion":"credentialprovider.kubelet.k8s.io/v1", +... "cacheKeyType":"Registry","cacheDuration":"1m", +... "auth":{"registry.example.invalid": +... {"username":"user","password":"pass"}}} +... JSON + + +*** Test Cases *** +Keys Absent Leaves Kubelet Unchanged + [Documentation] Without the keys, MicroShift starts and no credential provider is configured + [Setup] Run Keywords Remove Credential Provider Config AND Restart MicroShift With Cursor + Pattern Should Not Appear In Log Output ${CURSOR} ${CP_CONFIGURED_LOG} + +Valid Configuration Applies Kubelet Flags + [Documentation] With both keys set, MicroShift starts, logs the configured paths, reports them in + ... show-config, and keeps them out of the generated KubeletConfiguration + [Setup] Apply Credential Provider Config ${CP_VALID} + Pattern Should Appear In Log Output ${CURSOR} ${CP_CONFIGURED_LOG} + ${config}= Show Config effective + Should Be Equal As Strings ${config.kubelet.imageCredentialProviderConfigPath} ${CP_CONFIG_FILE} + Should Be Equal As Strings ${config.kubelet.imageCredentialProviderBinDir} ${CP_BIN_DIR} + # Reserved keys must not reach the KubeletConfiguration file kubelet loads + Command Should Fail grep -q imageCredentialProvider ${KUBELET_GENERATED_CONFIG} + [Teardown] Remove Credential Provider Config + +Missing Bin Directory Prevents Start + [Documentation] MicroShift fails to start when the bin directory does not exist + [Setup] Apply Invalid Credential Provider Config ${CP_MISSING_BIN_DIR} + Pattern Should Appear In Log Output ${CURSOR} imageCredentialProviderBinDir.*file or directory does not exist + [Teardown] Run Keywords Remove Credential Provider Config AND Restart MicroShift + +Only One Key Prevents Start + [Documentation] MicroShift fails to start when only one of the two keys is set + [Setup] Apply Invalid Credential Provider Config ${CP_ONLY_CONFIG_PATH} + Pattern Should Appear In Log Output + ... ${CURSOR} + ... imageCredentialProviderConfigPath and kubelet.imageCredentialProviderBinDir must be set together + [Teardown] Run Keywords Remove Credential Provider Config AND Restart MicroShift + +World Writable Bin Directory Prevents Start + [Documentation] MicroShift refuses to start when the bin directory is writable by others + [Setup] Run Keywords Command Should Work chmod o+w ${CP_BIN_DIR} + ... AND Apply Invalid Credential Provider Config ${CP_VALID} + Pattern Should Appear In Log Output + ... ${CURSOR} + ... imageCredentialProviderBinDir.*must be owned by root and not writable by group or others + [Teardown] Run Keywords Command Should Work chmod o-w ${CP_BIN_DIR} + ... AND Remove Credential Provider Config + ... AND Restart MicroShift + +Missing Provider Binary Prevents Start + [Documentation] MicroShift fails to start when a provider names a binary absent from the bin directory. + ... Validation fails before kubelet is configured, so the "configured" line must not appear. + [Setup] Run Keywords Upload String To File ${CP_BAD_PROVIDER_CONFIG} ${CP_CONFIG_FILE} + ... AND Apply Invalid Credential Provider Config ${CP_VALID} + Pattern Should Appear In Log Output + ... ${CURSOR} + ... imageCredentialProviderBinDir.*provider .+no-such-provider.+ .*has no executable at .*/no-such-provider + Pattern Should Not Appear In Log Output ${CURSOR} ${CP_CONFIGURED_LOG} + [Teardown] Run Keywords Upload String To File ${CP_PROVIDER_CONFIG} ${CP_CONFIG_FILE} + ... AND Remove Credential Provider Config + ... AND Restart MicroShift + +Empty Configuration Directory Prevents Start + [Documentation] MicroShift fails to start when the configuration directory holds no config files + [Setup] Run Keywords Command Should Work install -d -o root -g root -m 0755 ${CP_CONFIG_DIR} + ... AND Apply Invalid Credential Provider Config ${CP_EMPTY_DIR_CONFIG} + Pattern Should Appear In Log Output + ... ${CURSOR} + ... imageCredentialProviderConfigPath.*directory contains no .json, .yaml, or .yml configuration files + [Teardown] Run Keywords Command Should Work rm -rf ${CP_CONFIG_DIR} + ... AND Remove Credential Provider Config + ... AND Restart MicroShift + + +*** Keywords *** +Setup + [Documentation] Test suite setup: install a mock provider binary and a provider config + Check Required Env Variables + Login MicroShift Host + Setup Kubeconfig + Command Should Work install -d -o root -g root -m 0755 ${CP_BIN_DIR} + Upload String To File ${CP_MOCK_SCRIPT} ${CP_MOCK_PROVIDER} + Command Should Work chmod 0755 ${CP_MOCK_PROVIDER} + Upload String To File ${CP_PROVIDER_CONFIG} ${CP_CONFIG_FILE} + +Teardown + [Documentation] Remove the drop-in and fixtures, restart MicroShift to restore clean state + Remove Credential Provider Config + # Defensive: a failed test may leave the config dir behind; remove it too. + Command Should Work rm -rf ${CP_BIN_DIR} ${CP_CONFIG_FILE} ${CP_CONFIG_DIR} + Restart MicroShift + Remove Kubeconfig + Logout MicroShift Host + +Restart MicroShift With Cursor + [Documentation] Record the journal cursor, then restart MicroShift + ${cursor}= Get Journal Cursor + VAR ${CURSOR}= ${cursor} scope=TEST + Restart MicroShift + +Apply Credential Provider Config + [Documentation] Apply a drop-in config and restart MicroShift, recording the journal cursor + [Arguments] ${config} + Remove Drop In MicroShift Config ${CP_DROPIN} + Drop In MicroShift Config ${config} ${CP_DROPIN} + Restart MicroShift With Cursor + +Apply Invalid Credential Provider Config + [Documentation] Apply a drop-in config that should prevent MicroShift from starting + [Arguments] ${config} + Remove Drop In MicroShift Config ${CP_DROPIN} + Restart MicroShift + Drop In MicroShift Config ${config} ${CP_DROPIN} + ${cursor}= Get Journal Cursor + VAR ${CURSOR}= ${cursor} scope=TEST + Run Keyword And Expect Error 0 != 1 Restart MicroShift + +Remove Credential Provider Config + [Documentation] Remove the credential provider drop-in without restarting. + ... The next test's setup restarts MicroShift. + Remove Drop In MicroShift Config ${CP_DROPIN}