Repository: moby/moby
File: daemon/containerd/image_exporter.go (Go)

Code before:
	if err != nil {
		return errors.Wrap(err, "failed to decompress input tar archive")
	}
	defer decompressed.Close()

	pm := i.matchRequestedOrDefault(platforms.OnlyStrict, platform)

	opts := []containerd.ImportOpt{
		containerd.WithImportPlatform(pm),

		// Create an additional image with dangling name for imported images...
		containerd.WithDigestRef(danglingImageName),
		// ... but only if they don't have a name or it's invalid.
		containerd.WithSkipDigestRef(func(nameFromArchive string) bool {
			if nameFromArchive == "" {
				return false
			}

			ref, err := reference.ParseNormalizedNamed(nameFromArchive)
			if err != nil {
				return false
			}

			// Look up if there is an existing image with this name and ensure a dangling image exists.
			if img, err := i.resolveImage(ctx, ref.String()); err != nil && !errdefs.IsNotFound(err) {
				log.G(ctx).WithError(err).Warn("failed to resolve image: %w", err)
			} else {
				if err := i.ensureDanglingImage(ctx, img); err != nil {
					log.G(ctx).WithError(err).Warnf("failed to keep the previous image for %s as dangling", img.Name)
				}
			}
			return true
		}),
	}

	if platform == nil {
		// Allow variants to be missing if no specific platform is requested.
		opts = append(opts, containerd.WithSkipMissing())
	}

	imgs, err := i.client.Import(ctx, decompressed, opts...)
	if err != nil {
		if platform != nil {
			p := platforms.FormatAll(*platform)
			log.G(ctx).WithFields(log.Fields{"error": err, "platform": p}).Debug("failed to import image to containerd")

			// Note: ErrEmptyWalk will not be returned in most cases as
			// index.json will contain a descriptor of the actual OCI index or
			// Docker manifest list, so the walk is never empty.
			// Even in case of a single-platform image, the manifest descriptor
			// doesn't have a platform set, so it won't be filtered out by the

Review comment (suggestion):
Plain `else` would also match the `if err != nil && errdefs.IsNotFound(err)` case - which will create a dangling image if the image is not found.

(that will be solved by applying the previous suggestion 😅)

Code after:
	if err != nil {
		return errors.Wrap(err, "failed to decompress input tar archive")
	}
	defer decompressed.Close()

	pm := i.matchRequestedOrDefault(platforms.OnlyStrict, platform)

	opts := []containerd.ImportOpt{
		containerd.WithImportPlatform(pm),

		// Create an additional image with dangling name for imported images...
		containerd.WithDigestRef(danglingImageName),
		// ... but only if they don't have a name or it's invalid.
		containerd.WithSkipDigestRef(func(nameFromArchive string) bool {
			if nameFromArchive == "" {
				return false
			}

			ref, err := reference.ParseNormalizedNamed(nameFromArchive)
			if err != nil {
				return false
			}

			// Look up if there is an existing image with this name and ensure a dangling image exists.
			if img, err := i.images.Get(ctx, ref.String()); err == nil {
				if err := i.ensureDanglingImage(ctx, img); err != nil {
					log.G(ctx).WithError(err).Warnf("failed to keep the previous image for %s as dangling", img.Name)
				}
			} else if !errdefs.IsNotFound(err) {
				log.G(ctx).WithError(err).Warn("failed to retrieve image: %w", err)
			}
			return true
		}),
	}

	if platform == nil {
		// Allow variants to be missing if no specific platform is requested.
		opts = append(opts, containerd.WithSkipMissing())
	}

	imgs, err := i.client.Import(ctx, decompressed, opts...)
	if err != nil {
		if platform != nil {
			p := platforms.FormatAll(*platform)
			log.G(ctx).WithFields(log.Fields{"error": err, "platform": p}).Debug("failed to import image to containerd")

			// Note: ErrEmptyWalk will not be returned in most cases as
			// index.json will contain a descriptor of the actual OCI index or
			// Docker manifest list, so the walk is never empty.
			// Even in case of a single-platform image, the manifest descriptor
			// doesn't have a platform set, so it won't be filtered out by the