diff --git a/agent/docker_registry.go b/agent/docker_registry.go index 6c0a4772..5a4103f5 100644 --- a/agent/docker_registry.go +++ b/agent/docker_registry.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "strings" "time" @@ -37,7 +38,7 @@ func (dm *dockerManager) checkImageUpdate(image string) (bool, error) { repository := reference.Path(named) tag := named.(reference.Tagged).Tag() - localDigest, err := dm.inspectImageDigest(image, registry, repository) + localDigests, err := dm.inspectImageDigests(image, registry, repository) if err != nil { return false, err } @@ -47,48 +48,49 @@ func (dm *dockerManager) checkImageUpdate(image string) (bool, error) { return false, err } - return remoteDigest != localDigest, nil + return !slices.Contains(localDigests, remoteDigest), nil } -// inspectImageDigest reads Docker's image metadata without using dm.decode. +// inspectImageDigests reads Docker's image metadata without using dm.decode. // The checker runs in the image-discovery goroutine, so it must not hold any // of the container statistics locks while waiting on the Docker API. -func (dm *dockerManager) inspectImageDigest(image, registry, repository string) (string, error) { +func (dm *dockerManager) inspectImageDigests(image, registry, repository string) ([]string, error) { if dm.client == nil { - return "", fmt.Errorf("inspect image %q: Docker client is unavailable", image) + return nil, fmt.Errorf("inspect image %q: Docker client is unavailable", image) } endpoint := "http://localhost/images/" + url.PathEscape(image) + "/json" resp, err := dm.client.Get(endpoint) if err != nil { - return "", fmt.Errorf("inspect image %q: %w", image, err) + return nil, fmt.Errorf("inspect image %q: %w", image, err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("inspect image %q failed: %s", image, responseStatus(resp)) + return nil, fmt.Errorf("inspect image %q failed: %s", image, responseStatus(resp)) } var inspect struct { RepoDigests []string `json:"RepoDigests"` } if err := json.NewDecoder(resp.Body).Decode(&inspect); err != nil { - return "", fmt.Errorf("decode image inspect %q: %w", image, err) + return nil, fmt.Errorf("decode image inspect %q: %w", image, err) } if len(inspect.RepoDigests) == 0 { - return "", fmt.Errorf("inspect image %q returned no repository digests", image) + return nil, fmt.Errorf("inspect image %q returned no repository digests", image) } - localDigest, ok := matchingRepositoryDigest(inspect.RepoDigests, registry, repository) - if !ok { - return "", fmt.Errorf("inspect image %q returned no valid digest for %s/%s", image, registry, repository) + localDigests := matchingRepositoryDigests(inspect.RepoDigests, registry, repository) + if len(localDigests) == 0 { + return nil, fmt.Errorf("inspect image %q returned no valid digest for %s/%s", image, registry, repository) } - return localDigest, nil + return localDigests, nil } -// matchingRepositoryDigest returns a valid digest belonging to the requested -// repository. Docker can return multiple RepoDigests for one local image; an -// unrelated first entry must never be used for the comparison. -func matchingRepositoryDigest(repoDigests []string, registry, repository string) (string, bool) { +// matchingRepositoryDigests returns all valid digests belonging to the requested +// repository. Container engines can return both index and platform manifest digests for one +// local image, in either order. +func matchingRepositoryDigests(repoDigests []string, registry, repository string) []string { + var digests []string for _, repoDigest := range repoDigests { repoDigest = strings.TrimSpace(repoDigest) at := strings.LastIndexByte(repoDigest, '@') @@ -108,9 +110,9 @@ func matchingRepositoryDigest(repoDigests []string, registry, repository string) if err != nil { continue } - return d.String(), true + digests = append(digests, d.String()) } - return "", false + return digests } func sameRegistry(left, right string) bool { diff --git a/agent/docker_registry_test.go b/agent/docker_registry_test.go index 598be687..f29a6bbe 100644 --- a/agent/docker_registry_test.go +++ b/agent/docker_registry_test.go @@ -80,6 +80,45 @@ func TestCheckImageUpdateUsesInspectAndManifestDigests(t *testing.T) { require.EqualValues(t, 1, manifestCalls.Load()) } +func TestCheckImageUpdateMatchesAnyRepositoryDigest(t *testing.T) { + platform := registryDigest('a') + index := registryDigest('b') + other := registryDigest('c') + + for _, test := range []struct { + name string + digests []string + remote string + available bool + }{ + {name: "platform then index, remote index", digests: []string{platform, index}, remote: index}, + {name: "index then platform, remote index", digests: []string{index, platform}, remote: index}, + {name: "platform then index, remote platform", digests: []string{platform, index}, remote: platform}, + {name: "index then platform, remote platform", digests: []string{index, platform}, remote: platform}, + {name: "neither matches", digests: []string{platform, index}, remote: other, available: true}, + } { + t.Run(test.name, func(t *testing.T) { + inspect := fmt.Sprintf(`{"RepoDigests":["docker.io/library/busybox@%s","docker.io/library/alpine@%s","docker.io/library/alpine@sha256:invalid","docker.io/library/alpine@%s"]}`, test.remote, test.digests[0], test.digests[1]) + var manifestCalls atomic.Int32 + dm := newRegistryChecker(t, inspect, registryTransportFunc(func(req *http.Request) (*http.Response, error) { + if req.Method == http.MethodGet { + return registryResponse(http.StatusOK, `{"token":"test"}`), nil + } + manifestCalls.Add(1) + require.Equal(t, http.MethodHead, req.Method) + resp := registryResponse(http.StatusOK, "") + resp.Header.Set("Docker-Content-Digest", test.remote) + return resp, nil + })) + + available, err := dm.checkImageUpdate("alpine") + require.NoError(t, err) + require.Equal(t, test.available, available) + require.EqualValues(t, 1, manifestCalls.Load()) + }) + } +} + func TestCheckImageUpdateReportsUnknownInspectState(t *testing.T) { for _, test := range []struct { name string