diff options
27 files changed, 345 insertions, 220 deletions
diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index eca0da32b..96aeb08f4 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -525,8 +525,10 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string s.ContainerSecurityConfig.SelinuxOpts = append(s.ContainerSecurityConfig.SelinuxOpts, con[1]) s.Annotations[define.InspectAnnotationLabel] = strings.Join(s.ContainerSecurityConfig.SelinuxOpts, ",label=") case "apparmor": - s.ContainerSecurityConfig.ApparmorProfile = con[1] - s.Annotations[define.InspectAnnotationApparmor] = con[1] + if !c.Privileged { + s.ContainerSecurityConfig.ApparmorProfile = con[1] + s.Annotations[define.InspectAnnotationApparmor] = con[1] + } case "seccomp": s.SeccompProfilePath = con[1] s.Annotations[define.InspectAnnotationSeccomp] = con[1] diff --git a/cmd/podman/parse/json.go b/cmd/podman/parse/json.go new file mode 100644 index 000000000..95a6633b8 --- /dev/null +++ b/cmd/podman/parse/json.go @@ -0,0 +1,9 @@ +package parse + +import "regexp" + +var jsonFormatRegex = regexp.MustCompile(`^(\s*json\s*|\s*{{\s*json\s*\.\s*}}\s*)$`) + +func MatchesJSONFormat(s string) bool { + return jsonFormatRegex.Match([]byte(s)) +} diff --git a/cmd/podman/parse/json_test.go b/cmd/podman/parse/json_test.go new file mode 100644 index 000000000..5cad185fd --- /dev/null +++ b/cmd/podman/parse/json_test.go @@ -0,0 +1,30 @@ +package parse + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMatchesJSONFormat(t *testing.T) { + tests := []struct { + input string + expected bool + }{ + {"json", true}, + {" json", true}, + {"json ", true}, + {" json ", true}, + {"{{json .}}", true}, + {"{{ json .}}", true}, + {"{{json . }}", true}, + {" {{ json . }} ", true}, + {"{{json }}", false}, + {"{{json .", false}, + {"json . }}", false}, + } + + for _, tt := range tests { + assert.Equal(t, tt.expected, MatchesJSONFormat(tt.input)) + } +} diff --git a/cmd/podman/system/info.go b/cmd/podman/system/info.go index 699f7b55c..410b3455a 100644 --- a/cmd/podman/system/info.go +++ b/cmd/podman/system/info.go @@ -5,6 +5,7 @@ import ( "os" "text/template" + "github.com/containers/libpod/v2/cmd/podman/parse" "github.com/containers/libpod/v2/cmd/podman/registry" "github.com/containers/libpod/v2/cmd/podman/validate" "github.com/containers/libpod/v2/pkg/domain/entities" @@ -68,7 +69,7 @@ func info(cmd *cobra.Command, args []string) error { return err } - if inFormat == "json" { + if parse.MatchesJSONFormat(inFormat) { b, err := json.MarshalIndent(info, "", " ") if err != nil { return err diff --git a/cmd/podman/system/version.go b/cmd/podman/system/version.go index 5aac34699..9b70bc9f4 100644 --- a/cmd/podman/system/version.go +++ b/cmd/podman/system/version.go @@ -8,6 +8,7 @@ import ( "text/tabwriter" "github.com/containers/buildah/pkg/formats" + "github.com/containers/libpod/v2/cmd/podman/parse" "github.com/containers/libpod/v2/cmd/podman/registry" "github.com/containers/libpod/v2/cmd/podman/validate" "github.com/containers/libpod/v2/libpod/define" @@ -41,7 +42,7 @@ func version(cmd *cobra.Command, args []string) error { } switch { - case versionFormat == "json", versionFormat == "{{ json .}}": + case parse.MatchesJSONFormat(versionFormat): s, err := json.MarshalToString(versions) if err != nil { return err diff --git a/libpod/container_api.go b/libpod/container_api.go index c7df9d66c..487f75e67 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -29,7 +29,7 @@ import ( // Init requires that all dependency containers be started (e.g. pod infra // containers). The `recursive` parameter will, if set to true, start these // dependency containers before initializing this container. -func (c *Container) Init(ctx context.Context, recursive bool) (err error) { +func (c *Container) Init(ctx context.Context, recursive bool) error { span, _ := opentracing.StartSpanFromContext(ctx, "containerInit") span.SetTag("struct", "container") defer span.Finish() @@ -85,7 +85,7 @@ func (c *Container) Init(ctx context.Context, recursive bool) (err error) { // Start requites that all dependency containers (e.g. pod infra containers) be // running before being run. The recursive parameter, if set, will start all // dependencies before starting this container. -func (c *Container) Start(ctx context.Context, recursive bool) (err error) { +func (c *Container) Start(ctx context.Context, recursive bool) error { span, _ := opentracing.StartSpanFromContext(ctx, "containerStart") span.SetTag("struct", "container") defer span.Finish() @@ -112,7 +112,7 @@ func (c *Container) Start(ctx context.Context, recursive bool) (err error) { // Attach call occurs before Start). // In overall functionality, it is identical to the Start call, with the added // side effect that an attach session will also be started. -func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, recursive bool) (attachResChan <-chan error, err error) { +func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, recursive bool) (<-chan error, error) { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -150,7 +150,7 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachSt } // RestartWithTimeout restarts a running container and takes a given timeout in uint -func (c *Container) RestartWithTimeout(ctx context.Context, timeout uint) (err error) { +func (c *Container) RestartWithTimeout(ctx context.Context, timeout uint) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -160,7 +160,7 @@ func (c *Container) RestartWithTimeout(ctx context.Context, timeout uint) (err e } } - if err = c.checkDependenciesAndHandleError(); err != nil { + if err := c.checkDependenciesAndHandleError(); err != nil { return err } @@ -760,7 +760,7 @@ func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointO } // Restore restores a container -func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOptions) (err error) { +func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOptions) error { logrus.Debugf("Trying to restore container %s", c.ID()) if !c.batched { c.lock.Lock() diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 69da6fcfe..bd04ee9b9 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -779,25 +779,25 @@ func (c *Container) execOCILog(sessionID string) string { } // create a bundle path and associated files for an exec session -func (c *Container) createExecBundle(sessionID string) (err error) { +func (c *Container) createExecBundle(sessionID string) (retErr error) { bundlePath := c.execBundlePath(sessionID) - if createErr := os.MkdirAll(bundlePath, execDirPermission); createErr != nil { - return createErr + if err := os.MkdirAll(bundlePath, execDirPermission); err != nil { + return err } defer func() { - if err != nil { - if err2 := os.RemoveAll(bundlePath); err != nil { - logrus.Warnf("error removing exec bundle after creation caused another error: %v", err2) + if retErr != nil { + if err := os.RemoveAll(bundlePath); err != nil { + logrus.Warnf("error removing exec bundle after creation caused another error: %v", err) } } }() - if err2 := os.MkdirAll(c.execExitFileDir(sessionID), execDirPermission); err2 != nil { + if err := os.MkdirAll(c.execExitFileDir(sessionID), execDirPermission); err != nil { // The directory is allowed to exist - if !os.IsExist(err2) { - err = errors.Wrapf(err2, "error creating OCI runtime exit file path %s", c.execExitFileDir(sessionID)) + if !os.IsExist(err) { + return errors.Wrapf(err, "error creating OCI runtime exit file path %s", c.execExitFileDir(sessionID)) } } - return + return nil } // readExecExitCode reads the exit file for an exec session and returns diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 12132cf41..e98e20b9b 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -210,7 +210,7 @@ func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error { // Handle container restart policy. // This is called when a container has exited, and was not explicitly stopped by // an API call to stop the container or pod it is in. -func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, err error) { +func (c *Container) handleRestartPolicy(ctx context.Context) (_ bool, retErr error) { // If we did not get a restart policy match, exit immediately. // Do the same if we're not a policy that restarts. if !c.state.RestartPolicyMatch || @@ -241,7 +241,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er logrus.Debugf("Restarting container %s due to restart policy %s", c.ID(), c.config.RestartPolicy) // Need to check if dependencies are alive. - if err = c.checkDependenciesAndHandleError(); err != nil { + if err := c.checkDependenciesAndHandleError(); err != nil { return false, err } @@ -263,9 +263,9 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er } defer func() { - if err != nil { - if err2 := c.cleanup(ctx); err2 != nil { - logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) + if retErr != nil { + if err := c.cleanup(ctx); err != nil { + logrus.Errorf("error cleaning up container %s: %v", c.ID(), err) } } }() @@ -768,7 +768,7 @@ func (c *Container) save() error { // Checks the container is in the right state, then initializes the container in preparation to start the container. // If recursive is true, each of the containers dependencies will be started. // Otherwise, this function will return with error if there are dependencies of this container that aren't running. -func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err error) { +func (c *Container) prepareToStart(ctx context.Context, recursive bool) (retErr error) { // Container must be created or stopped to be started if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID()) @@ -785,9 +785,9 @@ func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err err } defer func() { - if err != nil { - if err2 := c.cleanup(ctx); err2 != nil { - logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) + if retErr != nil { + if err := c.cleanup(ctx); err != nil { + logrus.Errorf("error cleaning up container %s: %v", c.ID(), err) } } }() @@ -1133,7 +1133,7 @@ func (c *Container) reinit(ctx context.Context, retainRetries bool) error { // Initialize (if necessary) and start a container // Performs all necessary steps to start a container that is not running // Does not lock or check validity -func (c *Container) initAndStart(ctx context.Context) (err error) { +func (c *Container) initAndStart(ctx context.Context) (retErr error) { // If we are ContainerStateUnknown, throw an error if c.state.State == define.ContainerStateUnknown { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is in an unknown state", c.ID()) @@ -1151,9 +1151,9 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { } defer func() { - if err != nil { - if err2 := c.cleanup(ctx); err2 != nil { - logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) + if retErr != nil { + if err := c.cleanup(ctx); err != nil { + logrus.Errorf("error cleaning up container %s: %v", c.ID(), err) } } }() @@ -1335,7 +1335,7 @@ func (c *Container) unpause() error { } // Internal, non-locking function to restart a container -func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err error) { +func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (retErr error) { if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "unable to restart a container in a paused or unknown state") } @@ -1372,9 +1372,9 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e } } defer func() { - if err != nil { - if err2 := c.cleanup(ctx); err2 != nil { - logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) + if retErr != nil { + if err := c.cleanup(ctx); err != nil { + logrus.Errorf("error cleaning up container %s: %v", c.ID(), err) } } }() @@ -1696,7 +1696,7 @@ func (c *Container) cleanup(ctx context.Context) error { // delete deletes the container and runs any configured poststop // hooks. -func (c *Container) delete(ctx context.Context) (err error) { +func (c *Container) delete(ctx context.Context) error { span, _ := opentracing.StartSpanFromContext(ctx, "delete") span.SetTag("struct", "container") defer span.Finish() @@ -1715,7 +1715,7 @@ func (c *Container) delete(ctx context.Context) (err error) { // postDeleteHooks runs the poststop hooks (if any) as specified by // the OCI Runtime Specification (which requires them to run // post-delete, despite the stage name). -func (c *Container) postDeleteHooks(ctx context.Context) (err error) { +func (c *Container) postDeleteHooks(ctx context.Context) error { span, _ := opentracing.StartSpanFromContext(ctx, "postDeleteHooks") span.SetTag("struct", "container") defer span.Finish() @@ -1838,7 +1838,7 @@ func (c *Container) saveSpec(spec *spec.Spec) error { } // Warning: precreate hooks may alter 'config' in place. -func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (extensionStageHooks map[string][]spec.Hook, err error) { +func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[string][]spec.Hook, error) { allHooks := make(map[string][]spec.Hook) if c.runtime.config.Engine.HooksDir == nil { if rootless.IsRootless() { @@ -1952,7 +1952,7 @@ func (c *Container) checkReadyForRemoval() error { // writeJSONFile marshalls and writes the given data to a JSON file // in the bundle path -func (c *Container) writeJSONFile(v interface{}, file string) (err error) { +func (c *Container) writeJSONFile(v interface{}, file string) error { fileJSON, err := json.MarshalIndent(v, "", " ") if err != nil { return errors.Wrapf(err, "error writing JSON to %s for container %s", file, c.ID()) @@ -1967,7 +1967,7 @@ func (c *Container) writeJSONFile(v interface{}, file string) (err error) { // prepareCheckpointExport writes the config and spec to // JSON files for later export -func (c *Container) prepareCheckpointExport() (err error) { +func (c *Container) prepareCheckpointExport() error { // save live config if err := c.writeJSONFile(c.Config(), "config.dump"); err != nil { return err diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index a3a57ae0f..b2711745e 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -612,7 +612,7 @@ func (c *Container) addNamespaceContainer(g *generate.Generator, ns LinuxNS, ctr return nil } -func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) (err error) { +func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) error { if (len(c.config.NamedVolumes) > 0) || (len(c.Dependencies()) > 0) { return errors.Errorf("Cannot export checkpoints of containers with named volumes or dependencies") } @@ -723,7 +723,7 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) (err error) return nil } -func (c *Container) checkpointRestoreSupported() (err error) { +func (c *Container) checkpointRestoreSupported() error { if !criu.CheckForCriu() { return errors.Errorf("Checkpoint/Restore requires at least CRIU %d", criu.MinCriuVersion) } @@ -733,7 +733,7 @@ func (c *Container) checkpointRestoreSupported() (err error) { return nil } -func (c *Container) checkpointRestoreLabelLog(fileName string) (err error) { +func (c *Container) checkpointRestoreLabelLog(fileName string) error { // Create the CRIU log file and label it dumpLog := filepath.Join(c.bundlePath(), fileName) @@ -750,7 +750,7 @@ func (c *Container) checkpointRestoreLabelLog(fileName string) (err error) { return nil } -func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointOptions) (err error) { +func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointOptions) error { if err := c.checkpointRestoreSupported(); err != nil { return err } @@ -820,7 +820,7 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO return c.save() } -func (c *Container) importCheckpoint(input string) (err error) { +func (c *Container) importCheckpoint(input string) error { archiveFile, err := os.Open(input) if err != nil { return errors.Wrapf(err, "Failed to open checkpoint archive %s for import", input) @@ -849,8 +849,7 @@ func (c *Container) importCheckpoint(input string) (err error) { return nil } -func (c *Container) restore(ctx context.Context, options ContainerCheckpointOptions) (err error) { - +func (c *Container) restore(ctx context.Context, options ContainerCheckpointOptions) (retErr error) { if err := c.checkpointRestoreSupported(); err != nil { return err } @@ -860,7 +859,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti } if options.TargetFile != "" { - if err = c.importCheckpoint(options.TargetFile); err != nil { + if err := c.importCheckpoint(options.TargetFile); err != nil { return err } } @@ -946,9 +945,9 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti } defer func() { - if err != nil { - if err2 := c.cleanup(ctx); err2 != nil { - logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2) + if retErr != nil { + if err := c.cleanup(ctx); err != nil { + logrus.Errorf("error cleaning up container %s: %v", c.ID(), err) } } }() diff --git a/libpod/container_internal_unsupported.go b/libpod/container_internal_unsupported.go index a42c1d735..e6d94104c 100644 --- a/libpod/container_internal_unsupported.go +++ b/libpod/container_internal_unsupported.go @@ -18,7 +18,7 @@ func (c *Container) unmountSHM(mount string) error { return define.ErrNotImplemented } -func (c *Container) prepare() (err error) { +func (c *Container) prepare() error { return define.ErrNotImplemented } diff --git a/libpod/events/events_linux.go b/libpod/events/events_linux.go index ffb100be8..482d7d6dd 100644 --- a/libpod/events/events_linux.go +++ b/libpod/events/events_linux.go @@ -8,20 +8,20 @@ import ( ) // NewEventer creates an eventer based on the eventer type -func NewEventer(options EventerOptions) (eventer Eventer, err error) { +func NewEventer(options EventerOptions) (Eventer, error) { logrus.Debugf("Initializing event backend %s", options.EventerType) switch strings.ToUpper(options.EventerType) { case strings.ToUpper(Journald.String()): - eventer, err = newEventJournalD(options) + eventer, err := newEventJournalD(options) if err != nil { return nil, errors.Wrapf(err, "eventer creation") } + return eventer, nil case strings.ToUpper(LogFile.String()): - eventer = EventLogFile{options} + return EventLogFile{options}, nil case strings.ToUpper(Null.String()): - eventer = NewNullEventer() + return NewNullEventer(), nil default: return nil, errors.Errorf("unknown event logger type: %s", strings.ToUpper(options.EventerType)) } - return eventer, nil } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 7985e17e1..5a8faa7a4 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -141,18 +141,18 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re } // Create and configure a new network namespace for a container -func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result, err error) { +func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result, retErr error) { ctrNS, err := netns.NewNS() if err != nil { return nil, nil, errors.Wrapf(err, "error creating network namespace for container %s", ctr.ID()) } defer func() { - if err != nil { - if err2 := netns.UnmountNS(ctrNS); err2 != nil { - logrus.Errorf("Error unmounting partially created network namespace for container %s: %v", ctr.ID(), err2) + if retErr != nil { + if err := netns.UnmountNS(ctrNS); err != nil { + logrus.Errorf("Error unmounting partially created network namespace for container %s: %v", ctr.ID(), err) } - if err2 := ctrNS.Close(); err2 != nil { - logrus.Errorf("Error closing partially created network namespace for container %s: %v", ctr.ID(), err2) + if err := ctrNS.Close(); err != nil { + logrus.Errorf("Error closing partially created network namespace for container %s: %v", ctr.ID(), err) } } }() @@ -188,7 +188,7 @@ func checkSlirpFlags(path string) (*slirpFeatures, error) { } // Configure the network namespace for a rootless container -func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { +func (r *Runtime) setupRootlessNetNS(ctr *Container) error { path := r.config.Engine.NetworkCmdPath if path == "" { @@ -342,7 +342,7 @@ func waitForSync(syncR *os.File, cmd *exec.Cmd, logFile io.ReadSeeker, timeout t return nil } -func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) (err error) { +func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) error { syncR, syncW, err := os.Pipe() if err != nil { return errors.Wrapf(err, "failed to open pipe") @@ -420,7 +420,7 @@ func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) (er } // Configure the network namespace using the container process -func (r *Runtime) setupNetNS(ctr *Container) (err error) { +func (r *Runtime) setupNetNS(ctr *Container) error { nsProcess := fmt.Sprintf("/proc/%d/ns/net", ctr.state.PID) b := make([]byte, 16) diff --git a/libpod/networking_unsupported.go b/libpod/networking_unsupported.go index d5189709c..69f470ff7 100644 --- a/libpod/networking_unsupported.go +++ b/libpod/networking_unsupported.go @@ -4,11 +4,11 @@ package libpod import "github.com/containers/libpod/v2/libpod/define" -func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { +func (r *Runtime) setupRootlessNetNS(ctr *Container) error { return define.ErrNotImplemented } -func (r *Runtime) setupNetNS(ctr *Container) (err error) { +func (r *Runtime) setupNetNS(ctr *Container) error { return define.ErrNotImplemented } @@ -16,7 +16,7 @@ func (r *Runtime) teardownNetNS(ctr *Container) error { return define.ErrNotImplemented } -func (r *Runtime) createNetNS(ctr *Container) (err error) { +func (r *Runtime) createNetNS(ctr *Container) error { return define.ErrNotImplemented } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 534622382..bd6af5281 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -191,7 +191,7 @@ func hasCurrentUserMapped(ctr *Container) bool { } // CreateContainer creates a container. -func (r *ConmonOCIRuntime) CreateContainer(ctr *Container, restoreOptions *ContainerCheckpointOptions) (err error) { +func (r *ConmonOCIRuntime) CreateContainer(ctr *Container, restoreOptions *ContainerCheckpointOptions) error { if !hasCurrentUserMapped(ctr) { for _, i := range []string{ctr.state.RunDir, ctr.runtime.config.Engine.TmpDir, ctr.config.StaticDir, ctr.state.Mountpoint, ctr.runtime.config.Engine.VolumePath} { if err := makeAccessible(i, ctr.RootUID(), ctr.RootGID()); err != nil { @@ -853,7 +853,7 @@ func (r *ConmonOCIRuntime) getLogTag(ctr *Container) (string, error) { } // createOCIContainer generates this container's main conmon instance and prepares it for starting -func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *ContainerCheckpointOptions) (err error) { +func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *ContainerCheckpointOptions) error { var stderrBuf bytes.Buffer runtimeDir, err := util.GetRuntimeDir() @@ -1343,8 +1343,9 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec return nil } -// newPipe creates a unix socket pair for communication -func newPipe() (parent *os.File, child *os.File, err error) { +// newPipe creates a unix socket pair for communication. +// Returns two files - first is parent, second is child. +func newPipe() (*os.File, *os.File, error) { fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_SEQPACKET|unix.SOCK_CLOEXEC, 0) if err != nil { return nil, nil, err diff --git a/libpod/runtime.go b/libpod/runtime.go index 2575decc3..0075c0e13 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -126,7 +126,7 @@ func SetXdgDirs() error { // NewRuntime creates a new container runtime // Options can be passed to override the default configuration for the runtime -func NewRuntime(ctx context.Context, options ...RuntimeOption) (runtime *Runtime, err error) { +func NewRuntime(ctx context.Context, options ...RuntimeOption) (*Runtime, error) { conf, err := config.NewConfig("") if err != nil { return nil, err @@ -140,13 +140,13 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (runtime *Runtime // functions can be used to mutate this configuration further. // An error will be returned if the configuration file at the given path does // not exist or cannot be loaded -func NewRuntimeFromConfig(ctx context.Context, userConfig *config.Config, options ...RuntimeOption) (runtime *Runtime, err error) { +func NewRuntimeFromConfig(ctx context.Context, userConfig *config.Config, options ...RuntimeOption) (*Runtime, error) { return newRuntimeFromConfig(ctx, userConfig, options...) } -func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...RuntimeOption) (runtime *Runtime, err error) { - runtime = new(Runtime) +func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...RuntimeOption) (*Runtime, error) { + runtime := new(Runtime) if conf.Engine.OCIRuntime == "" { conf.Engine.OCIRuntime = "runc" @@ -236,7 +236,7 @@ func getLockManager(runtime *Runtime) (lock.Manager, error) { // Make a new runtime based on the given configuration // Sets up containers/storage, state store, OCI runtime -func makeRuntime(ctx context.Context, runtime *Runtime) (err error) { +func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) { // Find a working conmon binary cPath, err := runtime.config.FindConmon() if err != nil { @@ -316,12 +316,11 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (err error) { return err } defer func() { - if err != nil && store != nil { + if retErr != nil && store != nil { // Don't forcibly shut down // We could be opening a store in use by another libpod - _, err2 := store.Shutdown(false) - if err2 != nil { - logrus.Errorf("Error removing store for partially-created runtime: %s", err2) + if _, err := store.Shutdown(false); err != nil { + logrus.Errorf("Error removing store for partially-created runtime: %s", err) } } }() diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index e184df45d..d4508bf37 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -34,7 +34,7 @@ type CtrCreateOption func(*Container) error type ContainerFilter func(*Container) bool // NewContainer creates a new container from a given OCI config. -func (r *Runtime) NewContainer(ctx context.Context, rSpec *spec.Spec, options ...CtrCreateOption) (c *Container, err error) { +func (r *Runtime) NewContainer(ctx context.Context, rSpec *spec.Spec, options ...CtrCreateOption) (*Container, error) { r.lock.Lock() defer r.lock.Unlock() if !r.valid { @@ -44,7 +44,7 @@ func (r *Runtime) NewContainer(ctx context.Context, rSpec *spec.Spec, options .. } // RestoreContainer re-creates a container from an imported checkpoint -func (r *Runtime) RestoreContainer(ctx context.Context, rSpec *spec.Spec, config *ContainerConfig) (c *Container, err error) { +func (r *Runtime) RestoreContainer(ctx context.Context, rSpec *spec.Spec, config *ContainerConfig) (*Container, error) { r.lock.Lock() defer r.lock.Unlock() if !r.valid { @@ -68,7 +68,7 @@ func (r *Runtime) RestoreContainer(ctx context.Context, rSpec *spec.Spec, config return r.setupContainer(ctx, ctr) } -func (r *Runtime) initContainerVariables(rSpec *spec.Spec, config *ContainerConfig) (c *Container, err error) { +func (r *Runtime) initContainerVariables(rSpec *spec.Spec, config *ContainerConfig) (*Container, error) { if rSpec == nil { return nil, errors.Wrapf(define.ErrInvalidArg, "must provide a valid runtime spec to create container") } @@ -122,7 +122,7 @@ func (r *Runtime) initContainerVariables(rSpec *spec.Spec, config *ContainerConf return ctr, nil } -func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ...CtrCreateOption) (c *Container, err error) { +func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ...CtrCreateOption) (*Container, error) { span, _ := opentracing.StartSpanFromContext(ctx, "newContainer") span.SetTag("type", "runtime") defer span.Finish() @@ -141,7 +141,7 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. return r.setupContainer(ctx, ctr) } -func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Container, err error) { +func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Container, retErr error) { // Validate the container if err := ctr.validate(); err != nil { return nil, err @@ -157,9 +157,9 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai logrus.Debugf("Allocated lock %d for container %s", ctr.lock.ID(), ctr.ID()) defer func() { - if err != nil { - if err2 := ctr.lock.Free(); err2 != nil { - logrus.Errorf("Error freeing lock for container after creation failed: %v", err2) + if retErr != nil { + if err := ctr.lock.Free(); err != nil { + logrus.Errorf("Error freeing lock for container after creation failed: %v", err) } } }() @@ -272,9 +272,9 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai return nil, err } defer func() { - if err != nil { - if err2 := ctr.teardownStorage(); err2 != nil { - logrus.Errorf("Error removing partially-created container root filesystem: %s", err2) + if retErr != nil { + if err := ctr.teardownStorage(); err != nil { + logrus.Errorf("Error removing partially-created container root filesystem: %s", err) } } }() diff --git a/libpod/state_test.go b/libpod/state_test.go index 30ddf5e24..ef4f6f2be 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -34,13 +34,13 @@ var ( ) // Get an empty BoltDB state for use in tests -func getEmptyBoltState() (s State, p string, m lock.Manager, err error) { +func getEmptyBoltState() (_ State, _ string, _ lock.Manager, retErr error) { tmpDir, err := ioutil.TempDir("", tmpDirPrefix) if err != nil { return nil, "", nil, err } defer func() { - if err != nil { + if retErr != nil { os.RemoveAll(tmpDir) } }() @@ -66,13 +66,13 @@ func getEmptyBoltState() (s State, p string, m lock.Manager, err error) { } // Get an empty in-memory state for use in tests -func getEmptyInMemoryState() (s State, p string, m lock.Manager, err error) { +func getEmptyInMemoryState() (_ State, _ string, _ lock.Manager, retErr error) { tmpDir, err := ioutil.TempDir("", tmpDirPrefix) if err != nil { return nil, "", nil, err } defer func() { - if err != nil { + if retErr != nil { os.RemoveAll(tmpDir) } }() diff --git a/libpod/storage.go b/libpod/storage.go index be79b3fc0..e497d0daf 100644 --- a/libpod/storage.go +++ b/libpod/storage.go @@ -66,7 +66,7 @@ func (metadata *RuntimeContainerMetadata) SetMountLabel(mountLabel string) { // CreateContainerStorage creates the storage end of things. We already have the container spec created // TO-DO We should be passing in an Image object in the future. -func (r *storageService) CreateContainerStorage(ctx context.Context, systemContext *types.SystemContext, imageName, imageID, containerName, containerID string, options storage.ContainerOptions) (cinfo ContainerInfo, err error) { +func (r *storageService) CreateContainerStorage(ctx context.Context, systemContext *types.SystemContext, imageName, imageID, containerName, containerID string, options storage.ContainerOptions) (_ ContainerInfo, retErr error) { span, _ := opentracing.StartSpanFromContext(ctx, "createContainerStorage") span.SetTag("type", "storageService") defer span.Finish() @@ -132,9 +132,9 @@ func (r *storageService) CreateContainerStorage(ctx context.Context, systemConte // If anything fails after this point, we need to delete the incomplete // container before returning. defer func() { - if err != nil { - if err2 := r.store.DeleteContainer(container.ID); err2 != nil { - logrus.Infof("%v deleting partially-created container %q", err2, container.ID) + if retErr != nil { + if err := r.store.DeleteContainer(container.ID); err != nil { + logrus.Infof("%v deleting partially-created container %q", err, container.ID) return } diff --git a/pkg/api/handlers/compat/containers_attach.go b/pkg/api/handlers/compat/containers_attach.go index 325f96b40..71586fca4 100644 --- a/pkg/api/handlers/compat/containers_attach.go +++ b/pkg/api/handlers/compat/containers_attach.go @@ -1,23 +1,22 @@ package compat import ( + "bufio" "fmt" + "io" + "net" "net/http" + "strings" "github.com/containers/libpod/v2/libpod" "github.com/containers/libpod/v2/libpod/define" "github.com/containers/libpod/v2/pkg/api/handlers/utils" + "github.com/containers/libpod/v2/pkg/api/server/idletracker" "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -// AttachHeader is the literal header sent for upgraded/hijacked connections for -// attach, sourced from Docker at: -// https://raw.githubusercontent.com/moby/moby/b95fad8e51bd064be4f4e58a996924f343846c85/api/server/router/container/container_routes.go -// Using literally to ensure compatibility with existing clients. -const AttachHeader = "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n" - func AttachContainer(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) @@ -98,21 +97,11 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) { return } - // Hijack the connection - hijacker, ok := w.(http.Hijacker) - if !ok { - utils.InternalServerError(w, errors.Errorf("unable to hijack connection")) - return - } - - connection, buffer, err := hijacker.Hijack() + connection, buffer, err := AttachConnection(w, r) if err != nil { - utils.InternalServerError(w, errors.Wrapf(err, "error hijacking connection")) + utils.InternalServerError(w, err) return } - - fmt.Fprintf(connection, AttachHeader) - logrus.Debugf("Hijack for attach of container %s successful", ctr.ID()) // Perform HTTP attach. @@ -126,3 +115,57 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) { logrus.Debugf("Attach for container %s completed successfully", ctr.ID()) } + +type HijackedConnection struct { + net.Conn // Connection + idleTracker *idletracker.IdleTracker // Connection tracker +} + +func (c HijackedConnection) Close() error { + logrus.Debugf("Hijacked connection closed") + + c.idleTracker.TrackHijackedClosed() + return c.Conn.Close() +} + +func AttachConnection(w http.ResponseWriter, r *http.Request) (net.Conn, *bufio.ReadWriter, error) { + idleTracker := r.Context().Value("idletracker").(*idletracker.IdleTracker) + + // Hijack the connection + hijacker, ok := w.(http.Hijacker) + if !ok { + return nil, nil, errors.Errorf("unable to hijack connection") + } + + connection, buffer, err := hijacker.Hijack() + if err != nil { + return nil, nil, errors.Wrapf(err, "error hijacking connection") + } + trackedConnection := HijackedConnection{ + Conn: connection, + idleTracker: idleTracker, + } + + WriteAttachHeaders(r, trackedConnection) + + return trackedConnection, buffer, nil +} + +func WriteAttachHeaders(r *http.Request, connection io.Writer) { + // AttachHeader is the literal header sent for upgraded/hijacked connections for + // attach, sourced from Docker at: + // https://raw.githubusercontent.com/moby/moby/b95fad8e51bd064be4f4e58a996924f343846c85/api/server/router/container/container_routes.go + // Using literally to ensure compatibility with existing clients. + c := r.Header.Get("Connection") + proto := r.Header.Get("Upgrade") + if len(proto) == 0 || !strings.EqualFold(c, "Upgrade") { + // OK - can't upgrade if not requested or protocol is not specified + fmt.Fprintf(connection, + "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n") + } else { + // Upraded + fmt.Fprintf(connection, + "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: %s\r\n\r\n", + proto) + } +} diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index aee4196dd..a3b8cb573 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -173,21 +173,11 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { return } - // Hijack the connection - hijacker, ok := w.(http.Hijacker) - if !ok { - utils.InternalServerError(w, errors.Errorf("unable to hijack connection")) - return - } - - connection, buffer, err := hijacker.Hijack() + connection, buffer, err := AttachConnection(w, r) if err != nil { - utils.InternalServerError(w, errors.Wrapf(err, "error hijacking connection")) + utils.InternalServerError(w, err) return } - - fmt.Fprintf(connection, AttachHeader) - logrus.Debugf("Hijack for attach of container %s exec session %s successful", sessionCtr.ID(), sessionID) if err := sessionCtr.ExecHTTPStartAndAttach(sessionID, connection, buffer, nil, nil, nil); err != nil { diff --git a/pkg/api/server/handler_api.go b/pkg/api/server/handler_api.go index b0fd932ba..53fe8952b 100644 --- a/pkg/api/server/handler_api.go +++ b/pkg/api/server/handler_api.go @@ -37,6 +37,7 @@ func (s *APIServer) APIHandler(h http.HandlerFunc) http.HandlerFunc { c := context.WithValue(r.Context(), "decoder", s.Decoder) //nolint c = context.WithValue(c, "runtime", s.Runtime) //nolint c = context.WithValue(c, "shutdownFunc", s.Shutdown) //nolint + c = context.WithValue(c, "idletracker", s.idleTracker) //nolint r = r.WithContext(c) h(w, r) diff --git a/pkg/api/server/idletracker/idletracker.go b/pkg/api/server/idletracker/idletracker.go new file mode 100644 index 000000000..1ee905a99 --- /dev/null +++ b/pkg/api/server/idletracker/idletracker.go @@ -0,0 +1,74 @@ +package idletracker + +import ( + "net" + "net/http" + "sync" + "time" + + "github.com/sirupsen/logrus" +) + +type IdleTracker struct { + http map[net.Conn]struct{} + hijacked int + total int + mux sync.Mutex + timer *time.Timer + Duration time.Duration +} + +func NewIdleTracker(idle time.Duration) *IdleTracker { + return &IdleTracker{ + http: make(map[net.Conn]struct{}), + Duration: idle, + timer: time.NewTimer(idle), + } +} + +func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { + t.mux.Lock() + defer t.mux.Unlock() + + oldActive := t.ActiveConnections() + logrus.Debugf("IdleTracker %p:%v %d/%d connection(s)", conn, state, oldActive, t.TotalConnections()) + switch state { + case http.StateNew, http.StateActive: + t.http[conn] = struct{}{} + // stop the timer if we transitioned from idle + if oldActive == 0 { + t.timer.Stop() + } + t.total++ + case http.StateHijacked: + // hijacked connections are handled elsewhere + delete(t.http, conn) + t.hijacked++ + case http.StateIdle, http.StateClosed: + delete(t.http, conn) + // Restart the timer if we've become idle + if oldActive > 0 && len(t.http) == 0 { + t.timer.Stop() + t.timer.Reset(t.Duration) + } + } +} + +func (t *IdleTracker) TrackHijackedClosed() { + t.mux.Lock() + defer t.mux.Unlock() + + t.hijacked-- +} + +func (t *IdleTracker) ActiveConnections() int { + return len(t.http) + t.hijacked +} + +func (t *IdleTracker) TotalConnections() int { + return t.total +} + +func (t *IdleTracker) Done() <-chan time.Time { + return t.timer.C +} diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 8af6d3186..1c6007745 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -10,12 +10,12 @@ import ( "runtime" goRuntime "runtime" "strings" - "sync" "syscall" "time" "github.com/containers/libpod/v2/libpod" "github.com/containers/libpod/v2/pkg/api/handlers" + "github.com/containers/libpod/v2/pkg/api/server/idletracker" "github.com/coreos/go-systemd/v22/activation" "github.com/gorilla/mux" "github.com/gorilla/schema" @@ -24,14 +24,14 @@ import ( ) type APIServer struct { - http.Server // The HTTP work happens here - *schema.Decoder // Decoder for Query parameters to structs - context.Context // Context to carry objects to handlers - *libpod.Runtime // Where the real work happens - net.Listener // mux for routing HTTP API calls to libpod routines - context.CancelFunc // Stop APIServer - idleTracker *IdleTracker // Track connections to support idle shutdown - pprof *http.Server // Sidecar http server for providing performance data + http.Server // The HTTP work happens here + *schema.Decoder // Decoder for Query parameters to structs + context.Context // Context to carry objects to handlers + *libpod.Runtime // Where the real work happens + net.Listener // mux for routing HTTP API calls to libpod routines + context.CancelFunc // Stop APIServer + idleTracker *idletracker.IdleTracker // Track connections to support idle shutdown + pprof *http.Server // Sidecar http server for providing performance data } // Number of seconds to wait for next request, if exceeded shutdown server @@ -68,7 +68,7 @@ func newServer(runtime *libpod.Runtime, duration time.Duration, listener *net.Li } router := mux.NewRouter().UseEncodedPath() - idle := NewIdleTracker(duration) + idle := idletracker.NewIdleTracker(duration) server := APIServer{ Server: http.Server{ @@ -231,55 +231,3 @@ func (s *APIServer) Shutdown() error { func (s *APIServer) Close() error { return s.Server.Close() } - -type IdleTracker struct { - active map[net.Conn]struct{} - total int - mux sync.Mutex - timer *time.Timer - Duration time.Duration -} - -func NewIdleTracker(idle time.Duration) *IdleTracker { - return &IdleTracker{ - active: make(map[net.Conn]struct{}), - Duration: idle, - timer: time.NewTimer(idle), - } -} - -func (t *IdleTracker) ConnState(conn net.Conn, state http.ConnState) { - t.mux.Lock() - defer t.mux.Unlock() - - oldActive := len(t.active) - logrus.Debugf("IdleTracker %p:%v %d/%d connection(s)", conn, state, t.ActiveConnections(), t.TotalConnections()) - switch state { - case http.StateNew, http.StateActive, http.StateHijacked: - t.active[conn] = struct{}{} - // stop the timer if we transitioned from idle - if oldActive == 0 { - t.timer.Stop() - } - t.total++ - case http.StateIdle, http.StateClosed: - delete(t.active, conn) - // Restart the timer if we've become idle - if oldActive > 0 && len(t.active) == 0 { - t.timer.Stop() - t.timer.Reset(t.Duration) - } - } -} - -func (t *IdleTracker) ActiveConnections() int { - return len(t.active) -} - -func (t *IdleTracker) TotalConnections() int { - return t.total -} - -func (t *IdleTracker) Done() <-chan time.Time { - return t.timer.C -} diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index 077bb244f..297563688 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -457,15 +457,15 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, streams *define.A switch { case fd == 0: - if streams.AttachOutput { + if streams.AttachInput { + // Write STDIN to STDOUT (echoing characters + // typed by another attach session) if _, err := streams.OutputStream.Write(frame[0:l]); err != nil { return err } } case fd == 1: - if streams.AttachInput { - // Write STDIN to STDOUT (echoing characters - // typed by another attach session) + if streams.AttachOutput { if _, err := streams.OutputStream.Write(frame[0:l]); err != nil { return err } diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go index 4f744e401..0e60c4a7d 100644 --- a/test/e2e/exec_test.go +++ b/test/e2e/exec_test.go @@ -79,7 +79,6 @@ var _ = Describe("Podman exec", func() { }) It("podman exec environment test", func() { - Skip(v2remotefail) setup := podmanTest.RunTopContainer("test1") setup.WaitWithDefaultTimeout() Expect(setup.ExitCode()).To(Equal(0)) @@ -99,7 +98,6 @@ var _ = Describe("Podman exec", func() { It("podman exec os.Setenv env", func() { // remote doesn't properly interpret os.Setenv - SkipIfRemote() setup := podmanTest.RunTopContainer("test1") setup.WaitWithDefaultTimeout() Expect(setup.ExitCode()).To(Equal(0)) @@ -159,7 +157,6 @@ var _ = Describe("Podman exec", func() { }) It("podman exec with user only in container", func() { - Skip(v2remotefail) testUser := "test123" setup := podmanTest.Podman([]string{"run", "--name", "test1", "-d", fedoraMinimal, "sleep", "60"}) setup.WaitWithDefaultTimeout() @@ -176,7 +173,6 @@ var _ = Describe("Podman exec", func() { }) It("podman exec with user from run", func() { - Skip(v2remotefail) testUser := "guest" setup := podmanTest.Podman([]string{"run", "--user", testUser, "-d", ALPINE, "top"}) setup.WaitWithDefaultTimeout() @@ -196,7 +192,6 @@ var _ = Describe("Podman exec", func() { }) It("podman exec simple working directory test", func() { - Skip(v2remotefail) setup := podmanTest.RunTopContainer("test1") setup.WaitWithDefaultTimeout() Expect(setup.ExitCode()).To(Equal(0)) diff --git a/test/e2e/info_test.go b/test/e2e/info_test.go index e38ace53f..8aa9712fd 100644 --- a/test/e2e/info_test.go +++ b/test/e2e/info_test.go @@ -11,6 +11,7 @@ import ( . "github.com/containers/libpod/v2/test/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gexec" ) var _ = Describe("Podman Info", func() { @@ -35,11 +36,30 @@ var _ = Describe("Podman Info", func() { processTestResult(f) }) - It("podman info json output", func() { - session := podmanTest.Podman([]string{"info", "--format=json"}) - session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(0)) - + It("podman info --format json", func() { + tests := []struct { + input string + success bool + exitCode int + }{ + {"json", true, 0}, + {" json", true, 0}, + {"json ", true, 0}, + {" json ", true, 0}, + {"{{json .}}", true, 0}, + {"{{ json .}}", true, 0}, + {"{{json . }}", true, 0}, + {" {{ json . }} ", true, 0}, + {"{{json }}", false, 125}, + {"{{json .", false, 125}, + {"json . }}", false, 0}, // Note: this does NOT fail but produces garbage + } + for _, tt := range tests { + session := podmanTest.Podman([]string{"info", "--format", tt.input}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(tt.exitCode)) + Expect(session.IsJSONOutputValid()).To(Equal(tt.success)) + } }) It("podman info --format GO template", func() { diff --git a/test/e2e/version_test.go b/test/e2e/version_test.go index 775be6511..eb1d1b733 100644 --- a/test/e2e/version_test.go +++ b/test/e2e/version_test.go @@ -55,17 +55,29 @@ var _ = Describe("Podman version", func() { }) It("podman version --format json", func() { - session := podmanTest.Podman([]string{"version", "--format", "json"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - Expect(session.IsJSONOutputValid()).To(BeTrue()) - }) - - It("podman version --format json", func() { - session := podmanTest.Podman([]string{"version", "--format", "{{ json .}}"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - Expect(session.IsJSONOutputValid()).To(BeTrue()) + tests := []struct { + input string + success bool + exitCode int + }{ + {"json", true, 0}, + {" json", true, 0}, + {"json ", true, 0}, + {" json ", true, 0}, + {"{{json .}}", true, 0}, + {"{{ json .}}", true, 0}, + {"{{json . }}", true, 0}, + {" {{ json . }} ", true, 0}, + {"{{json }}", false, 125}, + {"{{json .", false, 125}, + {"json . }}", false, 0}, // Note: this does NOT fail but produces garbage + } + for _, tt := range tests { + session := podmanTest.Podman([]string{"version", "--format", tt.input}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(tt.exitCode)) + Expect(session.IsJSONOutputValid()).To(Equal(tt.success)) + } }) It("podman version --format GO template", func() { |