Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kube: configure proper account impersonation NS #492

Merged
merged 1 commit into from
Jun 7, 2022
Merged

kube: configure proper account impersonation NS #492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions controllers/helmrelease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
21 changes: 12 additions & 9 deletions internal/kube/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
}

Expand All @@ -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)
}
14 changes: 8 additions & 6 deletions internal/kube/builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}))

Expand All @@ -85,32 +85,34 @@ 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{}))

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"))
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{}))

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"))
g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:other-namespace:frank"))
})
}

Expand Down
24 changes: 13 additions & 11 deletions internal/kube/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
}
}

Expand All @@ -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
}

Expand Down
27 changes: 14 additions & 13 deletions internal/kube/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}))

Expand All @@ -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())
Expand All @@ -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{}))

Expand All @@ -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"))
Expand All @@ -131,19 +132,19 @@ 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) {
g := NewWithT(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,
})

Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())

Expand Down