Repository: containrrr/watchtower
Pull request #925 (2021-04-24 10:34:42)

config sanity check
This will check the container configuration for obvious errors before trying to recreate it, preventing at least some know fault conditions.

codecov[bot]: # [Codecov](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr) Report
> Merging [#925](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr) (ff1ee6c) into [main](https://codecov.io/gh/containrrr/watchtower/commit/b4cf17d33fc361988201a503d3e32a902c5d29da?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr) (b4cf17d) will **increase** coverage by `6.16%`.
> The diff coverage is `21.73%`.

[![Impacted file tree graph](https://codecov.io/gh/containrrr/watchtower/pull/925/graphs/tree.svg?width=650&height=150&src=pr&token=8pxWgB380Y&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr)](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr)

```diff
@@            Coverage Diff             @@
##             main     #925      +/-   ##
==========================================
+ Coverage   40.14%   46.30%   +6.16%     
==========================================
  Files          25       25              
  Lines        1415     1434      +19     
==========================================
+ Hits          568      664      +96     
+ Misses        785      702      -83     
- Partials       62       68       +6     
```


| [Impacted Files](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr) | Coverage Δ | |
|---|---|---|
| [pkg/container/container.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL2NvbnRhaW5lci9jb250YWluZXIuZ28=) | `32.22% <0.00%> (+32.22%)` | :arrow_up: |
| [internal/actions/update.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-aW50ZXJuYWwvYWN0aW9ucy91cGRhdGUuZ28=) | `50.47% <30.00%> (-0.55%)` | :arrow_down: |
| [pkg/notifications/shoutrrr.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL25vdGlmaWNhdGlvbnMvc2hvdXRycnIuZ28=) | `73.43% <100.00%> (+0.42%)` | :arrow_up: |
| [pkg/notifications/email.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL25vdGlmaWNhdGlvbnMvZW1haWwuZ28=) | `90.90% <0.00%> (+4.54%)` | :arrow_up: |
| [pkg/container/client.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL2NvbnRhaW5lci9jbGllbnQuZ28=) | `18.45% <0.00%> (+18.45%)` | :arrow_up: |
| [pkg/container/metadata.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL2NvbnRhaW5lci9tZXRhZGF0YS5nbw==) | `63.63% <0.00%> (+63.63%)` | :arrow_up: |
| [pkg/container/util.go](https://codecov.io/gh/containrrr/watchtower/pull/925/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr#diff-cGtnL2NvbnRhaW5lci91dGlsLmdv) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr). Last update [6f281b7...ff1ee6c](https://codecov.io/gh/containrrr/watchtower/pull/925?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containrrr).


Patch:
diff --git a/cmd/root.go b/cmd/root.go
index 6f84727..707c4fd 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -269,6 +269,9 @@ func writeStartupMessage(c *cobra.Command, sched time.Time, filtering string) {
 		}
 
 		log.Info("Watchtower ", version, "\n", notifs, "\n", filtering, "\n", schedMessage)
+		if log.IsLevelEnabled(log.TraceLevel) {
+			log.Warn("trace level enabled: log will include sensitive information as credentials and tokens")
+		}
 	}
 }
 
@@ -330,8 +333,10 @@ func runUpdatesWithNotifications(filter t.Filter) *metrics.Metric {
 	}
 	metricResults, err := actions.Update(client, updateParams)
 	if err != nil {
-		log.Println(err)
+		log.Error(err)
 	}
 	notifier.SendNotification()
+	log.Debugf("Session done: %v scanned, %v updated, %v failed",
+		metricResults.Scanned, metricResults.Updated, metricResults.Failed)
 	return metricResults
 }
diff --git a/internal/actions/mocks/container.go b/internal/actions/mocks/container.go
index a4ded79..0c0ee94 100644
--- a/internal/actions/mocks/container.go
+++ b/internal/actions/mocks/container.go
@@ -16,10 +16,13 @@ func CreateMockContainer(id string, name string, image string, created time.Time
 			Image:   image,
 			Name:    name,
 			Created: created.String(),
+			HostConfig: &container2.HostConfig{
+				PortBindings: map[nat.Port][]nat.PortBinding{},
+			},
 		},
 		Config: &container2.Config{
-			Image:  image,
-			Labels: make(map[string]string),
+			Image:        image,
+			Labels:       make(map[string]string),
 			ExposedPorts: map[nat.Port]struct{}{},
 		},
 	}
