kube: configure proper account impersonation NS

Fixing a regression introduced in #480 which would always pick the
namespace of the release. In addition, historically seen the
configuration of the impersonation username while making use of a
KubeConfig has never worked correctly, this has been adressed as well.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit is contained in:
Hidde Beydals 2022-06-07 12:25:29 +02:00
parent bf7406b2de
commit d19b470412
5 changed files with 53 additions and 42 deletions

View File

@ -474,10 +474,13 @@ func (r *HelmReleaseReconciler) checkDependencies(hr v2.HelmRelease) error {
}
func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) {
opts := []kube.ClientGetterOption{kube.WithClientOptions(r.ClientOpts)}
if hr.Spec.ServiceAccountName != "" {
opts = append(opts, kube.WithImpersonate(hr.Spec.ServiceAccountName))
opts := []kube.ClientGetterOption{
kube.WithClientOptions(r.ClientOpts),
// When ServiceAccountName is empty, it will fall back to the configured default.
// If this is not configured either, this option will result in a no-op.
kube.WithImpersonate(hr.Spec.ServiceAccountName, hr.GetNamespace()),
}
opts = append(opts)
if hr.Spec.KubeConfig != nil {
secretName := types.NamespacedName{
Namespace: hr.GetNamespace(),

View File

@ -33,11 +33,12 @@ const (
// clientGetterOptions used to BuildClientGetter.
type clientGetterOptions struct {
namespace string
kubeConfig []byte
impersonateAccount string
clientOptions client.Options
kubeConfigOptions client.KubeConfigOptions
namespace string
kubeConfig []byte
impersonateAccount string
impersonateNamespace string
clientOptions client.Options
kubeConfigOptions client.KubeConfigOptions
}
// ClientGetterOption configures a genericclioptions.RESTClientGetter.
@ -61,10 +62,12 @@ func WithClientOptions(opts client.Options) func(o *clientGetterOptions) {
}
// WithImpersonate configures the genericclioptions.RESTClientGetter to
// impersonate the provided account name.
func WithImpersonate(accountName string) func(o *clientGetterOptions) {
// impersonate with the given account name in the provided namespace.
// If the account name is empty, DefaultServiceAccountName is assumed.
func WithImpersonate(accountName, namespace string) func(o *clientGetterOptions) {
return func(o *clientGetterOptions) {
o.impersonateAccount = accountName
o.impersonateNamespace = namespace
}
}
@ -80,7 +83,7 @@ func BuildClientGetter(namespace string, opts ...ClientGetterOption) (genericcli
opt(o)
}
if len(o.kubeConfig) > 0 {
return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.clientOptions, o.kubeConfigOptions), nil
return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.impersonateNamespace, o.clientOptions, o.kubeConfigOptions), nil
}
return NewInClusterRESTClientGetter(namespace, o.impersonateAccount, &o.clientOptions)
return NewInClusterRESTClientGetter(namespace, o.impersonateAccount, o.impersonateNamespace, &o.clientOptions)
}

View File

@ -67,7 +67,7 @@ users:`)
cfgOpts := client.KubeConfigOptions{InsecureTLS: true}
impersonate := "jane"
getter, err := BuildClientGetter(namespace, WithClientOptions(clientOpts), WithKubeConfig(cfg, 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{}))
@ -85,7 +85,8 @@ users:`)
namespace := "a-namespace"
impersonate := "frank"
getter, err := BuildClientGetter(namespace, WithImpersonate(impersonate))
impersonateNS := "other-namespace"
getter, err := BuildClientGetter(namespace, WithImpersonate(impersonate, impersonateNS))
g.Expect(err).ToNot(HaveOccurred())
g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{}))
@ -93,16 +94,17 @@ users:`)
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"))
g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:other-namespace:frank"))
})
t.Run("with DefaultServiceAccount", func(t *testing.T) {
t.Run("with impersonate DefaultServiceAccount", func(t *testing.T) {
g := NewWithT(t)
ctrl.GetConfig = mockGetConfig
namespace := "a-namespace"
DefaultServiceAccountName = "frank"
getter, err := BuildClientGetter(namespace)
impersonateNS := "other-namespace"
getter, err := BuildClientGetter(namespace, WithImpersonate("", impersonateNS))
g.Expect(err).ToNot(HaveOccurred())
g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{}))
@ -110,7 +112,7 @@ users:`)
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"))
g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:other-namespace:frank"))
})
}

View File

