From 96fb47213efc62fd6b01a11375a388763f767161 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Wed, 14 Feb 2018 13:16:55 +0100 Subject: [PATCH] container_create: correctly set user We had a bug in ImageStatus where we weren't returning the default image user if set, thus running all containers as root despite a user being set in the image config. We weren't populating the Username field of ImageStatus. This patch fixes that along with the handling of multiple images based on the registry patch for multiple images. It also fixes ListImages to return Username as well. Signed-off-by: Antonio Murdaca --- pkg/storage/image.go | 34 +++++++++++++++++++ server/container_create.go | 2 +- server/image_list.go | 26 +++++++------- server/image_status.go | 69 ++++++++++++++++++++++++++++++-------- 4 files changed, 103 insertions(+), 28 deletions(-) diff --git a/pkg/storage/image.go b/pkg/storage/image.go index 5994d952..a94f95ad 100644 --- a/pkg/storage/image.go +++ b/pkg/storage/image.go @@ -39,6 +39,7 @@ type ImageResult struct { Size *uint64 Digest digest.Digest ConfigDigest digest.Digest + User string } type indexInfo struct { @@ -181,6 +182,16 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s 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 + } name, tags, digests := sortNamesByType(image.Names) imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID) results = append(results, ImageResult{ @@ -191,6 +202,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: imageConfig.Config.User, }) } } else { @@ -213,6 +225,16 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s 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 + } name, tags, digests := sortNamesByType(image.Names) imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID) results = append(results, ImageResult{ @@ -223,6 +245,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: imageConfig.Config.User, }) } } @@ -246,6 +269,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 +301,7 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI Size: size, Digest: imageDigest, ConfigDigest: configDigest, + User: imageConfig.Config.User, } return &result, nil 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, "" +}