Convert SpecGen values to be nullable where possible

SpecGen is our primary container creation abstraction, and is
used to connect our CLI to the Libpod container creation backend.
Because container creation has a million options (I exaggerate
only slightly), the struct is composed of several other structs,
many of which are quite large.

The core problem is that SpecGen is also an API type - it's used
in remote Podman. There, we have a client and a server, and we
want to respect the server's containers.conf. But how do we tell
what parts of SpecGen were set by the client explicitly, and what
parts were not? If we're not using nullable values, an explicit
empty string and a value never being set are identical - and we
can't tell if it's safe to grab a default from the server's
containers.conf.

Fortunately, we only really need to do this for booleans. An
empty string is sufficient to tell us that a string was unset
(even if the user explicitly gave us an empty string for an
option, filling in a default from the config file is acceptable).
This makes things a lot simpler. My initial attempt at this
changed everything, including strings, and it was far larger and
more painful.

Also, begin the first steps of removing all uses of
containers.conf defaults from client-side. Two are gone entirely,
the rest are marked as remove-when-possible.

[NO NEW TESTS NEEDED] This is just a refactor.

Signed-off-by: Matt Heon <mheon@redhat.com>

pkg/api/handlers/compat/containers_create.go

@@ -115,7 +115,8 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	// moby always create the working directory
-	sg.CreateWorkingDir = true
+	localTrue := true
+	sg.CreateWorkingDir = &localTrue
 	// moby doesn't inherit /etc/hosts from host
 	sg.BaseHostsFile = "none"
 

pkg/api/handlers/libpod/containers_create.go

@@ -30,11 +30,11 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	// we have to set the default before we decode to make sure the correct default is set when the field is unset
 	sg := specgen.SpecGenerator{
 		ContainerNetworkConfig: specgen.ContainerNetworkConfig{
-			UseImageHosts: conf.Containers.NoHosts,
+			UseImageHosts: &conf.Containers.NoHosts,
 		},
 		ContainerSecurityConfig: specgen.ContainerSecurityConfig{
 			Umask:      conf.Containers.Umask,
-			Privileged: conf.Containers.Privileged,
+			Privileged: &conf.Containers.Privileged,
 		},
 	}
 

pkg/bindings/test/attach_test.go

