From 4371610e4ba2b07e1e0c5641d1145ca3fb34ee07 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 May 2022 12:33:34 +0200 Subject: [PATCH 1/3] Cherry-pick kube changes from dev This is a partial cherry-pick of commit ae4f499e877f4, including changes around `kube`. This to include some of the changes around the construction of the ConfigFlags RESTClientGetter, as an attempt to solve token refresh issues. Signed-off-by: Hidde Beydals --- controllers/helmrelease_controller.go | 53 ++------ go.mod | 2 +- internal/kube/builder.go | 86 +++++++++++++ internal/kube/builder_test.go | 109 ++++++++++++++++ internal/kube/client.go | 57 +++++--- internal/kube/client_test.go | 179 ++++++++++++++++++++++++++ internal/kube/config.go | 51 ++++++++ internal/kube/config_test.go | 143 ++++++++++++++++++++ internal/kube/impersonate.go | 47 +++++++ internal/kube/impersonate_test.go | 75 +++++++++++ main.go | 19 ++- 11 files changed, 750 insertions(+), 71 deletions(-) create mode 100644 internal/kube/builder.go create mode 100644 internal/kube/builder_test.go create mode 100644 internal/kube/client_test.go create mode 100644 internal/kube/config.go create mode 100644 internal/kube/config_test.go create mode 100644 internal/kube/impersonate.go create mode 100644 internal/kube/impersonate_test.go diff --git a/controllers/helmrelease_controller.go b/controllers/helmrelease_controller.go index 0d386e30f..d42f2f372 100644 --- a/controllers/helmrelease_controller.go +++ b/controllers/helmrelease_controller.go @@ -295,7 +295,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, log := ctrl.LoggerFrom(ctx) // Initialize Helm action runner - getter, err := r.getRESTClientGetter(ctx, hr) + getter, err := r.buildRESTClientGetter(ctx, hr) if err != nil { return v2.HelmReleaseNotReady(hr, v2.InitFailedReason, err.Error()), err } @@ -472,23 +472,11 @@ func (r *HelmReleaseReconciler) checkDependencies(hr v2.HelmRelease) error { return nil } -func (r *HelmReleaseReconciler) setImpersonationConfig(restConfig *rest.Config, hr v2.HelmRelease) string { - name := r.DefaultServiceAccount - if sa := hr.Spec.ServiceAccountName; sa != "" { - name = sa +func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) { + var opts []kube.ClientGetterOption + if hr.Spec.ServiceAccountName != "" { + opts = append(opts, kube.WithImpersonate(hr.Spec.ServiceAccountName)) } - if name != "" { - username := fmt.Sprintf("system:serviceaccount:%s:%s", hr.GetNamespace(), name) - restConfig.Impersonate = rest.ImpersonationConfig{UserName: username} - return username - } - return "" -} - -func (r *HelmReleaseReconciler) getRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) { - config := *r.Config - impersonateAccount := r.setImpersonationConfig(&config, hr) - if hr.Spec.KubeConfig != nil { secretName := types.NamespacedName{ Namespace: hr.GetNamespace(), @@ -498,32 +486,13 @@ func (r *HelmReleaseReconciler) getRESTClientGetter(ctx context.Context, hr v2.H if err := r.Get(ctx, secretName, &secret); err != nil { return nil, fmt.Errorf("could not find KubeConfig secret '%s': %w", secretName, err) } - - var kubeConfig []byte - switch { - case hr.Spec.KubeConfig.SecretRef.Key != "": - key := hr.Spec.KubeConfig.SecretRef.Key - kubeConfig = secret.Data[key] - if kubeConfig == nil { - return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' key with a kubeconfig", secretName, key) - } - case secret.Data["value"] != nil: - kubeConfig = secret.Data["value"] - case secret.Data["value.yaml"] != nil: - kubeConfig = secret.Data["value.yaml"] - default: - // User did not specify a key, and the 'value' key was not defined. - return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a 'value' key with a kubeconfig", secretName) + kubeConfig, err := kube.ConfigFromSecret(&secret, hr.Spec.KubeConfig.SecretRef.Key) + if err != nil { + return nil, err } - - return kube.NewMemoryRESTClientGetter(kubeConfig, hr.GetReleaseNamespace(), impersonateAccount, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts), nil + opts = append(opts, kube.WithKubeConfig(kubeConfig, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts)) } - - if r.DefaultServiceAccount != "" || hr.Spec.ServiceAccountName != "" { - return kube.NewInClusterRESTClientGetter(&config, hr.GetReleaseNamespace()), nil - } - - return kube.NewInClusterRESTClientGetter(r.Config, hr.GetReleaseNamespace()), nil + return kube.BuildClientGetter(r.Config, hr.GetReleaseNamespace(), opts...), nil } @@ -653,7 +622,7 @@ func (r *HelmReleaseReconciler) reconcileDelete(ctx context.Context, hr v2.HelmR // Only uninstall the Helm Release if the resource is not suspended. if !hr.Spec.Suspend { - getter, err := r.getRESTClientGetter(ctx, hr) + getter, err := r.buildRESTClientGetter(ctx, hr) if err != nil { return ctrl.Result{}, err } diff --git a/go.mod b/go.mod index c294829b9..f211e6188 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( k8s.io/apimachinery v0.23.6 k8s.io/cli-runtime v0.23.6 k8s.io/client-go v0.23.6 + k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 sigs.k8s.io/controller-runtime v0.11.2 sigs.k8s.io/kustomize/api v0.11.4 sigs.k8s.io/yaml v1.3.0 @@ -165,7 +166,6 @@ require ( k8s.io/klog/v2 v2.50.0 // indirect k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65 // indirect k8s.io/kubectl v0.23.5 // indirect - k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect oras.land/oras-go v1.1.1 // indirect sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect sigs.k8s.io/kustomize/kyaml v0.13.6 // indirect diff --git a/internal/kube/builder.go b/internal/kube/builder.go new file mode 100644 index 000000000..de6928b53 --- /dev/null +++ b/internal/kube/builder.go @@ -0,0 +1,86 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" + + "github.com/fluxcd/pkg/runtime/client" +) + +const ( + // DefaultKubeConfigSecretKey is the default data key ConfigFromSecret + // looks at when no data key is provided. + DefaultKubeConfigSecretKey = "value" + // DefaultKubeConfigSecretKeyExt is the default data key ConfigFromSecret + // looks at when no data key is provided, and DefaultKubeConfigSecretKey + // does not exist. + DefaultKubeConfigSecretKeyExt = DefaultKubeConfigSecretKey + ".yaml" +) + +// clientGetterOptions used to BuildClientGetter. +type clientGetterOptions struct { + config *rest.Config + namespace string + kubeConfig []byte + burst int + qps float32 + impersonateAccount string + kubeConfigOptions client.KubeConfigOptions +} + +// ClientGetterOption configures a genericclioptions.RESTClientGetter. +type ClientGetterOption func(o *clientGetterOptions) + +// WithKubeConfig creates a MemoryRESTClientGetter configured with the provided +// KubeConfig and other values. +func WithKubeConfig(kubeConfig []byte, qps float32, burst int, opts client.KubeConfigOptions) func(o *clientGetterOptions) { + return func(o *clientGetterOptions) { + o.kubeConfig = kubeConfig + o.qps = qps + o.burst = burst + o.kubeConfigOptions = opts + } +} + +// WithImpersonate configures the genericclioptions.RESTClientGetter to +// impersonate the provided account name. +func WithImpersonate(accountName string) func(o *clientGetterOptions) { + return func(o *clientGetterOptions) { + o.impersonateAccount = accountName + } +} + +// BuildClientGetter builds a genericclioptions.RESTClientGetter based on the +// provided options and returns the result. config and namespace are mandatory, +// and not expected to be nil or empty. +func BuildClientGetter(config *rest.Config, namespace string, opts ...ClientGetterOption) genericclioptions.RESTClientGetter { + o := &clientGetterOptions{ + config: config, + namespace: namespace, + } + for _, opt := range opts { + opt(o) + } + if len(o.kubeConfig) > 0 { + return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.qps, o.burst, o.kubeConfigOptions) + } + cfg := *config + SetImpersonationConfig(&cfg, namespace, o.impersonateAccount) + return NewInClusterRESTClientGetter(&cfg, namespace) +} diff --git a/internal/kube/builder_test.go b/internal/kube/builder_test.go new file mode 100644 index 000000000..20c9b5a8b --- /dev/null +++ b/internal/kube/builder_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "testing" + + "github.com/fluxcd/pkg/runtime/client" + . "github.com/onsi/gomega" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" +) + +func TestBuildClientGetter(t *testing.T) { + t.Run("with config and namespace", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + BearerToken: "a-token", + } + namespace := "a-namespace" + getter := BuildClientGetter(cfg, namespace) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.BearerToken).ToNot(BeNil()) + g.Expect(*flags.BearerToken).To(Equal(cfg.BearerToken)) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + }) + + t.Run("with kubeconfig and impersonate", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + cfg := []byte(`apiVersion: v1 +clusters: +- cluster: + server: https://example.com + name: example-cluster +contexts: +- context: + cluster: example-cluster + namespace: flux-system +kind: Config +preferences: {} +users:`) + qps := float32(600) + burst := 1000 + cfgOpts := client.KubeConfigOptions{InsecureTLS: true} + + impersonate := "jane" + + getter := BuildClientGetter(&rest.Config{}, namespace, WithKubeConfig(cfg, qps, burst, cfgOpts), WithImpersonate(impersonate)) + g.Expect(getter).To(BeAssignableToTypeOf(&MemoryRESTClientGetter{})) + + got := getter.(*MemoryRESTClientGetter) + g.Expect(got.namespace).To(Equal(namespace)) + g.Expect(got.kubeConfig).To(Equal(cfg)) + g.Expect(got.qps).To(Equal(qps)) + g.Expect(got.burst).To(Equal(burst)) + g.Expect(got.kubeConfigOpts).To(Equal(cfgOpts)) + g.Expect(got.impersonateAccount).To(Equal(impersonate)) + }) + + t.Run("with config and impersonate account", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + impersonate := "frank" + getter := BuildClientGetter(&rest.Config{}, namespace, WithImpersonate(impersonate)) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) + }) + + t.Run("with config and DefaultServiceAccount", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + DefaultServiceAccountName = "frank" + getter := BuildClientGetter(&rest.Config{}, namespace) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) + }) +} diff --git a/internal/kube/client.go b/internal/kube/client.go index b8f35ecdf..98e5aba0a 100644 --- a/internal/kube/client.go +++ b/internal/kube/client.go @@ -24,53 +24,69 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/clientcmd" + "k8s.io/utils/pointer" "github.com/fluxcd/pkg/runtime/client" ) +// NewInClusterRESTClientGetter creates a new genericclioptions.RESTClientGetter +// using genericclioptions.NewConfigFlags, and configures it with the server, +// authentication, impersonation, and burst and QPS settings, and the provided +// namespace. func NewInClusterRESTClientGetter(cfg *rest.Config, namespace string) genericclioptions.RESTClientGetter { flags := genericclioptions.NewConfigFlags(false) - flags.APIServer = &cfg.Host - flags.BearerToken = &cfg.BearerToken - flags.CAFile = &cfg.CAFile - flags.Namespace = &namespace + flags.APIServer = pointer.String(cfg.Host) + flags.BearerToken = pointer.String(cfg.BearerToken) + flags.CAFile = pointer.String(cfg.CAFile) + flags.Namespace = pointer.String(namespace) flags.WithDiscoveryBurst(cfg.Burst) flags.WithDiscoveryQPS(cfg.QPS) if sa := cfg.Impersonate.UserName; sa != "" { - flags.Impersonate = &sa + flags.Impersonate = pointer.String(sa) } - return flags } // MemoryRESTClientGetter is an implementation of the genericclioptions.RESTClientGetter, // capable of working with an in-memory kubeconfig file. type MemoryRESTClientGetter struct { - kubeConfig []byte - namespace string + // kubeConfig used to load a rest.Config, after being sanitized. + kubeConfig []byte + // kubeConfigOpts control the sanitization of the kubeConfig. + kubeConfigOpts client.KubeConfigOptions + // namespace specifies the namespace the client is configured to. + namespace string + // impersonateAccount configures the rest.ImpersonationConfig account name. impersonateAccount string - qps float32 - burst int - kubeConfigOpts client.KubeConfigOptions + // qps configures the QPS on the discovery.DiscoveryClient. + qps float32 + // burst configures the burst on the discovery.DiscoveryClient. + burst int } +// NewMemoryRESTClientGetter returns a MemoryRESTClientGetter configured with +// the provided values and client.KubeConfigOptions. The provided KubeConfig is +// sanitized, configure the settings for this using client.KubeConfigOptions. func NewMemoryRESTClientGetter( kubeConfig []byte, namespace string, - impersonateAccount string, + impersonate string, qps float32, burst int, kubeConfigOpts client.KubeConfigOptions) genericclioptions.RESTClientGetter { return &MemoryRESTClientGetter{ kubeConfig: kubeConfig, namespace: namespace, - impersonateAccount: impersonateAccount, + impersonateAccount: impersonate, qps: qps, burst: burst, kubeConfigOpts: kubeConfigOpts, } } +// ToRESTConfig creates a rest.Config with the rest.ImpersonationConfig configured +// with to the impersonation account. It loads the config the KubeConfig bytes and +// sanitizes it using the client.KubeConfigOptions. func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) { cfg, err := clientcmd.RESTConfigFromKubeConfig(c.kubeConfig) if err != nil { @@ -83,23 +99,25 @@ func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) { return cfg, nil } +// ToDiscoveryClient returns a discovery.CachedDiscoveryInterface configured +// with ToRESTConfig, and the QPS and Burst settings. func (c *MemoryRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { config, err := c.ToRESTConfig() if err != nil { return nil, err } - if c.impersonateAccount != "" { - config.Impersonate = rest.ImpersonationConfig{UserName: c.impersonateAccount} - } - config.QPS = c.qps config.Burst = c.burst - discoveryClient, _ := discovery.NewDiscoveryClientForConfig(config) + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return nil, err + } return memory.NewMemCacheClient(discoveryClient), nil } +// ToRESTMapper returns a RESTMapper constructed from ToDiscoveryClient. func (c *MemoryRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { discoveryClient, err := c.ToDiscoveryClient() if err != nil { @@ -111,6 +129,9 @@ func (c *MemoryRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { return expander, nil } +// ToRawKubeConfigLoader returns a clientcmd.ClientConfig using +// clientcmd.DefaultClientConfig. With clientcmd.ClusterDefaults, namespace, and +// impersonate configured as overwrites. func (c *MemoryRESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig { loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() // use the standard defaults for this client command diff --git a/internal/kube/client_test.go b/internal/kube/client_test.go new file mode 100644 index 000000000..a232f7a3a --- /dev/null +++ b/internal/kube/client_test.go @@ -0,0 +1,179 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "testing" + + "github.com/fluxcd/pkg/runtime/client" + . "github.com/onsi/gomega" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" +) + +var cfg = []byte(`current-context: federal-context +apiVersion: v1 +clusters: +- cluster: + api-version: v1 + server: http://cow.org:8080 + insecure-skip-tls-verify: true + name: cow-cluster +contexts: +- context: + cluster: cow-cluster + user: blue-user + name: federal-context +kind: Config +users: +- name: blue-user + user: + token: foo`) + +func TestNewInClusterRESTClientGetter(t *testing.T) { + t.Run("api server config", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + Host: "https://example.com", + BearerToken: "chase-the-honey", + TLSClientConfig: rest.TLSClientConfig{ + CAFile: "afile", + }, + } + + got := NewInClusterRESTClientGetter(cfg, "") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + fields := map[*string]*string{ + flags.APIServer: &cfg.Host, + flags.BearerToken: &cfg.BearerToken, + flags.CAFile: &cfg.CAFile, + } + for f, ff := range fields { + g.Expect(f).ToNot(BeNil()) + g.Expect(f).To(Equal(ff)) + g.Expect(f).ToNot(BeIdenticalTo(ff)) + } + }) + + t.Run("namespace", func(t *testing.T) { + g := NewWithT(t) + + got := NewInClusterRESTClientGetter(&rest.Config{}, "a-space") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal("a-space")) + }) + + t.Run("impersonation", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + Impersonate: rest.ImpersonationConfig{ + UserName: "system:serviceaccount:namespace:foo", + }, + } + + got := NewInClusterRESTClientGetter(cfg, "") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal(cfg.Impersonate.UserName)) + }) +} + +func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { + t.Run("loads REST config from KubeConfig", func(t *testing.T) { + g := NewWithT(t) + getter := NewMemoryRESTClientGetter(cfg, "", "", 0, 0, client.KubeConfigOptions{}) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.Host).To(Equal("http://cow.org:8080")) + g.Expect(got.TLSClientConfig.Insecure).To(BeFalse()) + }) + + t.Run("sets ImpersonationConfig", func(t *testing.T) { + g := NewWithT(t) + getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{}) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.Impersonate.UserName).To(Equal("someone")) + }) + + t.Run("uses KubeConfigOptions", func(t *testing.T) { + g := NewWithT(t) + + agent := "a static string forever," + + "but static strings can have dreams and hope too" + getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{ + UserAgent: agent, + }) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.UserAgent).To(Equal(agent)) + }) + + t.Run("invalid config", func(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter([]byte(`invalid`), "", "", 0, 0, client.KubeConfigOptions{}) + got, err := getter.ToRESTConfig() + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) +} + +func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + got, err := getter.ToDiscoveryClient() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) +} + +func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + got, err := getter.ToRESTMapper() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) +} + +func TestMemoryRESTClientGetter_ToRawKubeConfigLoader(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", 0, 0, client.KubeConfigOptions{}) + got := getter.ToRawKubeConfigLoader() + g.Expect(got).ToNot(BeNil()) + + cfg, err := got.ClientConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cfg.Impersonate.UserName).To(Equal("impersonate")) + ns, _, err := got.Namespace() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ns).To(Equal("a-namespace")) +} diff --git a/internal/kube/config.go b/internal/kube/config.go new file mode 100644 index 000000000..d39e6eca0 --- /dev/null +++ b/internal/kube/config.go @@ -0,0 +1,51 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" +) + +// ConfigFromSecret returns the KubeConfig data from the provided key in the +// given Secret, or attempts to load the data from the default `value` and +// `value.yaml` keys. If a Secret is provided but no key with data can be +// found, an error is returned. The secret may be nil, in which case no bytes +// nor error are returned. Validation of the data is expected to happen while +// decoding the bytes. +func ConfigFromSecret(secret *corev1.Secret, key string) ([]byte, error) { + var kubeConfig []byte + if secret != nil { + secretName := fmt.Sprintf("%s/%s", secret.Namespace, secret.Name) + switch { + case key != "": + kubeConfig = secret.Data[key] + if kubeConfig == nil { + return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' key with data", secretName, key) + } + case secret.Data[DefaultKubeConfigSecretKey] != nil: + kubeConfig = secret.Data[DefaultKubeConfigSecretKey] + case secret.Data[DefaultKubeConfigSecretKeyExt] != nil: + kubeConfig = secret.Data[DefaultKubeConfigSecretKeyExt] + default: + // User did not specify a key, and the 'value' key was not defined. + return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' or '%s' key with data", secretName, DefaultKubeConfigSecretKey, DefaultKubeConfigSecretKeyExt) + } + } + return kubeConfig, nil +} diff --git a/internal/kube/config_test.go b/internal/kube/config_test.go new file mode 100644 index 000000000..58c52d5cf --- /dev/null +++ b/internal/kube/config_test.go @@ -0,0 +1,143 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestConfigFromSecret(t *testing.T) { + t.Run("with default key", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + DefaultKubeConfigSecretKey: []byte("good"), + // Also confirm priority. + DefaultKubeConfigSecretKeyExt: []byte("bad"), + }, + } + got, err := ConfigFromSecret(secret, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[DefaultKubeConfigSecretKey])) + }) + + t.Run("with default key with ext", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + DefaultKubeConfigSecretKeyExt: []byte("good"), + }, + } + got, err := ConfigFromSecret(secret, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[DefaultKubeConfigSecretKeyExt])) + }) + + t.Run("with key", func(t *testing.T) { + g := NewWithT(t) + + key := "cola.recipe" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + key: []byte("snow"), + }, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[key])) + }) + + t.Run("invalid key", func(t *testing.T) { + g := NewWithT(t) + + key := "black-hole" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{}, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("secret 'vault/super-secret' does not contain a 'black-hole' key ")) + g.Expect(got).To(Equal(secret.Data[key])) + }) + + t.Run("key without data", func(t *testing.T) { + g := NewWithT(t) + + key := "void" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + key: nil, + }, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("does not contain a 'void' key with data")) + }) + + t.Run("no keys", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{}, + } + + got, err := ConfigFromSecret(secret, "") + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("does not contain a 'value' or 'value.yaml'")) + }) + + t.Run("nil secret", func(t *testing.T) { + g := NewWithT(t) + + got, err := ConfigFromSecret(nil, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) +} diff --git a/internal/kube/impersonate.go b/internal/kube/impersonate.go new file mode 100644 index 000000000..e7176fc3f --- /dev/null +++ b/internal/kube/impersonate.go @@ -0,0 +1,47 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "fmt" + + "k8s.io/client-go/rest" +) + +// DefaultServiceAccountName can be set at runtime to enable a fallback account +// name when no service account name is provided to SetImpersonationConfig. +var DefaultServiceAccountName string + +// userNameFormat is the format of a system service account user name string. +// It formats into `system:serviceaccount:namespace:name`. +const userNameFormat = "system:serviceaccount:%s:%s" + +// SetImpersonationConfig configures the provided service account name if +// given, or the DefaultServiceAccountName as a fallback if set. It returns +// the configured impersonation username, or an empty string. +func SetImpersonationConfig(cfg *rest.Config, namespace, serviceAccount string) string { + name := DefaultServiceAccountName + if serviceAccount != "" { + name = serviceAccount + } + if name != "" && namespace != "" { + username := fmt.Sprintf(userNameFormat, namespace, name) + cfg.Impersonate = rest.ImpersonationConfig{UserName: username} + return username + } + return "" +} diff --git a/internal/kube/impersonate_test.go b/internal/kube/impersonate_test.go new file mode 100644 index 000000000..d9a2d4cd4 --- /dev/null +++ b/internal/kube/impersonate_test.go @@ -0,0 +1,75 @@ +/* +Copyright 2022 The Flux 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 kube + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/client-go/rest" +) + +func TestSetImpersonationConfig(t *testing.T) { + t.Run("DefaultServiceAccountName", func(t *testing.T) { + g := NewWithT(t) + + DefaultServiceAccountName = "default" + namespace := "test" + expect := "system:serviceaccount:" + namespace + ":" + DefaultServiceAccountName + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, namespace, "") + g.Expect(name).To(Equal(expect)) + g.Expect(cfg.Impersonate.UserName).ToNot(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(Equal(name)) + }) + + t.Run("overwrite DefaultServiceAccountName", func(t *testing.T) { + g := NewWithT(t) + + DefaultServiceAccountName = "default" + namespace := "test" + serviceAccount := "different" + expect := "system:serviceaccount:" + namespace + ":" + serviceAccount + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, namespace, serviceAccount) + g.Expect(name).To(Equal(expect)) + g.Expect(cfg.Impersonate.UserName).ToNot(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(Equal(name)) + }) + + t.Run("without namespace", func(t *testing.T) { + g := NewWithT(t) + + serviceAccount := "account" + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, "", serviceAccount) + g.Expect(name).To(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(BeEmpty()) + }) + + t.Run("no arguments", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, "", "") + g.Expect(name).To(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(BeEmpty()) + }) +} diff --git a/main.go b/main.go index 8e870406a..48da77720 100644 --- a/main.go +++ b/main.go @@ -43,6 +43,7 @@ import ( v2 "github.com/fluxcd/helm-controller/api/v2beta1" "github.com/fluxcd/helm-controller/controllers" + intkube "github.com/fluxcd/helm-controller/internal/kube" // +kubebuilder:scaffold:imports ) @@ -76,7 +77,6 @@ func main() { aclOptions acl.Options leaderElectionOptions leaderelection.Options rateLimiterOptions helper.RateLimiterOptions - defaultServiceAccount string ) flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") @@ -87,7 +87,7 @@ func main() { flag.BoolVar(&watchAllNamespaces, "watch-all-namespaces", true, "Watch for custom resources in all namespaces, if set to false it will only watch the runtime namespace.") flag.IntVar(&httpRetry, "http-retry", 9, "The maximum number of retries when failing to fetch artifacts over HTTP.") - flag.StringVar(&defaultServiceAccount, "default-service-account", "", "Default service account used for impersonation.") + flag.StringVar(&intkube.DefaultServiceAccountName, "default-service-account", "", "Default service account used for impersonation.") clientOptions.BindFlags(flag.CommandLine) logOptions.BindFlags(flag.CommandLine) aclOptions.BindFlags(flag.CommandLine) @@ -139,14 +139,13 @@ func main() { } if err = (&controllers.HelmReleaseReconciler{ - Client: mgr.GetClient(), - Config: mgr.GetConfig(), - Scheme: mgr.GetScheme(), - EventRecorder: eventRecorder, - MetricsRecorder: metricsRecorder, - NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, - DefaultServiceAccount: defaultServiceAccount, - KubeConfigOpts: kubeConfigOpts, + Client: mgr.GetClient(), + Config: mgr.GetConfig(), + Scheme: mgr.GetScheme(), + EventRecorder: eventRecorder, + MetricsRecorder: metricsRecorder, + NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, + KubeConfigOpts: kubeConfigOpts, }).SetupWithManager(mgr, controllers.HelmReleaseReconcilerOptions{ MaxConcurrentReconciles: concurrent, DependencyRequeueInterval: requeueDependency, From 5784f0644afa9f365383f469416b57093e2ffe7a Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 May 2022 12:38:10 +0200 Subject: [PATCH 2/3] kube: explicitly set ConfigFlags.CacheDir to nil Signed-off-by: Hidde Beydals --- controllers/helmrelease_controller.go | 1 - internal/kube/client.go | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/helmrelease_controller.go b/controllers/helmrelease_controller.go index d42f2f372..bc9ad48ce 100644 --- a/controllers/helmrelease_controller.go +++ b/controllers/helmrelease_controller.go @@ -493,7 +493,6 @@ func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2 opts = append(opts, kube.WithKubeConfig(kubeConfig, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts)) } return kube.BuildClientGetter(r.Config, hr.GetReleaseNamespace(), opts...), nil - } // composeValues attempts to resolve all v2beta1.ValuesReference resources diff --git a/internal/kube/client.go b/internal/kube/client.go index 98e5aba0a..8e483dcd7 100644 --- a/internal/kube/client.go +++ b/internal/kube/client.go @@ -44,6 +44,9 @@ func NewInClusterRESTClientGetter(cfg *rest.Config, namespace string) genericcli if sa := cfg.Impersonate.UserName; sa != "" { flags.Impersonate = pointer.String(sa) } + // In a container, we are not expected to be able to write to the + // home dir default. However, explicitly disabling this is better. + flags.CacheDir = nil return flags } From 1bed542fe4e31631b3e8cc51a6066ff82128841a Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 10 May 2022 15:24:13 +0200 Subject: [PATCH 3/3] internal/kube: get REST config from runtime Signed-off-by: Hidde Beydals --- controllers/helmrelease_controller.go | 7 ++-- internal/kube/builder.go | 34 +++++++-------- internal/kube/builder_test.go | 48 +++++++++++++--------- internal/kube/client.go | 41 +++++++++++-------- internal/kube/client_test.go | 59 +++++++++++++++++---------- main.go | 1 + 6 files changed, 112 insertions(+), 78 deletions(-) diff --git a/controllers/helmrelease_controller.go b/controllers/helmrelease_controller.go index bc9ad48ce..e83c74015 100644 --- a/controllers/helmrelease_controller.go +++ b/controllers/helmrelease_controller.go @@ -83,6 +83,7 @@ type HelmReleaseReconciler struct { MetricsRecorder *metrics.Recorder DefaultServiceAccount string NoCrossNamespaceRef bool + ClientOpts fluxClient.Options KubeConfigOpts fluxClient.KubeConfigOptions } @@ -473,7 +474,7 @@ func (r *HelmReleaseReconciler) checkDependencies(hr v2.HelmRelease) error { } func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) { - var opts []kube.ClientGetterOption + opts := []kube.ClientGetterOption{kube.WithClientOptions(r.ClientOpts)} if hr.Spec.ServiceAccountName != "" { opts = append(opts, kube.WithImpersonate(hr.Spec.ServiceAccountName)) } @@ -490,9 +491,9 @@ func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2 if err != nil { return nil, err } - opts = append(opts, kube.WithKubeConfig(kubeConfig, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts)) + opts = append(opts, kube.WithKubeConfig(kubeConfig, r.KubeConfigOpts)) } - return kube.BuildClientGetter(r.Config, hr.GetReleaseNamespace(), opts...), nil + return kube.BuildClientGetter(hr.GetReleaseNamespace(), opts...) } // composeValues attempts to resolve all v2beta1.ValuesReference resources diff --git a/internal/kube/builder.go b/internal/kube/builder.go index de6928b53..dc37f0642 100644 --- a/internal/kube/builder.go +++ b/internal/kube/builder.go @@ -17,10 +17,8 @@ limitations under the License. package kube import ( - "k8s.io/cli-runtime/pkg/genericclioptions" - "k8s.io/client-go/rest" - "github.com/fluxcd/pkg/runtime/client" + "k8s.io/cli-runtime/pkg/genericclioptions" ) const ( @@ -35,12 +33,10 @@ const ( // clientGetterOptions used to BuildClientGetter. type clientGetterOptions struct { - config *rest.Config namespace string kubeConfig []byte - burst int - qps float32 impersonateAccount string + clientOptions client.Options kubeConfigOptions client.KubeConfigOptions } @@ -49,15 +45,21 @@ type ClientGetterOption func(o *clientGetterOptions) // WithKubeConfig creates a MemoryRESTClientGetter configured with the provided // KubeConfig and other values. -func WithKubeConfig(kubeConfig []byte, qps float32, burst int, opts client.KubeConfigOptions) func(o *clientGetterOptions) { +func WithKubeConfig(kubeConfig []byte, opts client.KubeConfigOptions) func(o *clientGetterOptions) { return func(o *clientGetterOptions) { o.kubeConfig = kubeConfig - o.qps = qps - o.burst = burst o.kubeConfigOptions = opts } } +// WithClientOptions configures the genericclioptions.RESTClientGetter with +// provided options. +func WithClientOptions(opts client.Options) func(o *clientGetterOptions) { + return func(o *clientGetterOptions) { + o.clientOptions = opts + } +} + // WithImpersonate configures the genericclioptions.RESTClientGetter to // impersonate the provided account name. func WithImpersonate(accountName string) func(o *clientGetterOptions) { @@ -67,20 +69,18 @@ func WithImpersonate(accountName string) func(o *clientGetterOptions) { } // BuildClientGetter builds a genericclioptions.RESTClientGetter based on the -// provided options and returns the result. config and namespace are mandatory, -// and not expected to be nil or empty. -func BuildClientGetter(config *rest.Config, namespace string, opts ...ClientGetterOption) genericclioptions.RESTClientGetter { +// provided options and returns the result. Namespace is not expected to be +// empty. In case it fails to construct using NewInClusterRESTClientGetter, it +// returns an error. +func BuildClientGetter(namespace string, opts ...ClientGetterOption) (genericclioptions.RESTClientGetter, error) { o := &clientGetterOptions{ - config: config, namespace: namespace, } for _, opt := range opts { opt(o) } if len(o.kubeConfig) > 0 { - return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.qps, o.burst, o.kubeConfigOptions) + return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.clientOptions, o.kubeConfigOptions), nil } - cfg := *config - SetImpersonationConfig(&cfg, namespace, o.impersonateAccount) - return NewInClusterRESTClientGetter(&cfg, namespace) + return NewInClusterRESTClientGetter(namespace, o.impersonateAccount, &o.clientOptions) } diff --git a/internal/kube/builder_test.go b/internal/kube/builder_test.go index 20c9b5a8b..04785eb3c 100644 --- a/internal/kube/builder_test.go +++ b/internal/kube/builder_test.go @@ -17,34 +17,38 @@ limitations under the License. package kube import ( + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "testing" "github.com/fluxcd/pkg/runtime/client" . "github.com/onsi/gomega" "k8s.io/cli-runtime/pkg/genericclioptions" - "k8s.io/client-go/rest" ) func TestBuildClientGetter(t *testing.T) { - t.Run("with config and namespace", func(t *testing.T) { + t.Run("with namespace and retrieved config", func(t *testing.T) { g := NewWithT(t) - - cfg := &rest.Config{ - BearerToken: "a-token", + cfg := &rest.Config{Host: "https://example.com"} + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil } + namespace := "a-namespace" - getter := BuildClientGetter(cfg, namespace) + getter, err := BuildClientGetter(namespace) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := getter.(*genericclioptions.ConfigFlags) - g.Expect(flags.BearerToken).ToNot(BeNil()) - g.Expect(*flags.BearerToken).To(Equal(cfg.BearerToken)) g.Expect(flags.Namespace).ToNot(BeNil()) g.Expect(*flags.Namespace).To(Equal(namespace)) + g.Expect(flags.APIServer).ToNot(BeNil()) + g.Expect(*flags.APIServer).To(Equal(cfg.Host)) }) - t.Run("with kubeconfig and impersonate", func(t *testing.T) { + t.Run("with kubeconfig, impersonate and client options", func(t *testing.T) { g := NewWithT(t) + ctrl.GetConfig = mockGetConfig namespace := "a-namespace" cfg := []byte(`apiVersion: v1 @@ -59,30 +63,30 @@ contexts: kind: Config preferences: {} users:`) - qps := float32(600) - burst := 1000 + clientOpts := client.Options{QPS: 600, Burst: 1000} cfgOpts := client.KubeConfigOptions{InsecureTLS: true} - impersonate := "jane" - getter := BuildClientGetter(&rest.Config{}, namespace, WithKubeConfig(cfg, qps, burst, cfgOpts), WithImpersonate(impersonate)) + getter, err := BuildClientGetter(namespace, WithClientOptions(clientOpts), WithKubeConfig(cfg, cfgOpts), WithImpersonate(impersonate)) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(getter).To(BeAssignableToTypeOf(&MemoryRESTClientGetter{})) got := getter.(*MemoryRESTClientGetter) g.Expect(got.namespace).To(Equal(namespace)) g.Expect(got.kubeConfig).To(Equal(cfg)) - g.Expect(got.qps).To(Equal(qps)) - g.Expect(got.burst).To(Equal(burst)) + g.Expect(got.clientOpts).To(Equal(clientOpts)) g.Expect(got.kubeConfigOpts).To(Equal(cfgOpts)) g.Expect(got.impersonateAccount).To(Equal(impersonate)) }) - t.Run("with config and impersonate account", func(t *testing.T) { + t.Run("with impersonate account", func(t *testing.T) { g := NewWithT(t) + ctrl.GetConfig = mockGetConfig namespace := "a-namespace" impersonate := "frank" - getter := BuildClientGetter(&rest.Config{}, namespace, WithImpersonate(impersonate)) + getter, err := BuildClientGetter(namespace, WithImpersonate(impersonate)) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := getter.(*genericclioptions.ConfigFlags) @@ -92,12 +96,14 @@ users:`) g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) }) - t.Run("with config and DefaultServiceAccount", func(t *testing.T) { + t.Run("with DefaultServiceAccount", func(t *testing.T) { g := NewWithT(t) + ctrl.GetConfig = mockGetConfig namespace := "a-namespace" DefaultServiceAccountName = "frank" - getter := BuildClientGetter(&rest.Config{}, namespace) + getter, err := BuildClientGetter(namespace) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := getter.(*genericclioptions.ConfigFlags) @@ -107,3 +113,7 @@ users:`) g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) }) } + +func mockGetConfig() (*rest.Config, error) { + return &rest.Config{}, nil +} diff --git a/internal/kube/client.go b/internal/kube/client.go index 8e483dcd7..65692fbcd 100644 --- a/internal/kube/client.go +++ b/internal/kube/client.go @@ -17,6 +17,7 @@ limitations under the License. package kube import ( + "fmt" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/discovery" @@ -25,29 +26,38 @@ import ( "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/clientcmd" "k8s.io/utils/pointer" + controllerruntime "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/client" ) // NewInClusterRESTClientGetter creates a new genericclioptions.RESTClientGetter // using genericclioptions.NewConfigFlags, and configures it with the server, -// authentication, impersonation, and burst and QPS settings, and the provided -// namespace. -func NewInClusterRESTClientGetter(cfg *rest.Config, namespace string) genericclioptions.RESTClientGetter { +// authentication, impersonation, client options, and the provided namespace. +// It returns an error if it fails to retrieve a rest.Config. +func NewInClusterRESTClientGetter(namespace, impersonateAccount string, opts *client.Options) (genericclioptions.RESTClientGetter, error) { + cfg, err := controllerruntime.GetConfig() + if err != nil { + return nil, fmt.Errorf("failed to get config for in-cluster REST client: %w", err) + } + SetImpersonationConfig(cfg, namespace, impersonateAccount) + flags := genericclioptions.NewConfigFlags(false) flags.APIServer = pointer.String(cfg.Host) flags.BearerToken = pointer.String(cfg.BearerToken) flags.CAFile = pointer.String(cfg.CAFile) flags.Namespace = pointer.String(namespace) - flags.WithDiscoveryBurst(cfg.Burst) - flags.WithDiscoveryQPS(cfg.QPS) + if opts != nil { + flags.WithDiscoveryBurst(opts.Burst) + flags.WithDiscoveryQPS(opts.QPS) + } if sa := cfg.Impersonate.UserName; sa != "" { flags.Impersonate = pointer.String(sa) } // In a container, we are not expected to be able to write to the // home dir default. However, explicitly disabling this is better. flags.CacheDir = nil - return flags + return flags, nil } // MemoryRESTClientGetter is an implementation of the genericclioptions.RESTClientGetter, @@ -55,16 +65,14 @@ func NewInClusterRESTClientGetter(cfg *rest.Config, namespace string) genericcli type MemoryRESTClientGetter struct { // kubeConfig used to load a rest.Config, after being sanitized. kubeConfig []byte - // kubeConfigOpts control the sanitization of the kubeConfig. + // kubeConfigOpts controls the sanitization of the kubeConfig. kubeConfigOpts client.KubeConfigOptions + // clientOpts controls the kube client configuration. + clientOpts client.Options // namespace specifies the namespace the client is configured to. namespace string // impersonateAccount configures the rest.ImpersonationConfig account name. impersonateAccount string - // qps configures the QPS on the discovery.DiscoveryClient. - qps float32 - // burst configures the burst on the discovery.DiscoveryClient. - burst int } // NewMemoryRESTClientGetter returns a MemoryRESTClientGetter configured with @@ -74,15 +82,13 @@ func NewMemoryRESTClientGetter( kubeConfig []byte, namespace string, impersonate string, - qps float32, - burst int, + clientOpts client.Options, kubeConfigOpts client.KubeConfigOptions) genericclioptions.RESTClientGetter { return &MemoryRESTClientGetter{ kubeConfig: kubeConfig, namespace: namespace, impersonateAccount: impersonate, - qps: qps, - burst: burst, + clientOpts: clientOpts, kubeConfigOpts: kubeConfigOpts, } } @@ -110,8 +116,8 @@ func (c *MemoryRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryI return nil, err } - config.QPS = c.qps - config.Burst = c.burst + config.QPS = c.clientOpts.QPS + config.Burst = c.clientOpts.Burst discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) if err != nil { @@ -147,6 +153,5 @@ func (c *MemoryRESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig if c.impersonateAccount != "" { overrides.AuthInfo.Impersonate = c.impersonateAccount } - return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides) } diff --git a/internal/kube/client_test.go b/internal/kube/client_test.go index a232f7a3a..88e3e9ba2 100644 --- a/internal/kube/client_test.go +++ b/internal/kube/client_test.go @@ -17,12 +17,14 @@ limitations under the License. package kube import ( + "fmt" "testing" "github.com/fluxcd/pkg/runtime/client" . "github.com/onsi/gomega" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" ) var cfg = []byte(`current-context: federal-context @@ -45,7 +47,7 @@ users: token: foo`) func TestNewInClusterRESTClientGetter(t *testing.T) { - t.Run("api server config", func(t *testing.T) { + t.Run("discover config", func(t *testing.T) { g := NewWithT(t) cfg := &rest.Config{ @@ -55,8 +57,11 @@ func TestNewInClusterRESTClientGetter(t *testing.T) { CAFile: "afile", }, } - - got := NewInClusterRESTClientGetter(cfg, "") + ctrl.GetConfig = func() (*rest.Config, error) { + return cfg, nil + } + got, err := NewInClusterRESTClientGetter("", "", nil) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := got.(*genericclioptions.ConfigFlags) @@ -72,40 +77,52 @@ func TestNewInClusterRESTClientGetter(t *testing.T) { } }) + t.Run("config retrieval error", func(t *testing.T) { + g := NewWithT(t) + + ctrl.GetConfig = func() (*rest.Config, error) { + return nil, fmt.Errorf("error") + } + got, err := NewInClusterRESTClientGetter("", "", nil) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("failed to get config for in-cluster REST client")) + g.Expect(got).To(BeNil()) + }) + t.Run("namespace", func(t *testing.T) { g := NewWithT(t) - got := NewInClusterRESTClientGetter(&rest.Config{}, "a-space") + ctrl.GetConfig = mockGetConfig + namespace := "a-space" + got, err := NewInClusterRESTClientGetter(namespace, "", nil) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := got.(*genericclioptions.ConfigFlags) g.Expect(flags.Namespace).ToNot(BeNil()) - g.Expect(*flags.Namespace).To(Equal("a-space")) + g.Expect(*flags.Namespace).To(Equal(namespace)) }) t.Run("impersonation", func(t *testing.T) { g := NewWithT(t) - cfg := &rest.Config{ - Impersonate: rest.ImpersonationConfig{ - UserName: "system:serviceaccount:namespace:foo", - }, - } - - got := NewInClusterRESTClientGetter(cfg, "") + ctrl.GetConfig = mockGetConfig + ns := "a-namespace" + accountName := "foo" + got, err := NewInClusterRESTClientGetter(ns, accountName, nil) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) flags := got.(*genericclioptions.ConfigFlags) g.Expect(flags.Impersonate).ToNot(BeNil()) - g.Expect(*flags.Impersonate).To(Equal(cfg.Impersonate.UserName)) + g.Expect(*flags.Impersonate).To(Equal(fmt.Sprintf("system:serviceaccount:%s:%s", ns, accountName))) }) } func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { t.Run("loads REST config from KubeConfig", func(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter(cfg, "", "", 0, 0, client.KubeConfigOptions{}) - + getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{}, client.KubeConfigOptions{}) got, err := getter.ToRESTConfig() g.Expect(err).ToNot(HaveOccurred()) g.Expect(got.Host).To(Equal("http://cow.org:8080")) @@ -114,7 +131,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { t.Run("sets ImpersonationConfig", func(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{}) + getter := NewMemoryRESTClientGetter(cfg, "", "someone", client.Options{}, client.KubeConfigOptions{}) got, err := getter.ToRESTConfig() g.Expect(err).ToNot(HaveOccurred()) @@ -126,7 +143,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { agent := "a static string forever," + "but static strings can have dreams and hope too" - getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{ + getter := NewMemoryRESTClientGetter(cfg, "", "someone", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{ UserAgent: agent, }) @@ -138,7 +155,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { t.Run("invalid config", func(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter([]byte(`invalid`), "", "", 0, 0, client.KubeConfigOptions{}) + getter := NewMemoryRESTClientGetter([]byte("invalid"), "", "", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{}) got, err := getter.ToRESTConfig() g.Expect(err).To(HaveOccurred()) g.Expect(got).To(BeNil()) @@ -148,7 +165,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{}) got, err := getter.ToDiscoveryClient() g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).ToNot(BeNil()) @@ -157,7 +174,7 @@ func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) { func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{}) got, err := getter.ToRESTMapper() g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).ToNot(BeNil()) @@ -166,7 +183,7 @@ func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) { func TestMemoryRESTClientGetter_ToRawKubeConfigLoader(t *testing.T) { g := NewWithT(t) - getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", 0, 0, client.KubeConfigOptions{}) + getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{}) got := getter.ToRawKubeConfigLoader() g.Expect(got).ToNot(BeNil()) diff --git a/main.go b/main.go index 48da77720..7ebe6f848 100644 --- a/main.go +++ b/main.go @@ -145,6 +145,7 @@ func main() { EventRecorder: eventRecorder, MetricsRecorder: metricsRecorder, NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, + ClientOpts: clientOptions, KubeConfigOpts: kubeConfigOpts, }).SetupWithManager(mgr, controllers.HelmReleaseReconcilerOptions{ MaxConcurrentReconciles: concurrent,