diff --git a/pkg/spec/fix.go b/pkg/spec/fix.go index fbff93713..ca2466ebb 100644 --- a/pkg/spec/fix.go +++ b/pkg/spec/fix.go @@ -24,44 +24,54 @@ import ( ) func FixApp(app *App) error { + var err error // fix app.Services - if err := fixServices(app); err != nil { + app.Services, err = fixServices(app.Services, app.Name) + if err != nil { return errors.Wrap(err, "Unable to fix services") } // fix app.VolumeClaims - if err := fixVolumeClaims(app); err != nil { + app.VolumeClaims, err = fixVolumeClaims(app.VolumeClaims, app.Name) + if err != nil { return errors.Wrap(err, "Unable to fix persistentVolume") } // fix app.configMaps - if err := fixConfigMaps(app); err != nil { + app.ConfigMaps, err = fixConfigMaps(app.ConfigMaps, app.Name) + if err != nil { return errors.Wrap(err, "unable to fix configMaps") } - if err := fixContainers(app); err != nil { + app.Containers, err = fixContainers(app.Containers, app.Name) + if err != nil { return errors.Wrap(err, "unable to fix containers") } - if err := fixSecrets(app); err != nil { + app.InitContainers, err = fixContainers(app.InitContainers, app.Name) + if err != nil { + return errors.Wrap(err, "unable to fix init-containers") + } + + app.Secrets, err = fixSecrets(app.Secrets, app.Name) + if err != nil { return errors.Wrap(err, "unable to fix secrets") } return nil } -func fixServices(app *App) error { - for i, service := range app.Services { +func fixServices(services []ServiceSpecMod, appName string) ([]ServiceSpecMod, error) { + for i, service := range services { // auto populate service name if only one service is specified if service.Name == "" { - if len(app.Services) == 1 { - service.Name = app.Name + if len(services) == 1 { + service.Name = appName } else { - return errors.New("More than one service mentioned, please specify name for each one") + return nil, errors.New("More than one service mentioned, please specify name for each one") } } - app.Services[i] = service for i, servicePort := range service.Ports { // auto populate port names if not specified @@ -70,65 +80,69 @@ func fixServices(app *App) error { } service.Ports[i] = servicePort } + + // this should be the last statement in this for loop + services[i] = service } - return nil + return services, nil } -func fixVolumeClaims(app *App) error { - for i, pVolume := range app.VolumeClaims { +func fixVolumeClaims(volumeClaims []VolumeClaim, appName string) ([]VolumeClaim, error) { + for i, pVolume := range volumeClaims { if pVolume.Name == "" { - if len(app.VolumeClaims) == 1 { - pVolume.Name = app.Name + if len(volumeClaims) == 1 { + pVolume.Name = appName } else { - return errors.New("More than one persistent volume mentioned, please specify name for each one") + return nil, errors.New("More than one persistent volume mentioned," + + " please specify name for each one") } } - app.VolumeClaims[i] = pVolume + volumeClaims[i] = pVolume } - return nil + return volumeClaims, nil } -func fixConfigMaps(app *App) error { +func fixConfigMaps(configMaps []ConfigMapMod, appName string) ([]ConfigMapMod, error) { // if only one configMap is defined and its name is not specified - if len(app.ConfigMaps) == 1 && app.ConfigMaps[0].Name == "" { - app.ConfigMaps[0].Name = app.Name - } else if len(app.ConfigMaps) > 1 { + if len(configMaps) == 1 && configMaps[0].Name == "" { + configMaps[0].Name = appName + } else if len(configMaps) > 1 { // if multiple configMaps is defined then each should have a name - for cdn, cd := range app.ConfigMaps { + for cdn, cd := range configMaps { if cd.Name == "" { - return fmt.Errorf("name not specified for app.configMaps[%d]", cdn) + return nil, fmt.Errorf("name not specified for app.configMaps[%d]", cdn) } } } - return nil + return configMaps, nil } -func fixSecrets(app *App) error { +func fixSecrets(secrets []SecretMod, appName string) ([]SecretMod, error) { // populate secret name only if one secret is specified - if len(app.Secrets) == 1 && app.Secrets[0].Name == "" { - app.Secrets[0].Name = app.Name - } else if len(app.Secrets) > 1 { - for i, sec := range app.Secrets { + if len(secrets) == 1 && secrets[0].Name == "" { + secrets[0].Name = appName + } else if len(secrets) > 1 { + for i, sec := range secrets { if sec.Name == "" { - return fmt.Errorf("name not specified for app.secrets[%d]", i) + return nil, fmt.Errorf("name not specified for app.secrets[%d]", i) } } } - return nil + return secrets, nil } -func fixContainers(app *App) error { +func fixContainers(containers []Container, appName string) ([]Container, error) { // if only one container set name of it as app name - if len(app.Containers) == 1 && app.Containers[0].Name == "" { - app.Containers[0].Name = app.Name - } else if len(app.Containers) > 1 { + if len(containers) == 1 && containers[0].Name == "" { + containers[0].Name = appName + } else if len(containers) > 1 { // check if all the containers have a name // if not fail giving error - for cn, c := range app.Containers { + for cn, c := range containers { if c.Name == "" { - return fmt.Errorf("app %q: container name not defined for app.containers[%d]", app.Name, cn) + return nil, fmt.Errorf("app %q: container name not defined for app.containers[%d]", appName, cn) } } } - return nil + return containers, nil } diff --git a/pkg/spec/fix_test.go b/pkg/spec/fix_test.go new file mode 100644 index 000000000..2ebd593f2 --- /dev/null +++ b/pkg/spec/fix_test.go @@ -0,0 +1,188 @@ +/* +Copyright 2017 The Kedge Authors All rights reserved. + +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 spec + +import ( + "encoding/json" + "reflect" + "testing" + + api_v1 "k8s.io/client-go/pkg/api/v1" +) + +func TestFixServices(t *testing.T) { + failingTest := []ServiceSpecMod{ + {Ports: nil}, + {Ports: nil}, + {Ports: nil}, + } + _, err := fixServices(failingTest, "") + if err == nil { + t.Errorf("should have failed but passed") + } else { + t.Logf("failed with error: %v", err) + } + + appName := "test" + passingTests := []struct { + Name string + Input []ServiceSpecMod + Output []ServiceSpecMod + }{ + { + Name: "only one service given", + Input: []ServiceSpecMod{{}}, + Output: []ServiceSpecMod{{Name: appName}}, + }, + { + Name: "multiple ports and no port name given", + Input: []ServiceSpecMod{ + { + Ports: []ServicePortMod{ + {ServicePort: api_v1.ServicePort{Port: 8080}}, + {ServicePort: api_v1.ServicePort{Port: 8081}}, + }, + }, + }, + Output: []ServiceSpecMod{ + { + Name: appName, + Ports: []ServicePortMod{ + { + ServicePort: api_v1.ServicePort{ + Name: appName + "-8080", Port: 8080, + }, + }, + { + ServicePort: api_v1.ServicePort{ + Name: appName + "-8081", Port: 8081, + }, + }, + }, + }, + }, + }, + } + + for _, test := range passingTests { + t.Logf("Running test: %s", test.Name) + got, err := fixServices(test.Input, appName) + if err != nil { + t.Errorf("expected to pass but failed with: %v", err) + } + if !reflect.DeepEqual(got, test.Output) { + t.Errorf("expected: %s, got: %s", prettyPrintObjects(test.Output), + prettyPrintObjects(got)) + } + } +} + +func TestFixVolumeClaims(t *testing.T) { + failingTest := []VolumeClaim{{}, {}} + + _, err := fixVolumeClaims(failingTest, "") + if err == nil { + t.Errorf("should have failed but passed") + } else { + t.Logf("failed with error: %v", err) + } + + appName := "test" + passingTest := []VolumeClaim{{}} + expected := []VolumeClaim{{Name: appName}} + got, err := fixVolumeClaims(passingTest, appName) + if err != nil { + t.Errorf("expected to pass but failed with: %v", err) + } + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %s, got: %s", prettyPrintObjects(expected), + prettyPrintObjects(got)) + } +} + +func TestFixConfigMaps(t *testing.T) { + failingTest := []ConfigMapMod{{}, {}} + _, err := fixConfigMaps(failingTest, "") + if err == nil { + t.Errorf("should have failed but passed") + } else { + t.Logf("failed with error: %v", err) + } + + appName := "test" + passingTest := []ConfigMapMod{{}} + expected := []ConfigMapMod{{Name: appName}} + got, err := fixConfigMaps(passingTest, appName) + if err != nil { + t.Errorf("expected to pass but failed with: %v", err) + } + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %s, got: %s", prettyPrintObjects(expected), + prettyPrintObjects(got)) + } +} + +func TestFixSecrets(t *testing.T) { + failingTest := []SecretMod{{}, {}} + _, err := fixSecrets(failingTest, "") + if err == nil { + t.Errorf("should have failed but passed") + } else { + t.Logf("failed with error: %v", err) + } + + appName := "test" + passingTest := []SecretMod{{}} + expected := []SecretMod{{Name: appName}} + got, err := fixSecrets(passingTest, appName) + if err != nil { + t.Errorf("expected to pass but failed with: %v", err) + } + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %s, got: %s", prettyPrintObjects(expected), + prettyPrintObjects(got)) + } +} + +func TestFixContainers(t *testing.T) { + failingTest := []Container{{}, {}} + _, err := fixContainers(failingTest, "") + if err == nil { + t.Errorf("should have failed but passed") + } else { + t.Logf("failed with error: %v", err) + } + + appName := "test" + passingTest := []Container{{}} + expected := []Container{ + {Container: api_v1.Container{Name: appName}}, + } + got, err := fixContainers(passingTest, appName) + if err != nil { + t.Errorf("expected to pass but failed with: %v", err) + } + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %s, got: %s", prettyPrintObjects(expected), + prettyPrintObjects(got)) + } +} + +func prettyPrintObjects(v interface{}) string { + b, _ := json.MarshalIndent(v, "", " ") + return string(b) +}