From 88bb8085b0f55281f93cae3b16c2a2c6b2ca2708 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 15:11:42 -0800 Subject: [PATCH] Deduplicate the kubeValues data in InitKube [#67] Now that the InitKube initialization happens inside its own package, the private .values field can be populated at the same time, rather than having to wait for Prepare(). Also clarified the config/template filename fields (configFile vs. ConfigFile was particularly ambiguous). --- internal/run/initkube.go | 62 +++++++----------- internal/run/initkube_test.go | 120 +++++++++++++++++++--------------- 2 files changed, 92 insertions(+), 90 deletions(-) diff --git a/internal/run/initkube.go b/internal/run/initkube.go index f15ba75..cee9d28 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -11,17 +11,11 @@ import ( // InitKube is a step in a helm Plan that initializes the kubernetes config file. type InitKube struct { - SkipTLSVerify bool - Certificate string - APIServer string - ServiceAccount string - Token string - TemplateFile string - ConfigFile string - - template *template.Template - configFile io.WriteCloser - values kubeValues + templateFilename string + configFilename string + template *template.Template + configFile io.WriteCloser + values kubeValues } type kubeValues struct { @@ -36,20 +30,23 @@ type kubeValues struct { // NewInitKube creates a InitKube using the given Config and filepaths. No validation is performed at this time. func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { return &InitKube{ - SkipTLSVerify: cfg.SkipTLSVerify, - Certificate: cfg.Certificate, - APIServer: cfg.APIServer, - ServiceAccount: cfg.ServiceAccount, - Token: cfg.KubeToken, - TemplateFile: templateFile, - ConfigFile: configFile, + values: kubeValues{ + SkipTLSVerify: cfg.SkipTLSVerify, + Certificate: cfg.Certificate, + APIServer: cfg.APIServer, + Namespace: cfg.Namespace, + ServiceAccount: cfg.ServiceAccount, + Token: cfg.KubeToken, + }, + templateFilename: templateFile, + configFilename: configFile, } } // Execute generates a kubernetes config file from drone-helm3's template. func (i *InitKube) Execute(cfg Config) error { if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.ConfigFile) + fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.configFilename) } defer i.configFile.Close() return i.template.Execute(i.configFile, i.values) @@ -59,45 +56,36 @@ func (i *InitKube) Execute(cfg Config) error { func (i *InitKube) Prepare(cfg Config) error { var err error - if i.APIServer == "" { + if i.values.APIServer == "" { return errors.New("an API Server is needed to deploy") } - if i.Token == "" { + if i.values.Token == "" { return errors.New("token is needed to deploy") } - if i.ServiceAccount == "" { - i.ServiceAccount = "helm" + if i.values.ServiceAccount == "" { + i.values.ServiceAccount = "helm" } if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "loading kubeconfig template from %s\n", i.TemplateFile) + fmt.Fprintf(cfg.Stderr, "loading kubeconfig template from %s\n", i.templateFilename) } - i.template, err = template.ParseFiles(i.TemplateFile) + i.template, err = template.ParseFiles(i.templateFilename) if err != nil { return fmt.Errorf("could not load kubeconfig template: %w", err) } - i.values = kubeValues{ - SkipTLSVerify: i.SkipTLSVerify, - Certificate: i.Certificate, - APIServer: i.APIServer, - ServiceAccount: i.ServiceAccount, - Token: i.Token, - Namespace: cfg.Namespace, - } - if cfg.Debug { - if _, err := os.Stat(i.ConfigFile); err != nil { + if _, err := os.Stat(i.configFilename); err != nil { // non-nil err here isn't an actual error state; the kubeconfig just doesn't exist fmt.Fprint(cfg.Stderr, "creating ") } else { fmt.Fprint(cfg.Stderr, "truncating ") } - fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.ConfigFile) + fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.configFilename) } - i.configFile, err = os.Create(i.ConfigFile) + i.configFile, err = os.Create(i.configFilename) if err != nil { return fmt.Errorf("could not open kubeconfig file for writing: %w", err) } diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index df6d531..87e5398 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -29,13 +29,15 @@ func (suite *InitKubeTestSuite) TestNewInitKube() { init := NewInitKube(cfg, "conf.tpl", "conf.yml") suite.Equal(&InitKube{ - SkipTLSVerify: true, - Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", - APIServer: "98.765.43.21", - ServiceAccount: "greathelm", - Token: "b2YgbXkgYWZmZWN0aW9u", - TemplateFile: "conf.tpl", - ConfigFile: "conf.yml", + values: kubeValues{ + SkipTLSVerify: true, + Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", + APIServer: "98.765.43.21", + ServiceAccount: "greathelm", + Token: "b2YgbXkgYWZmZWN0aW9u", + }, + templateFilename: "conf.tpl", + configFilename: "conf.yml", }, init) } @@ -52,15 +54,16 @@ namespace: {{ .Namespace }} suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), - } - cfg := Config{ - Namespace: "Cisco", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + Namespace: "Cisco", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } + cfg := Config{} err = init.Prepare(cfg) suite.Require().Nil(err) @@ -85,16 +88,17 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { defer os.Remove(configFile.Name()) suite.Require().NoError(err) - cfg := Config{ - Namespace: "marshmallow", - } + cfg := Config{} init := InitKube{ - ConfigFile: configFile.Name(), - TemplateFile: "../../assets/kubeconfig.tpl", // the actual kubeconfig template - APIServer: "https://kube.cluster/peanut", - ServiceAccount: "chef", - Token: "eWVhaCB3ZSB0b2tpbic=", - Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", + configFilename: configFile.Name(), + templateFilename: "../../assets/kubeconfig.tpl", // the actual kubeconfig template + values: kubeValues{ + APIServer: "https://kube.cluster/peanut", + ServiceAccount: "chef", + Token: "eWVhaCB3ZSB0b2tpbic=", + Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", + Namespace: "marshmallow", + }, } suite.Require().NoError(init.Prepare(cfg)) suite.Require().NoError(init.Execute(cfg)) @@ -120,8 +124,8 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { suite.NoError(yaml.UnmarshalStrict(contents, &conf)) // test the other branch of the certificate/SkipTLSVerify conditional - init.SkipTLSVerify = true - init.Certificate = "" + init.values.SkipTLSVerify = true + init.values.Certificate = "" suite.Require().NoError(init.Prepare(cfg)) suite.Require().NoError(init.Execute(cfg)) @@ -139,10 +143,12 @@ func (suite *InitKubeTestSuite) TestPrepareParseError() { suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), } err = init.Prepare(Config{}) suite.Error(err) @@ -151,10 +157,12 @@ func (suite *InitKubeTestSuite) TestPrepareParseError() { func (suite *InitKubeTestSuite) TestPrepareNonexistentTemplateFile() { init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: "/usr/foreign/exclude/kubeprofig.tpl", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: "/usr/foreign/exclude/kubeprofig.tpl", } err := init.Prepare(Config{}) suite.Error(err) @@ -166,11 +174,13 @@ func (suite *InitKubeTestSuite) TestPrepareCannotOpenDestinationFile() { defer os.Remove(templateFile.Name()) suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: "/usr/foreign/exclude/kubeprofig", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: "/usr/foreign/exclude/kubeprofig", } cfg := Config{} @@ -190,22 +200,24 @@ func (suite *InitKubeTestSuite) TestPrepareRequiredConfig() { // initial config with all required fields present init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } cfg := Config{} suite.NoError(init.Prepare(cfg)) // consistency check; we should be starting in a happy state - init.APIServer = "" + init.values.APIServer = "" suite.Error(init.Prepare(cfg), "APIServer should be required.") - init.APIServer = "Sysadmin" - init.Token = "" + init.values.APIServer = "Sysadmin" + init.values.Token = "" suite.Error(init.Prepare(cfg), "Token should be required.") } @@ -219,17 +231,19 @@ func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } cfg := Config{} init.Prepare(cfg) - suite.Equal("helm", init.ServiceAccount) + suite.Equal("helm", init.values.ServiceAccount) } func tempfile(name, contents string) (*os.File, error) {