From 4a03e392227f4bf65f9a3f06568aa363406ba02b Mon Sep 17 00:00:00 2001 From: Sunil Arora Date: Thu, 1 Feb 2018 10:44:30 -0800 Subject: [PATCH] Kinflate: refactoring inputs flags forn config/map to common datastructure --- pkg/kinflate/commands/configmap.go | 30 +----- pkg/kinflate/commands/configmap_test.go | 62 ----------- pkg/kinflate/commands/data_config.go | 49 +++++++++ pkg/kinflate/commands/data_config_test.go | 83 +++++++++++++++ pkg/kinflate/commands/secret.go | 31 +----- pkg/kinflate/commands/secret_test.go | 124 +--------------------- 6 files changed, 136 insertions(+), 243 deletions(-) create mode 100644 pkg/kinflate/commands/data_config.go create mode 100644 pkg/kinflate/commands/data_config_test.go diff --git a/pkg/kinflate/commands/configmap.go b/pkg/kinflate/commands/configmap.go index 803b2839f..0721592c6 100644 --- a/pkg/kinflate/commands/configmap.go +++ b/pkg/kinflate/commands/configmap.go @@ -17,41 +17,13 @@ limitations under the License. package commands import ( - "fmt" "io" "github.com/spf13/cobra" ) -type addConfigMap struct { - // Name of configMap (required) - Name string - // FileSources to derive the configMap from (optional) - FileSources []string - // LiteralSources to derive the configMap from (optional) - LiteralSources []string - // EnvFileSource to derive the configMap from (optional) - EnvFileSource string -} - -// validate validates required fields are set to support structured generation. -func (a *addConfigMap) Validate(args []string) error { - if len(args) != 1 { - return fmt.Errorf("name must be specified once") - } - a.Name = args[0] - if len(a.EnvFileSource) == 0 && len(a.FileSources) == 0 && len(a.LiteralSources) == 0 { - return fmt.Errorf("at least from-env-file, or from-file or from-literal must be set") - } - if len(a.EnvFileSource) > 0 && (len(a.FileSources) > 0 || len(a.LiteralSources) > 0) { - return fmt.Errorf("from-env-file cannot be combined with from-file or from-literal") - } - // TODO: Should we check if the path exists? if it's valid, if it's within the same (sub-)directory? - return nil -} - func NewCmdAddConfigMap(errOut io.Writer) *cobra.Command { - var config addConfigMap + var config dataConfig cmd := &cobra.Command{ Use: "configmap NAME [--from-file=[key=]source] [--from-literal=key1=value1]", Short: "Adds a configmap to your manifest file.", diff --git a/pkg/kinflate/commands/configmap_test.go b/pkg/kinflate/commands/configmap_test.go index 8521ac676..d5d95128e 100644 --- a/pkg/kinflate/commands/configmap_test.go +++ b/pkg/kinflate/commands/configmap_test.go @@ -25,65 +25,3 @@ func TestNewAddConfigMapIsNotNil(t *testing.T) { t.Fatal("NewCmdAddConfigMap shouldn't be nil") } } - -func TestAddConfigValidation_NoName(t *testing.T) { - config := addConfigMap{} - - if config.Validate([]string{}) == nil { - t.Fatal("Validation should fail if no name is specified") - } -} - -func TestAddConfigValidation_MoreThanOneName(t *testing.T) { - config := addConfigMap{} - - if config.Validate([]string{"name", "othername"}) == nil { - t.Fatal("Validation should fail if more than one name is specified") - } -} - -func TestAddConfigValidation_Flags(t *testing.T) { - tests := []struct { - name string - config addConfigMap - shouldFail bool - }{ - { - name: "env-file-source and literal are both set", - config: addConfigMap{ - LiteralSources: []string{"one", "two"}, - EnvFileSource: "three", - }, - shouldFail: true, - }, - { - name: "env-file-source and from-file are both set", - config: addConfigMap{ - FileSources: []string{"one", "two"}, - EnvFileSource: "three", - }, - shouldFail: true, - }, - { - name: "we don't have any option set", - config: addConfigMap{}, - shouldFail: true, - }, - { - name: "we have from-file and literal ", - config: addConfigMap{ - LiteralSources: []string{"one", "two"}, - FileSources: []string{"three", "four"}, - }, - shouldFail: false, - }, - } - - for _, test := range tests { - if test.config.Validate([]string{"name"}) == nil && test.shouldFail { - t.Fatalf("Validation should fail if %s", test.name) - } else if test.config.Validate([]string{"name"}) != nil && !test.shouldFail { - t.Fatalf("Validation should succeed if %s", test.name) - } - } -} diff --git a/pkg/kinflate/commands/data_config.go b/pkg/kinflate/commands/data_config.go new file mode 100644 index 000000000..e48365ae4 --- /dev/null +++ b/pkg/kinflate/commands/data_config.go @@ -0,0 +1,49 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "fmt" +) + +// dataConfig encapsulates the options for add configmap/Secret commands. +type dataConfig struct { + // Name of configMap/Secret (required) + Name string + // FileSources to derive the configMap/Secret from (optional) + FileSources []string + // LiteralSources to derive the configMap/Secret from (optional) + LiteralSources []string + // EnvFileSource to derive the configMap/Secret from (optional) + EnvFileSource string +} + +// Validate validates required fields are set to support structured generation. +func (a *dataConfig) Validate(args []string) error { + if len(args) != 1 { + return fmt.Errorf("name must be specified once") + } + a.Name = args[0] + if len(a.EnvFileSource) == 0 && len(a.FileSources) == 0 && len(a.LiteralSources) == 0 { + return fmt.Errorf("at least from-env-file, or from-file or from-literal must be set") + } + if len(a.EnvFileSource) > 0 && (len(a.FileSources) > 0 || len(a.LiteralSources) > 0) { + return fmt.Errorf("from-env-file cannot be combined with from-file or from-literal") + } + // TODO: Should we check if the path exists? if it's valid, if it's within the same (sub-)directory? + return nil +} diff --git a/pkg/kinflate/commands/data_config_test.go b/pkg/kinflate/commands/data_config_test.go new file mode 100644 index 000000000..29154f4b0 --- /dev/null +++ b/pkg/kinflate/commands/data_config_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "testing" +) + +func TestDataConfigValidation_NoName(t *testing.T) { + config := dataConfig{} + + if config.Validate([]string{}) == nil { + t.Fatal("Validation should fail if no name is specified") + } +} + +func TestDataConfigValidation_MoreThanOneName(t *testing.T) { + config := dataConfig{} + + if config.Validate([]string{"name", "othername"}) == nil { + t.Fatal("Validation should fail if more than one name is specified") + } +} + +func TestDataConfigValidation_Flags(t *testing.T) { + tests := []struct { + name string + config dataConfig + shouldFail bool + }{ + { + name: "env-file-source and literal are both set", + config: dataConfig{ + LiteralSources: []string{"one", "two"}, + EnvFileSource: "three", + }, + shouldFail: true, + }, + { + name: "env-file-source and from-file are both set", + config: dataConfig{ + FileSources: []string{"one", "two"}, + EnvFileSource: "three", + }, + shouldFail: true, + }, + { + name: "we don't have any option set", + config: dataConfig{}, + shouldFail: true, + }, + { + name: "we have from-file and literal ", + config: dataConfig{ + LiteralSources: []string{"one", "two"}, + FileSources: []string{"three", "four"}, + }, + shouldFail: false, + }, + } + + for _, test := range tests { + if test.config.Validate([]string{"name"}) == nil && test.shouldFail { + t.Fatalf("Validation should fail if %s", test.name) + } else if test.config.Validate([]string{"name"}) != nil && !test.shouldFail { + t.Fatalf("Validation should succeed if %s", test.name) + } + } +} diff --git a/pkg/kinflate/commands/secret.go b/pkg/kinflate/commands/secret.go index 70ecfc90b..132cb531c 100644 --- a/pkg/kinflate/commands/secret.go +++ b/pkg/kinflate/commands/secret.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2018 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,35 +23,8 @@ import ( "github.com/spf13/cobra" ) -type addGenericSecret struct { - // Name of secret (required) - Name string - // FileSources to derive the secret from (optional) - FileSources []string - // LiteralSources to derive the secret from (optional) - LiteralSources []string - // EnvFileSource to derive the secret from (optional) - EnvFileSource string -} - -// Validate validates required fields are set to support structured generation. -func (a *addGenericSecret) Validate(args []string) error { - if len(args) != 1 { - return fmt.Errorf("name must be specified once") - } - a.Name = args[0] - if len(a.EnvFileSource) == 0 && len(a.FileSources) == 0 && len(a.LiteralSources) == 0 { - return fmt.Errorf("at least from-env-file, or from-file or from-literal must be set") - } - if len(a.EnvFileSource) > 0 && (len(a.FileSources) > 0 || len(a.LiteralSources) > 0) { - return fmt.Errorf("from-env-file cannot be combined with from-file or from-literal") - } - // TODO: Should we check if the path exists? if it's valid, if it's within the same (sub-)directory? - return nil -} - func newCmdAddSecretGeneric(errOut io.Writer) *cobra.Command { - var config addGenericSecret + var config dataConfig cmd := &cobra.Command{ Use: "generic NAME [--type=string] [--from-file=[key=]source] [--from-literal=key1=value1]", Short: "Adds a secret from a local file, directory or literal value.", diff --git a/pkg/kinflate/commands/secret_test.go b/pkg/kinflate/commands/secret_test.go index 7b3724581..e8f701ad5 100644 --- a/pkg/kinflate/commands/secret_test.go +++ b/pkg/kinflate/commands/secret_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2018 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -25,125 +25,3 @@ func TestNewAddSecretIsNotNil(t *testing.T) { t.Fatal("NewCmdAddSecret shouldn't be nil") } } - -func TestAddGenericSecretValidation_NoName(t *testing.T) { - config := addGenericSecret{} - - if config.Validate([]string{}) == nil { - t.Fatal("Validation should fail if no name is specified") - } -} - -func TestAddGenericSecretValidation_MoreThanOneName(t *testing.T) { - config := addGenericSecret{} - - if config.Validate([]string{"name", "othername"}) == nil { - t.Fatal("Validation should fail if more than one name is specified") - } -} - -func TestAddGenericSecretValidation_Flags(t *testing.T) { - tests := []struct { - name string - config addGenericSecret - shouldFail bool - }{ - { - name: "env-file-source and literal are both set", - config: addGenericSecret{ - LiteralSources: []string{"one", "two"}, - EnvFileSource: "three", - }, - shouldFail: true, - }, - { - name: "env-file-source and from-file are both set", - config: addGenericSecret{ - FileSources: []string{"one", "two"}, - EnvFileSource: "three", - }, - shouldFail: true, - }, - { - name: "we don't have any option set", - config: addGenericSecret{}, - shouldFail: true, - }, - { - name: "we have from-file and literal ", - config: addGenericSecret{ - LiteralSources: []string{"one", "two"}, - FileSources: []string{"three", "four"}, - }, - shouldFail: false, - }, - } - - for _, test := range tests { - if test.config.Validate([]string{"name"}) == nil && test.shouldFail { - t.Fatalf("Validation should fail if %s", test.name) - } else if test.config.Validate([]string{"name"}) != nil && !test.shouldFail { - t.Fatalf("Validation should succeed if %s", test.name) - } - } -} - -func TestAddTLSSecretValidation_NoName(t *testing.T) { - config := addTLSSecret{} - - if config.Validate([]string{}) == nil { - t.Fatal("Validation should fail if no name is specified") - } -} - -func TestAddTLSSecretValidation_MoreThanOneName(t *testing.T) { - config := addTLSSecret{} - - if config.Validate([]string{"name", "othername"}) == nil { - t.Fatal("Validation should fail if more than one name is specified") - } -} - -func TestAddTLSSecretValidation_Flags(t *testing.T) { - tests := []struct { - name string - config addTLSSecret - shouldFail bool - }{ - { - name: "cert and key are set", - config: addTLSSecret{ - Cert: "cert", - Key: "key", - }, - shouldFail: false, - }, - { - name: "cert is set, but not key", - config: addTLSSecret{ - Cert: "cert", - }, - shouldFail: true, - }, - { - name: "key is set, but not cert", - config: addTLSSecret{ - Key: "key", - }, - shouldFail: true, - }, - { - name: "neither key nor cert is set", - config: addTLSSecret{}, - shouldFail: true, - }, - } - - for _, test := range tests { - if test.config.Validate([]string{"name"}) == nil && test.shouldFail { - t.Fatalf("Validation should fail if %s", test.name) - } else if test.config.Validate([]string{"name"}) != nil && !test.shouldFail { - t.Fatalf("Validation should succeed if %s", test.name) - } - } -}