fix restore --rbac behavior when RBAC objects contains -, . or …

…any special characters new fix #930
Altinity · Jul 3, 2024 · 587e3ae · 587e3ae
1 parent da6da6c
commit 587e3ae
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 26 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,3 +1,7 @@
+# v2.5.19
+BUG FIXES
+- fix `restore --rbac` behavior when RBAC objects contains `-`, `.` or any special characters new fixes for [930](https://github.com/Altinity/clickhouse-backup/issues/930)
+
 # v2.5.18
 BUG FIXES
 - add `clean` command to `POST /backup/actions` API handler, fix [945](https://github.com/Altinity/clickhouse-backup/issues/945)

diff --git a/pkg/backup/restore.go b/pkg/backup/restore.go
@@ -593,10 +593,14 @@ func (b *Backuper) isRBACExists(ctx context.Context, kind string, name string, a
 	return false, "", ""
 }
 
+// https://github.com/Altinity/clickhouse-backup/issues/930
+var needQuoteRBACRE = regexp.MustCompile(`[^0-9a-zA-Z_]`)
+
 func (b *Backuper) dropExistsRBAC(ctx context.Context, kind string, name string, accessPath string, rbacType, rbacObjectId string, k *keeper.Keeper) error {
 	//sql
 	if rbacType == "sql" {
-		if strings.Contains(name, ".") && !strings.HasPrefix(name, "`") && !strings.HasPrefix(name, `"`) && !strings.HasPrefix(name, "'") && !strings.Contains(name, " ON ") {
+		// https://github.com/Altinity/clickhouse-backup/issues/930
+		if needQuoteRBACRE.MatchString(name) && !strings.HasPrefix(name, "`") && !strings.HasPrefix(name, `"`) && !strings.HasPrefix(name, "'") && !strings.Contains(name, " ON ") {
 			name = "`" + name + "`"
 		}
 		dropSQL := fmt.Sprintf("DROP %s IF EXISTS %s", kind, name)

diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go
@@ -460,27 +460,27 @@ func TestRBAC(t *testing.T) {
 
 		ch.queryWithNoError(r, "DROP TABLE IF EXISTS default.test_rbac")
 		ch.queryWithNoError(r, "CREATE TABLE default.test_rbac (v UInt64) ENGINE=MergeTree() ORDER BY tuple()")
-		ch.queryWithNoError(r, "DROP SETTINGS PROFILE IF EXISTS `test.rbac`")
-		ch.queryWithNoError(r, "DROP QUOTA IF EXISTS `test.rbac`")
-		ch.queryWithNoError(r, "DROP ROW POLICY IF EXISTS `test.rbac` ON default.test_rbac")
-		ch.queryWithNoError(r, "DROP ROLE IF EXISTS `test.rbac`")
-		ch.queryWithNoError(r, "DROP USER IF EXISTS `test.rbac`")
+		ch.queryWithNoError(r, "DROP SETTINGS PROFILE IF EXISTS `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP QUOTA IF EXISTS `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP ROW POLICY IF EXISTS `test.rbac-name` ON default.test_rbac")
+		ch.queryWithNoError(r, "DROP ROLE IF EXISTS `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP USER IF EXISTS `test.rbac-name`")
 
 		createRBACObjects := func(drop bool) {
 			if drop {
 				log.Info("drop all RBAC related objects")
-				ch.queryWithNoError(r, "DROP SETTINGS PROFILE `test.rbac`")
-				ch.queryWithNoError(r, "DROP QUOTA `test.rbac`")
-				ch.queryWithNoError(r, "DROP ROW POLICY `test.rbac` ON default.test_rbac")
-				ch.queryWithNoError(r, "DROP ROLE `test.rbac`")
-				ch.queryWithNoError(r, "DROP USER `test.rbac`")
+				ch.queryWithNoError(r, "DROP SETTINGS PROFILE `test.rbac-name`")
+				ch.queryWithNoError(r, "DROP QUOTA `test.rbac-name`")
+				ch.queryWithNoError(r, "DROP ROW POLICY `test.rbac-name` ON default.test_rbac")
+				ch.queryWithNoError(r, "DROP ROLE `test.rbac-name`")
+				ch.queryWithNoError(r, "DROP USER `test.rbac-name`")
 			}
 			log.Info("create RBAC related objects")
-			ch.queryWithNoError(r, "CREATE SETTINGS PROFILE `test.rbac` SETTINGS max_execution_time=60")
-			ch.queryWithNoError(r, "CREATE ROLE `test.rbac` SETTINGS PROFILE `test.rbac`")
-			ch.queryWithNoError(r, "CREATE USER `test.rbac` IDENTIFIED BY 'test_rbac_password' DEFAULT ROLE `test.rbac`")
-			ch.queryWithNoError(r, "CREATE QUOTA `test.rbac` KEYED BY user_name FOR INTERVAL 1 hour NO LIMITS TO `test.rbac`")
-			ch.queryWithNoError(r, "CREATE ROW POLICY `test.rbac` ON default.test_rbac USING 1=1 AS RESTRICTIVE TO `test.rbac`")
+			ch.queryWithNoError(r, "CREATE SETTINGS PROFILE `test.rbac-name` SETTINGS max_execution_time=60")
+			ch.queryWithNoError(r, "CREATE ROLE `test.rbac-name` SETTINGS PROFILE `test.rbac-name`")
+			ch.queryWithNoError(r, "CREATE USER `test.rbac-name` IDENTIFIED BY 'test_rbac_password' DEFAULT ROLE `test.rbac-name`")
+			ch.queryWithNoError(r, "CREATE QUOTA `test.rbac-name` KEYED BY user_name FOR INTERVAL 1 hour NO LIMITS TO `test.rbac-name`")
+			ch.queryWithNoError(r, "CREATE ROW POLICY `test.rbac-name` ON default.test_rbac USING 1=1 AS RESTRICTIVE TO `test.rbac-name`")
 		}
 		createRBACObjects(false)
 
@@ -515,11 +515,11 @@ func TestRBAC(t *testing.T) {
 		r.NoError(dockerExec("clickhouse", "ls", "-lah", "/var/lib/clickhouse/access"))
 
 		rbacTypes := map[string]string{
-			"PROFILES": "test.rbac",
-			"QUOTAS":   "test.rbac",
-			"POLICIES": "`test.rbac` ON default.test_rbac",
-			"ROLES":    "test.rbac",
-			"USERS":    "test.rbac",
+			"PROFILES": "test.rbac-name",
+			"QUOTAS":   "test.rbac-name",
+			"POLICIES": "`test.rbac-name` ON default.test_rbac",
+			"ROLES":    "test.rbac-name",
+			"USERS":    "test.rbac-name",
 		}
 		for rbacType, expectedValue := range rbacTypes {
 			var rbacRows []struct {
@@ -543,11 +543,11 @@ func TestRBAC(t *testing.T) {
 		r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", config, "delete", "local", "test_rbac_backup"))
 		r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", config, "delete", "remote", "test_rbac_backup"))
 
-		ch.queryWithNoError(r, "DROP SETTINGS PROFILE `test.rbac`")
-		ch.queryWithNoError(r, "DROP QUOTA `test.rbac`")
-		ch.queryWithNoError(r, "DROP ROW POLICY `test.rbac` ON default.test_rbac")
-		ch.queryWithNoError(r, "DROP ROLE `test.rbac`")
-		ch.queryWithNoError(r, "DROP USER `test.rbac`")
+		ch.queryWithNoError(r, "DROP SETTINGS PROFILE `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP QUOTA `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP ROW POLICY `test.rbac-name` ON default.test_rbac")
+		ch.queryWithNoError(r, "DROP ROLE `test.rbac-name`")
+		ch.queryWithNoError(r, "DROP USER `test.rbac-name`")
 		ch.queryWithNoError(r, "DROP TABLE IF EXISTS default.test_rbac")
 		ch.chbackend.Close()
 	}