fix healthcheck timeouts and ut8 coercion

this commit fixes two bugs and adds regression tests.

when getting healthcheck values from an image, if the image does not
have a timeout defined, this resulted in a 0 value for timeout.  The
default as described in the man pages is 30s.

when inspecting a container with a healthcheck command, a customer
observed that the &, <, and > characters were being converted into a
unicode escape value.  It turns out json marshalling will by default
coerce string values to ut8.

Fixes: bz2028408

Signed-off-by: Brent Baude <bbaude@redhat.com>

cmd/podman/common/create.go

@@ -257,31 +257,31 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		healthIntervalFlagName := "health-interval"
 		createFlags.StringVar(
 			&cf.HealthInterval,
-			healthIntervalFlagName, DefaultHealthCheckInterval,
+			healthIntervalFlagName, define.DefaultHealthCheckInterval,
 			"set an interval for the healthchecks (a value of disable results in no automatic timer setup)",
 		)
 		_ = cmd.RegisterFlagCompletionFunc(healthIntervalFlagName, completion.AutocompleteNone)
 
 		healthRetriesFlagName := "health-retries"
 		createFlags.UintVar(
 			&cf.HealthRetries,
-			healthRetriesFlagName, DefaultHealthCheckRetries,
+			healthRetriesFlagName, define.DefaultHealthCheckRetries,
 			"the number of retries allowed before a healthcheck is considered to be unhealthy",
 		)
 		_ = cmd.RegisterFlagCompletionFunc(healthRetriesFlagName, completion.AutocompleteNone)
 
 		healthStartPeriodFlagName := "health-start-period"
 		createFlags.StringVar(
 			&cf.HealthStartPeriod,
-			healthStartPeriodFlagName, DefaultHealthCheckStartPeriod,
+			healthStartPeriodFlagName, define.DefaultHealthCheckStartPeriod,
 			"the initialization time needed for a container to bootstrap",
 		)
 		_ = cmd.RegisterFlagCompletionFunc(healthStartPeriodFlagName, completion.AutocompleteNone)
 
 		healthTimeoutFlagName := "health-timeout"
 		createFlags.StringVar(
 			&cf.HealthTimeout,
-			healthTimeoutFlagName, DefaultHealthCheckTimeout,
+			healthTimeoutFlagName, define.DefaultHealthCheckTimeout,
 			"the maximum time allowed to complete the healthcheck before an interval is considered failed",
 		)
 		_ = cmd.RegisterFlagCompletionFunc(healthTimeoutFlagName, completion.AutocompleteNone)

cmd/podman/common/create_opts.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containers/common/pkg/cgroups"
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v3/cmd/podman/registry"
+	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/libpod/network/types"
 	"github.com/containers/podman/v3/pkg/api/handlers"
 	"github.com/containers/podman/v3/pkg/domain/entities"
@@ -305,10 +306,10 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c
 		VolumesFrom:       cc.HostConfig.VolumesFrom,
 		Workdir:           cc.Config.WorkingDir,
 		Net:               &netInfo,
