Make sure to port-forward all endpoints, regardless of exposure

Defining endpoints for Debug ports is required for Devfiles to work properly with `odo dev --debug`. This addresses the question of exposure for such endpoints. Tools like DevSpaces currently set 'exposure: none' for debug endpoints. This makes sense from the developer perspective, as 'odo dev' aims at emulating a local developer machine. As a consequence, `odo dev` should no longer ignore endpoints with 'exposure: none'. They should be port-forwarded just like other endpoints.
redhat-developer · Sep 13, 2022 · dd55ed7 · dd55ed7
1 parent 8b5986f
commit dd55ed7
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 92 deletions.
diff --git a/pkg/libdevfile/libdevfile.go b/pkg/libdevfile/libdevfile.go
@@ -269,7 +269,7 @@ func execDevfileEvent(devfileObj parser.DevfileObj, events []string, handler Han
 	return nil
 }
 
-// GetContainerEndpointMapping returns a map of container names and slice of its endpoints (in int) with exposure status other than none
+// GetContainerEndpointMapping returns a map of container names and slice of its endpoints (in int)
 func GetContainerEndpointMapping(containers []v1alpha2.Component) map[string][]int {
 	ceMapping := make(map[string][]int)
 	for _, container := range containers {
@@ -284,9 +284,7 @@ func GetContainerEndpointMapping(containers []v1alpha2.Component) map[string][]i
 
 		endpoints := container.Container.Endpoints
 		for _, e := range endpoints {
-			if e.Exposure != v1alpha2.NoneEndpointExposure {
-				ceMapping[k] = append(ceMapping[k], e.TargetPort)
-			}
+			ceMapping[k] = append(ceMapping[k], e.TargetPort)
 		}
 	}
 	return ceMapping

diff --git a/pkg/libdevfile/libdevfile_test.go b/pkg/libdevfile/libdevfile_test.go
@@ -530,6 +530,17 @@ func TestGetContainerEndpointMapping(t *testing.T) {
 		},
 	})
 
