fix(clickhousegrant): remote and local states comparison (#780)

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - Ignore `http.StatusBadRequest` on `ClickhouseGrant` deletion
 - Retry conflict error when k8s object saved to the storage
+- Fix `ClickhouseGrant` invalid remote and local privileges comparison
+- Fix `ClickhouseGrant`: doesn't escape role name to grant
 
 ## v0.22.0 - 2024-07-02
 

api/v1alpha1/clickhousegrant_types.go

@@ -237,7 +237,7 @@ func constructPrivilegesPart(g PrivilegeGrant) string {
 }
 
 func (g RoleGrant) ConstructParts(t chUtils.StatementType) (string, string, string) {
-	rolesPart := strings.Join(g.Roles, ", ")
+	rolesPart := strings.Join(escapeList(g.Roles), ", ")
 	granteesPart := constructGranteePart(g.Grantees)
 	options := ""
 	if g.WithAdminOption && t == chUtils.GRANT {
@@ -315,3 +315,11 @@ func escape(identifier string) string {
 	replacer := strings.NewReplacer("`", "\\`", "\\", "\\\\")
 	return "`" + replacer.Replace(identifier) + "`"
 }
+
+func escapeList(list []string) []string {
+	result := make([]string, len(list))
+	for i, v := range list {
+		result[i] = escape(v)
+	}
+	return result
+}

controllers/clickhousegrant_controller.go

@@ -4,10 +4,11 @@ package controllers
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"net/http"
 	"slices"
 	"strconv"
-	"strings"
 
 	"github.com/aiven/aiven-go-client/v2"
 	avngen "github.com/aiven/go-client-codegen"
@@ -158,44 +159,63 @@ func diffClickhouseGrantSpecToApi(specPrivilegeGrants []v1alpha1.PrivilegeGrant,
 	var roleGrantsToRevoke, roleGrantsToAdd []v1alpha1.RoleGrant
 
 	for _, apiGrant := range apiPrivilegeGrants {
-		if !containsPrivilegeGrant(specPrivilegeGrants, apiGrant) {
+		if !containsGrant(specPrivilegeGrants, apiGrant) {
 			privilegeGrantsToRevoke = append(privilegeGrantsToRevoke, apiGrant)
 		}
 	}
 
 	for _, specGrant := range specPrivilegeGrants {
-		if !containsPrivilegeGrant(apiPrivilegeGrants, specGrant) {
+		if !containsGrant(apiPrivilegeGrants, specGrant) {
 			privilegeGrantsToAdd = append(privilegeGrantsToAdd, specGrant)
 		}
 	}
 
 	for _, apiGrant := range apiRoleGrants {
-		if !containsRoleGrant(specRoleGrants, apiGrant) {
+		if !containsGrant(specRoleGrants, apiGrant) {
 			roleGrantsToRevoke = append(roleGrantsToRevoke, apiGrant)
 		}
 	}
 
 	for _, specGrant := range specRoleGrants {
-		if !containsRoleGrant(apiRoleGrants, specGrant) {
+		if !containsGrant(apiRoleGrants, specGrant) {
 			roleGrantsToAdd = append(roleGrantsToAdd, specGrant)
 		}
 	}
 
 	return privilegeGrantsToRevoke, privilegeGrantsToAdd, roleGrantsToRevoke, roleGrantsToAdd
 }
 
-func containsPrivilegeGrant(grants []v1alpha1.PrivilegeGrant, grant chUtils.Grant) bool {
-	for _, g := range grants {
-		if cmp.Equal(g, grant) {
-			return true
+// normalizeGrant v1alpha1.PrivilegeGrant and []v1alpha1.RoleGrant
+// have array fields with scalars and objects.
+// To compare two grants list fields must be sorted
+func normalizeGrant(grant any) (result map[string]any) {
+	b, _ := json.Marshal(grant)
+	_ = json.Unmarshal(b, &result)
+	return sortValues(result)
+}
+
+func sortValues[T map[string]any](m T) T {
+	for k, v := range m {
+		switch val := v.(type) {
+		case T:
+			m[k] = sortValues(val)
+		case []any:
+			sorted := make([]string, len(val))
+			for i, s := range val {
+				sorted[i] = fmt.Sprintf("%v", s)
+			}
+
+			slices.Sort(sorted)
+			m[k] = sorted
 		}
 	}
-	return false
+	return m
 }
 
-func containsRoleGrant(grants []v1alpha1.RoleGrant, grant v1alpha1.RoleGrant) bool {
+func containsGrant[T any](grants []T, grant T) bool {
+	s := normalizeGrant(grant)
 	for _, g := range grants {
-		if cmp.Equal(g, grant) {
+		if cmp.Equal(s, normalizeGrant(g)) {
 			return true
 		}
 	}

docs/docs/api-reference/clickhousegrant.md

@@ -2,9 +2,9 @@
 title: "ClickhouseGrant"
 ---
 
-## Usage example
+## Usage examples
 
-??? example 
+??? example "example_2"
     ```yaml
     apiVersion: aiven.io/v1alpha1
     kind: ClickhouseGrant
@@ -46,6 +46,94 @@ title: "ClickhouseGrant"
             - role: my-role
     ```
 
+??? example 
+    ```yaml
+    apiVersion: aiven.io/v1alpha1
+    kind: ClickhouseGrant
+    metadata:
+      name: my-clickhouse-grant
+    spec:
+      authSecretRef:
+        name: aiven-token
+        key: token
+
+      project: aiven-project-name
+      serviceName: my-clickhouse-service
+
+      privilegeGrants:
+        - grantees:
+            - role: my-clickhouse-role
+          privileges:
+            - INSERT
+            - SELECT
+            - CREATE TABLE
+            - CREATE VIEW
+          database: my-clickhouse-db
+      roleGrants:
+        - grantees:
+            - user: my-clickhouse-user
+          roles:
+            - my-clickhouse-role
+
+    ---
+
+    apiVersion: aiven.io/v1alpha1
+    kind: Clickhouse
+    metadata:
+      name: my-clickhouse-service
+    spec:
+      authSecretRef:
+        name: aiven-token
+        key: token
+
+      project: aiven-project-name
+      cloudName: google-europe-west1
+      plan: startup-16
+
+    ---
+
+    apiVersion: aiven.io/v1alpha1
+    kind: ClickhouseDatabase
+    metadata:
+      name: my-clickhouse-db
+    spec:
+      authSecretRef:
+        name: aiven-token
+        key: token
+
+      project: aiven-project-name
+      serviceName: my-clickhouse-service
+
+    ---
+
+    apiVersion: aiven.io/v1alpha1
+    kind: ClickhouseUser
+    metadata:
+      name: my-clickhouse-user
+    spec:
+      authSecretRef:
+        name: aiven-token
+        key: token
+
+      project: aiven-project-name
+      serviceName: my-clickhouse-service
+
+    ---
+
+    apiVersion: aiven.io/v1alpha1
+    kind: ClickhouseRole
+    metadata:
+      name: my-clickhouse-role
+    spec:
+      authSecretRef:
+        name: aiven-token
+        key: token
+
+      project: aiven-project-name
+      serviceName: my-clickhouse-service
+      role: my-clickhouse-role
+    ```
+
 !!! info
 	To create this resource, a `Secret` containing Aiven token must be [created](/aiven-operator/authentication.html) first.
 

docs/docs/api-reference/examples/clickhousegrant.example_2.yaml

@@ -0,0 +1,40 @@
+
+apiVersion: aiven.io/v1alpha1
+kind: ClickhouseGrant
+metadata:
+  name: demo-ch-grant
+spec:
+  authSecretRef:
+    name: aiven-token
+    key: token
+
+  project: my-aiven-project
+  serviceName: my-clickhouse
+
+  privilegeGrants:
+    - grantees:
+        - user: user1
+        - user: my-clickhouse-user-🦄
+      privileges:
+        - SELECT
+        - INSERT
+      database: my-db
+      # If table is omitted, the privileges are granted on all tables in the database
+      # If columns is omitted, the privileges are granted on all columns in the table
+    - grantees:
+        - role: my-role
+      privileges:
+        - SELECT
+      database: my-db
+      table: my-table
+      columns:
+        - col1
+        - col2
+
+  roleGrants:
+    - roles:
+        - other-role
+      grantees:
+        - user: my-user
+        - role: my-role
+

docs/docs/api-reference/examples/clickhousegrant.yaml

@@ -1,40 +1,84 @@
-
 apiVersion: aiven.io/v1alpha1
 kind: ClickhouseGrant
 metadata:
-  name: demo-ch-grant
+  name: my-clickhouse-grant
 spec:
   authSecretRef:
     name: aiven-token
     key: token
 
-  project: my-aiven-project
-  serviceName: my-clickhouse
+  project: aiven-project-name
+  serviceName: my-clickhouse-service
 
   privilegeGrants:
     - grantees:
-        - user: user1
-        - user: my-clickhouse-user-🦄
+        - role: my-clickhouse-role
       privileges:
-        - SELECT
         - INSERT
-      database: my-db
-      # If table is omitted, the privileges are granted on all tables in the database
-      # If columns is omitted, the privileges are granted on all columns in the table
-    - grantees:
-        - role: my-role
-      privileges:
         - SELECT
-      database: my-db
-      table: my-table
-      columns:
-        - col1
-        - col2
-
+        - CREATE TABLE
+        - CREATE VIEW
+      database: my-clickhouse-db
   roleGrants:
-    - roles:
-        - other-role
-      grantees:
-        - user: my-user
-        - role: my-role
+    - grantees:
+        - user: my-clickhouse-user
+      roles:
+        - my-clickhouse-role
+
+---
+
+apiVersion: aiven.io/v1alpha1
+kind: Clickhouse
+metadata:
+  name: my-clickhouse-service
+spec:
+  authSecretRef:
+    name: aiven-token
+    key: token
+
+  project: aiven-project-name
+  cloudName: google-europe-west1
+  plan: startup-16
+
+---
+
+apiVersion: aiven.io/v1alpha1
+kind: ClickhouseDatabase
+metadata:
+  name: my-clickhouse-db
+spec:
+  authSecretRef:
+    name: aiven-token
+    key: token
+
+  project: aiven-project-name
+  serviceName: my-clickhouse-service
+
+---
+
+apiVersion: aiven.io/v1alpha1
+kind: ClickhouseUser
+metadata:
+  name: my-clickhouse-user
+spec:
+  authSecretRef:
+    name: aiven-token
+    key: token
+
+  project: aiven-project-name
+  serviceName: my-clickhouse-service
+
+---
+
+apiVersion: aiven.io/v1alpha1
+kind: ClickhouseRole
+metadata:
+  name: my-clickhouse-role
+spec:
+  authSecretRef:
+    name: aiven-token
+    key: token
 
+  project: aiven-project-name
+  serviceName: my-clickhouse-service
+  role: my-clickhouse-role