Merge pull request #259 from TencentBlueKing/develop

v1.12.7

.github/workflows/go.yml

@@ -11,10 +11,10 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
 
     - name: Set up Go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v4
       with:
         go-version: 1.18.1
 
@@ -25,7 +25,6 @@ jobs:
       run: make test
 
     - name: Lint
-      uses: golangci/golangci-lint-action@v2
+      uses: golangci/golangci-lint-action@v3
       with:
         version: v1.46.2
-        skip-go-installation: true

.gitignore

@@ -44,3 +44,4 @@ build.yml
 .md_configs.data
 .me_configs.data
 .codecc
+.idea

VERSION

@@ -1 +1 @@
-1.12.6
+1.12.7

pkg/abac/pdp/evalctx/context.go

@@ -205,8 +205,9 @@ func (c *EvalContext) InitEnvironments(cond condition.Condition, currentTime tim
 
 // GenTimeEnvsFromCache will return the same time-related envs if the tz and timestamp are same!
 // NOTE: cache only if the envs is same for every request
-//       if you will change the envs later(e.g. set some value from request, do not cache it!)
-//       at that time, you should remove this func, use a new collection like sync.Pool
+//
+//	if you will change the envs later(e.g. set some value from request, do not cache it!)
+//	at that time, you should remove this func, use a new collection like sync.Pool
 func GenTimeEnvsFromCache(tz string, currentTime time.Time) (map[string]interface{}, error) {
 	key := tz + strconv.FormatInt(currentTime.Unix(), 10)
 

pkg/abac/prp/policy.go

@@ -120,8 +120,8 @@ func NewPolicyManager() PolicyManager {
 
 // ListBySubjectAction 查询用于鉴权的policy列表
 // policy有2个来源
-// 	1. 普通权限(自定义权限, 继承的用户组权限)
-// 	2. 临时权限(只来自个人)
+//  1. 普通权限(自定义权限, 继承的用户组权限)
+//  2. 临时权限(只来自个人)
 func (m *policyManager) ListBySubjectAction(
 	system string,
 	subject types.Subject,

pkg/api/model/handler/action_slz.go

@@ -207,8 +207,8 @@ func validateRelatedResourceTypes(data []relatedResourceType, actionID string) (
 }
 
 // validateActionAuthType will check the auth_type is valid or not
-// 1. if len(data.RelatedResourceTypes) == 0, auth_type should be "abac"
-// 2. if len(data.RelatedResourceTypes) > 0, auth_type should be "abac" OR "rbac"
+//  1. if len(data.RelatedResourceTypes) == 0, auth_type should be "abac"
+//  2. if len(data.RelatedResourceTypes) > 0, auth_type should be "abac" OR "rbac"
 //     2.1 if auth_type == "rbac", the related_resource_type[selection_mode] should be 'instance'
 func validateActionAuthType(authType string, relatedResourceTypes []relatedResourceType) (bool, string) {
 	if authType == types.AuthTypeRBACStr {

pkg/api/model/handler/query.go

@@ -46,6 +46,7 @@ const (
 // @Security AppCode
 // @Security AppSecret
 // @Router /api/v1/systems/{system_id}/query [get]
+//
 //nolint:gocognit
 func SystemInfoQuery(c *gin.Context) {
 	var query querySerializer
@@ -65,8 +66,9 @@ func SystemInfoQuery(c *gin.Context) {
 	BuildSystemInfoQueryResponse(c, systemID, fieldSet, false)
 }
 
-//nolint:gocognit
 // BuildSystemInfoQueryResponse will only the data requested
+//
+//nolint:gocognit
 func BuildSystemInfoQueryResponse(c *gin.Context, systemID string, fieldSet *set.StringSet, isModelSharing bool) {
 	// make the return data
 	data := gin.H{}

pkg/database/dao/expression.go

@@ -203,21 +203,27 @@ func (m *expressionManager) updateUnreferencedExpressionType(fromType int64, toT
 func (m *expressionManager) updateReferencedExpressionTypeBeforeUpdateAt(
 	fromType int64, toType int64, updatedAt int64,
 ) error {
-	sql := `UPDATE expression SET
-		type=?
-		WHERE type=?
-		AND updated_at < FROM_UNIXTIME(?)
-		AND pk IN (SELECT expression_pk FROM policy)`
+	sql := `UPDATE expression SET 
+		type=? 
+		WHERE pk IN (SELECT pk FROM 
+			(SELECT pk FROM expression 
+				WHERE type=?
+				AND updated_at < FROM_UNIXTIME(?)
+				AND pk IN (SELECT expression_pk FROM policy)
+			) AS e)`
 	return database.SqlxExec(m.DB, sql, toType, fromType, updatedAt)
 }
 
 func (m *expressionManager) deleteUnreferencedExpressionByTypeBeforeUpdateAt(
 	_type int64, updatedAt int64, limit int64,
 ) (int64, error) {
 	sql := `DELETE FROM expression
-		WHERE type=?
-		AND updated_at < FROM_UNIXTIME(?)
-		AND pk NOT IN (SELECT expression_pk FROM policy)
-		LIMIT ?`
+		WHERE pk IN (SELECT pk FROM 
+			(SELECT pk FROM expression 
+				WHERE type=?
+				AND updated_at < FROM_UNIXTIME(?)
+				AND pk NOT IN (SELECT expression_pk FROM policy)
+				LIMIT ?
+			) AS e)`
 	return database.SqlxDelete(m.DB, sql, _type, updatedAt, limit)
 }

pkg/database/dao/expression_test.go

@@ -198,7 +198,7 @@ func Test_expressionManager_ChangeReferencedExpressionTypeBeforeUpdateAt(t *test
 func Test_expressionManager_DeleteUnreferencedExpressionByTypeBeforeUpdateAt(t *testing.T) {
 	database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) {
 		mock.ExpectBegin()
-		mock.ExpectExec(`DELETE FROM expression WHERE type=`).WithArgs(
+		mock.ExpectExec(`DELETE FROM expression WHERE pk IN`).WithArgs(
 			int64(-1), int64(0), int64(10000),
 		).WillReturnResult(sqlmock.NewResult(1, 1))
 		mock.ExpectCommit()

pkg/database/dao/subject_system_group.go

@@ -13,6 +13,7 @@ package dao
 import (
 	"database/sql"
 	"errors"
+	"fmt"
 
 	"github.com/jmoiron/sqlx"
 
@@ -97,11 +98,11 @@ func (m *subjectSystemGroupManager) selectGroups(
 	systemID string,
 	subjectPKs []int64,
 ) error {
-	query := `SELECT
+	query := fmt.Sprintf(`SELECT
 		subject_pk,
-		groups
+		%s
 		FROM subject_system_group
-		WHERE system_id = ? AND subject_pk IN (?)`
+		WHERE system_id = ? AND subject_pk IN (?)`, "`groups`")
 	return database.SqlxSelect(m.DB, groups, query, systemID, subjectPKs)
 }
 
@@ -110,37 +111,37 @@ func (m *subjectSystemGroupManager) selectBySystemSubject(
 	systemID string,
 	subjectPK int64,
 ) error {
-	query := `SELECT
+	query := fmt.Sprintf(`SELECT
 		pk,
 		system_id,
 		subject_pk,
-		groups,
+		%s,
 		reversion
 		FROM subject_system_group
-		WHERE system_id = ? AND subject_pk = ?`
+		WHERE system_id = ? AND subject_pk = ?`, "`groups`")
 	return database.SqlxGet(m.DB, subjectSystemGroup, query, systemID, subjectPK)
 }
 
 func (m *subjectSystemGroupManager) insertWithTx(tx *sqlx.Tx, subjectSystemGroup *SubjectSystemGroup) error {
-	sql := `INSERT INTO subject_system_group (
+	sql := fmt.Sprintf(`INSERT INTO subject_system_group (
 		system_id, 
 		subject_pk, 
-		groups
+		%s
 	) VALUES (
 		:system_id,
 		:subject_pk,
 		:groups
-	)`
+	)`, "`groups`")
 	return database.SqlxInsertWithTx(tx, sql, subjectSystemGroup)
 }
 
 func (m *subjectSystemGroupManager) updateWithTx(tx *sqlx.Tx, subjectSystemGroup *SubjectSystemGroup) (int64, error) {
-	sql := `UPDATE subject_system_group SET
-		groups = :groups,
+	sql := fmt.Sprintf(`UPDATE subject_system_group SET
+		%s = :groups,
 		reversion = reversion + 1 
 		WHERE system_id = :system_id
 		AND subject_pk = :subject_pk
-		AND reversion = :reversion`
+		AND reversion = :reversion`, "`groups`")
 	return database.SqlxUpdateWithTx(tx, sql, subjectSystemGroup)
 }
 

pkg/database/dao/subject_system_group_test.go

@@ -11,6 +11,7 @@
 package dao
 
 import (
+	"fmt"
 	"testing"
 
 	sqlmock "github.com/DATA-DOG/go-sqlmock"
@@ -22,11 +23,11 @@ import (
 
 func Test_subjectSystemGroupManager_ListGroups(t *testing.T) {
 	database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) {
-		mockQuery := `^SELECT
+		mockQuery := fmt.Sprintf(`^SELECT
 		subject_pk,
-		groups
+		%s
 		FROM subject_system_group
-		WHERE system_id = (.*) AND subject_pk IN (.*)`
+		WHERE system_id = (.*) AND subject_pk IN (.*)`, "`groups`")
 		mockRows := sqlmock.NewRows([]string{"subject_pk", "groups"}).AddRow(int64(1), "test")
 		mock.ExpectQuery(mockQuery).WithArgs("system", int64(1)).WillReturnRows(mockRows)
 
@@ -58,14 +59,14 @@ func Test_subjectSystemGroupManager_DeleteBySystemSubjectWithTx(t *testing.T) {
 
 func Test_subjectSystemGroupManager_GetBySystemSubject(t *testing.T) {
 	database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) {
-		mockQuery := `^SELECT
+		mockQuery := fmt.Sprintf(`^SELECT
 		pk,
 		system_id,
 		subject_pk,
-		groups,
+		%s,
 		reversion
 		FROM subject_system_group
-		WHERE system_id = (.*) AND subject_pk = (.*)`
+		WHERE system_id = (.*) AND subject_pk = (.*)`, "`groups`")
 		mockRows := sqlmock.NewRows([]string{"system_id", "subject_pk", "groups", "reversion"}).
 			AddRow("test", int64(1), "[]", int64(2))
 		mock.ExpectQuery(mockQuery).WithArgs("system", int64(1)).WillReturnRows(mockRows)
@@ -108,7 +109,7 @@ func Test_subjectSystemGroupManager_CreateWithTx(t *testing.T) {
 func Test_subjectSystemGroupManager_UpdateWithTx(t *testing.T) {
 	database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) {
 		mock.ExpectBegin()
-		mock.ExpectExec(`^UPDATE subject_system_group SET groups = (.*)`).WithArgs(
+		mock.ExpectExec(fmt.Sprintf(`^UPDATE subject_system_group SET %s = (.*)`, "`groups`")).WithArgs(
 			"[]", "system", int64(1), int64(2),
 		).WillReturnResult(sqlmock.NewResult(1, 1))
 		mock.ExpectCommit()

pkg/logging/formatter_json.go

@@ -184,12 +184,12 @@ func (f *JSONFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 // This is to not silently overwrite `time`, `msg`, `func` and `level` fields when
 // dumping it. If this code wasn't there doing:
 //
-//  logrus.WithField("level", 1).Info("hello")
+//	logrus.WithField("level", 1).Info("hello")
 //
 // Would just silently drop the user provided level. Instead with this code
 // it'll logged as:
 //
-//  {"level": "info", "fields.level": 1, "msg": "hello", "time": "..."}
+//	{"level": "info", "fields.level": 1, "msg": "hello", "time": "..."}
 //
 // It's not exported because it's still using Data in an opinionated way. It's to
 // avoid code duplication between the two default formatters.

pkg/service/policy.go

@@ -653,8 +653,8 @@ func (s *policyService) DeleteUnreferencedExpressions() error {
 
 	// 2. 删除标记未被引用的expression
 	// 由于删除时可能数量较大，耗时长，锁行数据较多，影响鉴权，所以需要循环删除，限制每次删除的记录数，以及最多执行删除多少次
-	rowLimit := int64(5000)
-	maxAttempts := 100 // 相当于最多删除50万数据
+	rowLimit := int64(10000)
+	maxAttempts := 100 // 相当于最多删除100万数据
 
 	for i := 0; i < maxAttempts; i++ {
 		rowsAffected, err := s.expressionManger.DeleteUnreferencedExpressionByTypeBeforeUpdateAt(
@@ -673,6 +673,24 @@ func (s *policyService) DeleteUnreferencedExpressions() error {
 		}
 	}
 
+	// 清理自定义权限的未被引用的expression
+	for i := 0; i < maxAttempts; i++ {
+		rowsAffected, err := s.expressionManger.DeleteUnreferencedExpressionByTypeBeforeUpdateAt(
+			expressionTypeCustom,
+			updateAt,
+			rowLimit,
+		)
+		if err != nil {
+			return errorWrapf(err, "expressionManger.DeleteByTypeBeforeUpdateAt type=`%d`, updateAt=`%d`",
+				expressionTypeCustom, updateAt)
+		}
+
+		// 如果已经没有需要删除的了，就停止
+		if rowsAffected == 0 {
+			break
+		}
+	}
+
 	// 3. 标记未被引用的expression
 	err = s.expressionManger.ChangeUnreferencedExpressionType(expressionTypeTemplate, expressionTypeUnreferenced)
 	if err != nil {

pkg/task/handler/handler.go

@@ -161,7 +161,8 @@ func (h *groupAlterMessageHandler) alterSubjectActionGroupResource(subjectPK, ac
 			groupPK,
 			actionPK,
 		)
-		if err != nil {
+		// NOTE: action如果被删除, rbac_group_resource_policy中action_pks并没有清理, 这里可能出现操作查询不到的错误, 如果查询不到, 直接删除
+		if err != nil && !errors.Is(err, sql.ErrNoRows) {
 			return errorWrapf(err,
 				"cacheimpls.GetGroupActionAuthorizedResource fail, groupPK=`%d`, actionPK=`%d`",
 				groupPK, actionPK,

release.md

@@ -1,3 +1,8 @@
+# 1.12.7
+
+- bugfix: delete unreferenced expression
+- bugfix: rbac policy expression generate if action not found
+
 # 1.12.6
 
 - upgrade: /subjects-groups/belong api