@@ -67,7 +67,8 @@ var _ = Describe("Podman containers attach", func() {
 	It("can echo data via cat in container", func() {
 		s := specgen.NewSpecGenerator(alpine.name, false)
 		s.Name = "CatAttachTest"
-		s.Terminal = true
+		localTrue := true
+		s.Terminal = &localTrue
 		s.Command = []string{"/bin/cat"}
 		ctnr, err := containers.CreateWithSpec(bt.conn, s, nil)
 		Expect(err).ShouldNot(HaveOccurred())

pkg/bindings/test/common_test.go

@@ -203,7 +203,8 @@ func (b *bindingTest) restoreImageFromCache(i testImage) {
 // and add or append the alpine image to it
 func (b *bindingTest) RunTopContainer(containerName *string, podName *string) (string, error) {
 	s := specgen.NewSpecGenerator(alpine.name, false)
-	s.Terminal = false
+	terminal := false
+	s.Terminal = &terminal
 	s.Command = []string{"/usr/bin/top"}
 	if containerName != nil {
 		s.Name = *containerName

pkg/bindings/test/containers_test.go

@@ -360,7 +360,8 @@ var _ = Describe("Podman containers ", func() {
 	It("logging", func() {
 		stdoutChan := make(chan string, 10)
 		s := specgen.NewSpecGenerator(alpine.name, false)
-		s.Terminal = true
+		terminal := true
+		s.Terminal = &terminal
 		s.Command = []string{"date", "-R"}
 		r, err := containers.CreateWithSpec(bt.conn, s, nil)
 		Expect(err).ToNot(HaveOccurred())
@@ -774,7 +775,8 @@ var _ = Describe("Podman containers ", func() {
 		_, err = bt.RunTopContainer(&name2, nil)
 		Expect(err).ToNot(HaveOccurred())
 		s := specgen.NewSpecGenerator(alpine.name, false)
-		s.Terminal = true
+		terminal := true
+		s.Terminal = &terminal
 		s.Command = []string{"date", "-R"}
 		_, err = containers.CreateWithSpec(bt.conn, s, nil)
 		Expect(err).ToNot(HaveOccurred())

pkg/bindings/test/create_test.go

@@ -33,7 +33,8 @@ var _ = Describe("Create containers ", func() {
 	It("create a container running top", func() {
 		s := specgen.NewSpecGenerator(alpine.name, false)
 		s.Command = []string{"top"}
-		s.Terminal = true
+		terminal := true
+		s.Terminal = &terminal
 		s.Name = "top"
 		ctr, err := containers.CreateWithSpec(bt.conn, s, nil)
 		Expect(err).ToNot(HaveOccurred())

pkg/domain/infra/abi/containers.go

@@ -1701,7 +1701,8 @@ func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts enti
 	}
 
 	// if we do not pass term, running ctrs exit
-	spec.Terminal = c.Terminal()
+	localTerm := c.Terminal()
+	spec.Terminal = &localTerm
 
 	// Print warnings
 	if len(out) > 0 {

pkg/specgen/container_validate.go

@@ -31,7 +31,7 @@ func (s *SpecGenerator) Validate() error {
 		if len(s.Networks) > 0 {
 			return fmt.Errorf("networks must be defined when the pod is created: %w", define.ErrNetworkOnPodContainer)
 		}
-		if len(s.PortMappings) > 0 || s.PublishExposedPorts {
+		if len(s.PortMappings) > 0 || (s.PublishExposedPorts != nil && *s.PublishExposedPorts) {
 			return fmt.Errorf("published or exposed ports must be defined when the pod is created: %w", define.ErrNetworkOnPodContainer)
 		}
 		if len(s.HostAdd) > 0 {
@@ -102,7 +102,7 @@ func (s *SpecGenerator) Validate() error {
 	// ContainerNetworkConfig
 	//
 	// useimageresolveconf conflicts with dnsserver, dnssearch, dnsoption
-	if s.UseImageResolvConf {
+	if s.UseImageResolvConf != nil && *s.UseImageResolvConf {
 		if len(s.DNSServers) > 0 {
 			return exclusiveOptions("UseImageResolvConf", "DNSServer")
 		}
@@ -114,7 +114,7 @@ func (s *SpecGenerator) Validate() error {
 		}
 	}
 	// UseImageHosts and HostAdd are exclusive
-	if s.UseImageHosts && len(s.HostAdd) > 0 {
+	if (s.UseImageHosts != nil && *s.UseImageHosts) && len(s.HostAdd) > 0 {
 		return exclusiveOptions("UseImageHosts", "HostAdd")
 	}
 

pkg/specgen/generate/container.go

@@ -110,7 +110,15 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	}
 
 	// Get Default Environment from containers.conf
-	defaultEnvs, err := envLib.ParseSlice(rtc.GetDefaultEnvEx(s.EnvHost, s.HTTPProxy))
+	envHost := false
+	if s.EnvHost != nil {
+		envHost = *s.EnvHost
+	}
+	httpProxy := false
+	if s.HTTPProxy != nil {
+		httpProxy = *s.HTTPProxy
+	}
+	defaultEnvs, err := envLib.ParseSlice(rtc.GetDefaultEnvEx(envHost, httpProxy))
 	if err != nil {
 		return nil, fmt.Errorf("parsing fields in containers.conf: %w", err)
 	}
@@ -129,7 +137,7 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 
 	// add default terminal to env if tty flag is set
 	_, ok := defaultEnvs["TERM"]
-	if s.Terminal && !ok {
+	if (s.Terminal != nil && *s.Terminal) && !ok {
 		defaultEnvs["TERM"] = "xterm"
 	}
 
@@ -154,17 +162,17 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		delete(defaultEnvs, e)
 	}
 
-	if s.UnsetEnvAll {
+	if s.UnsetEnvAll != nil && *s.UnsetEnvAll {
 		defaultEnvs = make(map[string]string)
 	}
 	// First transform the os env into a map. We need it for the labels later in
 	// any case.
 	osEnv := envLib.Map(os.Environ())
 
 	// Caller Specified defaults
-	if s.EnvHost {
+	if envHost {
 		defaultEnvs = envLib.Join(defaultEnvs, osEnv)
-	} else if s.HTTPProxy {
+	} else if httpProxy {
 		for _, envSpec := range config.ProxyEnv {
 			if v, ok := osEnv[envSpec]; ok {
 				defaultEnvs[envSpec] = v
@@ -303,6 +311,10 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		warnings = append(warnings, "Port mappings have been discarded as one of the Host, Container, Pod, and None network modes are in use")
 	}
 
+	if len(s.ImageVolumeMode) == 0 {
+		s.ImageVolumeMode = rtc.Engine.ImageVolumeMode
+	}
+
 	return warnings, nil
 }
 
@@ -497,7 +509,7 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, containerID
 
 // mapSecurityConfig takes a libpod.ContainerSecurityConfig and converts it to a specgen.ContinerSecurityConfig
 func mapSecurityConfig(c *libpod.ContainerConfig, s *specgen.SpecGenerator) {
-	s.Privileged = c.Privileged
+	s.Privileged = &c.Privileged
 	s.SelinuxOpts = append(s.SelinuxOpts, c.LabelOpts...)
 	s.User = c.User
 	s.Groups = c.Groups

pkg/specgen/generate/container_create.go

@@ -152,7 +152,11 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 	}
 
 	if s.Rootfs != "" {
-		options = append(options, libpod.WithRootFS(s.Rootfs, s.RootfsOverlay, s.RootfsMapping))
+		rootfsOverlay := false
+		if s.RootfsOverlay != nil {
+			rootfsOverlay = *s.RootfsOverlay
+		}
+		options = append(options, libpod.WithRootFS(s.Rootfs, rootfsOverlay, s.RootfsMapping))
 	}
 
 	newImage, resolvedImageName, imageData, err := getImageFromSpec(ctx, rt, s)
@@ -358,7 +362,7 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 		options = append(options, libpod.WithPreserveFD(s.PreserveFD))
 	}
 
-	if s.Stdin {
+	if s.Stdin != nil && *s.Stdin {
 		options = append(options, libpod.WithStdin())
 	}
 
@@ -368,7 +372,7 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 	if s.Umask != "" {
 		options = append(options, libpod.WithUmask(s.Umask))
 	}
-	if s.Volatile {
+	if s.Volatile != nil && *s.Volatile {
 		options = append(options, libpod.WithVolatile())
 	}
 	if s.PasswdEntry != "" {
@@ -381,7 +385,7 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 		options = append(options, libpod.WithBaseHostsFile(s.BaseHostsFile))
 	}
 
-	if s.Privileged {
+	if s.IsPrivileged() {
 		options = append(options, libpod.WithMountAllDevices())
 	}
 
@@ -520,7 +524,7 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 	if s.WorkDir == "" {
 		s.WorkDir = "/"
 	}
-	if s.CreateWorkingDir {
+	if s.CreateWorkingDir != nil && *s.CreateWorkingDir {
 		options = append(options, libpod.WithCreateWorkingDir())
 	}
 	if s.StopSignal != nil {
@@ -547,8 +551,8 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 			options = append(options, libpod.WithLogDriver(s.LogConfiguration.Driver))
 		}
 	}
-	if s.ContainerSecurityConfig.LabelNested {
-		options = append(options, libpod.WithLabelNested(s.ContainerSecurityConfig.LabelNested))
+	if s.LabelNested != nil {
+		options = append(options, libpod.WithLabelNested(*s.LabelNested))
 	}
 	// Security options
 	if len(s.SelinuxOpts) > 0 {
@@ -567,8 +571,10 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 			options = append(options, libpod.WithSecLabels(selinuxOpts))
 		}
 	}
-	options = append(options, libpod.WithPrivileged(s.Privileged))
-	options = append(options, libpod.WithReadWriteTmpfs(s.ReadWriteTmpfs))
+	options = append(options, libpod.WithPrivileged(s.IsPrivileged()))
+	if s.ReadWriteTmpfs != nil {
+		options = append(options, libpod.WithReadWriteTmpfs(*s.ReadWriteTmpfs))
+	}
 
 	// Get namespace related options
 	namespaceOpts, err := namespaceOptions(s, rt, pod, imageData)
@@ -588,7 +594,11 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 		options = append(options, libpod.WithShmSizeSystemd(*s.ShmSizeSystemd))
 	}
 	if s.Rootfs != "" {
-		options = append(options, libpod.WithRootFS(s.Rootfs, s.RootfsOverlay, s.RootfsMapping))
+		rootfsOverlay := false
+		if s.RootfsOverlay != nil {
+			rootfsOverlay = *s.RootfsOverlay
+		}
+		options = append(options, libpod.WithRootFS(s.Rootfs, rootfsOverlay, s.RootfsMapping))
 	}
 	// Default used if not overridden on command line
 

pkg/specgen/generate/kube/kube.go

@@ -182,6 +182,8 @@ type CtrSpecGenOptions struct {
 }
 
 func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGenerator, error) {
+	localTrue := true
+
 	s := specgen.NewSpecGenerator(opts.Container.Image, false)
 
 	rtc, err := config.Default()
@@ -211,7 +213,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 
 	s.Name = fmt.Sprintf("%s-%s", opts.PodName, opts.Container.Name)
 
-	s.Terminal = opts.Container.TTY
+	s.Terminal = &opts.Container.TTY
 
 	s.Pod = opts.PodID
 
@@ -390,7 +392,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 
 	if label, ok := opts.Annotations[define.InspectAnnotationLabel+"/"+opts.Container.Name]; ok {
 		if label == "nested" {
-			s.ContainerSecurityConfig.LabelNested = true
+			s.ContainerSecurityConfig.LabelNested = &localTrue
 		}
 		if !slices.Contains(s.ContainerSecurityConfig.SelinuxOpts, label) {
 			s.ContainerSecurityConfig.SelinuxOpts = append(s.ContainerSecurityConfig.SelinuxOpts, label)
@@ -403,7 +405,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 		if err != nil {
 			return nil, err
 		}
-		s.Remove = autoremoveAsBool
+		s.Remove = &autoremoveAsBool
 		s.Annotations[define.InspectAnnotationAutoremove] = autoremove
 	}
 
@@ -413,7 +415,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 			return nil, err
 		}
 
-		s.Init = initAsBool
+		s.Init = &initAsBool
 		s.Annotations[define.InspectAnnotationInit] = init
 	}
 
@@ -423,7 +425,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 			if err != nil {
 				return nil, err
 			}
-			s.PublishExposedPorts = publishAllAsBool
+			s.PublishExposedPorts = &publishAllAsBool
 		}
 
 		s.Annotations[define.InspectAnnotationPublishAll] = publishAll
@@ -589,10 +591,12 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 	}
 
 	if ro := opts.ReadOnly; ro != itypes.OptionalBoolUndefined {
-		s.ReadOnlyFilesystem = ro == itypes.OptionalBoolTrue
+		localRO := ro == itypes.OptionalBoolTrue
+		s.ReadOnlyFilesystem = &localRO
 	}
 	// This should default to true for kubernetes yaml
-	s.ReadWriteTmpfs = true
+
+	s.ReadWriteTmpfs = &localTrue
 
 	// Make sure the container runs in a systemd unit which is
 	// stored as a label at container creation.
@@ -819,14 +823,15 @@ func setupSecurityContext(s *specgen.SpecGenerator, securityContext *v1.Security
 	}
 
 	if securityContext.ReadOnlyRootFilesystem != nil {
-		s.ReadOnlyFilesystem = *securityContext.ReadOnlyRootFilesystem
+		s.ReadOnlyFilesystem = securityContext.ReadOnlyRootFilesystem
 	}
 	if securityContext.Privileged != nil {
-		s.Privileged = *securityContext.Privileged
+		s.Privileged = securityContext.Privileged
 	}
 
 	if securityContext.AllowPrivilegeEscalation != nil {
-		s.NoNewPrivileges = !*securityContext.AllowPrivilegeEscalation
+		localNNP := !*securityContext.AllowPrivilegeEscalation
+		s.NoNewPrivileges = &localNNP
 	}
 
 	if securityContext.ProcMount != nil && *securityContext.ProcMount == v1.UnmaskedProcMount {

pkg/specgen/generate/namespaces.go

@@ -367,15 +367,15 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
 		toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", s.Networks))
 	}
 
-	if s.UseImageHosts {
+	if s.UseImageHosts != nil && *s.UseImageHosts {
 		toReturn = append(toReturn, libpod.WithUseImageHosts())
 	} else if len(s.HostAdd) > 0 {
 		toReturn = append(toReturn, libpod.WithHosts(s.HostAdd))
 	}
 	if len(s.DNSSearch) > 0 {
 		toReturn = append(toReturn, libpod.WithDNSSearch(s.DNSSearch))
 	}
-	if s.UseImageResolvConf {
+	if s.UseImageResolvConf != nil && *s.UseImageResolvConf {
 		toReturn = append(toReturn, libpod.WithUseImageResolvConf())
 	} else if len(s.DNSServers) > 0 {
 		var dnsServers []string

pkg/specgen/generate/namespaces_linux.go

@@ -152,7 +152,7 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
 	if g.Config.Annotations == nil {
 		g.Config.Annotations = make(map[string]string)
 	}
-	if s.PublishExposedPorts {
+	if s.PublishExposedPorts != nil && *s.PublishExposedPorts {
 		g.Config.Annotations[define.InspectAnnotationPublishAll] = define.InspectResponseTrue
 	}
 

pkg/specgen/generate/oci.go

@@ -50,7 +50,7 @@ func makeCommand(s *specgen.SpecGenerator, imageData *libimage.ImageData) []stri
 		finalCommand = []string{""}
 	}
 
-	if s.Init {
+	if s.Init != nil && *s.Init {
 		// bind mount for this binary is added in addContainerInitBinary()
 		finalCommand = append([]string{define.ContainerInitPath, "--"}, finalCommand...)
 	}