api: remove mapstructure tags fromPort struct

.changelog/12916.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+event_stream: fixed a bug where dynamic port values would fail to serialize in the event stream
+```

api/resources.go

@@ -98,9 +98,9 @@ func (r *Resources) Merge(other *Resources) {
 
 type Port struct {
 	Label       string `hcl:",label"`
-	Value       int    `mapstructure:"static" hcl:"static,optional"`
-	To          int    `mapstructure:"to" hcl:"to,optional"`
-	HostNetwork string `mapstructure:"host_network" hcl:"host_network,optional"`
+	Value       int    `hcl:"static,optional"`
+	To          int    `hcl:"to,optional"`
+	HostNetwork string `hcl:"host_network,optional"`
 }
 
 type DNSConfig struct {

command/agent/alloc_endpoint_test.go

@@ -1121,3 +1121,29 @@ func TestHTTP_ReadWsHandshake(t *testing.T) {
 		})
 	}
 }
+
+// TestHTTP_AllocPort_Parsing tests that removing the mapstructure tags from the
+// Port struct has no adverse affects on serialization.
+func TestHTTP_AllocPort_Parsing(t *testing.T) {
+	ci.Parallel(t)
+
+	httpTest(t, nil, func(s *TestAgent) {
+		state := s.Agent.server.State()
+		alloc := mock.Alloc()
+
+		require.NoError(t, state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc}))
+
+		req, err := http.NewRequest("GET", "/v1/allocation/"+alloc.ID, nil)
+		require.NoError(t, err)
+		respW := httptest.NewRecorder()
+
+		obj, err := s.Server.AllocSpecificRequest(respW, req)
+		require.NoError(t, err)
+
+		a := obj.(*structs.Allocation)
+		networkResource := a.AllocatedResources.Tasks["web"].Networks[0]
+		require.Equal(t, a.ID, alloc.ID)
+		require.Equal(t, 5000, networkResource.ReservedPorts[0].Value)
+		require.Equal(t, 9876, networkResource.DynamicPorts[0].Value)
+	})
+}

e2e/events/events_test.go

@@ -0,0 +1,127 @@
+package events
+
+import (
+	"context"
+	"fmt"
+	"github.com/hashicorp/nomad/api"
+	"github.com/hashicorp/nomad/e2e/e2eutil"
+	"github.com/hashicorp/nomad/helper/uuid"
+	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/testutil"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+// TestEventStream_Ports tests that removing the mapstructure tags from the Port
+// struct results in Port values being serialized correctly to the event stream.
+func TestEventStream_Ports(t *testing.T) {
+	nomadClient := e2eutil.NomadClient(t)
+	e2eutil.WaitForLeader(t, nomadClient)
+	e2eutil.WaitForNodesReady(t, nomadClient, 1)
+
+	testCases := []struct {
+		name     string
+		networks []*structs.NetworkResource
+	}{
+		{
+			name: "static-ports",
+			networks: []*structs.NetworkResource{
+				{
+					ReservedPorts: []structs.Port{
+						{
+							Label: "http",
+							Value: 9000,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "dynamic-ports",
+			networks: []*structs.NetworkResource{
+				{
+					DynamicPorts: []structs.Port{
+						{
+							Label: "http",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var jobIDs []string
+			t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs))
+
+			jobID := "event-stream-" + tc.name + "-" + uuid.Short()
+
+			err := e2eutil.Register(jobID, "./input/"+tc.name+".nomad")
+			require.NoError(t, err)
+			jobIDs = append(jobIDs, jobID)
+
+			err = e2eutil.WaitForAllocStatusExpected(jobID, "",
+				[]string{"running"})
+			require.NoError(t, err, "job should be running")
+
+			err = e2eutil.WaitForLastDeploymentStatus(jobID, "", "successful", nil)
+			require.NoError(t, err, "success", "deployment did not complete")
+
+			topics := map[api.Topic][]string{
+				api.TopicAllocation: {jobID},
+			}
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			events := nomadClient.EventStream()
+			streamCh, err := events.Stream(ctx, topics, 1, nil)
+			require.NoError(t, err)
+
+			var allocEvents []api.Event
+			// gather job alloc events
+			go func() {
+				for {
+					select {
+					case <-ctx.Done():
+						return
+					case event, ok := <-streamCh:
+						if !ok {
+							return
+						}
+						if event.IsHeartbeat() {
+							continue
+						}
+						allocEvents = append(allocEvents, event.Events...)
+					}
+				}
+			}()
+
+			var alloc *api.Allocation
+			testutil.WaitForResult(func() (bool, error) {
+				var got string
+				for _, e := range allocEvents {
+					if e.Type == "AllocationUpdated" {
+						alloc, err = e.Allocation()
+						return true, nil
+					}
+					got = e.Type
+				}
+				return false, fmt.Errorf("expected to receive allocation updated event, got: %#v", got)
+			}, func(e error) {
+				require.NoError(t, err)
+			})
+
+			require.NotNil(t, alloc)
+			require.Len(t, alloc.Resources.Networks, 1)
+			if len(tc.networks[0].ReservedPorts) == 1 {
+				require.Equal(t, tc.networks[0].ReservedPorts[0].Value, alloc.Resources.Networks[0].ReservedPorts[0].Value)
+			}
+
+			if len(tc.networks[0].DynamicPorts) == 1 {
+				require.NotEqual(t, 0, alloc.Resources.Networks[0].DynamicPorts[0].Value)
+			}
+		})
+	}
+}

e2e/events/input/dynamic-ports.nomad

@@ -0,0 +1,28 @@
+job "static-ports" {
+  datacenters = ["dc1"]
+  type        = "service"
+
+  constraint {
+    attribute = "${attr.kernel.name}"
+    value     = "linux"
+  }
+
+  group "group" {
+    count = 1
+
+    network {
+      port "db" {}
+    }
+
+    task "dynamic-port" {
+      driver = "docker"
+
+      config {
+        image   = "busybox:1"
+        command = "nc"
+        args    = ["-ll", "-p", "1234", "-e", "/bin/cat"]
+        ports   = ["db"]
+      }
+    }
+  }
+}

e2e/events/input/static-ports.nomad

@@ -0,0 +1,30 @@
+job "static-ports" {
+  datacenters = ["dc1"]
+  type        = "service"
+
+  constraint {
+    attribute = "${attr.kernel.name}"
+    value     = "linux"
+  }
+
+  group "group" {
+    count = 1
+
+    network {
+      port "db" {
+        static = 9000
+      }
+    }
+
+    task "static-port" {
+      driver = "docker"
+
+      config {
+        image   = "busybox:1"
+        command = "nc"
+        args    = ["-ll", "-p", "1234", "-e", "/bin/cat"]
+        ports   = ["db"]
+      }
+    }
+  }
+}

jobspec/parse_network.go

@@ -93,14 +93,12 @@ func parsePorts(networkObj *ast.ObjectList, nw *api.NetworkResource) error {
 		if knownPortLabels[l] {
 			return fmt.Errorf("found a port label collision: %s", label)
 		}
-		var p map[string]interface{}
+
 		var res api.Port
-		if err := hcl.DecodeObject(&p, port.Val); err != nil {
-			return err
-		}
-		if err := mapstructure.WeakDecode(p, &res); err != nil {
+		if err := hcl.DecodeObject(&res, port.Val); err != nil {
 			return err
 		}
+
 		res.Label = label
 		if res.Value > 0 {
 			nw.ReservedPorts = append(nw.ReservedPorts, res)

jobspec/parse_test.go

@@ -1865,3 +1865,26 @@ func TestIncorrectKey(t *testing.T) {
 		t.Fatalf("Expected key error; got %v", err)
 	}
 }
+
+// TestPortParsing validates that the removal of the mapstructure tags on the Port
+// struct don't cause issues with HCL 1 parsing.
+func TestPortParsing(t *testing.T) {
+	ci.Parallel(t)
+
+	var err error
+	var path string
+	var job *api.Job
+
+	path, err = filepath.Abs(filepath.Join("./test-fixtures", "parse-ports.hcl"))
+	require.NoError(t, err, "Can't get absolute path for file: parse-ports.hcl")
+
+	job, err = ParseFile(path)
+	require.NoError(t, err, "cannot parse job")
+	require.NotNil(t, job)
+	require.Len(t, job.TaskGroups, 1)
+	require.Len(t, job.TaskGroups[0].Networks, 1)
+	require.Len(t, job.TaskGroups[0].Networks[0].ReservedPorts, 1)
+	require.Len(t, job.TaskGroups[0].Networks[0].DynamicPorts, 1)
+	require.Equal(t, 9000, job.TaskGroups[0].Networks[0].ReservedPorts[0].Value)
+	require.Equal(t, 0, job.TaskGroups[0].Networks[0].DynamicPorts[0].Value)
+}

jobspec/test-fixtures/parse-ports.hcl

@@ -0,0 +1,11 @@
+job "parse-ports" {
+  group "group" {
+    network {
+      port "static" {
+        static = 9000
+      }
+
+      port "dynamic" {}
+    }
+  }
+}