review: modification based on code review.

client/allocrunner/alloc_runner_hooks.go

@@ -133,11 +133,14 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
 		return fmt.Errorf("failed to initialize network configurator: %v", err)
 	}
 
-	// Create a new taskenv.TaskEnv which is used by networkHook and
-	// groupServiceHook for interpolation.
+	// Create a new taskenv.Builder which is used and mutated by networkHook.
 	envBuilder := taskenv.NewBuilder(
 		config.Node, ar.Alloc(), nil, config.Region).SetAllocDir(ar.allocDir.AllocDir)
 
+	// Create a taskenv.TaskEnv which is used for read only purposes by the
+	// newNetworkHook.
+	builtTaskEnv := envBuilder.Build()
+
 	// Create the alloc directory hook. This is run first to ensure the
 	// directory path exists for other hooks.
 	alloc := ar.Alloc()
@@ -147,7 +150,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
 		newUpstreamAllocsHook(hookLogger, ar.prevAllocWatcher),
 		newDiskMigrationHook(hookLogger, ar.prevAllocMigrator, ar.allocDir),
 		newAllocHealthWatcherHook(hookLogger, alloc, hs, ar.Listener(), ar.consulClient),
-		newNetworkHook(hookLogger, ns, alloc, nm, nc, ar, envBuilder),
+		newNetworkHook(hookLogger, ns, alloc, nm, nc, ar, builtTaskEnv),
 		newGroupServiceHook(groupServiceHookConfig{
 			alloc:               alloc,
 			consul:              ar.consulClient,

client/allocrunner/network_hook.go

@@ -4,11 +4,11 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/asaskevich/govalidator"
 	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad/client/taskenv"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/drivers"
+	"github.com/miekg/dns"
 )
 
 const (
@@ -72,9 +72,8 @@ type networkHook struct {
 	// the alloc network has been created
 	networkConfigurator NetworkConfigurator
 
-	// taskEnvBuilder is used to perform interpolation within the network
-	// blocks.
-	taskEnvBuilder *taskenv.Builder
+	// taskEnv is used to perform interpolation within the network blocks.
+	taskEnv *taskenv.TaskEnv
 
 	logger hclog.Logger
 }
@@ -85,15 +84,15 @@ func newNetworkHook(logger hclog.Logger,
 	netManager drivers.DriverNetworkManager,
 	netConfigurator NetworkConfigurator,
 	networkStatusSetter networkStatusSetter,
-	envBuilder *taskenv.Builder,
+	taskEnv *taskenv.TaskEnv,
 ) *networkHook {
 	return &networkHook{
 		isolationSetter:     ns,
 		networkStatusSetter: networkStatusSetter,
 		alloc:               alloc,
 		manager:             netManager,
 		networkConfigurator: netConfigurator,
-		taskEnvBuilder:      envBuilder,
+		taskEnv:             taskEnv,
 		logger:              logger,
 	}
 }
@@ -114,24 +113,23 @@ func (h *networkHook) Prerun() error {
 	}
 
 	// Perform our networks block interpolation.
-	interpolatedNetworks := taskenv.InterpolateNetworks(h.taskEnvBuilder.Build(), tg.Networks)
+	interpolatedNetworks := taskenv.InterpolateNetworks(h.taskEnv, tg.Networks)
 
 	// Interpolated values need to be validated. It is also possible a user
 	// supplied hostname avoids the validation on job registrations because it
 	// looks like it includes interpolation, when it doesn't.
 	if interpolatedNetworks[0].Hostname != "" {
-		if !govalidator.IsDNSName(interpolatedNetworks[0].Hostname) {
+		if _, ok := dns.IsDomainName(interpolatedNetworks[0].Hostname); !ok {
 			return fmt.Errorf("network hostname %q is not a valid DNS name", interpolatedNetworks[0].Hostname)
 		}
 	}
 
 	// Our network create request.
 	networkCreateReq := drivers.NetworkCreateRequest{
-		AllocID:  h.alloc.ID,
 		Hostname: interpolatedNetworks[0].Hostname,
 	}
 
-	spec, created, err := h.manager.CreateNetwork(&networkCreateReq)
+	spec, created, err := h.manager.CreateNetwork(h.alloc.ID, &networkCreateReq)
 	if err != nil {
 		return fmt.Errorf("failed to create network for alloc: %v", err)
 	}

client/allocrunner/network_hook_test.go

@@ -57,8 +57,8 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
 	destroyCalled := false
 	nm := &testutils.MockDriver{
 		MockNetworkManager: testutils.MockNetworkManager{
-			CreateNetworkF: func(req *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
-				require.Equal(t, alloc.ID, req.AllocID)
+			CreateNetworkF: func(allocID string, req *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
+				require.Equal(t, alloc.ID, allocID)
 				return spec, false, nil
 			},
 
@@ -83,7 +83,7 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
 	envBuilder := taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region)
 
 	logger := testlog.HCLogger(t)
-	hook := newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder)
+	hook := newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder.Build())
 	require.NoError(hook.Prerun())
 	require.True(setter.called)
 	require.False(destroyCalled)
@@ -94,7 +94,7 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
 	setter.called = false
 	destroyCalled = false
 	alloc.Job.TaskGroups[0].Networks[0].Mode = "host"
-	hook = newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder)
+	hook = newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder.Build())
 	require.NoError(hook.Prerun())
 	require.False(setter.called)
 	require.False(destroyCalled)

client/allocrunner/network_manager_linux.go

@@ -96,6 +96,7 @@ func newNetworkManager(alloc *structs.Allocation, driverManager drivermanager.Ma
 			//  indicates only Docker supports this, which is true unless a
 			//  custom driver can which means this check still holds as true as
 			//  we can tell.
+			//  Please see: https://github.com/hashicorp/nomad/issues/11180
 			return nil, fmt.Errorf("hostname is not currently supported on driver %s", task.Driver)
 		}
 
@@ -112,13 +113,13 @@ type defaultNetworkManager struct{}
 // CreateNetwork is the CreateNetwork implementation of the
 // drivers.DriverNetworkManager interface function. It does not currently
 // support setting the hostname of the network namespace.
-func (*defaultNetworkManager) CreateNetwork(createSpec *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
-	netns, err := nsutil.NewNS(createSpec.AllocID)
+func (*defaultNetworkManager) CreateNetwork(allocID string, _ *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
+	netns, err := nsutil.NewNS(allocID)
 	if err != nil {
 		// when a client restarts, the namespace will already exist and
 		// there will be a namespace file in use by the task process
 		if e, ok := err.(*os.PathError); ok && e.Err == syscall.EPERM {
-			nsPath := path.Join(nsutil.NetNSRunDir, createSpec.AllocID)
+			nsPath := path.Join(nsutil.NetNSRunDir, allocID)
 			_, err := os.Stat(nsPath)
 			if err == nil {
 				return nil, false, nil

drivers/docker/network.go

@@ -21,7 +21,7 @@ const (
 	dockerNetSpecHostnameKey = "docker_sandbox_hostname"
 )
 
-func (d *Driver) CreateNetwork(createSpec *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
+func (d *Driver) CreateNetwork(allocID string, createSpec *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
 	// Initialize docker API clients
 	client, _, err := d.dockerClients()
 	if err != nil {
@@ -36,12 +36,12 @@ func (d *Driver) CreateNetwork(createSpec *drivers.NetworkCreateRequest) (*drive
 	if err != nil {
 		d.logger.Debug("auth failed for infra container image pull", "image", d.config.InfraImage, "error", err)
 	}
-	_, err = d.coordinator.PullImage(d.config.InfraImage, authOptions, createSpec.AllocID, noopLogEventFn, d.config.infraImagePullTimeoutDuration, d.config.pullActivityTimeoutDuration)
+	_, err = d.coordinator.PullImage(d.config.InfraImage, authOptions, allocID, noopLogEventFn, d.config.infraImagePullTimeoutDuration, d.config.pullActivityTimeoutDuration)
 	if err != nil {
 		return nil, false, err
 	}
 
-	config, err := d.createSandboxContainerConfig(createSpec)
+	config, err := d.createSandboxContainerConfig(allocID, createSpec)
 	if err != nil {
 		return nil, false, err
 	}
@@ -109,10 +109,10 @@ func (d *Driver) DestroyNetwork(allocID string, spec *drivers.NetworkIsolationSp
 
 // createSandboxContainerConfig creates a docker container configuration which
 // starts a container with an empty network namespace.
-func (d *Driver) createSandboxContainerConfig(createSpec *drivers.NetworkCreateRequest) (*docker.CreateContainerOptions, error) {
+func (d *Driver) createSandboxContainerConfig(allocID string, createSpec *drivers.NetworkCreateRequest) (*docker.CreateContainerOptions, error) {
 
 	return &docker.CreateContainerOptions{
-		Name: fmt.Sprintf("nomad_init_%s", createSpec.AllocID),
+		Name: fmt.Sprintf("nomad_init_%s", allocID),
 		Config: &docker.Config{
 			Image:    d.config.InfraImage,
 			Hostname: createSpec.Hostname,

drivers/docker/network_test.go

@@ -10,13 +10,14 @@ import (
 
 func TestDriver_createSandboxContainerConfig(t *testing.T) {
 	testCases := []struct {
+		inputAllocID              string
 		inputNetworkCreateRequest *drivers.NetworkCreateRequest
 		expectedOutputOpts        *docker.CreateContainerOptions
 		name                      string
 	}{
 		{
+			inputAllocID: "768b5e8c-a52e-825c-d564-51100230eb62",
 			inputNetworkCreateRequest: &drivers.NetworkCreateRequest{
-				AllocID:  "768b5e8c-a52e-825c-d564-51100230eb62",
 				Hostname: "",
 			},
 			expectedOutputOpts: &docker.CreateContainerOptions{
@@ -31,8 +32,8 @@ func TestDriver_createSandboxContainerConfig(t *testing.T) {
 			name: "no input hostname",
 		},
 		{
+			inputAllocID: "768b5e8c-a52e-825c-d564-51100230eb62",
 			inputNetworkCreateRequest: &drivers.NetworkCreateRequest{
-				AllocID:  "768b5e8c-a52e-825c-d564-51100230eb62",
 				Hostname: "linux",
 			},
 			expectedOutputOpts: &docker.CreateContainerOptions{
@@ -57,7 +58,7 @@ func TestDriver_createSandboxContainerConfig(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actualOutput, err := d.createSandboxContainerConfig(tc.inputNetworkCreateRequest)
+			actualOutput, err := d.createSandboxContainerConfig(tc.inputAllocID, tc.inputNetworkCreateRequest)
 			assert.Nil(t, err, tc.name)
 			assert.Equal(t, tc.expectedOutputOpts, actualOutput, tc.name)
 		})

go.mod

@@ -23,7 +23,6 @@ require (
 	github.com/NYTimes/gziphandler v1.0.1
 	github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e
 	github.com/armon/go-metrics v0.3.4
-	github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
 	github.com/aws/aws-sdk-go v1.38.20
 	github.com/boltdb/bolt v1.3.1
 	github.com/container-storage-interface/spec v1.4.0
@@ -94,6 +93,7 @@ require (
 	github.com/kr/pty v1.1.5
 	github.com/kr/text v0.2.0
 	github.com/mattn/go-colorable v0.1.7
+	github.com/miekg/dns v1.1.26
 	github.com/mitchellh/cli v1.1.0
 	github.com/mitchellh/colorstring v0.0.0-20150917214807-8631ce90f286
 	github.com/mitchellh/copystructure v1.1.1

go.sum

@@ -103,8 +103,6 @@ github.com/armon/go-metrics v0.3.4/go.mod h1:4O98XIr/9W0sxpJ8UaYkvjk10Iff7SnFrb4
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI=
 github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
-github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ=
-github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
 github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM=
 github.com/aws/aws-sdk-go v1.25.37/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
 github.com/aws/aws-sdk-go v1.25.41/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=

nomad/structs/structs.go

@@ -25,7 +25,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/asaskevich/govalidator"
 	"github.com/hashicorp/cronexpr"
 	"github.com/hashicorp/go-msgpack/codec"
 	"github.com/hashicorp/go-multierror"
@@ -40,6 +39,7 @@ import (
 	"github.com/hashicorp/nomad/lib/cpuset"
 	"github.com/hashicorp/nomad/lib/kheap"
 	psstructs "github.com/hashicorp/nomad/plugins/shared/structs"
+	"github.com/miekg/dns"
 	"github.com/mitchellh/copystructure"
 	"golang.org/x/crypto/blake2b"
 )
@@ -6323,7 +6323,7 @@ func (tg *TaskGroup) validateNetworks() error {
 		// would be nice to validate additional parameters, but this isn't the
 		// right place.
 		if net.Hostname != "" && !strings.Contains(net.Hostname, "${") {
-			if !govalidator.IsDNSName(net.Hostname) {
+			if _, ok := dns.IsDomainName(net.Hostname); !ok {
 				mErr.Errors = append(mErr.Errors, errors.New("Hostname is not a valid DNS name"))
 			}
 		}

nomad/structs/structs_test.go

@@ -1463,7 +1463,7 @@ func TestTaskGroupNetwork_Validate(t *testing.T) {
 				Networks: []*NetworkResource{
 					{
 						Mode:     "bridge",
-						Hostname: "/t/n/t/!!?!?!?!",
+						Hostname: "............",
 					},
 				},
 			},

plugins/drivers/driver.go

@@ -90,7 +90,7 @@ type ExecOptions struct {
 // network namespace for which tasks can join. This only needs to be implemented
 // if the driver MUST create the network namespace
 type DriverNetworkManager interface {
-	CreateNetwork(request *NetworkCreateRequest) (*NetworkIsolationSpec, bool, error)
+	CreateNetwork(allocID string, request *NetworkCreateRequest) (*NetworkIsolationSpec, bool, error)
 	DestroyNetwork(allocID string, spec *NetworkIsolationSpec) error
 }
 
@@ -215,9 +215,6 @@ type HostsConfig struct {
 // network via DriverNetworkManager.CreateNetwork.
 type NetworkCreateRequest struct {
 
-	// AllocID is the long form UUID of the allocation the network is for.
-	AllocID string
-
 	// Hostname is the hostname the user has specified that the network should
 	// be configured with.
 	Hostname string

plugins/drivers/server.go

@@ -388,7 +388,7 @@ func (b *driverPluginServer) CreateNetwork(ctx context.Context, req *proto.Creat
 		return nil, fmt.Errorf("CreateNetwork RPC not supported by driver")
 	}
 
-	spec, created, err := nm.CreateNetwork(networkCreateRequestFromProto(req))
+	spec, created, err := nm.CreateNetwork(req.GetAllocId(), networkCreateRequestFromProto(req))
 	if err != nil {
 		return nil, err
 	}

plugins/drivers/testutils/testing.go

@@ -199,12 +199,12 @@ type MockDriver struct {
 }
 
 type MockNetworkManager struct {
-	CreateNetworkF  func(*drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error)
+	CreateNetworkF  func(string, *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error)
 	DestroyNetworkF func(string, *drivers.NetworkIsolationSpec) error
 }
 
-func (m *MockNetworkManager) CreateNetwork(req *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
-	return m.CreateNetworkF(req)
+func (m *MockNetworkManager) CreateNetwork(allocID string, req *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
+	return m.CreateNetworkF(allocID, req)
 }
 func (m *MockNetworkManager) DestroyNetwork(id string, spec *drivers.NetworkIsolationSpec) error {
 	return m.DestroyNetworkF(id, spec)

plugins/drivers/utils.go

@@ -640,7 +640,6 @@ func networkCreateRequestFromProto(pb *proto.CreateNetworkRequest) *NetworkCreat
 		return nil
 	}
 	return &NetworkCreateRequest{
-		AllocID:  pb.GetAllocId(),
 		Hostname: pb.GetHostname(),
 	}
 }

plugins/drivers/utils_test.go

@@ -123,7 +123,6 @@ func Test_networkCreateRequestFromProto(t *testing.T) {
 				Hostname: "foobar",
 			},
 			expectedOutput: &NetworkCreateRequest{
-				AllocID:  "59598b74-86e9-16ee-eb54-24c62935cc7c",
 				Hostname: "foobar",
 			},
 			name: "generic 1",