-		HealthInterval:    DefaultHealthCheckInterval,
-		HealthRetries:     DefaultHealthCheckRetries,
-		HealthTimeout:     DefaultHealthCheckTimeout,
-		HealthStartPeriod: DefaultHealthCheckStartPeriod,
+		HealthInterval:    define.DefaultHealthCheckInterval,
+		HealthRetries:     define.DefaultHealthCheckRetries,
+		HealthTimeout:     define.DefaultHealthCheckTimeout,
+		HealthStartPeriod: define.DefaultHealthCheckStartPeriod,
 	}
 	if !rootless.IsRootless() {
 		var ulimits []string

cmd/podman/common/default.go

@@ -5,14 +5,7 @@ import (
 )
 
 var (
-	// DefaultHealthCheckInterval default value
-	DefaultHealthCheckInterval = "30s"
-	// DefaultHealthCheckRetries default value
-	DefaultHealthCheckRetries uint = 3
-	// DefaultHealthCheckStartPeriod default value
-	DefaultHealthCheckStartPeriod = "0s"
-	// DefaultHealthCheckTimeout default value
-	DefaultHealthCheckTimeout = "30s"
+
 	// DefaultImageVolume default value
 	DefaultImageVolume = "bind"
 	// Pull in configured json library

cmd/podman/inspect/inspect.go

@@ -237,12 +237,12 @@ func (i *inspector) inspect(namesOrIDs []string) error {
 }
 
 func printJSON(data []interface{}) error {
-	buf, err := json.MarshalIndent(data, "", "    ")
-	if err != nil {
-		return err
-	}
-	_, err = fmt.Println(string(buf))
-	return err
+	enc := json.NewEncoder(os.Stdout)
+	// by default, json marshallers will force utf=8 from
+	// a string. this breaks healthchecks that use <,>, &&.
+	enc.SetEscapeHTML(false)
+	enc.SetIndent("", "     ")
+	return enc.Encode(data)
 }
 
 func printTmpl(typ, row string, data []interface{}) error {

libpod/define/healthchecks.go

@@ -34,3 +34,16 @@ const (
 	// HealthCheckDefined means the healthcheck was found on the container
 	HealthCheckDefined HealthCheckStatus = iota
 )
+
+// Healthcheck defaults.  These are used both in the cli as well in
+// libpod and were moved from cmd/podman/common
+const (
+	// DefaultHealthCheckInterval default value
+	DefaultHealthCheckInterval = "30s"
+	// DefaultHealthCheckRetries default value
+	DefaultHealthCheckRetries uint = 3
+	// DefaultHealthCheckStartPeriod default value
+	DefaultHealthCheckStartPeriod = "0s"
+	// DefaultHealthCheckTimeout default value
+	DefaultHealthCheckTimeout = "30s"
+)

pkg/specgen/generate/container.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/containers/common/libimage"
 	"github.com/containers/podman/v3/libpod"
@@ -64,6 +65,13 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 			// NOTE: the health check is only set for Docker images
 			// but inspect will take care of it.
 			s.HealthConfig = inspectData.HealthCheck
+			if s.HealthConfig != nil && s.HealthConfig.Timeout == 0 {
+				hct, err := time.ParseDuration(define.DefaultHealthCheckTimeout)
+				if err != nil {
+					return nil, err
+				}
+				s.HealthConfig.Timeout = hct
+			}
 		}
 
 		// Image stop signal

test/e2e/healthcheck_run_test.go

@@ -2,7 +2,9 @@ package integration
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"time"
 
 	define "github.com/containers/podman/v3/libpod/define"
@@ -258,4 +260,40 @@ var _ = Describe("Podman healthcheck run", func() {
 		Expect(startAgain.OutputToString()).To(Equal("hc"))
 		Expect(startAgain.ErrorToString()).To(Equal(""))
 	})
+
+	It("Verify default time is used and no utf-8 escapes", func() {
+		cwd, err := os.Getwd()
+		Expect(err).To(BeNil())
+
+		podmanTest.AddImageToRWStore(ALPINE)
+		// Write target and fake files
+		targetPath, err := CreateTempDirInTempDir()
+		Expect(err).To(BeNil())
+		containerfile := fmt.Sprintf(`FROM %s
+HEALTHCHECK CMD ls -l / 2>&1`, ALPINE)
+		containerfilePath := filepath.Join(targetPath, "Containerfile")
+		err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
+		Expect(err).To(BeNil())
+		defer func() {
+			Expect(os.Chdir(cwd)).To(BeNil())
+			Expect(os.RemoveAll(targetPath)).To(BeNil())
+		}()
+
+		// make cwd as context root path
+		Expect(os.Chdir(targetPath)).To(BeNil())
+
+		session := podmanTest.Podman([]string{"build", "--format", "docker", "-t", "test", "."})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		run := podmanTest.Podman([]string{"run", "-dt", "--name", "hctest", "test", "ls"})
+		run.WaitWithDefaultTimeout()
+		Expect(run).Should(Exit(0))
+
+		inspect := podmanTest.InspectContainer("hctest")
+		// Check to make sure a default time value was added
+		Expect(inspect[0].Config.Healthcheck.Timeout).To(BeNumerically("==", 30000000000))
+		// Check to make sure characters were not coerced to utf8
+		Expect(inspect[0].Config.Healthcheck.Test).To(Equal([]string{"CMD-SHELL", "ls -l / 2>&1"}))
+	})
 })