diff --git a/internal/dockeragent/agent_image_digest_test.go b/internal/dockeragent/agent_image_digest_test.go index 0f32a8e1f..226869117 100644 --- a/internal/dockeragent/agent_image_digest_test.go +++ b/internal/dockeragent/agent_image_digest_test.go @@ -20,7 +20,8 @@ func TestAgent_getImageRepoDigest_Error(t *testing.T) { logger: zerolog.New(io.Discard), } - if got := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest"); got != "" { + got, _, _, _ := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest") + if got != "" { t.Fatalf("expected empty digest on error, got %q", got) } } @@ -35,7 +36,8 @@ func TestAgent_getImageRepoDigest_NoRepoDigests(t *testing.T) { logger: zerolog.New(io.Discard), } - if got := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest"); got != "" { + got, _, _, _ := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest") + if got != "" { t.Fatalf("expected empty digest for no RepoDigests, got %q", got) } } @@ -50,7 +52,8 @@ func TestAgent_getImageRepoDigest_Match(t *testing.T) { logger: zerolog.New(io.Discard), } - if got := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest"); got != "sha256:abc" { + got, _, _, _ := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest") + if got != "sha256:abc" { t.Fatalf("expected matching digest, got %q", got) } } @@ -68,7 +71,8 @@ func TestAgent_getImageRepoDigest_FallbackToFirst(t *testing.T) { logger: zerolog.New(io.Discard), } - if got := agent.getImageRepoDigest(context.Background(), "image-id", "custom:latest"); got != "sha256:first" { + got, _, _, _ := agent.getImageRepoDigest(context.Background(), "image-id", "custom:latest") + if got != "sha256:first" { t.Fatalf("expected fallback digest, got %q", got) } } @@ -83,7 +87,8 @@ func TestAgent_getImageRepoDigest_InvalidRepoDigest(t *testing.T) { logger: zerolog.New(io.Discard), } - if got := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest"); got != "" { + got, _, _, _ := agent.getImageRepoDigest(context.Background(), "image-id", "nginx:latest") + if got != "" { t.Fatalf("expected empty digest for invalid repo digest, got %q", got) } } diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index ff4362762..9a7e99115 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -357,12 +357,15 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum if a.registryChecker != nil && a.registryChecker.Enabled() { // Get the actual manifest digest (RepoDigest) from the image for accurate comparison. // The ImageID is a local content-addressable ID that differs from the registry manifest digest. - digestForComparison := a.getImageRepoDigest(containerCtx, summary.ImageID, summary.Image) + // We also get the architecture details to correctly resolve manifest lists from the registry. + digestForComparison, arch, os, variant := a.getImageRepoDigest(containerCtx, summary.ImageID, summary.Image) + if digestForComparison == "" { // Fall back to ImageID if we can't get RepoDigest (shouldn't compare as equal) digestForComparison = summary.ImageID } - result := a.registryChecker.CheckImageUpdate(ctx, container.Image, digestForComparison) + + result := a.registryChecker.CheckImageUpdate(ctx, container.Image, digestForComparison, arch, os, variant) if result != nil { container.UpdateStatus = &agentsdocker.UpdateStatus{ UpdateAvailable: result.UpdateAvailable, @@ -386,12 +389,9 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum return container, nil } -// getImageRepoDigest retrieves the RepoDigest for an image, which is the actual -// manifest digest from the registry. This is necessary because Docker's ImageID -// is a local content-addressable hash that differs from the registry manifest digest. -// For multi-arch images, the registry returns a manifest list digest, while Docker -// stores the platform-specific image config digest locally. -func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName string) string { +// getImageRepoDigest retrieves the RepoDigest for an image and its platform details. +// It returns the digest, architecture, OS, and variant. +func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName string) (string, string, string, string) { imageInspect, _, err := a.docker.ImageInspectWithRaw(ctx, imageID) if err != nil { a.logger.Debug(). @@ -399,12 +399,16 @@ func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName strin Str("imageID", imageID). Str("imageName", imageName). Msg("Failed to inspect image for RepoDigest") - return "" + return "", "", "", "" } + arch := imageInspect.Architecture + os := imageInspect.Os + variant := imageInspect.Variant + if len(imageInspect.RepoDigests) == 0 { // Locally built images won't have RepoDigests - return "" + return "", arch, os, variant } // Try to find a RepoDigest that matches the image reference @@ -418,7 +422,7 @@ func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName strin // Check if this RepoDigest matches our image reference // Normalize both for comparison if matchesImageReference(imageName, repoRef) { - return digest + return digest, arch, os, variant } } } @@ -426,10 +430,10 @@ func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName strin // If no exact match, return the first RepoDigest's digest // This handles cases where the image was pulled with a different tag if idx := strings.LastIndex(imageInspect.RepoDigests[0], "@"); idx >= 0 { - return imageInspect.RepoDigests[0][idx+1:] + return imageInspect.RepoDigests[0][idx+1:], arch, os, variant } - return "" + return "", arch, os, variant } // matchesImageReference checks if a RepoDigest repository matches an image reference. diff --git a/internal/dockeragent/registry.go b/internal/dockeragent/registry.go index c1c0c7268..9181b63b8 100644 --- a/internal/dockeragent/registry.go +++ b/internal/dockeragent/registry.go @@ -117,7 +117,7 @@ func (r *RegistryChecker) MarkChecked() { } // CheckImageUpdate checks if a newer version of the image is available. -func (r *RegistryChecker) CheckImageUpdate(ctx context.Context, image, currentDigest string) *ImageUpdateResult { +func (r *RegistryChecker) CheckImageUpdate(ctx context.Context, image, currentDigest, arch, os, variant string) *ImageUpdateResult { if !r.Enabled() { return nil } @@ -136,7 +136,8 @@ func (r *RegistryChecker) CheckImageUpdate(ctx context.Context, image, currentDi } // Check cache first - cacheKey := fmt.Sprintf("%s/%s:%s", registry, repository, tag) + // internal/dockeragent/registry.go + cacheKey := fmt.Sprintf("%s/%s:%s|%s/%s/%s", registry, repository, tag, arch, os, variant) if cached := r.getCached(cacheKey); cached != nil { if cached.err != "" { return &ImageUpdateResult{ @@ -157,7 +158,7 @@ func (r *RegistryChecker) CheckImageUpdate(ctx context.Context, image, currentDi } // Fetch latest digest from registry - latestDigest, err := r.fetchDigest(ctx, registry, repository, tag) + latestDigest, err := r.fetchDigest(ctx, registry, repository, tag, arch, os, variant) if err != nil { // Cache the error to avoid hammering the registry r.cacheError(cacheKey, err.Error()) @@ -203,7 +204,7 @@ func (r *RegistryChecker) digestsDiffer(current, latest string) bool { } // fetchDigest retrieves the digest for an image from the registry. -func (r *RegistryChecker) fetchDigest(ctx context.Context, registry, repository, tag string) (string, error) { +func (r *RegistryChecker) fetchDigest(ctx context.Context, registry, repository, tag, arch, os, variant string) (string, error) { // Get auth token if needed token, err := r.getAuthToken(ctx, registry, repository) if err != nil { @@ -260,6 +261,14 @@ func (r *RegistryChecker) fetchDigest(ctx context.Context, registry, repository, } } + contentType := resp.Header.Get("Content-Type") + isManifestList := strings.Contains(contentType, "manifest.list") || strings.Contains(contentType, "image.index") + + // If it's a manifest list and we have arch info, we need to resolve it + if isManifestList && arch != "" && os != "" { + return r.resolveManifestList(ctx, registry, repository, tag, arch, os, variant, token) + } + if digest == "" { return "", fmt.Errorf("no digest in response") } @@ -267,6 +276,79 @@ func (r *RegistryChecker) fetchDigest(ctx context.Context, registry, repository, return digest, nil } +// resolveManifestList fetches the manifest list and finds the matching digest for the architecture. +func (r *RegistryChecker) resolveManifestList(ctx context.Context, registry, repository, tag, arch, os, variant, token string) (string, error) { + manifestURL := fmt.Sprintf("https://%s/v2/%s/manifests/%s", registry, repository, tag) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, manifestURL, nil) + if err != nil { + return "", fmt.Errorf("create list request: %w", err) + } + + req.Header.Set("Accept", strings.Join([]string{ + "application/vnd.docker.distribution.manifest.list.v2+json", + "application/vnd.docker.distribution.manifest.v2+json", + "application/vnd.oci.image.manifest.v1+json", + "application/vnd.oci.image.index.v1+json", + }, ", ")) + + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + + resp, err := r.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("list request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("fetch manifest list failed: %d", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("read list body: %w", err) + } + + var list manifestList + if err := json.Unmarshal(body, &list); err != nil { + return "", fmt.Errorf("decode manifest list: %w", err) + } + + // Find the matching manifest + // We matched arch and os. Variant is tricky as it's not always passed or available clearly. + // We'll prioritize exact match including variant (if we had it), but for now standard match. + // Since we strictly want to match what the local image is, and we'll get that from ImageInspect. + + // Simple matching logic for now: exact match on Arch and OS + for _, m := range list.Manifests { + if m.Platform.Architecture == arch && m.Platform.OS == os { + if variant != "" && m.Platform.Variant != "" && variant != m.Platform.Variant { + continue + } + return m.Digest, nil + } + } + + return "", fmt.Errorf("no matching manifest found for %s/%s in list", os, arch) +} + +type manifestList struct { + Manifests []manifestDescriptor `json:"manifests"` +} + +type manifestDescriptor struct { + Digest string `json:"digest"` + Platform manifestPlatform `json:"platform"` +} + +type manifestPlatform struct { + Architecture string `json:"architecture"` + OS string `json:"os"` + Variant string `json:"variant,omitempty"` +} + // getAuthToken retrieves an auth token for the registry. func (r *RegistryChecker) getAuthToken(ctx context.Context, registry, repository string) (string, error) { // Docker Hub requires auth token even for public images diff --git a/internal/dockeragent/registry_coverage_test.go b/internal/dockeragent/registry_coverage_test.go index d68273df3..a8ea69454 100644 --- a/internal/dockeragent/registry_coverage_test.go +++ b/internal/dockeragent/registry_coverage_test.go @@ -47,10 +47,10 @@ func TestRegistryChecker_CheckImageUpdate_CacheHits(t *testing.T) { t.Run("cached error", func(t *testing.T) { checker := NewRegistryChecker(logger) - cacheKey := "example.test/repo:tag" + cacheKey := "example.test/repo:tag|//" checker.cacheError(cacheKey, "cached error") - result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current") + result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current", "", "", "") if result == nil { t.Fatal("Expected result for cached error") } @@ -64,10 +64,10 @@ func TestRegistryChecker_CheckImageUpdate_CacheHits(t *testing.T) { t.Run("cached digest", func(t *testing.T) { checker := NewRegistryChecker(logger) - cacheKey := "example.test/repo:tag" + cacheKey := "example.test/repo:tag|//" checker.cacheDigest(cacheKey, "sha256:latest") - result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current") + result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current", "", "", "") if result == nil { t.Fatal("Expected result for cached digest") } @@ -91,7 +91,7 @@ func TestRegistryChecker_CheckImageUpdate_FetchPaths(t *testing.T) { }), } - result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current") + result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current", "", "", "") if result == nil { t.Fatal("Expected result on fetch error") } @@ -99,7 +99,7 @@ func TestRegistryChecker_CheckImageUpdate_FetchPaths(t *testing.T) { t.Fatalf("Expected registry error, got %q", result.Error) } - cacheKey := "example.test/repo:tag" + cacheKey := "example.test/repo:tag|//" cached := checker.getCached(cacheKey) if cached == nil || cached.err != "registry error: 500" { t.Fatalf("Expected cached error to be stored, got %+v", cached) @@ -117,7 +117,7 @@ func TestRegistryChecker_CheckImageUpdate_FetchPaths(t *testing.T) { }), } - result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current") + result := checker.CheckImageUpdate(context.Background(), "example.test/repo:tag", "sha256:current", "", "", "") if result == nil { t.Fatal("Expected result on fetch success") } @@ -128,7 +128,7 @@ func TestRegistryChecker_CheckImageUpdate_FetchPaths(t *testing.T) { t.Fatal("Expected update available for new digest") } - cacheKey := "example.test/repo:tag" + cacheKey := "example.test/repo:tag|//" cached := checker.getCached(cacheKey) if cached == nil || cached.latestDigest != "sha256:latest" { t.Fatalf("Expected cached digest to be stored, got %+v", cached) @@ -200,7 +200,7 @@ func TestRegistryChecker_FetchDigest_StatusErrors(t *testing.T) { }, } - _, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag") + _, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag", "", "", "") if err == nil || err.Error() != tt.wantErr { t.Fatalf("Expected error %q, got %v", tt.wantErr, err) } @@ -220,7 +220,7 @@ func TestRegistryChecker_FetchDigest_RequestError(t *testing.T) { }, } - _, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag") + _, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag", "", "", "") if err == nil || !strings.Contains(err.Error(), "request:") { t.Fatalf("Expected request error, got %v", err) } @@ -229,7 +229,7 @@ func TestRegistryChecker_FetchDigest_RequestError(t *testing.T) { func TestRegistryChecker_FetchDigest_RequestCreationError(t *testing.T) { checker := &RegistryChecker{httpClient: &http.Client{}} - _, err := checker.fetchDigest(context.Background(), "bad host", "repo", "tag") + _, err := checker.fetchDigest(context.Background(), "bad host", "repo", "tag", "", "", "") if err == nil || !strings.Contains(err.Error(), "create request:") { t.Fatalf("Expected create request error, got %v", err) } @@ -257,7 +257,7 @@ func TestRegistryChecker_FetchDigest_DigestHeaders(t *testing.T) { }, } - digest, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag") + digest, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag", "", "", "") if err != nil { t.Fatalf("Expected digest, got error %v", err) } @@ -281,7 +281,7 @@ func TestRegistryChecker_FetchDigest_DigestHeaders(t *testing.T) { }, } - digest, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag") + digest, err := checker.fetchDigest(context.Background(), "example.test", "repo", "tag", "", "", "") if err != nil { t.Fatalf("Expected digest, got error %v", err) } @@ -304,7 +304,7 @@ func TestRegistryChecker_FetchDigest_AuthPaths(t *testing.T) { }, } - _, err := checker.fetchDigest(context.Background(), "registry-1.docker.io", "library/nginx", "latest") + _, err := checker.fetchDigest(context.Background(), "registry-1.docker.io", "library/nginx", "latest", "", "", "") if err == nil || err.Error() != "auth: token request failed: 500" { t.Fatalf("Expected auth error, got %v", err) } @@ -331,7 +331,7 @@ func TestRegistryChecker_FetchDigest_AuthPaths(t *testing.T) { }, } - digest, err := checker.fetchDigest(context.Background(), "registry-1.docker.io", "library/nginx", "latest") + digest, err := checker.fetchDigest(context.Background(), "registry-1.docker.io", "library/nginx", "latest", "", "", "") if err != nil { t.Fatalf("Expected digest, got error %v", err) } diff --git a/internal/dockeragent/registry_http_test.go b/internal/dockeragent/registry_http_test.go index 18f61c246..aaabe0fde 100644 --- a/internal/dockeragent/registry_http_test.go +++ b/internal/dockeragent/registry_http_test.go @@ -16,7 +16,7 @@ func TestRegistryChecker_CheckImageUpdate_Behavior(t *testing.T) { t.Run("disabled checker returns nil", func(t *testing.T) { checker := NewRegistryChecker(logger) checker.SetEnabled(false) - result := checker.CheckImageUpdate(context.Background(), "nginx:latest", "sha256:current") + result := checker.CheckImageUpdate(context.Background(), "nginx:latest", "sha256:current", "", "", "") if result != nil { t.Error("Expected nil result when checker is disabled") } @@ -24,7 +24,7 @@ func TestRegistryChecker_CheckImageUpdate_Behavior(t *testing.T) { t.Run("digest-pinned image skipped", func(t *testing.T) { checker := NewRegistryChecker(logger) - result := checker.CheckImageUpdate(context.Background(), "nginx@sha256:abc123", "sha256:abc123") + result := checker.CheckImageUpdate(context.Background(), "nginx@sha256:abc123", "sha256:abc123", "", "", "") if result == nil { t.Fatal("Expected result for digest-pinned image") } @@ -50,8 +50,8 @@ func TestRegistryChecker_CheckImageUpdate_Behavior(t *testing.T) { } }), } - - result := checker.CheckImageUpdate(context.Background(), "", "sha256:current") + + result := checker.CheckImageUpdate(context.Background(), "", "sha256:current", "", "", "") if result == nil { t.Fatal("Expected result for empty image") } diff --git a/internal/dockeragent/registry_manifest_test.go b/internal/dockeragent/registry_manifest_test.go new file mode 100644 index 000000000..cd922a979 --- /dev/null +++ b/internal/dockeragent/registry_manifest_test.go @@ -0,0 +1,50 @@ +package dockeragent + +import ( + "context" + "net/http" + "testing" + "github.com/rs/zerolog" +) + +func TestRegistryChecker_ResolveManifestList(t *testing.T) { + logger := zerolog.Nop() + t.Run("resolve manifest list", func(t *testing.T) { + checker := NewRegistryChecker(logger) + checker.httpClient = &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.Method == "HEAD" { + return newStringResponse(http.StatusOK, map[string]string{ + "Content-Type": "application/vnd.docker.distribution.manifest.list.v2+json", + }, ""), nil + } + // GET request for body + body := `{ + "manifests": [ + { + "digest": "sha256:armv7", + "platform": { "architecture": "arm", "os": "linux", "variant": "v7" } + }, + { + "digest": "sha256:amd64", + "platform": { "architecture": "amd64", "os": "linux" } + } + ] + }` + return newStringResponse(http.StatusOK, nil, body), nil + }), + } + + // Test matching amd64 + result := checker.CheckImageUpdate(context.Background(), "image:tag", "sha256:current", "amd64", "linux", "") + if result.LatestDigest != "sha256:amd64" { + t.Errorf("Expected sha256:amd64, got %s", result.LatestDigest) + } + + // Test matching arm/v7 + result = checker.CheckImageUpdate(context.Background(), "image:tag", "sha256:current", "arm", "linux", "v7") + if result.LatestDigest != "sha256:armv7" { + t.Errorf("Expected sha256:armv7, got %s", result.LatestDigest) + } + }) +}