+	containerWithOneNoneInternalEndpoint := generator.GetContainerComponent(generator.ContainerComponentParams{
+		Name: "container-none-endpoint",
+		Endpoints: []v1alpha2.Endpoint{
+			{
+				Name:       "debug",
+				TargetPort: 9099,
+				Exposure:   v1alpha2.NoneEndpointExposure,
+			},
+		},
+	})
+
 	tests := []struct {
 		name string
 		args args
@@ -552,16 +563,37 @@ func TestGetContainerEndpointMapping(t *testing.T) {
 		{
 			name: "multiple containers with varying types of endpoints",
 			args: args{
-				containers: []v1alpha2.Component{containerWithNoEndpoints, containerWithOnePublicEndpoint, containerWithOneInternalEndpoint},
+				containers: []v1alpha2.Component{
+					containerWithNoEndpoints,
+					containerWithOnePublicEndpoint,
+					containerWithOneInternalEndpoint,
+					containerWithOneNoneInternalEndpoint,
+				},
+			},
+			want: map[string][]int{
+				containerWithNoEndpoints.Name:             {},
+				containerWithOnePublicEndpoint.Name:       {8080},
+				containerWithOneInternalEndpoint.Name:     {9090},
+				containerWithOneNoneInternalEndpoint.Name: {9099},
 			},
-			want: map[string][]int{containerWithNoEndpoints.Name: {}, containerWithOnePublicEndpoint.Name: {8080}, containerWithOneInternalEndpoint.Name: {9090}},
 		},
 		{
 			name: "invalid input - one image component with rest being containers",
 			args: args{
-				containers: []v1alpha2.Component{containerWithNoEndpoints, containerWithOnePublicEndpoint, containerWithOneInternalEndpoint, imageComponent},
+				containers: []v1alpha2.Component{
+					containerWithNoEndpoints,
+					containerWithOnePublicEndpoint,
+					containerWithOneInternalEndpoint,
+					containerWithOneNoneInternalEndpoint,
+					imageComponent,
+				},
+			},
+			want: map[string][]int{
+				containerWithNoEndpoints.Name:             {},
+				containerWithOnePublicEndpoint.Name:       {8080},
+				containerWithOneInternalEndpoint.Name:     {9090},
+				containerWithOneNoneInternalEndpoint.Name: {9099},
 			},
-			want: map[string][]int{containerWithNoEndpoints.Name: {}, containerWithOnePublicEndpoint.Name: {8080}, containerWithOneInternalEndpoint.Name: {9090}},
 		},
 	}
 	for _, tt := range tests {

diff --git a/pkg/odo/cli/dev/dev.go b/pkg/odo/cli/dev/dev.go
@@ -307,7 +307,7 @@ func NewCmdDev(name, fullName string) *cobra.Command {
 		Use:   name,
 		Short: "Deploy component to development cluster",
 		Long: `odo dev is a long running command that will automatically sync your source to the cluster.
-It forwards endpoints with exposure values 'public' or 'internal' to a port on localhost.`,
+It forwards endpoints with any exposure values ('public', 'internal' or 'none') to a port on localhost.`,
 		Example: fmt.Sprintf(devExample, fullName),
 		Args:    cobra.MaximumNArgs(0),
 		Run: func(cmd *cobra.Command, args []string) {

diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go
@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -568,23 +569,23 @@ ComponentSettings:
 						devSession.WaitEnd()
 					})
 
-					It("should expose two endpoints on localhost", func() {
-						url1 := fmt.Sprintf("http://%s", ports["3000"])
-						url2 := fmt.Sprintf("http://%s", ports["4567"])
-
-						resp1, err := http.Get(url1)
-						Expect(err).ToNot(HaveOccurred())
-						defer resp1.Body.Close()
-
-						resp2, err := http.Get(url2)
-						Expect(err).ToNot(HaveOccurred())
-						defer resp2.Body.Close()
+					It("should expose all endpoints on localhost regardless of exposure", func() {
+						getServerResponse := func(p int) string {
+							resp, err := http.Get(fmt.Sprintf("http://%s", ports[strconv.Itoa(p)]))
+							Expect(err).ToNot(HaveOccurred())
+							defer resp.Body.Close()
 
-						body1, _ := io.ReadAll(resp1.Body)
-						helper.MatchAllInOutput(string(body1), []string{"Hello from Node.js Starter Application!"})
+							body, _ := io.ReadAll(resp.Body)
+							return string(body)
+						}
+						containerPorts := []int{3000, 4567, 7890}
 
-						body2, _ := io.ReadAll(resp2.Body)
-						helper.MatchAllInOutput(string(body2), []string{"Hello from Node.js Starter Application!"})
+						for _, p := range containerPorts {
+							By(fmt.Sprintf("exposing a port targeting container port %d", p), func() {
+								r := getServerResponse(p)
+								helper.MatchAllInOutput(r, []string{"Hello from Node.js Starter Application!"})
+							})
+						}
 
 						helper.ReplaceString("server.js", "Hello from Node.js", "H3110 from Node.js")
 
@@ -596,76 +597,17 @@ ComponentSettings:
 							devSession.PressKey('p')
 						}
 
-						_, _, _, err = devSession.WaitSync()
+						_, _, _, err := devSession.WaitSync()
 						Expect(err).Should(Succeed())
 
-						Eventually(func() bool {
-							resp3, err := http.Get(url1)
-							if err != nil {
-								return false
-							}
-							defer resp3.Body.Close()
-
-							resp4, err := http.Get(url2)
-							if err != nil {
-								return false
-							}
-							defer resp4.Body.Close()
-
-							body3, _ := io.ReadAll(resp3.Body)
-							if string(body3) != "H3110 from Node.js Starter Application!" {
-								return false
-							}
-
-							body4, _ := io.ReadAll(resp4.Body)
-							return string(body4) == "H3110 from Node.js Starter Application!"
-						}, 180, 10).Should(Equal(true))
-					})
-
-					When("an endpoint is added after first run of odo dev", func() {
-
-						BeforeEach(func() {
-							helper.ReplaceString("devfile.yaml", "exposure: none", "exposure: public")
-
-							if manual {
-								if os.Getenv("SKIP_KEY_PRESS") == "true" {
-									Skip("This is a unix-terminal specific scenario, skipping")
-								}
-
-								devSession.PressKey('p')
-							}
-
-							var err error
-							_, _, ports, err = devSession.WaitSync()
-							Expect(err).Should(Succeed())
-
-						})
-						It("should expose three endpoints on localhost", func() {
-							url1 := fmt.Sprintf("http://%s", ports["3000"])
-							url2 := fmt.Sprintf("http://%s", ports["4567"])
-							url3 := fmt.Sprintf("http://%s", ports["7890"])
-
-							resp1, err := http.Get(url1)
-							Expect(err).ToNot(HaveOccurred())
-							defer resp1.Body.Close()
-
-							resp2, err := http.Get(url2)
-							Expect(err).ToNot(HaveOccurred())
-							defer resp2.Body.Close()
-
-							resp3, err := http.Get(url3)
-							Expect(err).ToNot(HaveOccurred())
-							defer resp3.Body.Close()
-
-							body1, _ := io.ReadAll(resp1.Body)
-							helper.MatchAllInOutput(string(body1), []string{"Hello from Node.js Starter Application!"})
-
-							body2, _ := io.ReadAll(resp2.Body)
-							helper.MatchAllInOutput(string(body2), []string{"Hello from Node.js Starter Application!"})
-
-							body3, _ := io.ReadAll(resp3.Body)
-							helper.MatchAllInOutput(string(body3), []string{"Hello from Node.js Starter Application!"})
-						})
+						for _, p := range containerPorts {
+							By(fmt.Sprintf("returning the right response when querying port forwarded for container port %d", p),
+								func() {
+									Eventually(func() string {
+										return getServerResponse(p)
+									}, 180, 10).Should(Equal("H3110 from Node.js Starter Application!"))
+								})
+						}
 					})
 				})