fix(cloudid): cloud policy sync (#21173)

go.mod

@@ -89,12 +89,12 @@ require (
 	k8s.io/cluster-bootstrap v0.19.3
 	k8s.io/cri-api v0.22.17
 	moul.io/http2curl/v2 v2.3.0
-	yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240827090454-97d9840e0939
+	yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240906024748-a9bd6344aaf8
 	yunion.io/x/executor v0.0.0-20230705125604-c5ac3141db32
 	yunion.io/x/jsonutils v1.0.1-0.20240203102553-4096f103b401
 	yunion.io/x/log v1.0.1-0.20240305175729-7cf2d6cd5a91
 	yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900
-	yunion.io/x/pkg v1.10.1-0.20240826001854-ac73b70d75b7
+	yunion.io/x/pkg v1.10.1-0.20240905110705-77c46e716318
 	yunion.io/x/s3cli v0.0.0-20190917004522-13ac36d8687e
 	yunion.io/x/sqlchemy v1.1.3-0.20240831153043-4030cea8d4b9
 	yunion.io/x/structarg v0.0.0-20231017124457-df4d5009457c

go.sum

@@ -1214,8 +1214,8 @@ sigs.k8s.io/structured-merge-diff/v4 v4.0.1/go.mod h1:bJZC9H9iH24zzfZ/41RGcq60oK
 sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
 sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
 sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
-yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240827090454-97d9840e0939 h1:U30w5KcxiGXUnjCEauW7Wnf459TfwcxgitvDTRKzLMA=
-yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240827090454-97d9840e0939/go.mod h1:iLoBHVR2Eur/1WJSGcbZaEwpzh/iqXvbFCsX9/xt8CI=
+yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240906024748-a9bd6344aaf8 h1:lSB6Gm2QmAYZozENmA1AjwbM3/nQaEA6F3ZSVDhC+rw=
+yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240906024748-a9bd6344aaf8/go.mod h1:iLoBHVR2Eur/1WJSGcbZaEwpzh/iqXvbFCsX9/xt8CI=
 yunion.io/x/executor v0.0.0-20230705125604-c5ac3141db32 h1:v7POYkQwo1XzOxBoIoRVr/k0V9Y5JyjpshlIFa9raug=
 yunion.io/x/executor v0.0.0-20230705125604-c5ac3141db32/go.mod h1:Uxuou9WQIeJXNpy7t2fPLL0BYLvLiMvGQwY7Qc6aSws=
 yunion.io/x/jsonutils v0.0.0-20190625054549-a964e1e8a051/go.mod h1:4N0/RVzsYL3kH3WE/H1BjUQdFiWu50JGCFQuuy+Z634=
@@ -1229,8 +1229,8 @@ yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900 h1:Hu/4ERvoWaN6aiFs4h4/yvVB
 yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900/go.mod h1:0vLkNEhlmA64HViPBAnSTUMrx5QP1CLsxXmxDKQ80tc=
 yunion.io/x/pkg v0.0.0-20190620104149-945c25821dbf/go.mod h1:t6rEGG2sQ4J7DhFxSZVOTjNd0YO/KlfWQyK1W4tog+E=
 yunion.io/x/pkg v0.0.0-20190628082551-f4033ba2ea30/go.mod h1:t6rEGG2sQ4J7DhFxSZVOTjNd0YO/KlfWQyK1W4tog+E=
-yunion.io/x/pkg v1.10.1-0.20240826001854-ac73b70d75b7 h1:ceNLYp6CKYccbFw/ug9ohSaUbEk5THipaA4K+SAHEKc=
-yunion.io/x/pkg v1.10.1-0.20240826001854-ac73b70d75b7/go.mod h1:0Bwxqd9MA3ACi119/l02FprY/o9gHahmYC2bsSbnVpM=
+yunion.io/x/pkg v1.10.1-0.20240905110705-77c46e716318 h1:Fm7I8ypXHxeObY4u/VUGz78NsambemzTZ9fECyGKNi8=
+yunion.io/x/pkg v1.10.1-0.20240905110705-77c46e716318/go.mod h1:0Bwxqd9MA3ACi119/l02FprY/o9gHahmYC2bsSbnVpM=
 yunion.io/x/s3cli v0.0.0-20190917004522-13ac36d8687e h1:v+EzIadodSwkdZ/7bremd7J8J50Cise/HCylsOJngmo=
 yunion.io/x/s3cli v0.0.0-20190917004522-13ac36d8687e/go.mod h1:0iFKpOs1y4lbCxeOmq3Xx/0AcQoewVPwj62eRluioEo=
 yunion.io/x/sqlchemy v1.1.3-0.20240831153043-4030cea8d4b9 h1:vJSHj5jalKW7Vx5dqPmdI/jmpqMxR6nqHHtZSTQUZYM=

pkg/cloudid/drivers/base.go

@@ -247,8 +247,8 @@ func (base SProviderBaseProviderDriver) RequestSyncCloudproviderResources(ctx co
 	}()
 
 	func() {
-		lockman.LockRawObject(ctx, account.Id, models.SAMLProviderManager.Keyword())
-		defer lockman.ReleaseRawObject(ctx, account.Id, models.SAMLProviderManager.Keyword())
+		lockman.LockRawObject(ctx, cp.Id, models.SAMLProviderManager.Keyword())
+		defer lockman.ReleaseRawObject(ctx, cp.Id, models.SAMLProviderManager.Keyword())
 
 		samls, err := provider.GetICloudSAMLProviders()
 		if err != nil {
@@ -456,8 +456,8 @@ func (base SProviderBaseProviderDriver) RequestCreateSAMLProvider(ctx context.Co
 	}
 	for i := range providers {
 		err = func() error {
-			lockman.LockRawObject(ctx, account.Id, models.SAMLProviderManager.Keyword())
-			defer lockman.ReleaseRawObject(ctx, account.Id, models.SAMLProviderManager.Keyword())
+			lockman.LockRawObject(ctx, providers[i].Id, models.SAMLProviderManager.Keyword())
+			defer lockman.ReleaseRawObject(ctx, providers[i].Id, models.SAMLProviderManager.Keyword())
 
 			samlProviders, err := providers[i].GetSamlProviders()
 			if err != nil {
@@ -589,7 +589,7 @@ func (base SProviderBaseProviderDriver) RequestCreateRoleForSamlUser(ctx context
 		return errors.Wrapf(err, "GetProvider")
 	}
 	opts := &cloudprovider.SRoleCreateOptions{
-		Name:         fmt.Sprintf("%s-%s", user.Name, group.Name),
+		Name:         fmt.Sprintf("%s-%s", group.Name, utils.GenRequestId(5)),
 		Desc:         fmt.Sprintf("auto create by cloudpods"),
 		SAMLProvider: samlProvider.ExternalId,
 	}

pkg/cloudid/models/cloudaccount.go

@@ -27,7 +27,6 @@ import (
 	"yunion.io/x/pkg/tristate"
 
 	"yunion.io/x/onecloud/pkg/apis"
-	api "yunion.io/x/onecloud/pkg/apis/cloudid"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
 	"yunion.io/x/onecloud/pkg/cloudid/options"
@@ -109,31 +108,8 @@ func (self *SCloudaccount) GetDriver() (IProviderDriver, error) {
 	return GetProviderDriver(self.Provider)
 }
 
-func (self *SCloudaccount) GetCloudpolicies() ([]SCloudpolicy, error) {
+func (self *SCloudaccount) GetCloudpolicies(managerId string) ([]SCloudpolicy, error) {
 	q := CloudpolicyManager.Query().Equals("cloudaccount_id", self.Id)
-	policies := []SCloudpolicy{}
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
-func (self *SCloudaccount) GetSystemCloudpolicies(managerId string) ([]SCloudpolicy, error) {
-	q := CloudpolicyManager.Query().Equals("cloudaccount_id", self.Id).Equals("policy_type", api.CLOUD_POLICY_TYPE_SYSTEM)
-	if len(managerId) > 0 {
-		q = q.Equals("manager_id", managerId)
-	}
-	policies := []SCloudpolicy{}
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
-func (self *SCloudaccount) GetCustomCloudpolicies(managerId string) ([]SCloudpolicy, error) {
-	q := CloudpolicyManager.Query().Equals("cloudaccount_id", self.Id).Equals("policy_type", api.CLOUD_POLICY_TYPE_CUSTOM)
 	if len(managerId) > 0 {
 		q = q.Equals("manager_id", managerId)
 	}

pkg/cloudid/models/cloudgroup.go

@@ -17,6 +17,7 @@ package models
 import (
 	"context"
 	"database/sql"
+	"fmt"
 
 	"gopkg.in/fatih/set.v0"
 
@@ -382,26 +383,6 @@ func (self *SCloudgroup) GetCloudpolicies() ([]SCloudpolicy, error) {
 	return policies, nil
 }
 
-func (self *SCloudgroup) GetSystemCloudpolicies() ([]SCloudpolicy, error) {
-	policies := []SCloudpolicy{}
-	q := self.GetCloudpolicyQuery().Equals("policy_type", api.CLOUD_POLICY_TYPE_SYSTEM)
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
-func (self *SCloudgroup) GetCustomCloudpolicies() ([]SCloudpolicy, error) {
-	policies := []SCloudpolicy{}
-	q := self.GetCloudpolicyQuery().Equals("policy_type", api.CLOUD_POLICY_TYPE_CUSTOM)
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
 func (self *SCloudgroup) GetCloudpolicy(policyId string) (*SCloudpolicy, error) {
 	policies := []SCloudpolicy{}
 	q := self.GetCloudpolicyQuery().Equals("id", policyId)
@@ -773,8 +754,8 @@ func (self *SCloudgroup) attachPolicy(policyId string) error {
 }
 
 func (self *SCloudaccount) SyncCloudgroups(ctx context.Context, userCred mcclient.TokenCredential, iGroups []cloudprovider.ICloudgroup, managerId string) ([]SCloudgroup, []cloudprovider.ICloudgroup, compare.SyncResult) {
-	lockman.LockRawObject(ctx, CloudgroupManager.Keyword(), self.Id)
-	defer lockman.ReleaseRawObject(ctx, CloudgroupManager.Keyword(), self.Id)
+	lockman.LockRawObject(ctx, CloudgroupManager.Keyword(), fmt.Sprintf("%s-%s", self.Id, managerId))
+	defer lockman.ReleaseRawObject(ctx, CloudgroupManager.Keyword(), fmt.Sprintf("%s-%s", self.Id, managerId))
 
 	result := compare.SyncResult{}
 	dbGroups, err := self.GetCloudgroups(managerId)
@@ -889,9 +870,6 @@ func (group *SCloudgroup) SyncWithCloudgroup(ctx context.Context, userCred mccli
 }
 
 func (self *SCloudaccount) newCloudgroup(ctx context.Context, userCred mcclient.TokenCredential, iGroup cloudprovider.ICloudgroup, managerId string) (*SCloudgroup, error) {
-	lockman.LockObject(ctx, self)
-	defer lockman.ReleaseObject(ctx, self)
-
 	group := &SCloudgroup{}
 	group.SetModelManager(CloudgroupManager, group)
 	group.Name = iGroup.GetName()
@@ -1016,7 +994,7 @@ func (self *SCloudgroup) SyncPolicies(ctx context.Context, userCred mcclient.Tok
 			return q.Equals("cloudaccount_id", self.CloudaccountId)
 		})
 		if err != nil {
-			result.AddError(errors.Wrapf(err, "add %s", added[i].GetName()))
+			result.AddError(errors.Wrapf(err, "add %s(%s)", added[i].GetName(), added[i].GetGlobalId()))
 			continue
 		}
 		err = self.attachPolicy(policy.GetId())

pkg/cloudid/models/cloudpolicy.go

@@ -313,14 +313,18 @@ func (manager *SCloudpolicyManager) FetchCustomizeColumns(
 func (self *SCloudpolicy) SyncWithCloudpolicy(ctx context.Context, userCred mcclient.TokenCredential, iPolicy cloudprovider.ICloudpolicy) error {
 	_, err := db.Update(self, func() error {
 		self.Name = iPolicy.GetName()
-		self.Description = iPolicy.GetDescription()
+		if self.PolicyType == api.CLOUD_POLICY_TYPE_CUSTOM || len(self.Description) == 0 {
+			self.Description = iPolicy.GetDescription()
+		}
 		self.Status = apis.STATUS_AVAILABLE
 		self.IsPublic = true
-		doc, err := iPolicy.GetDocument()
-		if err != nil {
-			return errors.Wrapf(err, "GetDocument")
+		if self.PolicyType == api.CLOUD_POLICY_TYPE_CUSTOM || gotypes.IsNil(self.Document) {
+			doc, err := iPolicy.GetDocument()
+			if err != nil {
+				return errors.Wrapf(err, "GetDocument")
+			}
+			self.Document = doc
 		}
-		self.Document = doc
 		return nil
 	})
 	if err != nil {
@@ -330,9 +334,6 @@ func (self *SCloudpolicy) SyncWithCloudpolicy(ctx context.Context, userCred mccl
 }
 
 func (self *SCloudaccount) newCloudpolicy(ctx context.Context, userCred mcclient.TokenCredential, iPolicy cloudprovider.ICloudpolicy, managerId string) (*SCloudpolicy, error) {
-	lockman.LockObject(ctx, self)
-	defer lockman.ReleaseObject(ctx, self)
-
 	policy := &SCloudpolicy{}
 	policy.SetModelManager(CloudpolicyManager, policy)
 	doc, err := iPolicy.GetDocument()
@@ -352,14 +353,17 @@ func (self *SCloudaccount) newCloudpolicy(ctx context.Context, userCred mcclient
 }
 
 func (self *SCloudaccount) SyncPolicies(ctx context.Context, userCred mcclient.TokenCredential, iPolicies []cloudprovider.ICloudpolicy, managerId string) compare.SyncResult {
+	lockman.LockRawObject(ctx, CloudproviderManager.Keyword(), managerId)
+	defer lockman.ReleaseRawObject(ctx, CloudproviderManager.Keyword(), managerId)
+
 	result := compare.SyncResult{}
 
 	removed := make([]SCloudpolicy, 0)
 	commondb := make([]SCloudpolicy, 0)
 	commonext := make([]cloudprovider.ICloudpolicy, 0)
 	added := make([]cloudprovider.ICloudpolicy, 0)
 
-	dbPolicies, err := self.GetCloudpolicies()
+	dbPolicies, err := self.GetCloudpolicies(managerId)
 	if err != nil {
 		result.Error(errors.Wrapf(err, "GetCloudpolicies"))
 		return result

pkg/cloudid/models/clouduser.go

@@ -17,6 +17,7 @@ package models
 import (
 	"context"
 	"database/sql"
+	"fmt"
 
 	"gopkg.in/fatih/set.v0"
 
@@ -669,26 +670,6 @@ func (self *SClouduser) GetCloudpolicies() ([]SCloudpolicy, error) {
 	return policies, nil
 }
 
-func (self *SClouduser) GetSystemCloudpolicies() ([]SCloudpolicy, error) {
-	policies := []SCloudpolicy{}
-	q := self.GetCloudpolicyQuery().Equals("policy_type", api.CLOUD_POLICY_TYPE_SYSTEM)
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
-func (self *SClouduser) GetCustomCloudpolicies() ([]SCloudpolicy, error) {
-	policies := []SCloudpolicy{}
-	q := self.GetCloudpolicyQuery().Equals("policy_type", api.CLOUD_POLICY_TYPE_CUSTOM)
-	err := db.FetchModelObjects(CloudpolicyManager, q, &policies)
-	if err != nil {
-		return nil, errors.Wrap(err, "db.FetchModelObjects")
-	}
-	return policies, nil
-}
-
 func (self *SClouduser) joinGroup(groupId string) error {
 	gu := &SCloudgroupUser{}
 	gu.SetModelManager(CloudgroupUserManager, gu)
@@ -1142,8 +1123,8 @@ func (self *SCloudaccount) SyncCloudusers(
 	iUsers []cloudprovider.IClouduser,
 	managerId string,
 ) ([]SClouduser, []cloudprovider.IClouduser, compare.SyncResult) {
-	lockman.LockRawObject(ctx, ClouduserManager.Keyword(), self.Id)
-	defer lockman.ReleaseRawObject(ctx, ClouduserManager.Keyword(), self.Id)
+	lockman.LockRawObject(ctx, ClouduserManager.Keyword(), fmt.Sprintf("%s-%s", self.Id, managerId))
+	defer lockman.ReleaseRawObject(ctx, ClouduserManager.Keyword(), fmt.Sprintf("%s-%s", self.Id, managerId))
 
 	result := compare.SyncResult{}
 	dbUsers, err := self.GetCloudusers(managerId)

pkg/cloudid/saml/providers/aws/driver.go

@@ -60,7 +60,7 @@ func (d *SAWSSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCred
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/RoleSessionName",
 			friendlyName: "RoleSessionName",
-			value:        userCred.GetUserId(),
+			value:        userCred.GetUserName(),
 		},
 		{
 			name:         "urn:oid:1.3.6.1.4.1.5923.1.1.1.3",

pkg/cloudid/saml/providers/awscn/driver.go

@@ -60,7 +60,7 @@ func (d *SAWSCNSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCre
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/RoleSessionName",
 			friendlyName: "RoleSessionName",
-			value:        userCred.GetUserId(),
+			value:        userCred.GetUserName(),
 		},
 		{
 			name:         "urn:oid:1.3.6.1.4.1.5923.1.1.1.3",

pkg/cloudid/tasks/cloudaccount_sync_resources_task.go

@@ -36,7 +36,7 @@ var (
 )
 
 func init() {
-	CloudaccountSyncWorkerManager = appsrv.NewWorkerManager("CloudaccountSyncWorkerManager", 10, 1024, false)
+	CloudaccountSyncWorkerManager = appsrv.NewWorkerManager("CloudaccountSyncWorkerManager", 20, 1024, false)
 	taskman.RegisterTaskAndWorker(CloudaccountSyncResourcesTask{}, CloudaccountSyncWorkerManager)
 }
 

pkg/cloudid/tasks/cloudprovider_sync_resources_task.go

@@ -36,7 +36,7 @@ var (
 )
 
 func init() {
-	CloudproviderSyncWorkerManager = appsrv.NewWorkerManager("CloudproviderSyncWorkerManager", 10, 1024, false)
+	CloudproviderSyncWorkerManager = appsrv.NewWorkerManager("CloudproviderSyncWorkerManager", 30, 1024, false)
 	taskman.RegisterTaskAndWorker(CloudproviderSyncResourcesTask{}, CloudproviderSyncWorkerManager)
 }
 

vendor/modules.txt

@@ -1489,7 +1489,7 @@ sigs.k8s.io/structured-merge-diff/v4/value
 # sigs.k8s.io/yaml v1.2.0
 ## explicit; go 1.12
 sigs.k8s.io/yaml
-# yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240827090454-97d9840e0939
+# yunion.io/x/cloudmux v0.3.10-0-alpha.1.0.20240906024748-a9bd6344aaf8
 ## explicit; go 1.18
 yunion.io/x/cloudmux/pkg/apis
 yunion.io/x/cloudmux/pkg/apis/billing
@@ -1586,7 +1586,7 @@ yunion.io/x/log/hooks
 yunion.io/x/ovsdb/cli_util
 yunion.io/x/ovsdb/schema/ovn_nb
 yunion.io/x/ovsdb/types
-# yunion.io/x/pkg v1.10.1-0.20240826001854-ac73b70d75b7
+# yunion.io/x/pkg v1.10.1-0.20240905110705-77c46e716318
 ## explicit; go 1.18
 yunion.io/x/pkg/appctx
 yunion.io/x/pkg/errors