Skip to content

Commit 11edcfa

Browse files
committed
pkg/image: conditionally parse raw image manifest
the Image REST strategy logs errors when it cannot parse the raw json manifest when creating an Image object. however, this is not an error, as a call to this api might have already cleared the DockerImageManifest from the Image object.
1 parent e817f84 commit 11edcfa

File tree

1 file changed

+31
-19
lines changed

1 file changed

+31
-19
lines changed

pkg/image/apiserver/registry/image/strategy.go

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,28 +52,40 @@ func (s imageStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object)
5252
utilruntime.HandleError(fmt.Errorf("unable to update image metadata for %q: %v", newImage.Name, err))
5353
}
5454

55-
manifest, _, err := distribution.UnmarshalManifest(
56-
newImage.DockerImageManifestMediaType,
57-
[]byte(newImage.DockerImageManifest),
58-
)
59-
if err != nil {
60-
utilruntime.HandleError(fmt.Errorf("unable to parse manifest for %q: %v", newImage.Name, err))
61-
}
55+
// initially, the code in this function did not account for these
56+
// different contexts.
57+
// the code in this function is called from two contexts:
58+
// 1. image stream imports, which are also triggered by creating image
59+
// streams and adding tags to them;
60+
// 2. direct pushes to the internal registry, i.e. podman push
61+
// on 1. the raw manifest json is cleared from image.DockerImageManifest
62+
// to avoid unbound growth of Image objects in etcd; and on 2. the raw
63+
// manifest json is still set in the image.DockerImageManifest.
64+
// the code block below should not cause side effects on either path.
65+
if len(newImage.DockerImageManifest) != 0 {
66+
manifest, _, err := distribution.UnmarshalManifest(
67+
newImage.DockerImageManifestMediaType,
68+
[]byte(newImage.DockerImageManifest),
69+
)
70+
if err != nil {
71+
utilruntime.HandleError(fmt.Errorf("unable to parse manifest for %q: %v", newImage.Name, err))
72+
}
6273

63-
if mlist, ok := manifest.(*manifestlist.DeserializedManifestList); ok {
64-
subManifests := []imageapi.ImageManifest{}
65-
for _, m := range mlist.Manifests {
66-
im := imageapi.ImageManifest{
67-
Digest: m.Digest.String(),
68-
MediaType: m.MediaType,
69-
ManifestSize: m.Size,
70-
Architecture: m.Platform.Architecture,
71-
OS: m.Platform.OS,
72-
Variant: m.Platform.Variant,
74+
if mlist, ok := manifest.(*manifestlist.DeserializedManifestList); ok {
75+
subManifests := []imageapi.ImageManifest{}
76+
for _, m := range mlist.Manifests {
77+
im := imageapi.ImageManifest{
78+
Digest: m.Digest.String(),
79+
MediaType: m.MediaType,
80+
ManifestSize: m.Size,
81+
Architecture: m.Platform.Architecture,
82+
OS: m.Platform.OS,
83+
Variant: m.Platform.Variant,
84+
}
85+
subManifests = append(subManifests, im)
7386
}
74-
subManifests = append(subManifests, im)
87+
newImage.DockerImageManifests = subManifests
7588
}
76-
newImage.DockerImageManifests = subManifests
7789
}
7890

7991
if newImage.Annotations[imagev1.ImageManifestBlobStoredAnnotation] == "true" {

0 commit comments

Comments
 (0)