Fixed database by passing a default value (#97)

Since the docs says the database is optional, I made it in the code to option, because actually if you don't pass a value this following error occurs: ``` Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference) ... /home/runner/work/provider-sql/provider-sql/pkg/controller/mysql/grant/reconciler.go:143 +0x98 ``` Since we are quoting the database identifier we need remove back tick from the regex Signed-off-by: Augusto Melo <[email protected]> Signed-off-by: Augusto Melo <[email protected]> Co-authored-by: AUGMELO <[email protected]>
crossplane-contrib · Nov 21, 2022 · e4ab206 · e4ab206
1 parent 71bfe8c
commit e4ab206
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 20 deletions.
diff --git a/apis/mysql/v1alpha1/grant_types.go b/apis/mysql/v1alpha1/grant_types.go
@@ -75,13 +75,13 @@ type GrantParameters struct {
 	// +optional
 	UserSelector *xpv1.Selector `json:"userSelector,omitempty"`
 
-	// Tables this grant is for.
+	// Tables this grant is for, default *.
 	// +optional
 	Table *string `json:"table,omitempty" default:"*"`
 
-	// Database this grant is for.
+	// Database this grant is for, default *.
 	// +optional
-	Database *string `json:"database,omitempty"`
+	Database *string `json:"database,omitempty" default:"*"`
 
 	// DatabaseRef references the database object this grant it for.
 	// +immutable

diff --git a/package/crds/mysql.sql.crossplane.io_grants.yaml b/package/crds/mysql.sql.crossplane.io_grants.yaml
@@ -72,7 +72,7 @@ spec:
                   instance.
                 properties:
                   database:
-                    description: Database this grant is for.
+                    description: Database this grant is for, default *.
                     type: string
                   databaseRef:
                     description: DatabaseRef references the database object this grant
@@ -159,7 +159,7 @@ spec:
                     minItems: 1
                     type: array
                   table:
-                    description: Tables this grant is for.
+                    description: Tables this grant is for, default *.
                     type: string
                   user:
                     description: User this grant is for.

diff --git a/pkg/controller/mysql/grant/reconciler.go b/pkg/controller/mysql/grant/reconciler.go
@@ -61,7 +61,7 @@ const (
 )
 
 var (
-	grantRegex = regexp.MustCompile("^GRANT (.+) ON `(.+)`\\.(.+) TO .+")
+	grantRegex = regexp.MustCompile(`^GRANT (.+) ON (.+)\.(.+) TO .+`)
 )
 
 // Setup adds a controller that reconciles Grant managed resources.
@@ -140,8 +140,8 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 	}
 
 	username := *cr.Spec.ForProvider.User
-	dbname := *cr.Spec.ForProvider.Database
-	table := defaultTable(cr.Spec.ForProvider.Table)
+	dbname := defaultIdentifier(cr.Spec.ForProvider.Database)
+	table := defaultIdentifier(cr.Spec.ForProvider.Table)
 
 	privileges, result, err := c.getPrivileges(ctx, username, dbname, table)
 	if err != nil {
@@ -166,10 +166,11 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 	}, nil
 }
 
-func defaultTable(table *string) string {
-	if !(table == nil) {
-		return mysql.QuoteIdentifier(*table)
+func defaultIdentifier(identifier *string) string {
+	if identifier != nil && *identifier != "*" {
+		return mysql.QuoteIdentifier(*identifier)
 	}
+
 	return "*"
 }
 
@@ -244,8 +245,8 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 	}
 
 	username := *cr.Spec.ForProvider.User
-	dbname := *cr.Spec.ForProvider.Database
-	table := defaultTable(cr.Spec.ForProvider.Table)
+	dbname := defaultIdentifier(cr.Spec.ForProvider.Database)
+	table := defaultIdentifier(cr.Spec.ForProvider.Table)
 
 	privileges := strings.Join(cr.Spec.ForProvider.Privileges.ToStringSlice(), ", ")
 
@@ -264,8 +265,8 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 	}
 
 	username := *cr.Spec.ForProvider.User
-	dbname := *cr.Spec.ForProvider.Database
-	table := defaultTable(cr.Spec.ForProvider.Table)
+	dbname := defaultIdentifier(cr.Spec.ForProvider.Database)
+	table := defaultIdentifier(cr.Spec.ForProvider.Table)
 
 	privileges := strings.Join(cr.Spec.ForProvider.Privileges.ToStringSlice(), ", ")
 	username, host := mysql.SplitUserHost(username)
@@ -276,7 +277,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 	// Using a transaction is unfortunately not possible because a GRANT triggers
 	// an implicit commit: https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
 	query := fmt.Sprintf("REVOKE ALL ON %s.%s FROM %s@%s",
-		mysql.QuoteIdentifier(dbname),
+		dbname,
 		table,
 		mysql.QuoteValue(username),
 		mysql.QuoteValue(host),
@@ -297,7 +298,7 @@ func createGrantQuery(privileges, dbname, username string, table string) string
 	username, host := mysql.SplitUserHost(username)
 	result := fmt.Sprintf("GRANT %s ON %s.%s TO %s@%s",
 		privileges,
-		mysql.QuoteIdentifier(dbname),
+		dbname,
 		table,
 		mysql.QuoteValue(username),
 		mysql.QuoteValue(host),
@@ -313,15 +314,15 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {
 	}
 
 	username := *cr.Spec.ForProvider.User
-	dbname := *cr.Spec.ForProvider.Database
-	table := defaultTable(cr.Spec.ForProvider.Table)
+	dbname := defaultIdentifier(cr.Spec.ForProvider.Database)
+	table := defaultIdentifier(cr.Spec.ForProvider.Table)
 
 	privileges := strings.Join(cr.Spec.ForProvider.Privileges.ToStringSlice(), ", ")
 	username, host := mysql.SplitUserHost(username)
 
 	query := fmt.Sprintf("REVOKE %s ON %s.%s FROM %s@%s",
 		privileges,
-		mysql.QuoteIdentifier(dbname),
+		dbname,
 		table,
 		mysql.QuoteValue(username),
 		mysql.QuoteValue(host),

diff --git a/pkg/controller/mysql/grant/reconciler_test.go b/pkg/controller/mysql/grant/reconciler_test.go
@@ -381,6 +381,36 @@ func TestObserve(t *testing.T) {
 				err: nil,
 			},
 		},
+		"SuccessGrantNoDatabaseNoTable": {
+			reason: "We should return no error if no database and table were provided",
+			fields: fields{
+				db: mockDB{
+					MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) {
+						return mockRowsToSQLRows(
+							sqlmock.NewRows([]string{"Grants"}).
+								AddRow("GRANT CREATE, DROP ON *.* TO 'success-user'@%"),
+						), nil
+					},
+				},
+			},
+			args: args{
+				mg: &v1alpha1.Grant{
+					Spec: v1alpha1.GrantSpec{
+						ForProvider: v1alpha1.GrantParameters{
+							User:       pointer.StringPtr("success-user"),
+							Privileges: v1alpha1.GrantPrivileges{"DROP", "CREATE"},
+						},
+					},
+				},
+			},
+			want: want{
+				o: managed.ExternalObservation{
+					ResourceExists:   true,
+					ResourceUpToDate: true,
+				},
+				err: nil,
+			},
+		},
 		"SuccessGrantWithTables": {
 			reason: "We should see the grants in sync when using a table",
 			fields: fields{