Merge branch 'master' into unattended-flux-setup

Author: mumoshu

pkg/ctl/cmdutils/cmdutils.go

@@ -184,7 +184,7 @@ func AddUpdateAuthConfigMap(fs *pflag.FlagSet, updateAuthConfigMap *bool, descri
 
 // AddCommonFlagsForKubeconfig adds common flags for controlling how output kubeconfig is written
 func AddCommonFlagsForKubeconfig(fs *pflag.FlagSet, outputPath, authenticatorRoleARN *string, setContext, autoPath *bool, exampleName string) {
-	fs.StringVar(outputPath, "kubeconfig", kubeconfig.DefaultPath, "path to write kubeconfig (incompatible with --auto-kubeconfig)")
+	fs.StringVar(outputPath, "kubeconfig", kubeconfig.DefaultPath(), "path to write kubeconfig (incompatible with --auto-kubeconfig)")
 	fs.StringVar(authenticatorRoleARN, "authenticator-role-arn", "", "AWS IAM role to assume for authenticator")
 	fs.BoolVar(setContext, "set-kubeconfig-context", true, "if true then current-context will be set in kubeconfig; if a context is already set then it will be overwritten")
 	fs.BoolVar(autoPath, "auto-kubeconfig", false, fmt.Sprintf("save kubeconfig file by cluster name, e.g. %q", kubeconfig.AutoPath(exampleName)))

pkg/ctl/create/cluster.go

@@ -133,7 +133,7 @@ func doCreateCluster(cmd *cmdutils.Cmd, ng *api.NodeGroup, params *cmdutils.Crea
 	}
 
 	if params.AutoKubeconfigPath {
-		if params.KubeconfigPath != kubeconfig.DefaultPath {
+		if params.KubeconfigPath != kubeconfig.DefaultPath() {
 			return fmt.Errorf("--kubeconfig and --auto-kubeconfig %s", cmdutils.IncompatibleFlags)
 		}
 		params.KubeconfigPath = kubeconfig.AutoPath(meta.Name)

pkg/ctl/utils/write_kubeconfig.go

@@ -60,7 +60,7 @@ func doWriteKubeconfigCmd(cmd *cmdutils.Cmd, outputPath, roleARN string, setCont
 	}
 
 	if autoPath {
-		if outputPath != kubeconfig.DefaultPath {
+		if outputPath != kubeconfig.DefaultPath() {
 			return fmt.Errorf("--kubeconfig and --auto-kubeconfig %s", cmdutils.IncompatibleFlags)
 		}
 		outputPath = kubeconfig.AutoPath(cfg.Metadata.Name)

pkg/eks/client.go

@@ -21,19 +21,19 @@ import (
 
 // Client stores information about the client config
 type Client struct {
-	Config      *clientcmdapi.Config
-	ContextName string
+	Config *clientcmdapi.Config
 
 	rawConfig *restclient.Config
 }
 
 // NewClient creates a new client config by embedding the STS token
 func (c *ClusterProvider) NewClient(spec *api.ClusterConfig) (*Client, error) {
-	clientConfig, _, contextName := kubeconfig.New(spec, c.GetUsername(), "")
+	clientConfig := kubeconfig.
+		NewBuilder(spec.Metadata, spec.Status, c.GetUsername()).
+		Build()
 
 	config := &Client{
-		Config:      clientConfig,
-		ContextName: contextName,
+		Config: clientConfig,
 	}
 
 	return config.new(spec, c.Provider.STS())
@@ -73,7 +73,7 @@ func (c *Client) useEmbeddedToken(spec *api.ClusterConfig, stsclient stsiface.ST
 		return errors.Wrap(err, "could not get token")
 	}
 
-	c.Config.AuthInfos[c.ContextName].Token = tok.Token
+	c.Config.AuthInfos[c.Config.CurrentContext].Token = tok.Token
 	return nil
 }
 

pkg/nodebootstrap/userdata.go

@@ -67,7 +67,10 @@ func addFilesAndScripts(config *cloudconfig.CloudConfig, files configFiles, scri
 }
 
 func makeClientConfigData(spec *api.ClusterConfig, ng *api.NodeGroup, authenticatorCMD string) ([]byte, error) {
-	clientConfig, _, _ := kubeconfig.New(spec, "kubelet", configDir+"ca.crt")
+	clientConfig := kubeconfig.
+		NewBuilder(spec.Metadata, spec.Status, "kubelet").
+		UseCertificateAuthorityFile(configDir + "ca.crt").
+		Build()
 	kubeconfig.AppendAuthenticator(clientConfig, spec, authenticatorCMD, "", "")
 	clientConfigData, err := clientcmd.Write(*clientConfig)
 	if err != nil {

pkg/utils/kubeconfig/kubeconfig.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 
 	"os/exec"
@@ -17,18 +18,25 @@ import (
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 )
 
-// DefaultPath defines the default path
-var DefaultPath = clientcmd.RecommendedHomeFile
-
 const (
 	// AWSIAMAuthenticator defines the name of the AWS IAM authenticator
 	AWSIAMAuthenticator = "aws-iam-authenticator"
 	// HeptioAuthenticatorAWS defines the old name of AWS IAM authenticator
 	HeptioAuthenticatorAWS = "heptio-authenticator-aws"
 	// AWSEKSAuthenticator defines the recently added `aws eks get-token` command
 	AWSEKSAuthenticator = "aws"
+	// Shadowing the default kubeconfig path environment variable
+	RecommendedConfigPathEnvVar = clientcmd.RecommendedConfigPathEnvVar
 )
 
+// DefaultPath defines the default path
+func DefaultPath() string {
+	if env := os.Getenv(RecommendedConfigPathEnvVar); len(env) > 0 {
+		return env
+	}
+	return clientcmd.RecommendedHomeFile
+}
+
 // AuthenticatorCommands returns all of authenticator commands
 func AuthenticatorCommands() []string {
 	return []string{
@@ -38,22 +46,23 @@ func AuthenticatorCommands() []string {
 	}
 }
 
-// New creates Kubernetes client configuration for a given username
-// if certificateAuthorityPath is not empty, it is used instead of
-// embedded certificate-authority-data
-func New(spec *api.ClusterConfig, username, certificateAuthorityPath string) (*clientcmdapi.Config, string, string) {
-	clusterName := spec.Metadata.String()
-	contextName := fmt.Sprintf("%s@%s", username, clusterName)
+// ConfigBuilder can create a client-go clientcmd Config
+type ConfigBuilder struct {
+	cluster     clientcmdapi.Cluster
+	clusterName string
+	username    string
+}
 
-	c := &clientcmdapi.Config{
+// Build creates the Config with the ConfigBuilder settings
+func (cb *ConfigBuilder) Build() *clientcmdapi.Config {
+	contextName := fmt.Sprintf("%s@%s", cb.username, cb.clusterName)
+	return &clientcmdapi.Config{
 		Clusters: map[string]*clientcmdapi.Cluster{
-			clusterName: {
-				Server: spec.Status.Endpoint,
-			},
+			cb.clusterName: &cb.cluster,
 		},
 		Contexts: map[string]*clientcmdapi.Context{
 			contextName: {
-				Cluster:  clusterName,
+				Cluster:  cb.clusterName,
 				AuthInfo: contextName,
 			},
 		},
@@ -62,19 +71,32 @@ func New(spec *api.ClusterConfig, username, certificateAuthorityPath string) (*c
 		},
 		CurrentContext: contextName,
 	}
+}
 
-	if certificateAuthorityPath == "" {
-		c.Clusters[clusterName].CertificateAuthorityData = spec.Status.CertificateAuthorityData
-	} else {
-		c.Clusters[clusterName].CertificateAuthority = certificateAuthorityPath
+// NewBuilder returns a minimal ConfigBuilder
+func NewBuilder(metadata *api.ClusterMeta, status *api.ClusterStatus, username string) *ConfigBuilder {
+	cluster := clientcmdapi.Cluster{
+		Server:                   status.Endpoint,
+		CertificateAuthorityData: status.CertificateAuthorityData,
 	}
+	return &ConfigBuilder{
+		cluster:     cluster,
+		clusterName: metadata.String(),
+		username:    username,
+	}
+}
 
-	return c, clusterName, contextName
+// UseCertificateAuthorityFile sets the config to use CA from file instead
+// of the CA as retrieved from EKS
+func (cb *ConfigBuilder) UseCertificateAuthorityFile(path string) *ConfigBuilder {
+	cb.cluster.CertificateAuthority = path
+	cb.cluster.CertificateAuthorityData = []byte{}
+	return cb
 }
 
 // NewForKubectl creates configuration for kubectl using a suitable authenticator
 func NewForKubectl(spec *api.ClusterConfig, username, roleARN, profile string) *clientcmdapi.Config {
-	config, _, _ := New(spec, username, "")
+	config := NewBuilder(spec.Metadata, spec.Status, username).Build()
 	authenticator, found := LookupAuthenticator()
 	if !found {
 		// fall back to aws-iam-authenticator
@@ -139,21 +161,40 @@ func AppendAuthenticator(config *clientcmdapi.Config, spec *api.ClusterConfig, a
 	}
 }
 
+// ensureDirectory should probably be handled in flock
+func ensureDirectory(filePath string) error {
+	dir := filepath.Dir(filePath)
+	if _, err := os.Stat(dir); os.IsNotExist(err) {
+		if err = os.MkdirAll(dir, 0755); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func lockConfigFile(filePath string) error {
+	// Make sure the directory exists, otherwise flock fails
+	if err := ensureDirectory(filePath); err != nil {
+		return err
+	}
 	flock := flock.New(filePath)
 	err := flock.Lock()
 	if err != nil {
-		return errors.Wrap(err, "flock: failed to obtain exclusive lock existing kubeconfig file")
+		return errors.Wrap(err, "flock: failed to obtain exclusive lock on kubeconfig file")
 	}
 
 	return nil
 }
 
 func unlockConfigFile(filePath string) error {
+	// Make sure the directory exists, otherwise flock fails
+	if err := ensureDirectory(filePath); err != nil {
+		return err
+	}
 	flock := flock.New(filePath)
 	err := flock.Unlock()
 	if err != nil {
-		return errors.Wrap(err, "flock: failed to release exclusive lock on existing kubeconfig file")
+		return errors.Wrap(err, "flock: failed to release exclusive lock on kubeconfig file")
 	}
 
 	return nil
@@ -199,7 +240,7 @@ func Write(path string, newConfig clientcmdapi.Config, setContext bool) (string,
 
 func getConfigAccess(explicitPath string) clientcmd.ConfigAccess {
 	pathOptions := clientcmd.NewDefaultPathOptions()
-	if explicitPath != "" && explicitPath != DefaultPath {
+	if explicitPath != "" && explicitPath != DefaultPath() {
 		pathOptions.LoadingRules.ExplicitPath = explicitPath
 	}
 
@@ -275,7 +316,7 @@ func MaybeDeleteConfig(meta *api.ClusterMeta) {
 		return
 	}
 
-	configAccess := getConfigAccess(DefaultPath)
+	configAccess := getConfigAccess(DefaultPath())
 	defaultFilename := configAccess.GetDefaultFilename()
 	err := lockConfigFile(defaultFilename)
 	if err != nil {
@@ -290,7 +331,7 @@ func MaybeDeleteConfig(meta *api.ClusterMeta) {
 
 	config, err := configAccess.GetStartingConfig()
 	if err != nil {
-		logger.Debug("error reading kubeconfig file %q: %s", DefaultPath, err.Error())
+		logger.Debug("error reading kubeconfig file %q: %s", DefaultPath(), err.Error())
 		return
 	}
 
@@ -299,7 +340,7 @@ func MaybeDeleteConfig(meta *api.ClusterMeta) {
 	}
 
 	if err := clientcmd.ModifyConfig(configAccess, *config, true); err != nil {
-		logger.Debug("ignoring error while failing to update config file %q: %s", DefaultPath, err.Error())
+		logger.Debug("ignoring error while failing to update config file %q: %s", DefaultPath(), err.Error())
 	} else {
 		logger.Success("kubeconfig has been updated")
 	}

pkg/utils/kubeconfig/kubeconfig_test.go

@@ -3,6 +3,7 @@ package kubeconfig_test
 import (
 	"io/ioutil"
 	"os"
+	"path"
 	"sync"
 
 	eksctlapi "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
@@ -70,6 +71,13 @@ var _ = Describe("Kubeconfig", func() {
 		Expect(filename).To(BeEmpty())
 	})
 
+	It("creating new Kubeconfig in non-existent directory", func() {
+		tempDir, _ := ioutil.TempDir("", "")
+		filename, err := kubeconfig.Write(path.Join(tempDir, "nonexistentdir", "kubeconfig"), testConfig, false)
+		Expect(err).To(BeNil())
+		Expect(filename).ToNot(BeEmpty())
+	})
+
 	It("sets new Kubeconfig context", func() {
 		testConfigContext := testConfig
 		testConfigContext.CurrentContext = "test-context"
@@ -138,6 +146,20 @@ var _ = Describe("Kubeconfig", func() {
 		Expect(readConfig.CurrentContext).To(Equal("minikube"))
 	})
 
+	var (
+		kubeconfigPathToRestore string
+		hasKubeconfigPath       bool
+	)
+
+	ChangeKubeconfig := func() {
+		if _, err := os.Stat(configFile.Name()); os.IsNotExist(err) {
+			GinkgoT().Fatal(err)
+		}
+
+		kubeconfigPathToRestore, hasKubeconfigPath = os.LookupEnv("KUBECONFIG")
+		os.Setenv("KUBECONFIG", configFile.Name())
+	}
+
 	Context("delete config", func() {
 		// Default cluster name is 'foo' and region is 'us-west-2'
 		var apiClusterConfigSample = eksctlapi.ClusterConfig{
@@ -185,8 +207,6 @@ var _ = Describe("Kubeconfig", func() {
 			twoClustersAsBytes                []byte
 			oneClusterWithoutContextAsBytes   []byte
 			oneClusterWithStaleContextAsBytes []byte
-			kubeconfigPathToRestore           string
-			hasKubeconfigPath                 bool
 		)
 
 		// Returns an ClusterConfig with a cluster name equal to the provided clusterName.
@@ -196,15 +216,6 @@ var _ = Describe("Kubeconfig", func() {
 			return &apiClusterConfig
 		}
 
-		ChangeKubeconfig := func() {
-			if _, err := os.Stat(configFile.Name()); os.IsNotExist(err) {
-				GinkgoT().Fatal(err)
-			}
-
-			kubeconfigPathToRestore, hasKubeconfigPath = os.LookupEnv("KUBECONFIG")
-			os.Setenv("KUBECONFIG", configFile.Name())
-		}
-
 		RestoreKubeconfig := func() {
 			if hasKubeconfigPath {
 				os.Setenv("KUBECONFIG", kubeconfigPathToRestore)
@@ -297,28 +308,45 @@ var _ = Describe("Kubeconfig", func() {
 			Expect(err).To(BeNil())
 			Expect(configFileAsBytes).To(MatchYAML(twoClustersAsBytes), "Should not change")
 		})
+	})
+
+	It("safely handles concurrent read-modify-write operations", func() {
+		var (
+			oneClusterAsBytes  []byte
+			twoClustersAsBytes []byte
+		)
+		ChangeKubeconfig()
+
+		var err error
+
+		if oneClusterAsBytes, err = ioutil.ReadFile("testdata/one_cluster.golden"); err != nil {
+			GinkgoT().Fatalf("failed reading .golden: %v", err)
+		}
+
+		if twoClustersAsBytes, err = ioutil.ReadFile("testdata/two_clusters.golden"); err != nil {
+			GinkgoT().Fatalf("failed reading .golden: %v", err)
+		}
 
-		It("safely handles concurrent read-modify-write operations", func() {
-			var wg sync.WaitGroup
-			multiplier := 3
-			iters := 100
-			for i := 0; i < multiplier; i++ {
-				for k := 0; k < iters; k++ {
-					wg.Add(2)
-					go func() {
-						defer wg.Done()
-						_, err := configFile.Write(oneClusterAsBytes)
-						Expect(err).To(BeNil())
-					}()
-					go func() {
-						defer wg.Done()
-						_, err := configFile.Write(twoClustersAsBytes)
-						Expect(err).To(BeNil())
-					}()
-				}
+		var wg sync.WaitGroup
+		multiplier := 3
+		iters := 100
+		for i := 0; i < multiplier; i++ {
+			for k := 0; k < iters; k++ {
+				wg.Add(2)
+				go func() {
+					defer wg.Done()
+					_, err := configFile.Write(oneClusterAsBytes)
+					Expect(err).To(BeNil())
+				}()
+				go func() {
+					defer wg.Done()
+					_, err := configFile.Write(twoClustersAsBytes)
+					Expect(err).To(BeNil())
+				}()
 			}
+		}
+
+		wg.Wait()
 
-			wg.Wait()
-		})
 	})
 })