postgres: Expand grant privileges where required (#144)

* postgres: Expand grant privileges where required

Inside postgresql, some privileges are altered[0] when issuing a `GRANT`
- for example `ALL` is expanded to `CREATE, TEMPORARY, CONNECT`.

When we observe the grant, we query privileges on the database to see if
the ones in the grant exist. Given that they are expanded, we can't
simply query for what the user gave us. Expand the privileges which need
expanding before we make the query.

Closes: #92

[0]: https://www.postgresql.org/docs/15/ddl-priv.html

Signed-off-by: Iain Lane <[email protected]>

* Sort the grants privileges before comparing them

When we use them in the observation the order doesn't matter, but in the
test we are generating a diff where it does, so compare them in the
test.

Signed-off-by: Iain Lane <[email protected]>

---------

Signed-off-by: Iain Lane <[email protected]>
Signed-off-by: Iain Lane <[email protected]>
Signed-off-by: Timotej Avsec <[email protected]>

apis/postgresql/v1alpha1/grant_types.go

@@ -43,6 +43,39 @@ type GrantPrivilege string
 // +kubebuilder:validation:MinItems:=1
 type GrantPrivileges []GrantPrivilege
 
+// Some privileges are shorthands for multiple privileges. These translations
+// happen internally inside postgresql when making grants. When we query the
+// privileges back, we need to look for the expanded set.
+// https://www.postgresql.org/docs/15/ddl-priv.html
+var grantReplacements = map[GrantPrivilege]GrantPrivileges{
+	"ALL":            {"CREATE", "TEMPORARY", "CONNECT"},
+	"ALL PRIVILEGES": {"CREATE", "TEMPORARY", "CONNECT"},
+	"TEMP":           {"TEMPORARY"},
+}
+
+// ExpandPrivileges expands any shorthand privileges to their full equivalents.
+func (gp *GrantPrivileges) ExpandPrivileges() GrantPrivileges {
+	privilegeSet := make(map[GrantPrivilege]struct{})
+
+	// Replace any shorthand privileges with their full equivalents
+	for _, p := range *gp {
+		if _, ok := grantReplacements[p]; ok {
+			for _, rp := range grantReplacements[p] {
+				privilegeSet[rp] = struct{}{}
+			}
+		} else {
+			privilegeSet[p] = struct{}{}
+		}
+	}
+
+	privileges := make([]GrantPrivilege, 0, len(privilegeSet))
+	for p := range privilegeSet {
+		privileges = append(privileges, p)
+	}
+
+	return privileges
+}
+
 // ToStringSlice converts the slice of privileges to strings
 func (gp *GrantPrivileges) ToStringSlice() []string {
 	if gp == nil {

pkg/controller/postgresql/grant/reconciler.go

@@ -191,7 +191,9 @@ func selectGrantQuery(gp v1alpha1.GrantParameters, q *xsql.Query) error {
 		return nil
 	case roleDatabase:
 		gro := gp.WithOption != nil && *gp.WithOption == v1alpha1.GrantOptionGrant
-		sp := gp.Privileges.ToStringSlice()
+
+		ep := gp.Privileges.ExpandPrivileges()
+		sp := ep.ToStringSlice()
 		// Join grantee. Filter by database name and grantee name.
 		// Finally, perform a permission comparison against expected
 		// permissions.

pkg/controller/postgresql/grant/reconciler_test.go

@@ -19,12 +19,15 @@ package grant
 import (
 	"context"
 	"database/sql"
+	"fmt"
+	"sort"
 
 	"testing"
 
 	"github.com/crossplane-contrib/provider-sql/apis/postgresql/v1alpha1"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/lib/pq"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/utils/pointer"
@@ -241,6 +244,55 @@ func TestObserve(t *testing.T) {
 				o: managed.ExternalObservation{ResourceExists: false},
 			},
 		},
+		"AllMapsToExpandedPrivileges": {
+			reason: "We expand ALL to CREATE, TEMPORARY, CONNECT when checking for existing grants",
+			fields: fields{
+				db: mockDB{
+					MockScan: func(ctx context.Context, q xsql.Query, dest ...interface{}) error {
+						privileges := q.Parameters[3]
+
+						privs, ok := privileges.(*pq.StringArray)
+						if !ok {
+							return fmt.Errorf("expected Scan parameter to be pq.StringArray, got %T", privileges)
+						}
+
+						// The order is not guaranteed, so sort the slices before comparing
+						sort.Strings(*privs)
+
+						// Return if there's a diff between the expected and actual privileges
+						diff := cmp.Diff(&pq.StringArray{"CONNECT", "CREATE", "TEMPORARY"}, privileges)
+
+						bv := dest[0].(*bool)
+						*bv = diff == ""
+
+						// Extra logging in case this test is going to fail
+						if diff != "" {
+							t.Logf("expected empty diff, got: %s", diff)
+						}
+
+						return nil
+					},
+				},
+			},
+			args: args{
+				mg: &v1alpha1.Grant{
+					Spec: v1alpha1.GrantSpec{
+						ForProvider: v1alpha1.GrantParameters{
+							Database:   pointer.StringPtr("test-example"),
+							Role:       pointer.StringPtr("test-example"),
+							Privileges: v1alpha1.GrantPrivileges{"ALL"},
+						},
+					},
+				},
+			},
+			want: want{
+				o: managed.ExternalObservation{
+					ResourceExists:   true,
+					ResourceUpToDate: true,
+				},
+				err: nil,
+			},
+		},
 		"ErrSelectGrant": {
 			reason: "We should return any errors encountered while trying to show the grant",
 			fields: fields{