pass only required parameter to functions

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
 }

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)
+}