diff --git a/pkg/storage/image.go b/pkg/storage/image.go index 5994d952..9df7ffbc 100644 --- a/pkg/storage/image.go +++ b/pkg/storage/image.go @@ -5,6 +5,7 @@ import ( "net" "path" "strings" + "sync" "github.com/containers/image/copy" "github.com/containers/image/docker/reference" @@ -39,6 +40,7 @@ type ImageResult struct { Size *uint64 Digest digest.Digest ConfigDigest digest.Digest + User string } type indexInfo struct { @@ -46,12 +48,23 @@ type indexInfo struct { secure bool } +// A set of information that we prefer to cache about images, so that we can +// avoid having to reread them every time we need to return information about +// images. +type imageCacheItem struct { + user string + size *uint64 + configDigest digest.Digest +} + type imageService struct { store storage.Store defaultTransport string insecureRegistryCIDRs []*net.IPNet indexConfigs map[string]*indexInfo registries []string + imageCache map[string]imageCacheItem + imageCacheLock sync.Mutex } // sizer knows its size. @@ -171,15 +184,38 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s return nil, err } if image, err := istorage.Transport.GetStoreImage(svc.store, ref); err == nil { - img, err := ref.NewImageSource(systemContext) - if err != nil { - return nil, err - } - size := imageSize(img) - configDigest, err := imageConfigDigest(img, nil) - img.Close() - if err != nil { - return nil, err + var user string + var size *uint64 + var configDigest digest.Digest + if cacheItem, ok := svc.imageCache[image.ID]; ok { + user, size, configDigest = cacheItem.user, cacheItem.size, cacheItem.configDigest + } else { + img, err := ref.NewImageSource(systemContext) + if err != nil { + return nil, err + } + size = imageSize(img) + configDigest, err = imageConfigDigest(img, nil) + img.Close() + if err != nil { + return nil, err + } + imageFull, err := ref.NewImage(systemContext) + if err != nil { + return nil, err + } + defer imageFull.Close() + imageConfig, err := imageFull.OCIConfig() + if err != nil { + return nil, err + } + user = imageConfig.Config.User + cacheItem := imageCacheItem{ + user: user, + size: size, + configDigest: configDigest, + } + svc.imageCache[image.ID] = cacheItem } name, tags, digests := sortNamesByType(image.Names) imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID) @@ -191,6 +227,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: user, }) } } else { @@ -198,20 +235,72 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s if err != nil { return nil, err } + visited := make(map[string]struct{}) + defer func() { + // We built a map using IDs of images that we looked + // at, so remove any items from the cache that don't + // correspond to any of those IDs. + removedIDs := make([]string, 0, len(svc.imageCache)) + for imageID := range svc.imageCache { + if _, keep := visited[imageID]; !keep { + // We have cached data for an image + // with this ID, but it's not in the + // list of images now, so the image has + // been removed. + removedIDs = append(removedIDs, imageID) + } + } + // Handle the removals. + svc.imageCacheLock.Lock() + for _, removedID := range removedIDs { + delete(svc.imageCache, removedID) + } + svc.imageCacheLock.Unlock() + }() for _, image := range images { - ref, err := istorage.Transport.ParseStoreReference(svc.store, "@"+image.ID) - if err != nil { - return nil, err - } - img, err := ref.NewImageSource(systemContext) - if err != nil { - return nil, err - } - size := imageSize(img) - configDigest, err := imageConfigDigest(img, nil) - img.Close() - if err != nil { - return nil, err + visited[image.ID] = struct{}{} + var user string + var size *uint64 + var configDigest digest.Digest + svc.imageCacheLock.Lock() + cacheItem, ok := svc.imageCache[image.ID] + svc.imageCacheLock.Unlock() + if ok { + user, size, configDigest = cacheItem.user, cacheItem.size, cacheItem.configDigest + } else { + ref, err := istorage.Transport.ParseStoreReference(svc.store, "@"+image.ID) + if err != nil { + return nil, err + } + img, err := ref.NewImageSource(systemContext) + if err != nil { + return nil, err + } + size = imageSize(img) + configDigest, err = imageConfigDigest(img, nil) + img.Close() + if err != nil { + return nil, err + } + imageFull, err := ref.NewImage(systemContext) + if err != nil { + return nil, err + } + defer imageFull.Close() + + imageConfig, err := imageFull.OCIConfig() + if err != nil { + return nil, err + } + user = imageConfig.Config.User + cacheItem := imageCacheItem{ + user: user, + size: size, + configDigest: configDigest, + } + svc.imageCacheLock.Lock() + svc.imageCache[image.ID] = cacheItem + svc.imageCacheLock.Unlock() } name, tags, digests := sortNamesByType(image.Names) imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID) @@ -223,6 +312,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: user, }) } } @@ -246,6 +336,16 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI if err != nil { return nil, err } + imageFull, err := ref.NewImage(systemContext) + if err != nil { + return nil, err + } + defer imageFull.Close() + + imageConfig, err := imageFull.OCIConfig() + if err != nil { + return nil, err + } img, err := ref.NewImageSource(systemContext) if err != nil { @@ -268,6 +368,7 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: imageConfig.Config.User, } return &result, nil @@ -585,6 +686,7 @@ func GetImageService(store storage.Store, defaultTransport string, insecureRegis indexConfigs: make(map[string]*indexInfo, 0), insecureRegistryCIDRs: make([]*net.IPNet, 0), registries: cleanRegistries, + imageCache: make(map[string]imageCacheItem), } insecureRegistries = append(insecureRegistries, "127.0.0.0/8") diff --git a/server/container_create.go b/server/container_create.go index 2de24f67..07b6ee67 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -449,7 +449,7 @@ func setupContainerUser(specgen *generate.Generator, rootfs string, sc *pb.Linux containerUser := "" // Case 1: run as user is set by kubelet if sc.GetRunAsUser() != nil { - containerUser = strconv.FormatInt(sc.GetRunAsUser().Value, 10) + containerUser = strconv.FormatInt(sc.GetRunAsUser().GetValue(), 10) } else { // Case 2: run as username is set by kubelet userName := sc.GetRunAsUsername() diff --git a/server/image_list.go b/server/image_list.go index bcdc1036..ac75ff9a 100644 --- a/server/image_list.go +++ b/server/image_list.go @@ -31,20 +31,20 @@ func (s *Server) ListImages(ctx context.Context, req *pb.ListImagesRequest) (res } resp = &pb.ListImagesResponse{} for _, result := range results { - if result.Size != nil { - resp.Images = append(resp.Images, &pb.Image{ - Id: result.ID, - RepoTags: result.RepoTags, - RepoDigests: result.RepoDigests, - Size_: *result.Size, - }) - } else { - resp.Images = append(resp.Images, &pb.Image{ - Id: result.ID, - RepoTags: result.RepoTags, - RepoDigests: result.RepoDigests, - }) + resImg := &pb.Image{ + Id: result.ID, + RepoTags: result.RepoTags, + RepoDigests: result.RepoDigests, } + uid, username := getUserFromImage(result.User) + if uid != nil { + resImg.Uid = &pb.Int64Value{Value: *uid} + } + resImg.Username = username + if result.Size != nil { + resImg.Size_ = *result.Size + } + resp.Images = append(resp.Images, resImg) } logrus.Debugf("ListImagesResponse: %+v", resp) return resp, nil diff --git a/server/image_status.go b/server/image_status.go index 4e2e6a0e..842f612d 100644 --- a/server/image_status.go +++ b/server/image_status.go @@ -2,6 +2,8 @@ package server import ( "fmt" + "strconv" + "strings" "time" "github.com/containers/storage" @@ -37,23 +39,62 @@ func (s *Server) ImageStatus(ctx context.Context, req *pb.ImageStatusRequest) (r return nil, err } } - // match just the first registry as that's what kube meant - image = images[0] - status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image) - if err != nil { - if errors.Cause(err) == storage.ErrImageUnknown { - return &pb.ImageStatusResponse{}, nil + var ( + notfound bool + lastErr error + ) + for _, image := range images { + status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image) + if err != nil { + if errors.Cause(err) == storage.ErrImageUnknown { + logrus.Warnf("imageStatus: can't find %s", image) + notfound = true + continue + } + logrus.Warnf("imageStatus: error getting status from %s: %v", image, err) + lastErr = err + continue } - return nil, err + resp = &pb.ImageStatusResponse{ + Image: &pb.Image{ + Id: status.ID, + RepoTags: status.RepoTags, + RepoDigests: status.RepoDigests, + Size_: *status.Size, + }, + } + uid, username := getUserFromImage(status.User) + if uid != nil { + resp.Image.Uid = &pb.Int64Value{Value: *uid} + } + resp.Image.Username = username + break } - resp = &pb.ImageStatusResponse{ - Image: &pb.Image{ - Id: status.ID, - RepoTags: status.RepoTags, - RepoDigests: status.RepoDigests, - Size_: *status.Size, - }, + if lastErr != nil && resp == nil { + return nil, lastErr + } + if notfound && resp == nil { + return &pb.ImageStatusResponse{}, nil } logrus.Debugf("ImageStatusResponse: %+v", resp) return resp, nil } + +// getUserFromImage gets uid or user name of the image user. +// If user is numeric, it will be treated as uid; or else, it is treated as user name. +func getUserFromImage(user string) (*int64, string) { + // return both empty if user is not specified in the image. + if user == "" { + return nil, "" + } + // split instances where the id may contain user:group + user = strings.Split(user, ":")[0] + // user could be either uid or user name. Try to interpret as numeric uid. + uid, err := strconv.ParseInt(user, 10, 64) + if err != nil { + // If user is non numeric, assume it's user name. + return nil, user + } + // If user is a numeric uid. + return &uid, "" +}