Skip to content

Commit 8a912a1

Browse files
Merge pull request #582 from openshift-cherrypick-robot/cherry-pick-559-to-release-4.19
[release-4.19] OCPBUGS-65962: pkg/image: conditionally parse raw image manifest
2 parents 101bce5 + e5b8672 commit 8a912a1

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 := make([]imageapi.ImageManifest, 0, len(mlist.Manifests))
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)