change: allow different protocols on the same port (manager#1566)

pkg/buildclient/filesyncclient.go

@@ -172,7 +172,7 @@ func prepareSyncedDirs(localDirs map[string]string, dirNames []string, followPat
 			}
 		}
 	}
-	resetUIDAndGID := func(p string, st *fstypes.Stat) fsutil.MapResult {
+	resetUIDAndGID := func(_ string, st *fstypes.Stat) fsutil.MapResult {
 		st.Uid = 0
 		st.Gid = 0
 		return fsutil.MapResultKeep

pkg/cli/completion.go

@@ -281,7 +281,7 @@ func secretsCompletion(ctx context.Context, c client.Client, toComplete string)
 }
 
 func projectsCompletion(f ClientFactory) completionFunc {
-	return func(ctx context.Context, c client.Client, toComplete string) ([]string, error) {
+	return func(ctx context.Context, _ client.Client, toComplete string) ([]string, error) {
 		var acornConfigFile string
 		if f != nil {
 			acornConfigFile = f.AcornConfigFile()

pkg/cli/run.go

@@ -93,7 +93,7 @@ Volume Syntax
 	}
 	cmd.Flags().SetInterspersed(false)
 
-	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
+	cmd.SetHelpFunc(func(cmd *cobra.Command, _ []string) {
 		fmt.Println(cmd.Short + "\n")
 		fmt.Println(cmd.UsageString())
 	})

pkg/cli/run_test.go

@@ -43,7 +43,7 @@ func TestRunArgs_Env(t *testing.T) {
 func TestRun(t *testing.T) {
 	baseMock := func(f *mocks.MockClient) {
 		f.EXPECT().AppGet(gomock.Any(), gomock.Any()).DoAndReturn(
-			func(ctx context.Context, name string) (*apiv1.App, error) {
+			func(_ context.Context, name string) (*apiv1.App, error) {
 				switch name {
 				case "dne":
 					return nil, fmt.Errorf("error: app %s does not exist", name)
@@ -62,7 +62,7 @@ func TestRun(t *testing.T) {
 				return nil, nil
 			}).AnyTimes()
 		f.EXPECT().AppRun(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
-			func(ctx context.Context, image string, opts *client.AppRunOptions) (*apiv1.App, error) {
+			func(_ context.Context, image string, _ *client.AppRunOptions) (*apiv1.App, error) {
 				switch image {
 				case "dne":
 					return nil, fmt.Errorf("error: app %s does not exist", image)

pkg/cli/version.go

@@ -12,7 +12,7 @@ func NewVersion() *cobra.Command {
 		Use:     "version",
 		Short:   "Version information for acorn",
 		Example: "acorn version",
-		Run: func(cmd *cobra.Command, args []string) {
+		Run: func(_ *cobra.Command, _ []string) {
 			fmt.Printf("acorn version %s\n", version.Get().String())
 		},
 		Args: cobra.NoArgs,

pkg/cli/volumes_test.go

@@ -36,7 +36,7 @@ func TestVolume(t *testing.T) {
 				Status: apiv1.VolumeStatus{AppPublicName: "found", AppName: "found", VolumeName: "vol"},
 			}}, nil).AnyTimes()
 		f.EXPECT().VolumeGet(gomock.Any(), gomock.Any()).DoAndReturn(
-			func(ctx context.Context, name string) (*apiv1.Volume, error) {
+			func(_ context.Context, name string) (*apiv1.Volume, error) {
 				potentialVol := apiv1.Volume{TypeMeta: metav1.TypeMeta{},
 					ObjectMeta: metav1.ObjectMeta{Name: "found.vol",
 						Labels: map[string]string{
@@ -58,7 +58,7 @@ func TestVolume(t *testing.T) {
 				return nil, nil
 			}).AnyTimes()
 		f.EXPECT().VolumeDelete(gomock.Any(), gomock.Any()).DoAndReturn(
-			func(ctx context.Context, name string) (*apiv1.Volume, error) {
+			func(_ context.Context, name string) (*apiv1.Volume, error) {
 				switch name {
 				case "dne":
 					return nil, nil

pkg/client/client.go

@@ -516,7 +516,7 @@ func (c *DefaultClient) KubeProxyAddress(ctx context.Context, opts *KubeProxyAdd
 
 	srv := &http.Server{
 		Handler: handler,
-		BaseContext: func(listener net.Listener) context.Context {
+		BaseContext: func(_ net.Listener) context.Context {
 			return ctx
 		},
 	}

pkg/k8schannel/websocket.go

@@ -13,7 +13,7 @@ import (
 	"github.com/gorilla/websocket"
 )
 
-var Upgrader = &websocket.Upgrader{CheckOrigin: func(req *http.Request) bool {
+var Upgrader = &websocket.Upgrader{CheckOrigin: func(_ *http.Request) bool {
 	return true
 }, HandshakeTimeout: 15 * time.Second}
 

pkg/local/docker.go

@@ -296,7 +296,7 @@ func (c *Container) Wait(ctx context.Context) error {
 	ns.Infof("Waiting for local project")
 	w := watcher.New[*v1.Project](kc)
 	for {
-		_, err = w.ByName(ctx, "", "local", func(obj *v1.Project) (bool, error) {
+		_, err = w.ByName(ctx, "", "local", func(_ *v1.Project) (bool, error) {
 			return true, nil
 		})
 		if err != nil {

pkg/portforward/portforward.go

@@ -54,11 +54,11 @@ func PortForward(ctx context.Context, c client.Client, containerName string, add
 
 	p := tcpproxy.Proxy{}
 	p.AddRoute(listenAddress, &tcpproxy.DialProxy{
-		DialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+		DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
 			return dialer(ctx)
 		},
 	})
-	p.ListenFunc = func(_, laddr string) (net.Listener, error) {
+	p.ListenFunc = func(_, _ string) (net.Listener, error) {
 		fmt.Printf("Forwarding %s => %d for container [%s]\n", listener.Addr().String(), port, containerName)
 		return listener, err
 	}

pkg/ports/ports_test.go

@@ -132,11 +132,69 @@ func TestCollectPorts(t *testing.T) {
 				{TargetPort: 7070, Port: 7000, Hostname: "myapp3.local"},
 			},
 		},
+		// same port mappings on different hostnames
+		{
+			name: "same target ports, same ports, different hostnames",
+			ports: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Hostname: "myapp.local"},
+				{TargetPort: 8080, Port: 8080, Hostname: "myapp2.local"},
+			},
+			expected: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Hostname: "myapp.local"},
+				{TargetPort: 8080, Port: 8080, Hostname: "myapp2.local"},
+			},
+		},
+		{
+			name: "same target ports, same ports, different protocols, different hostnames - tcp twice ",
+			ports: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP, Hostname: "myapp.local"},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP, Hostname: "myapp2.local"},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolHTTP2, Hostname: "myapp3.local"},
+			},
+			expected: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP, Hostname: "myapp.local"},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP, Hostname: "myapp2.local"},
+			},
+		},
+		{
+			name: "same target ports, same ports, different protocols - tcp twice",
+			ports: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolHTTP2},
+			},
+			expected: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+			},
+		},
+		{
+			name: "same target ports, same ports, same protocol twice",
+			ports: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP},
+			},
+			expected: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolTCP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+			},
+		},
+		{
+			name: "same target ports, same ports, udp twice",
+			ports: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+			},
+			expected: []v1.PortDef{
+				{TargetPort: 8080, Port: 8080, Protocol: v1.ProtocolUDP},
+			},
+		},
 	}
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-			seen := map[int32][]int32{}
+			seen := map[int32][]v1.PortDef{}
 			seenHostname := map[string]struct{}{}
 			assert.Equal(t, tt.expected, collectPorts(seen, seenHostname, tt.ports, false))
 		})

pkg/ports/publish.go

@@ -198,7 +198,14 @@ func matches(binding v1.PortPublish, port v1.PortDef) bool {
 		portMatches(binding, port)
 }
 
-func collectPorts(seen map[int32][]int32, seenHostnames map[string]struct{}, ports []v1.PortDef, devMode bool) (result []v1.PortDef) {
+/*
+collectPorts collects all port definitions, deduplicates them and drops disallowed combinations
+
+  - no duplicate hostnames allowed
+  - same protocol on same port only allowed if using different hostnames
+  - only 1x UDP and 1 variant of TCP (tcp/http/http2) allowed
+*/
+func collectPorts(seen map[int32][]v1.PortDef, seenHostnames map[string]struct{}, ports []v1.PortDef, devMode bool) (result []v1.PortDef) {
 	for _, port := range ports {
 		if !devMode && port.Dev {
 			continue
@@ -216,23 +223,38 @@ func collectPorts(seen map[int32][]int32, seenHostnames map[string]struct{}, por
 			port.Port = port.TargetPort
 		}
 
-		if targets, ok := seen[port.Port]; ok {
-			// Check for special case: the same port is exposed on multiple hostnames, so keep both.
-			if port.Hostname != "" {
-				for _, t := range targets {
-					if t == port.TargetPort {
-						// Same port and target port but different hostnames, so keep both
-						seen[port.Port] = append(targets, port.TargetPort)
-						seenHostnames[port.Hostname] = struct{}{}
-						result = append(result, port)
+		if seenPortDefs, ok := seen[port.Port]; ok {
+			discard := false
+			for _, p := range seenPortDefs {
+				if port.TargetPort == p.TargetPort {
+					if (port.Protocol != v1.ProtocolUDP && p.Protocol != v1.ProtocolUDP) || (port.Protocol == v1.ProtocolUDP && p.Protocol == v1.ProtocolUDP) {
+						if port.Protocol == p.Protocol && port.Hostname != "" {
+							// OK: Same port and target port and protocol but different hostnames (deduplicated before), so keep both
+							continue
+						}
+						// NOT OK: Same port, target port, and protocol (variants of TCP are considered the same, i.e. TCP/HTTP/HTTP2)
+						// The only case that's OK is if one is UDP and the other is not (some variant of TCP)
+						discard = true
 						break
 					}
+				} else {
+					// NOT OK: Same port but different target port
+					discard = true
+					break
+				}
+			}
+
+			if !discard {
+				seen[port.Port] = append(seen[port.Port], port)
+				if port.Hostname != "" {
+					seenHostnames[port.Hostname] = struct{}{}
 				}
+				result = append(result, port)
 			}
 			continue
 		}
 
-		seen[port.Port] = []int32{port.TargetPort}
+		seen[port.Port] = []v1.PortDef{port}
 		if port.Hostname != "" {
 			seenHostnames[port.Hostname] = struct{}{}
 		}
@@ -253,7 +275,7 @@ func FilterDevPorts(ports []v1.PortDef, devMode bool) (result []v1.PortDef) {
 
 func CollectContainerPorts(container *v1.Container, devMode bool) (result []v1.PortDef) {
 	// seen represents a mapping of public port numbers to target port numbers
-	seen := map[int32][]int32{}
+	seen := map[int32][]v1.PortDef{}
 	seenHostnames := map[string]struct{}{}
 
 	result = append(result, collectPorts(seen, seenHostnames, container.Ports, devMode)...)

pkg/replace/interpolate.go

@@ -23,7 +23,7 @@ func Interpolate(data any, s string) (string, error) {
 		out := json.RawMessage{}
 		err = aml.Unmarshal([]byte(s), &out, aml.DecoderOption{
 			SourceName: "inline",
-			GlobalsLookup: func(ctx context.Context, key string, parent eval.Scope) (value.Value, bool, error) {
+			GlobalsLookup: func(_ context.Context, key string, _ eval.Scope) (value.Value, bool, error) {
 				return value.Lookup(val, value.NewValue(key))
 			},
 		})

pkg/replace/replace_test.go

@@ -82,7 +82,7 @@ func TestReplace(t *testing.T) {
 				s:          "start@{inner}end",
 				startToken: "@{",
 				endToken:   "}",
-				replace: func(s string) (string, bool, error) {
+				replace: func(_ string) (string, bool, error) {
 					return "", false, nil
 				},
 			},

pkg/server/registry/apigroups/acorn/apps/icon.go

@@ -81,18 +81,18 @@ func (i *Icon) Connect(ctx context.Context, id string, _ runtime.Object, _ rest.
 	case ".gif":
 		contentType = "image/gif"
 	default:
-		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		return http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
 			rw.WriteHeader(http.StatusNotFound)
 		}), nil
 	}
 
 	if len(icon) == 0 {
-		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		return http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
 			rw.WriteHeader(http.StatusNotFound)
 		}), nil
 	}
 
-	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+	return http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
 		rw.Header().Set("Content-Type", contentType)
 		_, _ = rw.Write(icon)
 	}), nil

pkg/server/registry/apigroups/acorn/builders/port.go

@@ -37,7 +37,7 @@ func NewBuilderPort(client kclient.WithWatch, transport http.RoundTripper) (*Bui
 		proxy: httputil.ReverseProxy{
 			Transport:     transport,
 			FlushInterval: 200 * time.Millisecond,
-			Director:      func(request *http.Request) {},
+			Director:      func(_ *http.Request) {},
 		},
 		httpClient: &http.Client{
 			Transport: transport,

pkg/server/registry/apigroups/acorn/containers/exec.go

@@ -70,7 +70,7 @@ func NewContainerExec(client kclient.WithWatch, cfg *rest.Config) (*ContainerExe
 		proxy: httputil.ReverseProxy{
 			FlushInterval: 200 * time.Millisecond,
 			Transport:     transport,
-			Director:      func(request *http.Request) {},
+			Director:      func(_ *http.Request) {},
 		},
 		RESTClient: k8s.CoreV1().RESTClient(),
 		rbac:       apps.NewRBACValidator(client),

pkg/server/registry/apigroups/acorn/containers/port_forward.go

@@ -50,7 +50,7 @@ func NewPortForward(client kclient.WithWatch, cfg *rest.Config) (*PortForward, e
 		proxy: httputil.ReverseProxy{
 			FlushInterval: 200 * time.Millisecond,
 			Transport:     transport,
-			Director:      func(request *http.Request) {},
+			Director:      func(_ *http.Request) {},
 		},
 		RESTClient: k8s.CoreV1().RESTClient(),
 	}, nil