Merge pull request #6363 from swordqiu/hotfix/qj-image-upload-public-scope

fix: set public_scope to system for image with is_public=true

cmd/climc/shell/image/images.go

@@ -34,6 +34,7 @@ type ImageOptionalOptions struct {
 	Unprotected        bool     `help:"Allow image to be deleted"`
 	Standard           bool     `help:"Mark image as a standard image"`
 	Nonstandard        bool     `help:"Mark image as a non-standard image"`
+	Public             bool     `help:"make image public"`
 	MinDisk            int64    `help:"Disk size after expanded, in MB" metavar:"MIN_DISK_SIZE_MB"`
 	MinRam             int64    `help:"Minimal memory size required" metavar:"MIN_RAM_MB"`
 	VirtualSize        int64    `help:"Disk size after expanded, in MB"`
@@ -57,7 +58,6 @@ type ImageOptionalOptions struct {
 
 func addImageOptionalOptions(s *mcclient.ClientSession, params *jsonutils.JSONDict, args ImageOptionalOptions) error {
 	if len(args.Format) > 0 {
-		params.Add(jsonutils.NewString("bare"), "container-format")
 		params.Add(jsonutils.NewString(args.Format), "disk-format")
 	}
 	if args.Protected && !args.Unprotected {
@@ -70,6 +70,9 @@ func addImageOptionalOptions(s *mcclient.ClientSession, params *jsonutils.JSONDi
 	} else if !args.Standard && args.Nonstandard {
 		params.Add(jsonutils.JSONFalse, "is_standard")
 	}
+	if args.Public {
+		params.Add(jsonutils.JSONTrue, "is_public")
+	}
 	if args.MinDisk > 0 {
 		params.Add(jsonutils.NewString(fmt.Sprintf("%d", args.MinDisk)), "min_disk")
 	}
@@ -244,22 +247,6 @@ func init() {
 		return nil
 	})
 
-	type ImageMembershipOptions struct {
-		IMAGE   string `help:"ID or name of image to share"`
-		PROJECT string `help:"ID or name of project to share image with"`
-	}
-	type ImageMembershipAddOptions struct {
-		ImageMembershipOptions
-		CanShare bool `help:"Indicating whether the project can share the image with others"`
-	}
-	R(&ImageMembershipAddOptions{}, "image-add-project", "Add a project to private image's membership list", func(s *mcclient.ClientSession, args *ImageMembershipAddOptions) error {
-		return modules.Images.AddMembership(s, args.IMAGE, args.PROJECT, args.CanShare)
-	})
-
-	R(&ImageMembershipOptions{}, "image-remove-project", "Remove a project from private image's membership list", func(s *mcclient.ClientSession, args *ImageMembershipOptions) error {
-		return modules.Images.RemoveMembership(s, args.IMAGE, args.PROJECT)
-	})
-
 	type ImageDetailOptions struct {
 		ID string `help:"Image ID or name"`
 	}
@@ -278,19 +265,6 @@ func init() {
 		return nil
 	})
 
-	R(&ImageDetailOptions{}, "image-list-project", "List image members", func(s *mcclient.ClientSession, args *ImageDetailOptions) error {
-		imgId, e := modules.Images.GetId(s, args.ID, nil)
-		if e != nil {
-			return e
-		}
-		result, e := modules.Images.ListMemberProjects(s, imgId)
-		if e != nil {
-			return e
-		}
-		printList(result, modules.Projects.GetColumns(s))
-		return nil
-	})
-
 	type ImageCancelDeleteOptions struct {
 		ID string `help:"Image id or name" metavar:"IMAGE"`
 	}

pkg/apis/image/guestimage.go

@@ -26,7 +26,7 @@ type GuestImageDetails struct {
 	apis.SharableVirtualResourceDetails
 	SGuestImage
 
-	ImageIds jsonutils.JSONObject `json:"image_ids"`
+	ImageIds []string `json:"image_ids"`
 
 	//Status     string               `json:"status"`
 	Size       int64          `json:"size"`

pkg/apis/image/image.go

@@ -61,3 +61,27 @@ type ImageDetails struct {
 	DisableDelete bool `json:"disable_delete"`
 	//OssChecksum   string    `json:"oss_checksum"`
 }
+
+type ImageCreateInput struct {
+	apis.SharableVirtualResourceCreateInput
+
+	// 镜像大小, 单位Byte
+	Size *int64 `json:"size"`
+	// 镜像格式
+	DiskFormat string `json:"disk_format"`
+	// 最小系统盘要求
+	MinDiskMB *int32 `json:"min_disk"`
+	// 最小内存要求
+	MinRamMB *int32 `json:"min_ram"`
+	// 是否有删除保护
+	Protected *bool `json:"protected"`
+	// 是否是标准镜像
+	IsStandard *bool `json:"is_standard"`
+	// 是否是主机镜像
+	IsGuestImage *bool `json:"is_guest_image"`
+	// 是否是数据盘镜像
+	IsData *bool `json:"is_data"`
+
+	// 镜像属性
+	Properties map[string]string `json:"properties"`
+}

pkg/cloudcommon/db/sharablebase.go

@@ -153,9 +153,6 @@ func SharableManagerValidateCreateData(
 	query jsonutils.JSONObject,
 	input apis.SharableResourceBaseCreateInput,
 ) (apis.SharableResourceBaseCreateInput, error) {
-	if len(input.PublicScope) == 0 {
-		return input, nil
-	}
 	resScope := manager.ResourceScope()
 	reqScope := resScope
 	isPublic := true
@@ -167,10 +164,10 @@ func SharableManagerValidateCreateData(
 		} else if input.PublicScope == string(rbacutils.ScopeDomain) {
 			input.IsPublic = &isPublic
 			reqScope = rbacutils.ScopeDomain
-		} else if input.IsPublic != nil && *input.IsPublic {
-			// return input, errors.Wrap(httperrors.ErrNotSupported, "project level resource can be shared to domain or system ONLY")
-			input.IsPublic = nil
-			input.PublicScope = string(rbacutils.ScopeNone)
+		} else if input.IsPublic != nil && *input.IsPublic && len(input.PublicScope) == 0 {
+			// backward compatible, if only is_public is true, make it share to system
+			input.IsPublic = &isPublic
+			input.PublicScope = string(rbacutils.ScopeSystem)
 		} else {
 			input.IsPublic = nil
 			input.PublicScope = string(rbacutils.ScopeNone)
@@ -179,10 +176,10 @@ func SharableManagerValidateCreateData(
 		if input.PublicScope == string(rbacutils.ScopeSystem) {
 			input.IsPublic = &isPublic
 			reqScope = rbacutils.ScopeSystem
-		} else if input.IsPublic != nil && *input.IsPublic {
-			// return input, errors.Wrap(httperrors.ErrNotSupported, "domain level resource can be shared to system ONLY")
-			input.IsPublic = nil
-			input.PublicScope = string(rbacutils.ScopeNone)
+		} else if input.IsPublic != nil && *input.IsPublic && len(input.PublicScope) == 0 {
+			// backward compatible, if only is_public is true, make it share to system
+			input.IsPublic = &isPublic
+			input.PublicScope = string(rbacutils.ScopeSystem)
 		} else {
 			input.IsPublic = nil
 			input.PublicScope = string(rbacutils.ScopeNone)

pkg/image/models/image_guest.go

@@ -79,7 +79,7 @@ func (manager *SGuestImageManager) ValidateCreateData(ctx context.Context, userC
 
 	pendingUsage := SQuota{Image: int(imageNum)}
 	data.Set("disk_format", jsonutils.NewString("qcow2"))
-	keys := imageCreateInput2QuotaKeys(data, ownerId)
+	keys := imageCreateInput2QuotaKeys("qcow2", ownerId)
 	pendingUsage.SetKeys(keys)
 	if err := quotas.CheckSetPendingQuota(ctx, userCred, &pendingUsage); err != nil {
 
@@ -114,9 +114,8 @@ func (gi *SGuestImage) PostCreate(ctx context.Context, userCred mcclient.TokenCr
 	}
 	images, _ := kwargs.GetArray("images")
 	kwargs.Remove("images")
-	kwargs.Add(jsonutils.NewString(gi.Id), "guest_image_id")
+	// kwargs.Add(jsonutils.NewString(gi.Id), "guest_image_id")
 
-	imageIds := jsonutils.NewArray()
 	suc := true
 
 	// HACK
@@ -130,6 +129,7 @@ func (gi *SGuestImage) PostCreate(ctx context.Context, userCred mcclient.TokenCr
 			tmp, _ := image.Get(key)
 			params.Add(tmp, key)
 		}
+		params.Add(jsonutils.JSONTrue, "is_guest_image")
 		if i == len(images)-1 {
 			params.Add(jsonutils.NewString(fmt.Sprintf("%s-%s", gi.Name, "root")), "generate_name")
 		} else {
@@ -138,7 +138,6 @@ func (gi *SGuestImage) PostCreate(ctx context.Context, userCred mcclient.TokenCr
 		}
 		model, err := db.DoCreate(ImageManager, ctx, userCred, query, params, ownerId)
 		if err != nil {
-			imageIds.Add(jsonutils.NewString(""))
 			suc = false
 			break
 		} else {
@@ -148,12 +147,15 @@ func (gi *SGuestImage) PostCreate(ctx context.Context, userCred mcclient.TokenCr
 
 				model.PostCreate(ctx, userCred, ownerId, query, data)
 			}()
-			imageIds.Add(jsonutils.NewString(model.GetId()))
+			_, err := GuestImageJointManager.CreateGuestImageJoint(ctx, gi.Id, model.GetId())
+			if err != nil {
+				model.(*SImage).OnJointFailed(ctx, userCred)
+			}
 		}
 	}
 
 	pendingUsage := SQuota{Image: int(imageNumber)}
-	keys := imageCreateInput2QuotaKeys(data, ownerId)
+	keys := imageCreateInput2QuotaKeys("qcow2", ownerId)
 	pendingUsage.SetKeys(keys)
 	quotas.CancelPendingUsage(ctx, userCred, &pendingUsage, &pendingUsage, true)
 
@@ -162,9 +164,6 @@ func (gi *SGuestImage) PostCreate(ctx context.Context, userCred mcclient.TokenCr
 	}
 
 	gi.SetStatus(userCred, api.IMAGE_STATUS_SAVING, "")
-	// HACK
-	tmp := query.(*jsonutils.JSONDict)
-	tmp.Add(imageIds, "image_ids")
 }
 
 func (gi *SGuestImage) ValidateDeleteCondition(ctx context.Context) error {
@@ -276,10 +275,6 @@ func (gi *SGuestImage) DoCancelPendingDelete(ctx context.Context, userCred mccli
 func (self *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject,
 	out api.GuestImageDetails) api.GuestImageDetails {
 
-	if query.Contains("image_ids") {
-		out.ImageIds, _ = query.Get("image_ids")
-	}
-
 	if self.Status != api.IMAGE_STATUS_ACTIVE {
 		self.checkStatus(ctx, userCred)
 		out.Status = self.Status
@@ -297,6 +292,7 @@ func (self *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.T
 	var rootImage api.SubImageInfo
 	for i := range images {
 		image := images[i]
+		out.ImageIds = append(out.ImageIds, image.Id)
 		size += image.Size
 		if !image.IsData.IsTrue() {
 			rootImage = api.SubImageInfo{

pkg/image/models/image_guest_joint.go

@@ -52,7 +52,7 @@ func init() {
 }
 
 func (gm *SGuestImageJointManager) GetByGuestImageId(guestImageId string) ([]SGuestImageJoint, error) {
-	q := gm.Query().Equals("guest_image_id", guestImageId)
+	q := gm.Query().Equals("guest_image_id", guestImageId).Asc("row_id") // order by row_id ascending
 	ret := make([]SGuestImageJoint, 0, 1)
 	err := db.FetchModelObjects(gm, q, &ret)
 	if err != nil {

pkg/image/models/images.go

@@ -150,7 +150,8 @@ func (manager *SImageManager) CustomizeHandlerInfo(info *appsrv.SHandlerInfo) {
 }
 
 func (manager *SImageManager) FetchCreateHeaderData(ctx context.Context, header http.Header) (jsonutils.JSONObject, error) {
-	return modules.FetchImageMeta(header), nil
+	data := modules.FetchImageMeta(header)
+	return data, nil
 }
 
 func (manager *SImageManager) FetchUpdateHeaderData(ctx context.Context, header http.Header) (jsonutils.JSONObject, error) {
@@ -372,31 +373,26 @@ func (self *SImage) GetExtraDetailsHeaders(ctx context.Context, userCred mcclien
 	return headers
 }
 
-func (manager *SImageManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
-	input := apis.SharableVirtualResourceCreateInput{}
-	err := data.Unmarshal(&input)
+func (manager *SImageManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, input api.ImageCreateInput) (api.ImageCreateInput, error) {
+	var err error
+	input.SharableVirtualResourceCreateInput, err = manager.SSharableVirtualResourceBaseManager.ValidateCreateData(ctx, userCred, ownerId, query, input.SharableVirtualResourceCreateInput)
 	if err != nil {
-		return nil, httperrors.NewInternalServerError("unmarshal StandaloneResourceCreateInput fail %s", err)
+		return input, errors.Wrap(err, "SSharableVirtualResourceBaseManager.ValidateCreateData")
 	}
-	input, err = manager.SSharableVirtualResourceBaseManager.ValidateCreateData(ctx, userCred, ownerId, query, input)
-	if err != nil {
-		return nil, err
-	}
-	data.Update(jsonutils.Marshal(input))
 
 	// If this image is the part of guest image (contains "guest_image_id"),
 	// we do not need to check and set pending quota
 	// because that pending quota has been checked and set in SGuestImage.ValidateCreateData
-	if !data.Contains("guest_image_id") {
+	if input.IsGuestImage == nil || !*input.IsGuestImage {
 		pendingUsage := SQuota{Image: 1}
-		keys := imageCreateInput2QuotaKeys(data, ownerId)
+		keys := imageCreateInput2QuotaKeys(input.DiskFormat, ownerId)
 		pendingUsage.SetKeys(keys)
 		if err := quotas.CheckSetPendingQuota(ctx, userCred, &pendingUsage); err != nil {
-			return nil, httperrors.NewOutOfQuotaError("%s", err)
+			return input, httperrors.NewOutOfQuotaError("%s", err)
 		}
 	}
 
-	return data, nil
+	return input, nil
 }
 
 func (self *SImage) CustomizeCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) error {
@@ -406,10 +402,6 @@ func (self *SImage) CustomizeCreate(ctx context.Context, userCred mcclient.Token
 	}
 	self.Status = api.IMAGE_STATUS_QUEUED
 	self.Owner = self.ProjectId
-	// if belong to a guest image,
-	if data.Contains("guest_image_id") {
-		self.IsGuestImage = tristate.True
-	}
 	return nil
 }
 
@@ -421,9 +413,17 @@ func (self *SImage) GetPath(format string) string {
 	return path
 }
 
+func (self *SImage) unprotectImage() {
+	db.Update(self, func() error {
+		self.Protected = tristate.False
+		return nil
+	})
+}
+
 func (self *SImage) OnJointFailed(ctx context.Context, userCred mcclient.TokenCredential) {
 	log.Errorf("create joint of image and guest image failed")
 	self.SetStatus(userCred, api.IMAGE_STATUS_KILLED, "")
+	self.unprotectImage()
 }
 
 func (self *SImage) OnSaveFailed(ctx context.Context, userCred mcclient.TokenCredential, msg string) {
@@ -455,6 +455,7 @@ func (self *SImage) saveSuccess(userCred mcclient.TokenCredential, msg string) {
 func (self *SImage) saveFailed(userCred mcclient.TokenCredential, msg string) {
 	log.Errorf(msg)
 	self.SetStatus(userCred, api.IMAGE_STATUS_KILLED, msg)
+	self.unprotectImage()
 	db.OpsLog.LogEvent(self, db.ACT_SAVE_FAIL, msg, userCred)
 }
 
@@ -537,7 +538,7 @@ func (self *SImage) PostCreate(ctx context.Context, userCred mcclient.TokenCrede
 	// if SImage belong to a guest image, pending quota will not be set.
 	if self.IsGuestImage.IsFalse() {
 		pendingUsage := SQuota{Image: 1}
-		keys := imageCreateInput2QuotaKeys(data, ownerId)
+		keys := imageCreateInput2QuotaKeys(self.DiskFormat, ownerId)
 		pendingUsage.SetKeys(keys)
 		cancelUsage := SQuota{Image: 1}
 		keys = self.GetQuotaKeys()
@@ -574,15 +575,6 @@ func (self *SImage) PostCreate(ctx context.Context, userCred mcclient.TokenCrede
 		}
 	}
 
-	// This image is belong to some guestImage.
-	// Code below must be run without upload.
-	if data.Contains("guest_image_id") {
-		guestImageId, _ := data.GetString("guest_image_id")
-		_, err := GuestImageJointManager.CreateGuestImageJoint(ctx, guestImageId, self.Id)
-		if err != nil {
-			self.OnJointFailed(ctx, userCred)
-		}
-	}
 }
 
 // After image probe and customization, image size and checksum changed
@@ -1420,10 +1412,9 @@ func (img *SImage) GetQuotaKeys() quotas.IQuotaKeys {
 	return keys
 }
 
-func imageCreateInput2QuotaKeys(data jsonutils.JSONObject, ownerId mcclient.IIdentityProvider) quotas.IQuotaKeys {
+func imageCreateInput2QuotaKeys(format string, ownerId mcclient.IIdentityProvider) quotas.IQuotaKeys {
 	keys := SImageQuotaKeys{}
 	keys.SBaseProjectQuotaKeys = quotas.OwnerIdProjectQuotaKeys(rbacutils.ScopeProject, ownerId)
-	format, _ := data.GetString("disk_format")
 	if format == string(api.ImageTypeISO) {
 		keys.Type = string(api.ImageTypeISO)
 	} else if len(format) > 0 {

pkg/mcclient/modules/mod_images.go

@@ -423,24 +423,6 @@ func (this *ImageManager) Upload(s *mcclient.ClientSession, params jsonutils.JSO
 	return this._create(s, params, body, size)
 }
 
-func (this *ImageManager) IsNameDuplicate(s *mcclient.ClientSession, name string) (bool, error) {
-	dupName := true
-	_, e := this.GetByName(s, name, nil)
-	if e != nil {
-		switch e.(type) {
-		case *httputils.JSONClientError:
-			je := e.(*httputils.JSONClientError)
-			if je.Code == 404 {
-				dupName = false
-			}
-		default:
-			log.Errorf("GetByName fail %s", e)
-			return false, e
-		}
-	}
-	return dupName, nil
-}
-
 func (this *ImageManager) _create(s *mcclient.ClientSession, params jsonutils.JSONObject, body io.Reader, size int64) (jsonutils.JSONObject, error) {
 	/*format, _ := params.GetString("disk-format")
 	if len(format) == 0 {
@@ -461,16 +443,9 @@ func (this *ImageManager) _create(s *mcclient.ClientSession, params jsonutils.JS
 		if len(name) == 0 {
 			return nil, httperrors.NewMissingParameterError("name")
 		}
-		dupName, e := this.IsNameDuplicate(s, name)
-		if dupName {
-			return nil, httperrors.NewDuplicateNameError("name", name)
-		}
-		if e != nil {
-			return nil, fmt.Errorf("Check name duplicate error %s", e)
-		}
 	} else {
 		path = fmt.Sprintf("/%s/%s", this.URLPath(), imageId)
-		method = "PUT"
+		method = httputils.PUT
 	}
 	headers, e := setImageMeta(params)
 	if e != nil {
@@ -485,7 +460,6 @@ func (this *ImageManager) _create(s *mcclient.ClientSession, params jsonutils.JS
 		size = 0
 		headers.Set(IMAGE_META_COPY_FROM, copyFromUrl)
 	}
-	headers.Set(fmt.Sprintf("%s%s", IMAGE_META, utils.Capitalize("container-format")), "bare")
 	if body != nil {
 		headers.Add("Content-Type", "application/octet-stream")
 		if size > 0 {