Repository: moby/moby
File: daemon/container.go (Go)

Code before:
		return err
	}

	if hostConfig != nil && hostConfig.NetworkMode == "" {
		hostConfig.NetworkMode = networktypes.NetworkDefault
	}
	container.HostConfig = hostConfig
	return nil
}

// verifyContainerSettings performs validation of the hostconfig and config
// structures.
func (daemon *Daemon) verifyContainerSettings(daemonCfg *configStore, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
	// First perform verification of settings common across all platforms.
	if err = validateContainerConfig(config); err != nil {
		return warnings, err
	}

	warns, err := validateHostConfig(hostConfig)
	warnings = append(warnings, warns...)
	if err != nil {
		return warnings, err
	}

	// Now do platform-specific verification
	warns, err = verifyPlatformContainerSettings(daemon, daemonCfg, hostConfig, update)
	warnings = append(warnings, warns...)

	return warnings, err
}

func validateContainerConfig(config *containertypes.Config) error {
	if config == nil {
		return nil
	}
	if err := translateWorkingDir(config); err != nil {
		return err
	}
	if len(config.StopSignal) > 0 {
		if _, err := signal.ParseSignal(config.StopSignal); err != nil {
			return err
		}
	}
	// Validate if Env contains empty variable or not (e.g., ``, `=foo`)
	for _, env := range config.Env {
		if _, err := opts.ValidateEnv(env); err != nil {
			return err
		}
	}
	return validateHealthCheck(config.Healthcheck)
}

Review comment (bug):
Perhaps OK as a follow-up, but we may want to preserve the debug logs somewhere. Doing that in the currentl location was perhaps not ideal (as this code also gets called implicitly for, e.g., swarm service tasks), but maybe on the caller side? (Which could be in the API); https://github.com/moby/moby/blob/6856bdd5a67007c1ea110eebd73ad03001cac063/api/server/router/container/container_routes.go#L659

We can decide to either only log on "success" (warnings, but no "fatal" error), which could perhaps be done in a defer. If we use structured logs, I guess we can log them all at once, instead of one per warning;

```go
var warnings []string
defer func() {
	// Log warnings for debugging, regardless if the request was successful or not.
	if len(warnings) > 0 {
		log.G(ctx).WithField("warnings", warnings).Debug("warnings encountered during container create request")
	}
}()
```

Or if we only want to log them if the request itself was successful, we can do that at the very end; https://github.com/moby/moby/blob/6856bdd5a67007c1ea110eebd73ad03001cac063/api/server/router/container/container_routes.go#L691-L695

```go
if err != nil {
	return err
}
ccr.Warnings = append(ccr.Warnings, warnings...)

// Log warnings for debugging, regardless if the request was successful or not.
if len(ccr.Warnings) > 0 {
    log.G(ctx).WithField("warnings", ccr.Warnings).Debug("warnings encountered during container create request")
}
return httputils.WriteJSON(w, http.StatusCreated, ccr)
```


Code after:
	if err := daemon.registerLinks(container, hostConfig); err != nil {
		return err
	}

	if hostConfig != nil && hostConfig.NetworkMode == "" {
		hostConfig.NetworkMode = networktypes.NetworkDefault
	}
	container.HostConfig = hostConfig
	return nil
}

// verifyContainerSettings performs validation of the hostconfig and config
// structures.
func (daemon *Daemon) verifyContainerSettings(daemonCfg *configStore, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
	// First perform verification of settings common across all platforms.
	if err = validateContainerConfig(config); err != nil {
		return warnings, err
	}

	warns, err := validateHostConfig(hostConfig)
	warnings = append(warnings, warns...)
	if err != nil {
		return warnings, err
	}

	// Now do platform-specific verification
	warns, err = verifyPlatformContainerSettings(daemon, daemonCfg, hostConfig, update)
	warnings = append(warnings, warns...)

	return warnings, err
}

func validateContainerConfig(config *containertypes.Config) error {
	if config == nil {
		return nil
	}
	if err := translateWorkingDir(config); err != nil {
		return err
	}
	if len(config.StopSignal) > 0 {
		if _, err := signal.ParseSignal(config.StopSignal); err != nil {
			return err
		}
	}
	// Validate if Env contains empty variable or not (e.g., ``, `=foo`)
	for _, env := range config.Env {
		if _, err := opts.ValidateEnv(env); err != nil {
			return err
		}
	}
	return validateHealthCheck(config.Healthcheck)