@ -35,12 +35,12 @@ import (
// using genericclioptions.NewConfigFlags, and configures it with the server,
// 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) {
func NewInClusterRESTClientGetter(namespace, impersonateAccount, impersonateNamespace 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)
SetImpersonationConfig(cfg, impersonateNamespace, impersonateAccount)
flags := genericclioptions.NewConfigFlags(false)
flags.APIServer = pointer.String(cfg.Host)
@ -73,6 +73,8 @@ type MemoryRESTClientGetter struct {
namespace string
// impersonateAccount configures the rest.ImpersonationConfig account name.
impersonateAccount string
// impersonateAccount configures the rest.ImpersonationConfig account namespace.
impersonateNamespace string
}
// NewMemoryRESTClientGetter returns a MemoryRESTClientGetter configured with
@ -81,15 +83,17 @@ type MemoryRESTClientGetter struct {
func NewMemoryRESTClientGetter(
kubeConfig []byte,
namespace string,
impersonate string,
impersonateAccount string,
impersonateNamespace string,
clientOpts client.Options,
kubeConfigOpts client.KubeConfigOptions) genericclioptions.RESTClientGetter {
return &MemoryRESTClientGetter{
kubeConfig: kubeConfig,
namespace: namespace,
impersonateAccount: impersonate,
clientOpts: clientOpts,
kubeConfigOpts: kubeConfigOpts,
kubeConfig: kubeConfig,
namespace: namespace,
impersonateAccount: impersonateAccount,
impersonateNamespace: impersonateNamespace,
clientOpts: clientOpts,
kubeConfigOpts: kubeConfigOpts,
}
}
@ -102,9 +106,7 @@ func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) {
return nil, err
}
cfg = client.KubeConfig(cfg, c.kubeConfigOpts)
if c.impersonateAccount != "" {
cfg.Impersonate = rest.ImpersonationConfig{UserName: c.impersonateAccount}
}
SetImpersonationConfig(cfg, c.impersonateNamespace, c.impersonateAccount)
return cfg, nil
}

View File

@ -60,7 +60,7 @@ func TestNewInClusterRESTClientGetter(t *testing.T) {
ctrl.GetConfig = func() (*rest.Config, error) {
return cfg, nil
}
got, err := NewInClusterRESTClientGetter("", "", nil)
got, err := NewInClusterRESTClientGetter("", "", "", nil)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{}))
@ -83,7 +83,7 @@ func TestNewInClusterRESTClientGetter(t *testing.T) {
ctrl.GetConfig = func() (*rest.Config, error) {
return nil, fmt.Errorf("error")
}
got, err := NewInClusterRESTClientGetter("", "", nil)
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())
@ -94,7 +94,7 @@ func TestNewInClusterRESTClientGetter(t *testing.T) {
ctrl.GetConfig = mockGetConfig
namespace := "a-space"
got, err := NewInClusterRESTClientGetter(namespace, "", nil)
got, err := NewInClusterRESTClientGetter(namespace, "", "", nil)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{}))
@ -109,20 +109,21 @@ func TestNewInClusterRESTClientGetter(t *testing.T) {
ctrl.GetConfig = mockGetConfig
ns := "a-namespace"
accountName := "foo"
got, err := NewInClusterRESTClientGetter(ns, accountName, nil)
accountNamespace := "another-namespace"
got, err := NewInClusterRESTClientGetter(ns, accountName, accountNamespace, 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(fmt.Sprintf("system:serviceaccount:%s:%s", ns, accountName)))
g.Expect(*flags.Impersonate).To(Equal(fmt.Sprintf("system:serviceaccount:%s:%s", accountNamespace, accountName)))
})
}
func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) {
t.Run("loads REST config from KubeConfig", func(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{}, 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"))
@ -131,11 +132,11 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) {
t.Run("sets ImpersonationConfig", func(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter(cfg, "", "someone", client.Options{}, client.KubeConfigOptions{})
getter := NewMemoryRESTClientGetter(cfg, "", "someone", "a-namespace", client.Options{}, client.KubeConfigOptions{})
got, err := getter.ToRESTConfig()
g.Expect(err).ToNot(HaveOccurred())
g.Expect(got.Impersonate.UserName).To(Equal("someone"))
g.Expect(got.Impersonate.UserName).To(Equal("system:serviceaccount:a-namespace:someone"))
})
t.Run("uses KubeConfigOptions", func(t *testing.T) {
@ -143,7 +144,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", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{
getter := NewMemoryRESTClientGetter(cfg, "", "someone", "", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{
UserAgent: agent,
})
@ -155,7 +156,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) {
t.Run("invalid config", func(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter([]byte("invalid"), "", "", client.Options{QPS: 400, Burst: 800}, 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())
@ -165,7 +166,7 @@ func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) {
func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{QPS: 400, Burst: 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())
@ -174,7 +175,7 @@ func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) {
func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter(cfg, "", "", client.Options{QPS: 400, Burst: 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())
@ -183,7 +184,7 @@ func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) {
func TestMemoryRESTClientGetter_ToRawKubeConfigLoader(t *testing.T) {
g := NewWithT(t)
getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{})
getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", "other-namespace", client.Options{QPS: 400, Burst: 800}, client.KubeConfigOptions{})
got := getter.ToRawKubeConfigLoader()
g.Expect(got).ToNot(BeNil())