diff --git a/internal/actions/update.go b/internal/actions/update.go
index 06bb345..66a28f1 100644
--- a/internal/actions/update.go
+++ b/internal/actions/update.go
@@ -1,7 +1,6 @@
 package actions
 
 import (
-	"errors"
 	"github.com/containrrr/watchtower/internal/util"
 	"github.com/containrrr/watchtower/pkg/container"
 	"github.com/containrrr/watchtower/pkg/lifecycle"
@@ -33,11 +32,23 @@ func Update(client container.Client, params types.UpdateParams) (*metrics2.Metri
 
 	for i, targetContainer := range containers {
 		stale, err := client.IsContainerStale(targetContainer)
-		if stale && !params.NoRestart && !params.MonitorOnly && !targetContainer.IsMonitorOnly() && !targetContainer.HasImageInfo() {
-			err = errors.New("no available image info")
+		shouldUpdate := stale && !params.NoRestart && !params.MonitorOnly && !targetContainer.IsMonitorOnly()
+		if err == nil && shouldUpdate {
+			// Check to make sure we have all the necessary information for recreating the container
+			err = targetContainer.VerifyConfiguration()
+			// If the image information is incomplete and trace logging is enabled, log it for further diagnosis
+			if err != nil && log.IsLevelEnabled(log.TraceLevel) {
+				imageInfo := targetContainer.ImageInfo()
+				log.Tracef("Image info: %#v", imageInfo)
+				log.Tracef("Container info: %#v", targetContainer.ContainerInfo())
+				if imageInfo != nil {
+					log.Tracef("Image config: %#v", imageInfo.Config)
+				}
+			}
 		}
+
 		if err != nil {
-			log.Infof("Unable to update container %q: %v. Proceeding to next.", containers[i].Name(), err)
+			log.Infof("Unable to update container %q: %v. Proceeding to next.", targetContainer.Name(), err)
 			stale = false
 			staleCheckFailed++
 			metric.Failed++
diff --git a/pkg/container/container.go b/pkg/container/container.go
index 7631b5e..92abec2 100644
--- a/pkg/container/container.go
+++ b/pkg/container/container.go
@@ -258,3 +258,32 @@ func (c Container) HasImageInfo() bool {
 func (c Container) ImageInfo() *types.ImageInspect {
 	return c.imageInfo
 }
+
+// VerifyConfiguration checks the container and image configurations for nil references to make sure
+// that the container can be recreated once deleted
+func (c Container) VerifyConfiguration() error {
+	if c.imageInfo == nil {
+		return errorNoImageInfo
+	}
+
+	containerInfo := c.ContainerInfo()
+	if containerInfo == nil {
+		return errorInvalidConfig
+	}
+
+	containerConfig := containerInfo.Config
+	if containerConfig == nil {
+		return errorInvalidConfig
+	}
+
+	hostConfig := containerInfo.HostConfig
+	if hostConfig == nil {
+		return errorInvalidConfig
+	}
+
+	if len(hostConfig.PortBindings) > 0 && containerConfig.ExposedPorts == nil {
+		return errorNoExposedPorts
+	}
+
+	return nil
+}
diff --git a/pkg/container/errors.go b/pkg/container/errors.go
new file mode 100644
index 0000000..b927220
--- /dev/null
+++ b/pkg/container/errors.go
@@ -0,0 +1,7 @@
+package container
+
+import "errors"
+
+var errorNoImageInfo = errors.New("no available image info")
+var errorNoExposedPorts = errors.New("exposed ports does not match port bindings")
+var errorInvalidConfig = errors.New("container configuration missing or invalid")
diff --git a/pkg/registry/digest/digest.go b/pkg/registry/digest/digest.go
index 894c162..4634688 100644
--- a/pkg/registry/digest/digest.go
+++ b/pkg/registry/digest/digest.go
@@ -10,8 +10,10 @@ import (
 	"github.com/containrrr/watchtower/pkg/registry/manifest"
 	"github.com/containrrr/watchtower/pkg/types"
 	"github.com/sirupsen/logrus"
+	"net"
 	"net/http"
 	"strings"
+	"time"
 )
 
 // ContentDigestHeader is the key for the key-value pair containing the digest header
@@ -69,7 +71,17 @@ func TransformAuth(registryAuth string) string {
 // GetDigest from registry using a HEAD request to prevent rate limiting
 func GetDigest(url string, token string) (string, error) {
 	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+		}).DialContext,
+		ForceAttemptHTTP2:     true,
+		MaxIdleConns:          100,
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   10 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+		TLSClientConfig:       &tls.Config{InsecureSkipVerify: true},
 	}
 	client := &http.Client{Transport: tr}
 


Test patch:
diff --git a/pkg/container/container_test.go b/pkg/container/container_test.go
index 8ddeb8b..8f22044 100644
--- a/pkg/container/container_test.go
+++ b/pkg/container/container_test.go
@@ -6,6 +6,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	cli "github.com/docker/docker/client"
+	"github.com/docker/go-connections/nat"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -32,14 +33,14 @@ var _ = Describe("the container", func() {
 			containerKnown := *mockContainerWithImageName("docker.io/prefix/imagename:latest")
 
 			When("warn on head failure is set to \"always\"", func() {
-				c := NewClient(false, false, false, false, false, "always")
+				c := newClientNoAPI(false, false, false, false, false, "always")
 				It("should always return true", func() {
 					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeTrue())
 					Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
 				})
 			})
 			When("warn on head failure is set to \"auto\"", func() {
-				c := NewClient(false, false, false, false, false, "auto")
+				c := newClientNoAPI(false, false, false, false, false, "auto")
 				It("should always return true", func() {
 					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
 				})
@@ -48,7 +49,7 @@ var _ = Describe("the container", func() {
 				})
 			})
 			When("warn on head failure is set to \"never\"", func() {
-				c := NewClient(false, false, false, false, false, "never")
+				c := newClientNoAPI(false, false, false, false, false, "never")
 				It("should never return true", func() {
 					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
 					Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse())
@@ -130,6 +131,63 @@ var _ = Describe("the container", func() {
 			})
 		})
 	})
+	Describe("VerifyConfiguration", func() {
+		When("verifying a container with no image info", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.imageInfo = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorNoImageInfo))
+			})
+		})
+		When("verifying a container with no container info", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no config", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo.Config = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no host config", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo.HostConfig = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no port bindings", func() {
+			It("should not return an error", func() {
+				c := mockContainerWithPortBindings()
+				err := c.VerifyConfiguration()
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+		When("verifying a container with port bindings, but no exposed ports", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings("80/tcp")
+				c.containerInfo.Config.ExposedPorts = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorNoExposedPorts))
+			})
+		})
+		When("verifying a container with port bindings and exposed ports is non-nil", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings("80/tcp")
+				c.containerInfo.Config.ExposedPorts = map[nat.Port]struct{}{"80/tcp": {}}
+				err := c.VerifyConfiguration()
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+	})
 	When("asked for metadata", func() {
 		var c *Container
 		BeforeEach(func() {
@@ -281,10 +339,23 @@ var _ = Describe("the container", func() {
 	})
 })
 
+func mockContainerWithPortBindings(portBindingSources ...string) *Container {
+	mockContainer := mockContainerWithLabels(nil)
+	mockContainer.imageInfo = &types.ImageInspect{}
+	hostConfig := &container.HostConfig{
+		PortBindings: nat.PortMap{},
+	}
+	for _, pbs := range portBindingSources {
+		hostConfig.PortBindings[nat.Port(pbs)] = []nat.PortBinding{}
+	}
+	mockContainer.containerInfo.HostConfig = hostConfig
+	return mockContainer
+}
+
 func mockContainerWithImageName(name string) *Container {
-	container := mockContainerWithLabels(nil)
-	container.containerInfo.Config.Image = name
-	return container
+	mockContainer := mockContainerWithLabels(nil)
+	mockContainer.containerInfo.Config.Image = name
+	return mockContainer
 }
 
 func mockContainerWithLinks(links []string) *Container {
@@ -317,3 +388,15 @@ func mockContainerWithLabels(labels map[string]string) *Container {
 	}
 	return NewContainer(&content, nil)
 }
+
+func newClientNoAPI(pullImages, includeStopped, reviveStopped, removeVolumes, includeRestarting bool, warnOnHeadFailed string) Client {
+	return dockerClient{
+		api:               nil,
+		pullImages:        pullImages,
+		removeVolumes:     removeVolumes,
+		includeStopped:    includeStopped,
+		reviveStopped:     reviveStopped,
+		includeRestarting: includeRestarting,
+		warnOnHeadFailed:  warnOnHeadFailed,
+	}
+}