Merge pull request #5862 from rhatdan/v2

Fix up handling of image data as well as setting Linux Capabilties correctly

8 files changed, 197 insertions, 178 deletions

diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go
index b8526993c..7550bf784 100644
--- a/cmd/podman/common/specgen.go
+++ b/cmd/podman/common/specgen.go
@@ -424,6 +424,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 		}
 	}
 
+	s.SeccompPolicy = c.SeccompPolicy
 	// TODO any idea why this was done
 	// storage.go from spec/
 	// grab it
diff --git a/cmd/podman/images/inspect.go b/cmd/podman/images/inspect.go
index 4482ceee5..11bef02ba 100644
--- a/cmd/podman/images/inspect.go
+++ b/cmd/podman/images/inspect.go
@@ -20,11 +20,13 @@ import (
 var (
 	// Command: podman image _inspect_
 	inspectCmd = &cobra.Command{
-		Use:     "inspect [flags] IMAGE",
-		Short:   "Display the configuration of an image",
-		Long:    `Displays the low-level information on an image identified by name or ID.`,
-		RunE:    inspect,
-		Example: `podman image inspect alpine`,
+		Use:   "inspect [flags] IMAGE",
+		Short: "Display the configuration of an image",
+		Long:  `Displays the low-level information on an image identified by name or ID.`,
+		RunE:  inspect,
+		Example: `podman inspect alpine
+  podman inspect --format "imageId: {{.Id}} size: {{.Size}}" alpine
+  podman inspect --format "image: {{.ImageName}} driver: {{.Driver}}" myctr`,
 	}
 	inspectOpts *entities.InspectOptions
 )
@@ -39,14 +41,14 @@ func init() {
 }
 
 func inspect(cmd *cobra.Command, args []string) error {
-	latestContainer := inspectOpts.Latest
-
-	if len(args) == 0 && !latestContainer {
-		return errors.Errorf("container or image name must be specified: podman inspect [options [...]] name")
+	if inspectOpts.Size {
+		return fmt.Errorf("--size can only be used for containers")
 	}
-
-	if len(args) > 0 && latestContainer {
-		return errors.Errorf("you cannot provide additional arguments with --latest")
+	if inspectOpts.Latest {
+		return fmt.Errorf("--latest can only be used for containers")
+	}
+	if len(args) == 0 {
+		return errors.Errorf("image name must be specified: podman image inspect [options [...]] name")
 	}
 
 	results, err := registry.ImageEngine().Inspect(context.Background(), args, *inspectOpts)
diff --git a/cmd/podman/inspect.go b/cmd/podman/inspect.go
index e67bc326b..93bf58bdd 100644
--- a/cmd/podman/inspect.go
+++ b/cmd/podman/inspect.go
@@ -1,10 +1,8 @@
 package main
 
 import (
-	"context"
 	"fmt"
 
-	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/libpod/cmd/podman/common"
 	"github.com/containers/libpod/cmd/podman/containers"
 	"github.com/containers/libpod/cmd/podman/images"
@@ -21,11 +19,12 @@ var (
 	// Command: podman _inspect_ Object_ID
 	inspectCmd = &cobra.Command{
 		Use:              "inspect [flags] {CONTAINER_ID | IMAGE_ID}",
-		Args:             cobra.ExactArgs(1),
 		Short:            "Display the configuration of object denoted by ID",
 		Long:             "Displays the low-level information on an object identified by name or ID",
 		TraverseChildren: true,
 		RunE:             inspect,
+		Example: `podman inspect alpine
+  podman inspect --format "imageId: {{.Id}} size: {{.Size}}" alpine`,
 	}
 )
 
@@ -35,21 +34,25 @@ func init() {
 		Command: inspectCmd,
 	})
 	inspectOpts = common.AddInspectFlagSet(inspectCmd)
+	flags := inspectCmd.Flags()
+	flags.StringVarP(&inspectOpts.Type, "type", "t", "", "Return JSON for specified type, (image or container) (default \"all\")")
+	if !registry.IsRemote() {
+		flags.BoolVarP(&inspectOpts.Latest, "latest", "l", false, "Act on the latest container podman is aware of (containers only)")
+	}
 }
 
 func inspect(cmd *cobra.Command, args []string) error {
-	// First check if the input is even valid for an image
-	if _, err := reference.Parse(args[0]); err == nil {
-		if found, err := registry.ImageEngine().Exists(context.Background(), args[0]); err != nil {
-			return err
-		} else if found.Value {
-			return images.Inspect(cmd, args, inspectOpts)
+	switch inspectOpts.Type {
+	case "image":
+		return images.Inspect(cmd, args, inspectOpts)
+	case "container":
+		return containers.Inspect(cmd, args, inspectOpts)
+	case "":
+		if err := images.Inspect(cmd, args, inspectOpts); err == nil {
+			return nil
 		}
-	}
-	if found, err := registry.ContainerEngine().ContainerExists(context.Background(), args[0]); err != nil {
-		return err
-	} else if found.Value {
 		return containers.Inspect(cmd, args, inspectOpts)
+	default:
+		return fmt.Errorf("invalid type %q is must be 'container' or 'image'", inspectOpts.Type)
 	}
-	return fmt.Errorf("%s not found on system", args[0])
 }
diff --git a/libpod/image/image.go b/libpod/image/image.go
index 7198a42a3..bbf803056 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -867,22 +867,77 @@ func (i *Image) Intermediate(ctx context.Context) (bool, error) {
 	return false, nil
 }
 
+// User returns the image's user
+func (i *Image) User(ctx context.Context) (string, error) {
+	imgInspect, err := i.inspect(ctx, false)
+	if err != nil {
+		return "", err
+	}
+	return imgInspect.Config.User, nil
+}
+
+// StopSignal returns the image's StopSignal
+func (i *Image) StopSignal(ctx context.Context) (string, error) {
+	imgInspect, err := i.inspect(ctx, false)
+	if err != nil {
+		return "", err
+	}
+	return imgInspect.Config.StopSignal, nil
+}
+
+// WorkingDir returns the image's WorkingDir
+func (i *Image) WorkingDir(ctx context.Context) (string, error) {
+	imgInspect, err := i.inspect(ctx, false)
+	if err != nil {
+		return "", err
+	}
+	return imgInspect.Config.WorkingDir, nil
+}
+
+// Cmd returns the image's cmd
+func (i *Image) Cmd(ctx context.Context) ([]string, error) {
+	imgInspect, err := i.inspect(ctx, false)
+	if err != nil {
+		return nil, err
+	}
+	return imgInspect.Config.Cmd, nil
+}
+
+// Entrypoint returns the image's entrypoint
+func (i *Image) Entrypoint(ctx context.Context) ([]string, error) {
+	imgInspect, err := i.inspect(ctx, false)
+	if err != nil {
+		return nil, err
+	}
+	return imgInspect.Config.Entrypoint, nil
+}
+
+// Env returns the image's env
+func (i *Image) Env(ctx context.Context) ([]string, error) {
+	imgInspect, err := i.imageInspectInfo(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return imgInspect.Env, nil
+}
+
 // Labels returns the image's labels
 func (i *Image) Labels(ctx context.Context) (map[string]string, error) {
 	imgInspect, err := i.imageInspectInfo(ctx)
 	if err != nil {
-		return nil, nil
+		return nil, err
 	}
 	return imgInspect.Labels, nil
 }
 
 // GetLabel Returns a case-insensitive match of a given label
 func (i *Image) GetLabel(ctx context.Context, label string) (string, error) {
-	imageLabels, err := i.Labels(ctx)
+	labels, err := i.Labels(ctx)
 	if err != nil {
 		return "", err
 	}
-	for k, v := range imageLabels {
+
+	for k, v := range labels {
 		if strings.ToLower(k) == strings.ToLower(label) {
 			return v, nil
 		}
diff --git a/pkg/domain/entities/types.go b/pkg/domain/entities/types.go
index b89aa869a..096af2df2 100644
--- a/pkg/domain/entities/types.go
+++ b/pkg/domain/entities/types.go
@@ -50,6 +50,7 @@ type InspectOptions struct {
 	Format string `json:",omitempty"`
 	Latest bool   `json:",omitempty"`
 	Size   bool   `json:",omitempty"`
+	Type   string `json:",omitempty"`
 }
 
 // All API and CLI diff commands and diff sub-commands use the same options
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
index 7233acb8a..31465d8bf 100644
--- a/pkg/specgen/generate/container.go
+++ b/pkg/specgen/generate/container.go
@@ -20,17 +20,27 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	}
 
 	// Image stop signal
-	if s.StopSignal == nil && newImage.Config != nil {
-		sig, err := signal.ParseSignalNameOrNumber(newImage.Config.StopSignal)
+	if s.StopSignal == nil {
+		stopSignal, err := newImage.StopSignal(ctx)
+		if err != nil {
+			return err
+		}
+		sig, err := signal.ParseSignalNameOrNumber(stopSignal)
 		if err != nil {
 			return err
 		}
 		s.StopSignal = &sig
 	}
+
 	// Image envs from the image if they don't exist
 	// already
-	if newImage.Config != nil && len(newImage.Config.Env) > 0 {
-		envs, err := envLib.ParseSlice(newImage.Config.Env)
+	env, err := newImage.Env(ctx)
+	if err != nil {
+		return err
+	}
+
+	if len(env) > 0 {
+		envs, err := envLib.ParseSlice(env)
 		if err != nil {
 			return err
 		}
@@ -41,12 +51,15 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		}
 	}
 
+	labels, err := newImage.Labels(ctx)
+	if err != nil {
+		return err
+	}
+
 	// labels from the image that dont exist already
-	if config := newImage.Config; config != nil {
-		for k, v := range config.Labels {
-			if _, exists := s.Labels[k]; !exists {
-				s.Labels[k] = v
-			}
+	for k, v := range labels {
+		if _, exists := s.Labels[k]; !exists {
+			s.Labels[k] = v
 		}
 	}
 
@@ -75,20 +88,30 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	}
 
 	// entrypoint
-	if config := newImage.Config; config != nil {
-		if len(s.Entrypoint) < 1 && len(config.Entrypoint) > 0 {
-			s.Entrypoint = config.Entrypoint
-		}
-		if len(s.Command) < 1 && len(config.Cmd) > 0 {
-			s.Command = config.Cmd
-		}
-		if len(s.Command) < 1 && len(s.Entrypoint) < 1 {
-			return errors.Errorf("No command provided or as CMD or ENTRYPOINT in this image")
-		}
-		// workdir
-		if len(s.WorkDir) < 1 && len(config.WorkingDir) > 1 {
-			s.WorkDir = config.WorkingDir
-		}
+	entrypoint, err := newImage.Entrypoint(ctx)
+	if err != nil {
+		return err
+	}
+	if len(s.Entrypoint) < 1 && len(entrypoint) > 0 {
+		s.Entrypoint = entrypoint
+	}
+	command, err := newImage.Cmd(ctx)
+	if err != nil {
+		return err
+	}
+	if len(s.Command) < 1 && len(command) > 0 {
+		s.Command = command
+	}
+	if len(s.Command) < 1 && len(s.Entrypoint) < 1 {
+		return errors.Errorf("No command provided or as CMD or ENTRYPOINT in this image")
+	}
+	// workdir
+	workingDir, err := newImage.WorkingDir(ctx)
+	if err != nil {
+		return err
+	}
+	if len(s.WorkDir) < 1 && len(workingDir) > 1 {
+		s.WorkDir = workingDir
 	}
 
 	if len(s.SeccompProfilePath) < 1 {
@@ -99,15 +122,17 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		s.SeccompProfilePath = p
 	}
 
-	if user := s.User; len(user) == 0 {
-		switch {
+	if len(s.User) == 0 {
+		s.User, err = newImage.User(ctx)
+		if err != nil {
+			return err
+		}
+
 		// TODO This should be enabled when namespaces actually work
 		//case usernsMode.IsKeepID():
 		//	user = fmt.Sprintf("%d:%d", rootless.GetRootlessUID(), rootless.GetRootlessGID())
-		case newImage.Config == nil || (newImage.Config != nil && len(newImage.Config.User) == 0):
+		if len(s.User) == 0 {
 			s.User = "0"
-		default:
-			s.User = newImage.Config.User
 		}
 	}
 	if err := finishThrottleDevices(s); err != nil {
@@ -116,7 +141,7 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	// Unless already set via the CLI, check if we need to disable process
 	// labels or set the defaults.
 	if len(s.SelinuxOpts) == 0 {
-		if err := SetLabelOpts(s, r, s.PidNS, s.IpcNS); err != nil {
+		if err := setLabelOpts(s, r, s.PidNS, s.IpcNS); err != nil {
 			return err
 		}
 	}
diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go
index cdd7d86da..53ae335c3 100644
--- a/pkg/specgen/generate/namespaces.go
+++ b/pkg/specgen/generate/namespaces.go
@@ -3,9 +3,7 @@ package generate
 import (
 	"os"
 
-	"github.com/containers/common/pkg/capabilities"
 	"github.com/containers/libpod/libpod"
-	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/specgen"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
@@ -324,66 +322,6 @@ func userConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator) err
 	return nil
 }
 
-func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image) error {
-	// HANDLE CAPABILITIES
-	// NOTE: Must happen before SECCOMP
-	if s.Privileged {
-		g.SetupPrivileged(true)
-	}
-
-	useNotRoot := func(user string) bool {
-		if user == "" || user == "root" || user == "0" {
-			return false
-		}
-		return true
-	}
-	configSpec := g.Config
-	var err error
-	var caplist []string
-	bounding := configSpec.Process.Capabilities.Bounding
-	if useNotRoot(s.User) {
-		configSpec.Process.Capabilities.Bounding = caplist
-	}
-	caplist, err = capabilities.MergeCapabilities(configSpec.Process.Capabilities.Bounding, s.CapAdd, s.CapDrop)
-	if err != nil {
-		return err
-	}
-
-	configSpec.Process.Capabilities.Bounding = caplist
-	configSpec.Process.Capabilities.Permitted = caplist
-	configSpec.Process.Capabilities.Inheritable = caplist
-	configSpec.Process.Capabilities.Effective = caplist
-	configSpec.Process.Capabilities.Ambient = caplist
-	if useNotRoot(s.User) {
-		caplist, err = capabilities.MergeCapabilities(bounding, s.CapAdd, s.CapDrop)
-		if err != nil {
-			return err
-		}
-	}
-	configSpec.Process.Capabilities.Bounding = caplist
-
-	// HANDLE SECCOMP
-	if s.SeccompProfilePath != "unconfined" {
-		seccompConfig, err := getSeccompConfig(s, configSpec, newImage)
-		if err != nil {
-			return err
-		}
-		configSpec.Linux.Seccomp = seccompConfig
-	}
-
-	// Clear default Seccomp profile from Generator for privileged containers
-	if s.SeccompProfilePath == "unconfined" || s.Privileged {
-		configSpec.Linux.Seccomp = nil
-	}
-
-	g.SetRootReadonly(s.ReadOnlyFilesystem)
-	for sysctlKey, sysctlVal := range s.Sysctl {
-		g.AddLinuxSysctl(sysctlKey, sysctlVal)
-	}
-
-	return nil
-}
-
 // GetNamespaceOptions transforms a slice of kernel namespaces
 // into a slice of pod create options. Currently, not all
 // kernel namespaces are supported, and they will be returned in an error
diff --git a/pkg/specgen/generate/security.go b/pkg/specgen/generate/security.go
index ef4b3b47a..e2da9e976 100644
--- a/pkg/specgen/generate/security.go
+++ b/pkg/specgen/generate/security.go
@@ -1,15 +1,22 @@
 package generate
 
 import (
+	"strings"
+
+	"github.com/containers/common/pkg/capabilities"
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/specgen"
+	"github.com/containers/libpod/pkg/util"
+	"github.com/opencontainers/runtime-tools/generate"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
-// SetLabelOpts sets the label options of the SecurityConfig according to the
+// setLabelOpts sets the label options of the SecurityConfig according to the
 // input.
-func SetLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig specgen.Namespace, ipcConfig specgen.Namespace) error {
+func setLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig specgen.Namespace, ipcConfig specgen.Namespace) error {
 	if !runtime.EnableLabeling() || s.Privileged {
 		s.SelinuxOpts = label.DisableSecOpt()
 		return nil
@@ -48,12 +55,10 @@ func SetLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig s
 	return nil
 }
 
-// ConfigureGenerator configures the generator according to the input.
-/*
-func (c *SecurityConfig) ConfigureGenerator(g *generate.Generator, user *UserConfig) error {
+func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image) error {
 	// HANDLE CAPABILITIES
 	// NOTE: Must happen before SECCOMP
-	if c.Privileged {
+	if s.Privileged {
 		g.SetupPrivileged(true)
 	}
 
@@ -63,56 +68,66 @@ func (c *SecurityConfig) ConfigureGenerator(g *generate.Generator, user *UserCon
 		}
 		return true
 	}
-
 	configSpec := g.Config
 	var err error
-	var defaultCaplist []string
+	var caplist []string
 	bounding := configSpec.Process.Capabilities.Bounding
-	if useNotRoot(user.User) {
-		configSpec.Process.Capabilities.Bounding = defaultCaplist
+	if useNotRoot(s.User) {
+		configSpec.Process.Capabilities.Bounding = caplist
 	}
-	defaultCaplist, err = capabilities.MergeCapabilities(configSpec.Process.Capabilities.Bounding, c.CapAdd, c.CapDrop)
+	caplist, err = capabilities.MergeCapabilities(configSpec.Process.Capabilities.Bounding, s.CapAdd, s.CapDrop)
 	if err != nil {
 		return err
 	}
+	privCapsRequired := []string{}
+
+	// If the container image specifies an label with a
+	// capabilities.ContainerImageLabel then split the comma separated list
+	// of capabilities and record them.  This list indicates the only
+	// capabilities, required to run the container.
+	var capsRequiredRequested []string
+	for key, val := range s.Labels {
+		if util.StringInSlice(key, capabilities.ContainerImageLabels) {
+			capsRequiredRequested = strings.Split(val, ",")
+		}
+	}
+	if !s.Privileged && len(capsRequiredRequested) > 0 {
 
-	privCapRequired := []string{}
-
-	if !c.Privileged && len(c.CapRequired) > 0 {
-		// Pass CapRequired in CapAdd field to normalize capabilities names
-		capRequired, err := capabilities.MergeCapabilities(nil, c.CapRequired, nil)
+		// Pass capRequiredRequested in CapAdd field to normalize capabilities names
+		capsRequired, err := capabilities.MergeCapabilities(nil, capsRequiredRequested, nil)
 		if err != nil {
-			logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(c.CapRequired, ","))
+			logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(capsRequired, ","))
 		} else {
-			// Verify all capRequiered are in the defaultCapList
-			for _, cap := range capRequired {
-				if !util.StringInSlice(cap, defaultCaplist) {
-					privCapRequired = append(privCapRequired, cap)
+			// Verify all capRequiered are in the capList
+			for _, cap := range capsRequired {
+				if !util.StringInSlice(cap, caplist) {
+					privCapsRequired = append(privCapsRequired, cap)
 				}
 			}
 		}
-		if len(privCapRequired) == 0 {
-			defaultCaplist = capRequired
+		if len(privCapsRequired) == 0 {
+			caplist = capsRequired
 		} else {
-			logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapRequired, ","))
+			logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapsRequired, ","))
 		}
 	}
-	configSpec.Process.Capabilities.Bounding = defaultCaplist
-	configSpec.Process.Capabilities.Permitted = defaultCaplist
-	configSpec.Process.Capabilities.Inheritable = defaultCaplist
-	configSpec.Process.Capabilities.Effective = defaultCaplist
-	configSpec.Process.Capabilities.Ambient = defaultCaplist
-	if useNotRoot(user.User) {
-		defaultCaplist, err = capabilities.MergeCapabilities(bounding, c.CapAdd, c.CapDrop)
+
+	configSpec.Process.Capabilities.Bounding = caplist
+	configSpec.Process.Capabilities.Permitted = caplist
+	configSpec.Process.Capabilities.Inheritable = caplist
+	configSpec.Process.Capabilities.Effective = caplist
+	configSpec.Process.Capabilities.Ambient = caplist
+	if useNotRoot(s.User) {
+		caplist, err = capabilities.MergeCapabilities(bounding, s.CapAdd, s.CapDrop)
 		if err != nil {
 			return err
 		}
 	}
-	configSpec.Process.Capabilities.Bounding = defaultCaplist
+	configSpec.Process.Capabilities.Bounding = caplist
 
 	// HANDLE SECCOMP
-	if c.SeccompProfilePath != "unconfined" {
-		seccompConfig, err := getSeccompConfig(c, configSpec)
+	if s.SeccompProfilePath != "unconfined" {
+		seccompConfig, err := getSeccompConfig(s, configSpec, newImage)
 		if err != nil {
 			return err
 		}
@@ -120,35 +135,14 @@ func (c *SecurityConfig) ConfigureGenerator(g *generate.Generator, user *UserCon
 	}
 
 	// Clear default Seccomp profile from Generator for privileged containers
-	if c.SeccompProfilePath == "unconfined" || c.Privileged {
+	if s.SeccompProfilePath == "unconfined" || s.Privileged {
 		configSpec.Linux.Seccomp = nil
 	}
 
-	for _, opt := range c.SecurityOpts {
-		// Split on both : and =
-		splitOpt := strings.Split(opt, "=")
-		if len(splitOpt) == 1 {
-			splitOpt = strings.Split(opt, ":")
-		}
-		if len(splitOpt) < 2 {
-			continue
-		}
-		switch splitOpt[0] {
-		case "label":
-			configSpec.Annotations[libpod.InspectAnnotationLabel] = splitOpt[1]
-		case "seccomp":
-			configSpec.Annotations[libpod.InspectAnnotationSeccomp] = splitOpt[1]
-		case "apparmor":
-			configSpec.Annotations[libpod.InspectAnnotationApparmor] = splitOpt[1]
-		}
-	}
-
-	g.SetRootReadonly(c.ReadOnlyRootfs)
-	for sysctlKey, sysctlVal := range c.Sysctl {
+	g.SetRootReadonly(s.ReadOnlyFilesystem)
+	for sysctlKey, sysctlVal := range s.Sysctl {
 		g.AddLinuxSysctl(sysctlKey, sysctlVal)
 	}
 
 	return nil
 }
-
-*/