From e3bf18660e34b24d59f4e830da18c6ea1cb41c94 Mon Sep 17 00:00:00 2001 From: Antoine Date: Wed, 3 Jun 2020 02:01:25 +0200 Subject: [PATCH] :ok_hand: PR review Use `log.Fatalln(err)` instead of `log.Errorln(err)` + `os.Exit(1)` Use Label prefix instead of LabelName suffix Rename all secret occurence with token --- cmd/create/createCluster.go | 8 ++++---- cmd/get/getClusterToken.go | 10 +++++----- cmd/root.go | 6 ++---- docs/usage/commands.md | 2 +- pkg/cluster/cluster.go | 36 ++++++++++++++++++------------------ pkg/tools/tools.go | 2 +- pkg/types/types.go | 10 +++++----- pkg/util/files.go | 2 +- pkg/util/randomString.go | 2 +- 9 files changed, 38 insertions(+), 40 deletions(-) diff --git a/cmd/create/createCluster.go b/cmd/create/createCluster.go index a0a62904..f05e2c57 100644 --- a/cmd/create/createCluster.go +++ b/cmd/create/createCluster.go @@ -115,7 +115,7 @@ func NewCmdCreateCluster() *cobra.Command { cmd.Flags().IntP("workers", "w", 0, "Specify how many workers you want to create") cmd.Flags().StringP("image", "i", fmt.Sprintf("%s:%s", k3d.DefaultK3sImageRepo, version.GetK3sVersion(false)), "Specify k3s image that you want to use for the nodes") cmd.Flags().String("network", "", "Join an existing network") - cmd.Flags().String("secret", "", "Specify a cluster secret. By default, we generate one.") + cmd.Flags().String("token", "", "Specify a cluster token. By default, we generate one.") cmd.Flags().StringArrayP("volume", "v", nil, "Mount volumes into the nodes (Format: `--volume [SOURCE:]DEST[@NODEFILTER[;NODEFILTER...]]`\n - Example: `k3d create -w 2 -v /my/path@worker[0,1] -v /tmp/test:/tmp/other@master[0]`") cmd.Flags().StringArrayP("port", "p", nil, "Map ports from the node containers to the host (Format: `[HOST:][HOSTPORT:]CONTAINERPORT[/PROTOCOL][@NODEFILTER]`)\n - Example: `k3d create -w 2 -p 8080:80@worker[0] -p 8081@worker[1]`") cmd.Flags().BoolVar(&createClusterOpts.WaitForMaster, "wait", false, "Wait for the master(s) to be ready before returning. Use '--timeout DURATION' to not wait forever.") @@ -206,8 +206,8 @@ func parseCreateClusterCmd(cmd *cobra.Command, args []string, createClusterOpts log.Fatalln("Can only run a single node in hostnetwork mode") } - // --secret - secret, err := cmd.Flags().GetString("secret") + // --token + token, err := cmd.Flags().GetString("token") if err != nil { log.Fatalln(err) } @@ -309,7 +309,7 @@ func parseCreateClusterCmd(cmd *cobra.Command, args []string, createClusterOpts cluster := &k3d.Cluster{ Name: clustername, Network: network, - Secret: secret, + Token: token, CreateClusterOpts: createClusterOpts, ExposeAPI: exposeAPI, } diff --git a/cmd/get/getClusterToken.go b/cmd/get/getClusterToken.go index 006528c4..cf065115 100644 --- a/cmd/get/getClusterToken.go +++ b/cmd/get/getClusterToken.go @@ -74,8 +74,8 @@ func NewCmdGetClusterToken() *cobra.Command { } } - // pretty print secret - printSecret(clusters, getClusterTokenFlags.noHeader) + // pretty print token + printToken(clusters, getClusterTokenFlags.noHeader) }, } @@ -87,13 +87,13 @@ func NewCmdGetClusterToken() *cobra.Command { return cmd } -func printSecret(clusters []*k3d.Cluster, headersOff bool) { +func printToken(clusters []*k3d.Cluster, headersOff bool) { tabwriter := tabwriter.NewWriter(os.Stdout, 6, 4, 3, ' ', tabwriter.RememberWidths) defer tabwriter.Flush() if !headersOff { - headers := []string{"CLUSTER", "SECRET"} + headers := []string{"CLUSTER", "TOKEN"} _, err := fmt.Fprintf(tabwriter, "%s\n", strings.Join(headers, "\t")) if err != nil { log.Fatalln("Failed to print headers") @@ -106,6 +106,6 @@ func printSecret(clusters []*k3d.Cluster, headersOff bool) { }) for _, cluster := range clusters { - fmt.Fprintf(tabwriter, "%s\t%s\n", cluster.Name, string(cluster.Secret)) + fmt.Fprintf(tabwriter, "%s\t%s\n", cluster.Name, string(cluster.Token)) } } diff --git a/cmd/root.go b/cmd/root.go index 2646a0de..8173c561 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -67,8 +67,7 @@ All Nodes of a k3d cluster are part of the same docker network.`, printVersion() } else { if err := cmd.Usage(); err != nil { - log.Errorln(err) - os.Exit(1) + log.Fatalln(err) } } }, @@ -78,8 +77,7 @@ All Nodes of a k3d cluster are part of the same docker network.`, // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { if err := rootCmd.Execute(); err != nil { - log.Errorln(err) - os.Exit(1) + log.Fatalln(err) } } diff --git a/docs/usage/commands.md b/docs/usage/commands.md index 12e8e430..5c2de08a 100644 --- a/docs/usage/commands.md +++ b/docs/usage/commands.md @@ -14,7 +14,7 @@ k3d --network # specify a network you want to connect to --no-image-volume # disable the creation of a volume for storing images (used for the 'k3d load image' command) -p, --port # add some more port mappings - --secret # specify a cluster secret (default: auto-generated) + --token # specify a cluster token (default: auto-generated) --timeout # specify a timeout, after which the cluster creation will be interrupted and changes rolled back --update-kubeconfig # enable the automated update of the default kubeconfig with the details of the newly created cluster (also sets '--wait=true') --switch # (implies --update-kubeconfig) automatically sets the current-context of your default kubeconfig to the new cluster's context diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 43056c6c..ea0b6c96 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -79,19 +79,19 @@ func CreateCluster(ctx context.Context, runtime k3drt.Runtime, cluster *k3d.Clus } cluster.Network.Name = networkID extraLabels := map[string]string{ - k3d.NetworkLabelName: networkID, - k3d.NetworkExternalLabelName: strconv.FormatBool(cluster.Network.External), + k3d.LabelNetwork: networkID, + k3d.LabelNetworkExternal: strconv.FormatBool(cluster.Network.External), } if networkExists { - extraLabels[k3d.NetworkExternalLabelName] = "true" // if the network wasn't created, we say that it's managed externally (important for cluster deletion) + extraLabels[k3d.LabelNetworkExternal] = "true" // if the network wasn't created, we say that it's managed externally (important for cluster deletion) } /* - * Cluster Secret + * Cluster Token */ - if cluster.Secret == "" { - cluster.Secret = GenerateClusterSecret() + if cluster.Token == "" { + cluster.Token = GenerateClusterToken() } /* @@ -105,7 +105,7 @@ func CreateCluster(ctx context.Context, runtime k3drt.Runtime, cluster *k3d.Clus return err } - extraLabels[k3d.ImageVolumeLabelName] = imageVolumeName + extraLabels[k3d.LabelImageVolume] = imageVolumeName // attach volume to nodes for _, node := range cluster.Nodes { @@ -127,8 +127,8 @@ func CreateCluster(ctx context.Context, runtime k3drt.Runtime, cluster *k3d.Clus node.Labels = make(map[string]string) // TODO: maybe create an init function? } node.Labels["k3d.cluster"] = cluster.Name - node.Env = append(node.Env, fmt.Sprintf("K3S_TOKEN=%s", cluster.Secret)) - node.Labels[k3d.SecretLabelName] = cluster.Secret + node.Env = append(node.Env, fmt.Sprintf("K3S_TOKEN=%s", cluster.Token)) + node.Labels[k3d.LabelToken] = cluster.Token node.Labels["k3d.cluster.url"] = connectionURL // append extra labels @@ -419,7 +419,7 @@ func populateClusterFieldsFromLabels(cluster *k3d.Cluster) error { // get the name of the cluster network if cluster.Network.Name == "" { - if networkName, ok := node.Labels[k3d.NetworkLabelName]; ok { + if networkName, ok := node.Labels[k3d.LabelNetwork]; ok { cluster.Network.Name = networkName } } @@ -427,7 +427,7 @@ func populateClusterFieldsFromLabels(cluster *k3d.Cluster) error { // check if the network is external // since the struct value is a bool, initialized as false, we cannot check if it's unset if !cluster.Network.External && !networkExternalSet { - if networkExternalString, ok := node.Labels[k3d.NetworkExternalLabelName]; ok { + if networkExternalString, ok := node.Labels[k3d.LabelNetworkExternal]; ok { if networkExternal, err := strconv.ParseBool(networkExternalString); err == nil { cluster.Network.External = networkExternal networkExternalSet = true @@ -437,15 +437,15 @@ func populateClusterFieldsFromLabels(cluster *k3d.Cluster) error { // get image volume // TODO: enable external image volumes the same way we do it with networks if cluster.ImageVolume == "" { - if imageVolumeName, ok := node.Labels[k3d.ImageVolumeLabelName]; ok { + if imageVolumeName, ok := node.Labels[k3d.LabelImageVolume]; ok { cluster.ImageVolume = imageVolumeName } } - // get k3s cluster's secret - if cluster.Secret == "" { - if secretToken, ok := node.Labels[k3d.SecretLabelName]; ok { - cluster.Secret = secretToken + // get k3s cluster's token + if cluster.Token == "" { + if token, ok := node.Labels[k3d.LabelToken]; ok { + cluster.Token = token } } } @@ -493,8 +493,8 @@ func GetCluster(ctx context.Context, runtime k3drt.Runtime, cluster *k3d.Cluster return cluster, nil } -// GenerateClusterSecret generates a random 20 character string -func GenerateClusterSecret() string { +// GenerateClusterToken generates a random 20 character string +func GenerateClusterToken() string { return util.GenerateRandomString(20) } diff --git a/pkg/tools/tools.go b/pkg/tools/tools.go index 5704b550..21d87bbe 100644 --- a/pkg/tools/tools.go +++ b/pkg/tools/tools.go @@ -50,7 +50,7 @@ func LoadImagesIntoCluster(ctx context.Context, runtime runtimes.Runtime, images var ok bool for _, node := range cluster.Nodes { if node.Role == k3d.MasterRole || node.Role == k3d.WorkerRole { - if imageVolume, ok = node.Labels[k3d.ImageVolumeLabelName]; ok { + if imageVolume, ok = node.Labels[k3d.LabelImageVolume]; ok { break } } diff --git a/pkg/types/types.go b/pkg/types/types.go index 469219e6..cb06a182 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -76,10 +76,10 @@ var DefaultObjectLabels = map[string]string{ // List of k3d technical label name const ( - SecretLabelName string = "k3d.cluster.secret" - ImageVolumeLabelName string = "k3d.cluster.imageVolume" - NetworkExternalLabelName string = "k3d.cluster.network.external" - NetworkLabelName string = "k3d.cluster.network" + LabelToken string = "k3d.cluster.token" + LabelImageVolume string = "k3d.cluster.imageVolume" + LabelNetworkExternal string = "k3d.cluster.network.external" + LabelNetwork string = "k3d.cluster.network" ) // DefaultRoleCmds maps the node roles to their respective default commands @@ -160,7 +160,7 @@ type ClusterNetwork struct { type Cluster struct { Name string `yaml:"name" json:"name,omitempty"` Network ClusterNetwork `yaml:"network" json:"network,omitempty"` - Secret string `yaml:"cluster_secret" json:"clusterSecret,omitempty"` + Token string `yaml:"cluster_token" json:"clusterToken,omitempty"` Nodes []*Node `yaml:"nodes" json:"nodes,omitempty"` InitNode *Node // init master node ExternalDatastore ExternalDatastore `yaml:"external_datastore" json:"externalDatastore,omitempty"` diff --git a/pkg/util/files.go b/pkg/util/files.go index 2eb48c2d..2276a3a8 100644 --- a/pkg/util/files.go +++ b/pkg/util/files.go @@ -43,7 +43,7 @@ func GetConfigDirOrCreate() (string, error) { // create directories if necessary if err := createDirIfNotExists(configDir); err != nil { - log.Errorln("Failed to create config path '%s'", configDir) + log.Errorf("Failed to create config path '%s'", configDir) return "", err } diff --git a/pkg/util/randomString.go b/pkg/util/randomString.go index 03ff2ee4..def2d959 100644 --- a/pkg/util/randomString.go +++ b/pkg/util/randomString.go @@ -37,7 +37,7 @@ const ( var src = rand.NewSource(time.Now().UnixNano()) // GenerateRandomString thanks to https://stackoverflow.com/a/31832326/6450189 -// GenerateRandomString is used to generate a random string that is used as a cluster secret +// GenerateRandomString is used to generate a random string that is used as a cluster token func GenerateRandomString(n int) string { sb := strings.Builder{}