Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Simplify clientset/kubeconfig handling
- fix issue with accidential use of authenticator
  - it was causing infinite loops when authenticator not present
  - it was due to too many deep references
- reduce overall code complexity
  • Loading branch information
errordeveloper committed Feb 21, 2019
1 parent b1186f4 commit d94e4df
Show file tree
Hide file tree
Showing 9 changed files with 183 additions and 92 deletions.
21 changes: 11 additions & 10 deletions pkg/ctl/create/cluster.go
Expand Up @@ -111,7 +111,6 @@ func skipNodeGroupsIfRequested(cfg *api.ClusterConfig) {
}
}


// checkEachNodeGroup iterates over each nodegroup and calls check function
// (this is need to avoid common goroutine-for-loop pitfall)
func checkEachNodeGroup(cfg *api.ClusterConfig, check func(i int, ng *api.NodeGroup) error) error {
Expand Down Expand Up @@ -490,25 +489,27 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, nameArg stri
// obtain cluster credentials, write kubeconfig

{ // post-creation action
clientConfigBase, err := ctl.NewClientConfig(cfg)
if err != nil {
return err
}
var kubeconfigContextName string

if writeKubeconfig {
config := clientConfigBase.WithExecAuthenticator()
kubeconfigPath, err = kubeconfig.Write(kubeconfigPath, config.Client, setContext)
clientConfig, err := ctl.NewClientConfig(cfg, false)
if err != nil {
return errors.Wrap(err, "writing kubeconfig")
return err
}
kubeconfigContextName = clientConfig.ContextName
logger.Debug("clientConfig(withAuthenticator) = %#v", clientConfig)

kubeconfigPath, err = kubeconfig.Write(kubeconfigPath, *clientConfig.Client, setContext)
if err != nil {
return errors.Wrap(err, "writing kubeconfig")
}
logger.Success("saved kubeconfig as %q", kubeconfigPath)
} else {
kubeconfigPath = ""
}

// create Kubernetes client
clientSet, err := clientConfigBase.NewClientSetWithEmbeddedToken()
clientSet, err := ctl.NewClientSet(cfg)
if err != nil {
return err
}
Expand Down Expand Up @@ -559,7 +560,7 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, nameArg stri
if err != nil {
return err
}
if err := utils.CheckAllCommands(kubeconfigPath, setContext, clientConfigBase.ContextName, env); err != nil {
if err := utils.CheckAllCommands(kubeconfigPath, setContext, kubeconfigContextName, env); err != nil {
logger.Critical("%s\n", err.Error())
logger.Info("cluster should be functional despite missing (or misconfigured) client binaries")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ctl/create/nodegroup.go
Expand Up @@ -276,7 +276,7 @@ func doCreateNodeGroups(p *api.ProviderConfig, cfg *api.ClusterConfig, nameArg s
}

{ // post-creation action
clientSet, err := ctl.NewStandardClientSet(cfg)
clientSet, err := ctl.NewClientSet(cfg)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ctl/delete/nodegroup.go
Expand Up @@ -111,7 +111,7 @@ func doDeleteNodeGroup(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.No

// post-deletion action
if updateAuthConfigMap {
clientSet, err := ctl.NewStandardClientSet(cfg)
clientSet, err := ctl.NewClientSet(cfg)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/ctl/utils/write_kubeconfig.go
Expand Up @@ -83,13 +83,12 @@ func doWriteKubeconfigCmd(p *api.ProviderConfig, cfg *api.ClusterConfig, nameArg
return err
}

clientConfigBase, err := ctl.NewClientConfig(cfg)
config, err := ctl.NewClientConfig(cfg, false)
if err != nil {
return err
}

config := clientConfigBase.WithExecAuthenticator()
filename, err := kubeconfig.Write(writeKubeconfigOutputPath, config.Client, writeKubeconfigSetContext)
filename, err := kubeconfig.Write(writeKubeconfigOutputPath, *config.Client, writeKubeconfigSetContext)
if err != nil {
return errors.Wrap(err, "writing kubeconfig")
}
Expand Down
86 changes: 23 additions & 63 deletions pkg/eks/auth.go
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/sts"
"github.com/aws/aws-sdk-go/service/sts/stsiface"
"github.com/kris-nova/logger"
"github.com/kubernetes-sigs/aws-iam-authenticator/pkg/token"
"github.com/pkg/errors"
Expand Down Expand Up @@ -152,84 +151,44 @@ func (c *ClusterProvider) getUsername() string {
// ClientConfig stores information about the client config
type ClientConfig struct {
Client *clientcmdapi.Config
Cluster *api.ClusterConfig
ClusterName string
ContextName string
roleARN string
sts stsiface.STSAPI
profile string
}

// NewClientConfig creates a new client config
// based on "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
// these are small, so we can copy these, and no need to deal with k/k as dependency
func (c *ClusterProvider) NewClientConfig(spec *api.ClusterConfig) (*ClientConfig, error) {
client, clusterName, contextName := kubeconfig.New(spec, c.getUsername(), "")
clientConfig := &ClientConfig{
Cluster: spec,
// NewClientConfig creates a new client config, if withEmbeddedToken is true
// it will embed the STS toke, otherwise it will use authenticator exec plugin
// and ensures that AWS_PROFILE environment variable gets set also
func (c *ClusterProvider) NewClientConfig(spec *api.ClusterConfig, withEmbeddedToken bool) (*ClientConfig, error) {
client, _, contextName := kubeconfig.New(spec, c.getUsername(), "")

config := &ClientConfig{
Client: client,
ClusterName: clusterName,
ContextName: contextName,
roleARN: c.Status.iamRoleARN,
sts: c.Provider.STS(),
profile: c.Provider.Profile(),
}

return clientConfig, nil
}

// NewStandardClientSet returns a new clientset using emebedded token
func (c *ClusterProvider) NewStandardClientSet(spec *api.ClusterConfig) (*clientset.Clientset, error) {
clientConfigBase, err := c.NewClientConfig(spec)
if err != nil {
return nil, err
}

clientConfig, err := clientConfigBase.WithEmbeddedToken()
if err != nil {
return nil, err
}

return clientConfig.NewClientSetWithEmbeddedToken()
}

// WithExecAuthenticator creates a copy of ClientConfig with authenticator exec plugin
// it ensures that AWS_PROFILE environment variable gets added to config also
func (c *ClientConfig) WithExecAuthenticator() *ClientConfig {
clientConfigCopy := *c

kubeconfig.AppendAuthenticator(clientConfigCopy.Client, c.Cluster, utils.DetectAuthenticator())

if len(c.profile) > 0 {
clientConfigCopy.Client.AuthInfos[c.ContextName].Exec.Env = []clientcmdapi.ExecEnvVar{
clientcmdapi.ExecEnvVar{
Name: "AWS_PROFILE",
Value: c.profile,
},
if withEmbeddedToken {
if err := config.useEmbeddedToken(spec, c.Provider.STS().(*sts.STS)); err != nil {
return nil, err
}
} else {
kubeconfig.AppendAuthenticator(config.Client, spec, utils.DetectAuthenticator(), c.Provider.Profile())
}

return &clientConfigCopy
return config, nil
}

// WithEmbeddedToken embeds the STS token
func (c *ClientConfig) WithEmbeddedToken() (*ClientConfig, error) {
clientConfigCopy := *c

func (c *ClientConfig) useEmbeddedToken(spec *api.ClusterConfig, sts *sts.STS) error {
gen, err := token.NewGenerator(true)
if err != nil {
return nil, errors.Wrap(err, "could not get token generator")
return errors.Wrap(err, "could not get token generator")
}

tok, err := gen.GetWithSTS(c.Cluster.Metadata.Name, c.sts.(*sts.STS))
tok, err := gen.GetWithSTS(spec.Metadata.Name, sts)
if err != nil {
return nil, errors.Wrap(err, "could not get token")
return errors.Wrap(err, "could not get token")
}

x := c.Client.AuthInfos[c.ContextName]
x.Token = tok

return &clientConfigCopy, nil
c.Client.AuthInfos[c.ContextName].Token = tok
return nil
}

// NewClientSet creates a new API client
Expand All @@ -246,12 +205,13 @@ func (c *ClientConfig) NewClientSet() (*clientset.Clientset, error) {
return client, nil
}

// NewClientSetWithEmbeddedToken creates a new API client with an embedded STS token
func (c *ClientConfig) NewClientSetWithEmbeddedToken() (*clientset.Clientset, error) {
clientConfig, err := c.WithEmbeddedToken()
// NewClientSet creates a new API client in one go with an embedded STS token
func (c *ClusterProvider) NewClientSet(spec *api.ClusterConfig) (*clientset.Clientset, error) {
clientConfig, err := c.NewClientConfig(spec, true)
if err != nil {
return nil, errors.Wrap(err, "creating Kubernetes client config with embedded token")
}

clientSet, err := clientConfig.NewClientSet()
if err != nil {
return nil, errors.Wrap(err, "creating Kubernetes client")
Expand Down
119 changes: 119 additions & 0 deletions pkg/eks/auth_test.go
@@ -0,0 +1,119 @@
package eks_test

import (
"strings"

awseks "github.com/aws/aws-sdk-go/service/eks"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/mock"
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha4"
. "github.com/weaveworks/eksctl/pkg/eks"
"github.com/weaveworks/eksctl/pkg/testutils"
"github.com/weaveworks/eksctl/pkg/testutils/mockprovider"
)

var _ = Describe("eks auth helpers", func() {
var ctl *ClusterProvider

Describe("constuct client config", func() {
Context("With a cluster name", func() {
clusterName := "auth-test-cluster"

BeforeEach(func() {

p := mockprovider.NewMockProvider()

ctl = &ClusterProvider{
Provider: p,
Status: &ProviderStatus{},
}

p.MockEKS().On("DescribeCluster", mock.MatchedBy(func(input *awseks.DescribeClusterInput) bool {
return *input.Name == clusterName
})).Return(&awseks.DescribeClusterOutput{
Cluster: testutils.NewFakeCluster(clusterName, awseks.ClusterStatusActive),
}, nil)

p.MockSTS().On("DescribeCluster", mock.MatchedBy(func(input *awseks.DescribeClusterInput) bool {
return *input.Name == clusterName
})).Return(&awseks.DescribeClusterOutput{
Cluster: testutils.NewFakeCluster(clusterName, awseks.ClusterStatusActive),
}, nil)

})

Context("xxx", func() {
cfg := &api.ClusterConfig{
Metadata: &api.ClusterMeta{
Name: clusterName,
Region: "eu-west-3",
},
Status: &api.ClusterStatus{
Endpoint: "https://TEST.aws",
CertificateAuthorityData: []byte("123"),
},
}

It("should create config with authenticator", func() {
clientConfig, err := ctl.NewClientConfig(cfg, false)

Expect(err).To(Not(HaveOccurred()))

Expect(clientConfig).To(Not(BeNil()))
ctx := clientConfig.ContextName
cluster := strings.Split(ctx, "@")[1]
Expect(ctx).To(Equal("iam-root-account@auth-test-cluster.eu-west-3.eksctl.io"))

k := clientConfig.Client

Expect(k.CurrentContext).To(Equal(ctx))

Expect(k.Contexts).To(HaveKey(ctx))
Expect(k.Contexts).To(HaveLen(1))

Expect(k.Contexts[ctx].Cluster).To(Equal(cluster))
Expect(k.Contexts[ctx].AuthInfo).To(Equal(ctx))

Expect(k.Contexts[ctx].LocationOfOrigin).To(BeEmpty())
Expect(k.Contexts[ctx].Namespace).To(BeEmpty())
Expect(k.Contexts[ctx].Extensions).To(BeNil())

Expect(k.AuthInfos).To(HaveKey(ctx))
Expect(k.AuthInfos).To(HaveLen(1))

Expect(k.AuthInfos[ctx].Token).To(BeEmpty())
Expect(k.AuthInfos[ctx].Exec).To(Not(BeNil()))

Expect(k.AuthInfos[ctx].Exec.Command).To(MatchRegexp("(heptio-authenticator-aws|aws-iam-authenticator)"))

Expect(strings.Join(k.AuthInfos[ctx].Exec.Args, " ")).To(Equal("token -i auth-test-cluster"))

Expect(k.Clusters).To(HaveKey(cluster))
Expect(k.Clusters).To(HaveLen(1))

Expect(k.Clusters[cluster].InsecureSkipTLSVerify).To(BeFalse())
Expect(k.Clusters[cluster].Server).To(Equal(cfg.Status.Endpoint))
Expect(k.Clusters[cluster].CertificateAuthorityData).To(Equal(cfg.Status.CertificateAuthorityData))
})

It("should create config with embedded token", func() {
// TODO: cannot test this, as token generator uses STS directly, we cannot pass the interface
// we can probably fix the package itself
})

It("should create clientset", func() {
clientConfig, err := ctl.NewClientConfig(cfg, false)

Expect(err).To(Not(HaveOccurred()))
Expect(clientConfig).To(Not(BeNil()))

clientSet, err := clientConfig.NewClientSet()

Expect(err).To(Not(HaveOccurred()))
Expect(clientSet).To(Not(BeNil()))
})
})
})
})
})
2 changes: 1 addition & 1 deletion pkg/nodebootstrap/userdata.go
Expand Up @@ -76,7 +76,7 @@ func makeClientConfigData(spec *api.ClusterConfig, ng *api.NodeGroup) ([]byte, e
if ng.AMIFamily == ami.ImageFamilyUbuntu1804 {
authenticator = kubeconfig.HeptioAuthenticatorAWS
}
kubeconfig.AppendAuthenticator(clientConfig, spec, authenticator)
kubeconfig.AppendAuthenticator(clientConfig, spec, authenticator, "")
clientConfigData, err := clientcmd.Write(*clientConfig)
if err != nil {
return nil, errors.Wrap(err, "serialising kubeconfig for nodegroup")
Expand Down
28 changes: 20 additions & 8 deletions pkg/utils/kubeconfig/kubeconfig.go
Expand Up @@ -59,23 +59,35 @@ func New(spec *api.ClusterConfig, username, certificateAuthorityPath string) (*c
return c, clusterName, contextName
}

// AppendAuthenticator appends the AWS IAM authenticator
func AppendAuthenticator(c *clientcmdapi.Config, spec *api.ClusterConfig, command string) {
c.AuthInfos[c.CurrentContext].Exec = &clientcmdapi.ExecConfig{
// AppendAuthenticator appends the AWS IAM authenticator, and
// if profile is non-empty string it sets AWS_PROFILE environment
// variable also
func AppendAuthenticator(c *clientcmdapi.Config, spec *api.ClusterConfig, command, profile string) {
execConfig := &clientcmdapi.ExecConfig{
APIVersion: "client.authentication.k8s.io/v1alpha1",
Command: command,
Args: []string{"token", "-i", spec.Metadata.Name},
/*
Args: []string{"token", "-i", c.Cluster.ClusterName, "-r", c.roleARN},
*/
}

if profile != "" {
execConfig.Env = []clientcmdapi.ExecEnvVar{
clientcmdapi.ExecEnvVar{
Name: "AWS_PROFILE",
Value: profile,
},
}
}

c.AuthInfos[c.CurrentContext] = &clientcmdapi.AuthInfo{
Exec: execConfig,
}
}

// Write will write Kubernetes client configuration to a file.
// If path isn't specified then the path will be determined by client-go.
// If file pointed to by path doesn't exist it will be created.
// If the file already exists then the configuration will be merged with the existing file.
func Write(path string, newConfig *clientcmdapi.Config, setContext bool) (string, error) {
func Write(path string, newConfig clientcmdapi.Config, setContext bool) (string, error) {
configAccess := getConfigAccess(path)

config, err := configAccess.GetStartingConfig()
Expand All @@ -84,7 +96,7 @@ func Write(path string, newConfig *clientcmdapi.Config, setContext bool) (string
}

logger.Debug("merging kubeconfig files")
merged := merge(config, newConfig)
merged := merge(config, &newConfig)

if setContext && newConfig.CurrentContext != "" {
logger.Debug("setting current-context to %s", newConfig.CurrentContext)
Expand Down

0 comments on commit d94e4df

Please sign in to comment.