From 209cda28d22ab4f9be8c5ee0f40dc1ea4c450a0e Mon Sep 17 00:00:00 2001
From: Suraj Narwade <surajnarwade353@gmail.com>
Date: Fri, 5 May 2017 19:40:35 +0530
Subject: [PATCH] Failing when port is specified with labels

Resolves #522
Kompose will give FATAL error if labels are given but ports are not defined
---
 docs/user-guide.md                            |  3 ++-
 pkg/loader/compose/compose_test.go            | 24 +++++++++++++++++++
 pkg/loader/compose/v1v2.go                    | 11 +++++++++
 script/test/cmd/tests.sh                      |  4 ++++
 .../fixtures/label-port/docker-compose.yml    |  6 +++++
 5 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 script/test/fixtures/label-port/docker-compose.yml

diff --git a/docs/user-guide.md b/docs/user-guide.md
index 943f23db..a889dd34 100644
--- a/docs/user-guide.md
+++ b/docs/user-guide.md
@@ -367,6 +367,7 @@ The currently supported options are:
 | kompose.service.type | nodeport / clusterip / loadbalancer |
 | kompose.service.expose| true / hostname |
 
+**Note**: `kompose.service.type` label should be defined with `ports` only, otherwise `kompose` will fail.
 
 ## Restart
 
@@ -400,4 +401,4 @@ If the Docker Compose file has a volume specified for a service, the Deployment
 
 If the Docker Compose file has service name with `_` in it (eg.`web_service`), then it will be replaced by `-` and the service name will be renamed accordingly (eg.`web-service`). Kompose does this because "Kubernetes" doesn't allow `_` in object name.
 
-Please note that changing service name might break some `docker-compose` files.
+Please note that changing service name might break some `docker-compose` files.
\ No newline at end of file
diff --git a/pkg/loader/compose/compose_test.go b/pkg/loader/compose/compose_test.go
index 9b8b566b..56dea79f 100644
--- a/pkg/loader/compose/compose_test.go
+++ b/pkg/loader/compose/compose_test.go
@@ -323,3 +323,27 @@ func TestNormalizeServiceNames(t *testing.T) {
 		}
 	}
 }
+
+func TestCheckLabelsPorts(t *testing.T) {
+	testCases := []struct {
+		name        string
+		noOfPort    int
+		labels      string
+		svcName     string
+		expectError bool
+	}{
+		{"ports is defined", 1, "NodePort", "foo", false},
+		{"ports is not defined", 0, "NodePort", "foo", true},
+	}
+
+	var err error
+	for _, testcase := range testCases {
+		t.Log(testcase.name)
+		err = checkLabelsPorts(testcase.noOfPort, testcase.labels, testcase.svcName)
+		if testcase.expectError && err == nil {
+			t.Log("Expected error, got ", err)
+
+		}
+
+	}
+}
diff --git a/pkg/loader/compose/v1v2.go b/pkg/loader/compose/v1v2.go
index ee041d93..fe5c2a25 100644
--- a/pkg/loader/compose/v1v2.go
+++ b/pkg/loader/compose/v1v2.go
@@ -242,6 +242,10 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
 				serviceConfig.ExposeService = strings.ToLower(value)
 			}
 		}
+		err = checkLabelsPorts(len(serviceConfig.Port), composeServiceConfig.Labels["kompose.service.type"], name)
+		if err != nil {
+			return kobject.KomposeObject{}, errors.Wrap(err, "kompose.service.type can't be set if service doesn't expose any ports.")
+		}
 
 		// convert compose labels to annotations
 		serviceConfig.Annotations = map[string]string(composeServiceConfig.Labels)
@@ -266,3 +270,10 @@ func libComposeToKomposeMapping(composeObject *project.Project) (kobject.Kompose
 	}
 	return komposeObject, nil
 }
+
+func checkLabelsPorts(noOfPort int, labels string, svcName string) error {
+	if noOfPort == 0 && labels == "NodePort" || labels == "LoadBalancer" {
+		return errors.Errorf("%s defined in service %s with no ports present. Issues may occur when bringing up artifacts.", labels, svcName)
+	}
+	return nil
+}
diff --git a/script/test/cmd/tests.sh b/script/test/cmd/tests.sh
index 2ec00aab..1d64c34e 100755
--- a/script/test/cmd/tests.sh
+++ b/script/test/cmd/tests.sh
@@ -200,6 +200,10 @@ convert::expect_success_and_warning "kompose --provider openshift -f $KOMPOSE_RO
 # Test regarding validating dockerfilepath
 convert::expect_failure "kompose -f $KOMPOSE_ROOT/script/test/fixtures/dockerfilepath/docker-compose.yml convert --stdout"
 
+# Test regarding while label (nodeport or loadbalancer ) is provided but not ports
+convert::expect_failure "kompose -f $KOMPOSE_ROOT/script/test/fixtures/label-port/docker-compose.yml convert --stdout"
+
+
 ######
 # Test the output file behavior of kompose convert
 # Default behavior without -o
diff --git a/script/test/fixtures/label-port/docker-compose.yml b/script/test/fixtures/label-port/docker-compose.yml
new file mode 100644
index 00000000..ce80f958
--- /dev/null
+++ b/script/test/fixtures/label-port/docker-compose.yml
@@ -0,0 +1,6 @@
+version: "2"
+services:
+  webapiapplication:
+    image: webapiapplication
+    labels:
+      kompose.service.type: NodePort