Skip to content

Commit b26300f

Browse files
committed
fixed an issue when searching for default privileges
Signed-off-by: Joaquín Fernández Campo <[email protected]>
1 parent 65128d0 commit b26300f

File tree

2 files changed

+64
-19
lines changed

2 files changed

+64
-19
lines changed

pkg/controller/postgresql/default_privileges/reconciler.go

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -245,19 +245,39 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
245245
var query xsql.Query
246246
selectDefaultPrivilegesQuery(gp, &query)
247247

248-
var grants []string
249-
err := c.db.Scan(ctx, query, &grants)
250-
if err != nil && !xsql.IsNoRows(err) {
248+
var defaultPrivileges []string
249+
250+
rows, err := c.db.Query(ctx, query)
251+
if xsql.IsNoRows(err) {
252+
return managed.ExternalObservation{ResourceExists: false}, nil
253+
}
254+
255+
if err != nil {
256+
return managed.ExternalObservation{}, errors.Wrap(err, errSelectDefaultPrivileges)
257+
}
258+
defer rows.Close()
259+
for rows.Next() {
260+
var privilege string
261+
if err := rows.Scan(&privilege); err != nil {
262+
return managed.ExternalObservation{}, errors.Wrap(err, errSelectDefaultPrivileges)
263+
}
264+
defaultPrivileges = append(defaultPrivileges, privilege)
265+
}
266+
267+
// Check for any errors encountered during iteration
268+
if err := rows.Err(); err != nil {
251269
return managed.ExternalObservation{}, errors.Wrap(err, errSelectDefaultPrivileges)
252270
}
253-
if len(grants) == 0 {
271+
272+
// If no default privileges are found, the resource does not exist.
273+
// Maybe this is covered by the xsql.IsNoRows(err) check above?
274+
if len(defaultPrivileges) == 0 {
254275
return managed.ExternalObservation{ResourceExists: false}, nil
255276
}
256277

257-
// Grants have no way of being 'not up to date' - if they exist, they are up to date
258278
cr.SetConditions(xpv1.Available())
259279

260-
resourceMatches := matchingGrants(grants, gp.Privileges.ToStringSlice())
280+
resourceMatches := matchingGrants(defaultPrivileges, gp.Privileges.ToStringSlice())
261281
return managed.ExternalObservation{
262282
ResourceLateInitialized: false,
263283
// check that the list of grants matches the expected grants
@@ -286,15 +306,8 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
286306
err := c.db.ExecTx(ctx, []xsql.Query{
287307
deleteQuery, createQuery,
288308
})
289-
errString := errCreateDefaultPrivileges
290-
if err != nil {
291-
errString = fmt.Sprintf(`
292-
%s
293-
delete: |%s|
294-
create: |%s|
295-
`, errString, deleteQuery.String, createQuery.String)
296-
}
297-
return managed.ExternalCreation{}, errors.Wrap(err, errString)
309+
310+
return managed.ExternalCreation{}, errors.Wrap(err, errCreateDefaultPrivileges)
298311
}
299312

300313
func (c *external) Update(

pkg/controller/postgresql/default_privileges/reconciler_test.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"database/sql"
2222
"testing"
2323

24+
"github.com/DATA-DOG/go-sqlmock"
2425
"github.com/crossplane-contrib/provider-sql/apis/postgresql/v1alpha1"
2526
"github.com/google/go-cmp/cmp"
2627
"github.com/google/go-cmp/cmp/cmpopts"
@@ -221,6 +222,9 @@ func TestObserve(t *testing.T) {
221222
reason: "We should return ResourceExists: false when no default grant is found",
222223
fields: fields{
223224
db: mockDB{
225+
MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) {
226+
return mockRowsToSQLRows(sqlmock.NewRows([]string{})), nil
227+
},
224228
MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error {
225229
// Default value is empty, so we don't need to do anything here
226230
return nil
@@ -248,6 +252,12 @@ func TestObserve(t *testing.T) {
248252
reason: "We should return any errors encountered while trying to show the default grant",
249253
fields: fields{
250254
db: mockDB{
255+
MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) {
256+
r := sqlmock.NewRows([]string{"PRIVILEGE"}).
257+
AddRow("UPDATE").
258+
AddRow("SELECT")
259+
return mockRowsToSQLRows(r), errBoom
260+
},
251261
MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error {
252262
return errBoom
253263
},
@@ -275,11 +285,22 @@ func TestObserve(t *testing.T) {
275285
reason: "We should return no error if we can find the right permissions in the default grant",
276286
fields: fields{
277287
db: mockDB{
278-
MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error {
279-
bv := dest[0].(*[]string)
280-
*bv = []string{"SELECT", "UPDATE"}
281-
return nil
288+
MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) {
289+
r := sqlmock.NewRows([]string{"PRIVILEGE"}).
290+
AddRow("UPDATE").
291+
AddRow("SELECT")
292+
return mockRowsToSQLRows(r), nil
282293
},
294+
// MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error {
295+
// if len(dest) == 0 {
296+
// runtime.Breakpoint()
297+
// return nil
298+
// }
299+
// // populate the dest slice with the expected values
300+
// // so we can compare them in the test
301+
// *dest[0].(*string) = "SELECT"
302+
// return nil
303+
// },
283304
},
284305
},
285306
args: args{
@@ -320,6 +341,17 @@ func TestObserve(t *testing.T) {
320341
}
321342
}
322343

344+
func mockRowsToSQLRows(mockRows *sqlmock.Rows) *sql.Rows {
345+
db, mock, _ := sqlmock.New()
346+
mock.ExpectQuery("select").WillReturnRows(mockRows)
347+
rows, err := db.Query("select")
348+
if err != nil {
349+
println("%v", err)
350+
return nil
351+
}
352+
return rows
353+
}
354+
323355
func TestCreate(t *testing.T) {
324356
errBoom := errors.New("boom")
325357

0 commit comments

Comments
 (0)