diff --git a/go.mod b/go.mod index 13434162f..c9019479d 100644 --- a/go.mod +++ b/go.mod @@ -53,6 +53,8 @@ require ( sigs.k8s.io/yaml v1.6.0 ) +require github.com/prometheus/common v0.67.5 + require ( dario.cat/mergo v1.0.1 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect @@ -121,7 +123,6 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect - github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/otlptranslator v1.0.0 // indirect github.com/prometheus/procfs v0.20.1 // indirect github.com/redis/go-redis/extra/rediscmd/v9 v9.0.5 // indirect diff --git a/internal/chart/v3/metadata.go b/internal/chart/v3/metadata.go index 5f7cea897..bc28d3540 100644 --- a/internal/chart/v3/metadata.go +++ b/internal/chart/v3/metadata.go @@ -17,12 +17,24 @@ package v3 import ( "path/filepath" + "slices" "strings" "unicode" "github.com/Masterminds/semver/v3" ) +// KubeFeatureGateComponents are the Kubernetes component names accepted as +// top-level keys in Metadata.KubeFeatureGates. +var KubeFeatureGateComponents = []string{ + "apiserver", + "kubelet", + "scheduler", + "controllerManager", + "cloudControllerManager", + "kubeProxy", +} + // Maintainer describes a Chart maintainer. type Maintainer struct { // Name is a user name or organization name @@ -77,6 +89,8 @@ type Metadata struct { Annotations map[string]string `json:"annotations,omitempty"` // KubeVersion is a SemVer constraint specifying the version of Kubernetes required. KubeVersion string `json:"kubeVersion,omitempty"` + // KubeFeatureGates specifies Kubernetes feature gates required for this chart. + KubeFeatureGates map[string]map[string]bool `json:"kubeFeatureGates,omitempty"` // Dependencies are a list of dependencies for a chart. Dependencies []*Dependency `json:"dependencies,omitempty"` // Specifies the chart type: application or library @@ -129,6 +143,17 @@ func (md *Metadata) Validate() error { return ValidationError("chart.metadata.type must be application or library") } + for component, gates := range md.KubeFeatureGates { + if !slices.Contains(KubeFeatureGateComponents, component) { + return ValidationErrorf("chart.metadata.kubeFeatureGates has unknown component %q, must be one of %s", component, strings.Join(KubeFeatureGateComponents, ", ")) + } + for gate := range gates { + if gate == "" { + return ValidationErrorf("chart.metadata.kubeFeatureGates.%s has an empty feature gate name", component) + } + } + } + for _, m := range md.Maintainers { if err := m.Validate(); err != nil { return err diff --git a/internal/chart/v3/metadata_test.go b/internal/chart/v3/metadata_test.go index 91b917980..054bec2e8 100644 --- a/internal/chart/v3/metadata_test.go +++ b/internal/chart/v3/metadata_test.go @@ -16,6 +16,7 @@ limitations under the License. package v3 import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -190,6 +191,47 @@ func TestValidate(t *testing.T) { &Metadata{APIVersion: "3", Name: "test", Version: "1.2.3.4"}, ValidationError("chart.metadata.version \"1.2.3.4\" is invalid"), }, + { + "kubeFeatureGates with unknown component", + &Metadata{ + Name: "test", + APIVersion: "v3", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "unknownComponent": {"SomeGate": true}, + }, + }, + ValidationErrorf("chart.metadata.kubeFeatureGates has unknown component %q, must be one of %s", "unknownComponent", strings.Join(KubeFeatureGateComponents, ", ")), + }, + { + "kubeFeatureGates with empty gate name", + &Metadata{ + Name: "test", + APIVersion: "v3", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "apiserver": {"": true}, + }, + }, + ValidationErrorf("chart.metadata.kubeFeatureGates.%s has an empty feature gate name", "apiserver"), + }, + { + "kubeFeatureGates valid across multiple components", + &Metadata{ + Name: "test", + APIVersion: "v3", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "apiserver": {"ConstrainedImpersonation": true}, + "kubelet": {"SidecarContainers": true}, + "scheduler": {"ComponentFlagz": false}, + }, + }, + nil, + }, } for _, tt := range tests { diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 056c539a5..975d6fa22 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -256,6 +256,12 @@ func withKube(version string) chartOption { } } +func withKubeFeatureGates(gates map[string]map[string]bool) chartOption { + return func(opts *chartOptions) { + opts.Metadata.KubeFeatureGates = gates + } +} + // releaseStub creates a release stub, complete with the chartStub as its chart. func releaseStub() *release.Release { return namedReleaseStub("angry-panda", rcommon.StatusDeployed) diff --git a/pkg/action/install.go b/pkg/action/install.go index 605c423bc..d674196bf 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -315,6 +315,19 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st return nil, fmt.Errorf("chart dependencies processing failed: %w", err) } + // Verified before CRDs are installed: CRDs are not rolled back on failure + // and no release is recorded, so a chart whose gates can't be satisfied + // must never get that far. + wantedFeatureGates, err := mergedKubeFeatureGates(chrt) + if err != nil { + return nil, err + } + if len(wantedFeatureGates) > 0 && interactWithServer(i.DryRunStrategy) { + if err := i.cfg.checkKubeFeatureGates(ctx, wantedFeatureGates); err != nil { + return nil, err + } + } + // Pre-install anything in the crd/ directory. We do this before Helm // contacts the upstream server and builds the capabilities object. if crds := chrt.CRDObjects(); interactWithServer(i.DryRunStrategy) && !i.SkipCRDs && len(crds) > 0 { diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 2d83abe27..3677e8a60 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -42,6 +42,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" ci "helm.sh/helm/v4/pkg/chart" @@ -609,6 +610,86 @@ func TestInstallRelease_KubeVersion(t *testing.T) { is.ErrorContains(err, "chart requires kubeVersion: >=99.0.0 which is incompatible with Kubernetes v1.20.") } +func TestInstallRelease_KubeFeatureGates(t *testing.T) { + req := require.New(t) + + // A chart declaring only a component Helm cannot yet actively verify + // (see supportedKubeFeatureGateComponents) must never block install, even + // though the default test fixture has no RESTClientGetter configured. + instAction := installAction(t) + vals := map[string]any{} + _, err := instAction.Run(buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "scheduler": {"ComponentFlagz": false}, + })), vals) + req.NoError(err, "unsupported components must warn, not block") + + // A supported component that Helm cannot reach (no cluster configured) + // blocks install, since Helm cannot positively confirm the chart's + // requirement is satisfied. + instAction = installAction(t) + instAction.ReleaseName = "should-fail-unreachable" + _, err = instAction.Run(buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + })), vals) + req.Error(err) + req.ErrorContains(err, "apiserver: could not verify feature gates") + + // With a reachable apiserver reporting the requested gates, a match + // installs cleanly and a mismatch blocks with a clear error. + server := newFeatureGateTestServer(t) + cfg := actionConfigFixture(t) + cfg.RESTClientGetter = &fakeRESTClientGetter{restConfig: &rest.Config{Host: server.URL}} + + matchAction := installActionWithConfig(cfg) + _, err = matchAction.Run(buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true, "ClusterTrustBundle": false}, + })), vals) + req.NoError(err, "matching gate state must install cleanly") + + mismatchAction := installActionWithConfig(cfg) + mismatchAction.ReleaseName = "should-also-fail" + _, err = mismatchAction.Run(buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"ClusterTrustBundle": true}, + })), vals) + req.Error(err) + req.ErrorContains(err, "apiserver.ClusterTrustBundle=true (cluster reports false)") +} + +func TestInstallRelease_KubeFeatureGates_RespectsInteractWithRemote(t *testing.T) { + req := require.New(t) + + instAction := installAction(t) + instAction.cfg.RESTClientGetter = &poisonRESTClientGetter{t: t} + instAction.DryRunStrategy = DryRunClient + + _, err := instAction.Run(buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + })), map[string]any{}) + req.NoError(err) +} + +func TestInstallRelease_KubeFeatureGates_RunsBeforeCRDInstall(t *testing.T) { + req := require.New(t) + + cfg := actionConfigFixture(t) + failer := cfg.KubeClient.(*kubefake.FailingKubeClient) + failer.CreateError = errors.New("CRD create should not have been attempted") + cfg.KubeClient = failer + + crdFile := common.File{ + Name: "crds/example.yaml", + Data: []byte("apiVersion: apiextensions.k8s.io/v1\nkind: CustomResourceDefinition\nmetadata:\n name: examples.example.com\n"), + } + ch := buildChart(withFile(crdFile), withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + })) + + _, err := installActionWithConfig(cfg).Run(ch, map[string]any{}) + req.Error(err) + req.ErrorContains(err, "apiserver: could not verify feature gates") + req.NotErrorIs(err, failer.CreateError) +} + func TestInstallRelease_Wait(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/action/kube_feature_gates.go b/pkg/action/kube_feature_gates.go new file mode 100644 index 000000000..eef958bb5 --- /dev/null +++ b/pkg/action/kube_feature_gates.go @@ -0,0 +1,283 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "context" + "errors" + "fmt" + "log/slog" + "slices" + "strings" + "time" + + "github.com/prometheus/common/expfmt" + "github.com/prometheus/common/model" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +// featureGateMetricName is the Prometheus gauge every Kubernetes component +// built on k8s.io/component-base exposes on its own /metrics endpoint, +// recording the enablement state of every feature gate known to that +// process. See: +// https://kubernetes.io/docs/tasks/administer-cluster/configure-feature-gates/#check-via-metrics-endpoint +const featureGateMetricName = "kubernetes_feature_enabled" + +// defaultKubeletPort is used when a Node does not report a kubelet port. +const defaultKubeletPort = 10250 + +// featureGateFetchTimeout bounds each network call in this file: rendering +// happens outside the window helm's --timeout flag covers. +const featureGateFetchTimeout = 10 * time.Second + +// supportedKubeFeatureGateComponents are the components this version of Helm +// can actively verify feature gates for. Components accepted by chart +// validation (chart.KubeFeatureGateComponents) but not listed here are only +// ever warned about, never enforced: checking them requires machinery Helm +// does not have today (pod discovery plus a port-forward/exec tunnel), and +// they are unreachable on most managed Kubernetes offerings regardless. +var supportedKubeFeatureGateComponents = map[string]bool{ + "apiserver": true, + "kubelet": true, +} + +// mergedKubeFeatureGates collects the kubeFeatureGates requirements declared +// by chrt and all of its dependencies (recursively) into a single map, so a +// component is only ever polled once regardless of how many chart levels +// declare requirements for it. Conflicting requirements for the same gate +// from different chart levels are a chart authoring error, not a cluster +// state to warn about, so they fail immediately. +func mergedKubeFeatureGates(chrt *chart.Chart) (map[string]map[string]bool, error) { + type declaration struct { + chart string + want bool + } + declaredBy := map[string]map[string]declaration{} + + var walk func(c *chart.Chart) error + walk = func(c *chart.Chart) error { + for component, gates := range c.Metadata.KubeFeatureGates { + if declaredBy[component] == nil { + declaredBy[component] = map[string]declaration{} + } + for gate, want := range gates { + if prior, ok := declaredBy[component][gate]; ok && prior.want != want { + return fmt.Errorf("chart %q requires kubeFeatureGates.%s.%s=%t but chart %q requires %t", c.Name(), component, gate, want, prior.chart, prior.want) + } + declaredBy[component][gate] = declaration{chart: c.Name(), want: want} + } + } + for _, dep := range c.Dependencies() { + if err := walk(dep); err != nil { + return err + } + } + return nil + } + if err := walk(chrt); err != nil { + return nil, err + } + + merged := make(map[string]map[string]bool, len(declaredBy)) + for component, gates := range declaredBy { + g := make(map[string]bool, len(gates)) + for gate, d := range gates { + g[gate] = d.want + } + merged[component] = g + } + return merged, nil +} + +// checkKubeFeatureGates verifies that the feature gates required by the +// chart (keyed by component name, then by gate name) match the state +// reported by the cluster. A component missing from wanted is not checked at +// all. +// +// A component this version of Helm does not yet support checking is only +// logged as a warning and does not block installation. For a component Helm +// does support (apiserver, kubelet), any failure to positively confirm a +// required gate -- RBAC denied, component unreachable, timeout, a parse +// error, or the gate simply not being reported -- blocks installation, the +// same as a definitive mismatch. +func (cfg *Configuration) checkKubeFeatureGates(ctx context.Context, wanted map[string]map[string]bool) error { + components := make([]string, 0, len(wanted)) + for component := range wanted { + components = append(components, component) + } + slices.Sort(components) + + var problems []string + for _, component := range components { + gates := wanted[component] + if len(gates) == 0 { + continue + } + + if !supportedKubeFeatureGateComponents[component] { + cfg.Logger().Warn( + "chart requires kubeFeatureGates for a component Helm cannot yet verify; skipping", + slog.String("component", component), + ) + continue + } + + actual, err := cfg.fetchComponentFeatureGates(ctx, component) + if err != nil { + problems = append(problems, fmt.Sprintf("%s: could not verify feature gates: %s", component, err)) + continue + } + + gateNames := make([]string, 0, len(gates)) + for gate := range gates { + gateNames = append(gateNames, gate) + } + slices.Sort(gateNames) + + for _, gate := range gateNames { + want := gates[gate] + got, known := actual[gate] + if !known { + problems = append(problems, fmt.Sprintf("%s.%s: not reported by the cluster", component, gate)) + continue + } + if got != want { + problems = append(problems, fmt.Sprintf("%s.%s=%t (cluster reports %t)", component, gate, want, got)) + } + } + } + + if len(problems) > 0 { + return fmt.Errorf("chart requires kubeFeatureGates that could not be satisfied or verified: %s", strings.Join(problems, ", ")) + } + return nil +} + +// fetchComponentFeatureGates retrieves and parses the feature gate state +// reported by the given component's /metrics endpoint. +func (cfg *Configuration) fetchComponentFeatureGates(ctx context.Context, component string) (map[string]bool, error) { + if cfg.RESTClientGetter == nil { + return nil, errors.New("no Kubernetes cluster is configured") + } + + clientset, err := cfg.KubernetesClientSet() + if err != nil { + return nil, err + } + + var raw []byte + switch component { + case "apiserver": + apiserverCtx, cancel := context.WithTimeout(ctx, featureGateFetchTimeout) + defer cancel() + raw, err = clientset.CoreV1().RESTClient().Get().RequestURI("/metrics").Do(apiserverCtx).Raw() + case "kubelet": + raw, err = fetchKubeletMetrics(ctx, clientset) + default: + return nil, fmt.Errorf("component %q is not supported", component) + } + if err != nil { + return nil, err + } + + return parseFeatureGateMetrics(raw) +} + +// fetchKubeletMetrics polls the /metrics endpoint of a single representative +// Ready node, via the API server's node proxy subresource. kubelet feature +// gates are technically per-node; Helm treats one Ready node as +// representative of the cluster rather than polling every node. +func fetchKubeletMetrics(ctx context.Context, clientset kubernetes.Interface) ([]byte, error) { + listCtx, cancelList := context.WithTimeout(ctx, featureGateFetchTimeout) + defer cancelList() + nodes, err := clientset.CoreV1().Nodes().List(listCtx, metav1.ListOptions{}) + if err != nil { + return nil, fmt.Errorf("listing nodes: %w", err) + } + + node, ok := firstReadyNode(nodes.Items) + if !ok { + return nil, errors.New("no Ready node found to check kubelet feature gates against") + } + + port := node.Status.DaemonEndpoints.KubeletEndpoint.Port + if port == 0 { + port = defaultKubeletPort + } + + metricsCtx, cancelMetrics := context.WithTimeout(ctx, featureGateFetchTimeout) + defer cancelMetrics() + return clientset.CoreV1().RESTClient().Get(). + Resource("nodes"). + SubResource("proxy"). + Name(fmt.Sprintf("%s:%d", node.Name, port)). + Suffix("metrics"). + Do(metricsCtx).Raw() +} + +// firstReadyNode returns the first node in the list reporting a True Ready +// condition. +func firstReadyNode(nodes []corev1.Node) (corev1.Node, bool) { + for _, node := range nodes { + for _, cond := range node.Status.Conditions { + if cond.Type == corev1.NodeReady && cond.Status == corev1.ConditionTrue { + return node, true + } + } + } + return corev1.Node{}, false +} + +// parseFeatureGateMetrics parses the raw Prometheus text exposition format +// scraped from a component's /metrics endpoint and extracts the enablement +// state of every feature gate it reports. +func parseFeatureGateMetrics(raw []byte) (map[string]bool, error) { + // kubernetes_feature_enabled and its labels always use the classic + // Prometheus name character set; expfmt.TextParser requires an explicit + // scheme (its zero value is invalid) since it added UTF-8 name support. + parser := expfmt.NewTextParser(model.LegacyValidation) + families, err := parser.TextToMetricFamilies(bytes.NewReader(raw)) + if err != nil { + return nil, fmt.Errorf("parsing metrics: %w", err) + } + + family, ok := families[featureGateMetricName] + if !ok { + return map[string]bool{}, nil + } + + gates := make(map[string]bool, len(family.Metric)) + for _, m := range family.Metric { + var name string + for _, label := range m.Label { + if label.GetName() == "name" { + name = label.GetValue() + break + } + } + if name == "" || m.Gauge == nil { + continue + } + gates[name] = m.Gauge.GetValue() != 0 + } + return gates, nil +} diff --git a/pkg/action/kube_feature_gates_test.go b/pkg/action/kube_feature_gates_test.go new file mode 100644 index 000000000..72c6e298a --- /dev/null +++ b/pkg/action/kube_feature_gates_test.go @@ -0,0 +1,257 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" +) + +// fakeRESTClientGetter is a minimal RESTClientGetter that only supports +// ToRESTConfig, enough to exercise code paths that build a Kubernetes +// clientset directly against a local httptest server. +type fakeRESTClientGetter struct { + restConfig *rest.Config +} + +func (f *fakeRESTClientGetter) ToRESTConfig() (*rest.Config, error) { + return f.restConfig, nil +} + +func (f *fakeRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + return nil, errors.New("not implemented") +} + +func (f *fakeRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { + return nil, errors.New("not implemented") +} + +// poisonRESTClientGetter fails the test if any of its methods are called, +// used to prove a code path never contacts the cluster. +type poisonRESTClientGetter struct { + t *testing.T +} + +func (p *poisonRESTClientGetter) ToRESTConfig() (*rest.Config, error) { + p.t.Fatal("ToRESTConfig should not be called") + return nil, nil +} + +func (p *poisonRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + p.t.Fatal("ToDiscoveryClient should not be called") + return nil, nil +} + +func (p *poisonRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { + p.t.Fatal("ToRESTMapper should not be called") + return nil, nil +} + +const sampleFeatureGateMetrics = `# HELP kubernetes_feature_enabled [BETA] This metric records the data about the stage and enablement of a k8s feature. +# TYPE kubernetes_feature_enabled gauge +kubernetes_feature_enabled{name="ClusterTrustBundle",stage="BETA"} 0 +kubernetes_feature_enabled{name="SidecarContainers",stage=""} 1 +` + +func TestParseFeatureGateMetrics(t *testing.T) { + tests := []struct { + name string + raw string + want map[string]bool + wantErr bool + }{ + { + name: "parses enabled and disabled gates", + raw: sampleFeatureGateMetrics, + want: map[string]bool{ + "ClusterTrustBundle": false, + "SidecarContainers": true, + }, + }, + { + name: "metric family absent", + raw: "# HELP unrelated_metric a metric we don't care about\n# TYPE unrelated_metric gauge\nunrelated_metric 1\n", + want: map[string]bool{}, + }, + { + name: "malformed input", + raw: "kubernetes_feature_enabled{name=\"X\" 1\n", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseFeatureGateMetrics([]byte(tt.raw)) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +// newFeatureGateTestServer returns an httptest server whose /metrics handler +// serves sampleFeatureGateMetrics, standing in for kube-apiserver. +func newFeatureGateTestServer(t *testing.T) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/metrics", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain; version=0.0.4") + _, _ = w.Write([]byte(sampleFeatureGateMetrics)) + }) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + return server +} + +func TestCheckKubeFeatureGates(t *testing.T) { + t.Run("no component declared is a no-op", func(t *testing.T) { + cfg := actionConfigFixture(t) + require.NoError(t, cfg.checkKubeFeatureGates(t.Context(), nil)) + }) + + t.Run("unsupported component only warns, never blocks", func(t *testing.T) { + cfg := actionConfigFixture(t) + // actionConfigFixture never sets RESTClientGetter; if this component were + // (incorrectly) treated as supported, fetching would fail loudly instead + // of being skipped outright. + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "scheduler": {"ComponentFlagz": false}, + }) + require.NoError(t, err) + }) + + t.Run("no cluster configured blocks with a clear error", func(t *testing.T) { + cfg := actionConfigFixture(t) + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "apiserver: could not verify feature gates") + }) + + server := newFeatureGateTestServer(t) + cfg := actionConfigFixture(t) + cfg.RESTClientGetter = &fakeRESTClientGetter{restConfig: &rest.Config{Host: server.URL}} + + t.Run("matching gate state passes", func(t *testing.T) { + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "apiserver": { + "ClusterTrustBundle": false, + "SidecarContainers": true, + }, + }) + require.NoError(t, err) + }) + + t.Run("mismatching gate state blocks with a clear error", func(t *testing.T) { + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "apiserver": {"ClusterTrustBundle": true}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "apiserver.ClusterTrustBundle=true (cluster reports false)") + }) + + t.Run("gate absent from scraped output blocks with a clear error", func(t *testing.T) { + // Covers the issue's primary scenario: an older cluster (or one + // predating the gate) that doesn't report it at all must not be + // treated as satisfying the requirement. + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "apiserver": {"SomeGateThisServerDoesNotReport": true}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "apiserver.SomeGateThisServerDoesNotReport: not reported by the cluster") + }) +} + +func TestCheckKubeFeatureGates_ClusterPredatesFeatureGateMetric(t *testing.T) { + // Simulates a Kubernetes version older than 1.26, which doesn't expose + // kubernetes_feature_enabled at all -- the issue's primary scenario of a + // lower K8s version must not be treated as satisfying the requirement. + mux := http.NewServeMux() + mux.HandleFunc("/metrics", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain; version=0.0.4") + _, _ = w.Write([]byte("# HELP unrelated_metric a metric with nothing to do with feature gates\n# TYPE unrelated_metric gauge\nunrelated_metric 1\n")) + }) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + cfg := actionConfigFixture(t) + cfg.RESTClientGetter = &fakeRESTClientGetter{restConfig: &rest.Config{Host: server.URL}} + + err := cfg.checkKubeFeatureGates(t.Context(), map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "apiserver.SidecarContainers: not reported by the cluster") +} + +func TestMergedKubeFeatureGates(t *testing.T) { + t.Run("no dependencies returns the root chart's own requirements", func(t *testing.T) { + ch := buildChart(withKubeFeatureGates(map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + })) + merged, err := mergedKubeFeatureGates(ch) + require.NoError(t, err) + assert.Equal(t, map[string]map[string]bool{"apiserver": {"SidecarContainers": true}}, merged) + }) + + t.Run("combines root and dependency requirements for different gates", func(t *testing.T) { + ch := buildChart( + withKubeFeatureGates(map[string]map[string]bool{"apiserver": {"SidecarContainers": true}}), + withDependency(withName("sub"), withKubeFeatureGates(map[string]map[string]bool{"kubelet": {"NodeSwap": false}})), + ) + merged, err := mergedKubeFeatureGates(ch) + require.NoError(t, err) + assert.Equal(t, map[string]map[string]bool{ + "apiserver": {"SidecarContainers": true}, + "kubelet": {"NodeSwap": false}, + }, merged) + }) + + t.Run("agreeing requirements for the same gate from multiple levels are fine", func(t *testing.T) { + ch := buildChart( + withKubeFeatureGates(map[string]map[string]bool{"apiserver": {"SidecarContainers": true}}), + withDependency(withName("sub"), withKubeFeatureGates(map[string]map[string]bool{"apiserver": {"SidecarContainers": true}})), + ) + merged, err := mergedKubeFeatureGates(ch) + require.NoError(t, err) + assert.Equal(t, map[string]map[string]bool{"apiserver": {"SidecarContainers": true}}, merged) + }) + + t.Run("conflicting requirements for the same gate error out", func(t *testing.T) { + ch := buildChart( + withName("root"), + withKubeFeatureGates(map[string]map[string]bool{"apiserver": {"SidecarContainers": true}}), + withDependency(withName("sub"), withKubeFeatureGates(map[string]map[string]bool{"apiserver": {"SidecarContainers": false}})), + ) + _, err := mergedKubeFeatureGates(ch) + require.Error(t, err) + assert.Contains(t, err.Error(), "apiserver.SidecarContainers") + }) +} diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..5f05e97f8 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -299,6 +299,16 @@ func (u *Upgrade) prepareUpgrade(ctx context.Context, name string, chart *chartv return nil, nil, false, err } + wantedFeatureGates, err := mergedKubeFeatureGates(chart) + if err != nil { + return nil, nil, false, err + } + if len(wantedFeatureGates) > 0 && interactWithServer(u.DryRunStrategy) { + if err := u.cfg.checkKubeFeatureGates(ctx, wantedFeatureGates); err != nil { + return nil, nil, false, err + } + } + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(ctx, chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy) if err != nil { return nil, nil, false, err diff --git a/pkg/chart/v2/metadata.go b/pkg/chart/v2/metadata.go index 5df307321..0325c3a75 100644 --- a/pkg/chart/v2/metadata.go +++ b/pkg/chart/v2/metadata.go @@ -17,12 +17,24 @@ package v2 import ( "path/filepath" + "slices" "strings" "unicode" "github.com/Masterminds/semver/v3" ) +// KubeFeatureGateComponents are the Kubernetes component names accepted as +// top-level keys in Metadata.KubeFeatureGates. +var KubeFeatureGateComponents = []string{ + "apiserver", + "kubelet", + "scheduler", + "controllerManager", + "cloudControllerManager", + "kubeProxy", +} + // Maintainer describes a Chart maintainer. type Maintainer struct { // Name is a user name or organization name @@ -77,6 +89,8 @@ type Metadata struct { Annotations map[string]string `json:"annotations,omitempty"` // KubeVersion is a SemVer constraint specifying the version of Kubernetes required. KubeVersion string `json:"kubeVersion,omitempty"` + // KubeFeatureGates specifies Kubernetes feature gates required for this chart. + KubeFeatureGates map[string]map[string]bool `json:"kubeFeatureGates,omitempty"` // Dependencies are a list of dependencies for a chart. Dependencies []*Dependency `json:"dependencies,omitempty"` // Specifies the chart type: application or library @@ -129,6 +143,17 @@ func (md *Metadata) Validate() error { return ValidationError("chart.metadata.type must be application or library") } + for component, gates := range md.KubeFeatureGates { + if !slices.Contains(KubeFeatureGateComponents, component) { + return ValidationErrorf("chart.metadata.kubeFeatureGates has unknown component %q, must be one of %s", component, strings.Join(KubeFeatureGateComponents, ", ")) + } + for gate := range gates { + if gate == "" { + return ValidationErrorf("chart.metadata.kubeFeatureGates.%s has an empty feature gate name", component) + } + } + } + for _, m := range md.Maintainers { if err := m.Validate(); err != nil { return err diff --git a/pkg/chart/v2/metadata_test.go b/pkg/chart/v2/metadata_test.go index 720ca4886..a46615882 100644 --- a/pkg/chart/v2/metadata_test.go +++ b/pkg/chart/v2/metadata_test.go @@ -16,6 +16,7 @@ limitations under the License. package v2 import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -190,6 +191,47 @@ func TestValidate(t *testing.T) { &Metadata{APIVersion: "v2", Name: "test", Version: "1.2.3.4"}, ValidationError("chart.metadata.version \"1.2.3.4\" is invalid"), }, + { + "kubeFeatureGates with unknown component", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "unknownComponent": {"SomeGate": true}, + }, + }, + ValidationErrorf("chart.metadata.kubeFeatureGates has unknown component %q, must be one of %s", "unknownComponent", strings.Join(KubeFeatureGateComponents, ", ")), + }, + { + "kubeFeatureGates with empty gate name", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "apiserver": {"": true}, + }, + }, + ValidationErrorf("chart.metadata.kubeFeatureGates.%s has an empty feature gate name", "apiserver"), + }, + { + "kubeFeatureGates valid across multiple components", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + KubeFeatureGates: map[string]map[string]bool{ + "apiserver": {"ConstrainedImpersonation": true}, + "kubelet": {"SidecarContainers": true}, + "scheduler": {"ComponentFlagz": false}, + }, + }, + nil, + }, } for _, tt := range tests {