feat(postgresql): add revoke public logic to grant and schema resourc…

…es (#207) - added a field called revokePublicOnDb in grant resource; if true, it will revoke from public access to DB; This usually a DBA requirement. - added a field called revokePublicOnSchema in schema resource; if true, it will revoke from public access to a schema; This usually a DBA requirement. Signed-off-by: oliver.zokra <oliver.zokra@kyriba.com>
crossplane-contrib · Jan 13, 2025 · fcd50ac · fcd50ac
1 parent cee2aea
commit fcd50ac
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 17 deletions.
diff --git a/apis/postgresql/v1alpha1/grant_types.go b/apis/postgresql/v1alpha1/grant_types.go
@@ -154,6 +154,10 @@ type GrantParameters struct {
 	// +immutable
 	// +optional
 	MemberOfSelector *xpv1.Selector `json:"memberOfSelector,omitempty"`
+
+	// RevokePublicOnDb apply the statement "REVOKE ALL ON DATABASE %s FROM PUBLIC" to make database unreachable from public
+	// +optional
+	RevokePublicOnDb *bool `json:"revokePublicOnDb,omitempty" default:"false"`
 }
 
 // A GrantStatus represents the observed state of a Grant.

diff --git a/apis/postgresql/v1alpha1/schema_types.go b/apis/postgresql/v1alpha1/schema_types.go
@@ -59,6 +59,10 @@ type SchemaParameters struct {
 	// +immutable
 	// +optional
 	DatabaseSelector *xpv1.Selector `json:"databaseSelector,omitempty"`
+
+	// RevokePublicOnSchema apply a "REVOKE ALL ON SCHEMA public FROM public" statement
+	// +optional
+	RevokePublicOnSchema *bool `json:"revokePublicOnSchema,omitempty" default:"false"`
 }
 
 // A SchemaStatus represents the observed state of a Schema.

diff --git a/apis/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/postgresql/v1alpha1/zz_generated.deepcopy.go
diff --git a/package/crds/postgresql.sql.crossplane.io_grants.yaml b/package/crds/postgresql.sql.crossplane.io_grants.yaml
@@ -252,6 +252,11 @@ spec:
                       type: string
                     minItems: 1
                     type: array
+                  revokePublicOnDb:
+                    description: RevokePublicOnDb apply the statement "REVOKE ALL
+                      ON DATABASE %s FROM PUBLIC" to make database unreachable from
+                      public
+                    type: boolean
                   role:
                     description: Role this grant is for.
                     type: string

diff --git a/package/crds/postgresql.sql.crossplane.io_schemas.yaml b/package/crds/postgresql.sql.crossplane.io_schemas.yaml
@@ -156,6 +156,10 @@ spec:
                             type: string
                         type: object
                     type: object
+                  revokePublicOnSchema:
+                    description: RevokePublicOnSchema apply a "REVOKE ALL ON SCHEMA
+                      public FROM public" statement
+                    type: boolean
                   role:
                     description: Role for ownership of this schema.
                     type: string

diff --git a/pkg/controller/postgresql/grant/reconciler.go b/pkg/controller/postgresql/grant/reconciler.go
@@ -276,6 +276,14 @@ func createGrantQueries(gp v1alpha1.GrantParameters, ql *[]xsql.Query) error { /
 				withOption(gp.WithOption),
 			)},
 		)
+		if gp.RevokePublicOnDb != nil && *gp.RevokePublicOnDb {
+			*ql = append(*ql,
+				// REVOKE FROM PUBLIC
+				xsql.Query{String: fmt.Sprintf("REVOKE ALL ON DATABASE %s FROM PUBLIC",
+					db,
+				)},
+			)
+		}
 		return nil
 	}
 	return errors.New(errUnknownGrant)

diff --git a/pkg/controller/postgresql/schema/reconciler.go b/pkg/controller/postgresql/schema/reconciler.go
@@ -167,16 +167,14 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 		return managed.ExternalCreation{}, errors.New(errNotSchema)
 	}
 
-	var b strings.Builder
-	b.WriteString("CREATE SCHEMA IF NOT EXISTS ")
-	b.WriteString(pq.QuoteIdentifier(meta.GetExternalName(cr)))
+	var queries []xsql.Query
 
-	if cr.Spec.ForProvider.Role != nil {
-		b.WriteString(" AUTHORIZATION ")
-		b.WriteString(pq.QuoteIdentifier(*cr.Spec.ForProvider.Role))
-	}
+	cr.SetConditions(xpv1.Creating())
 
-	return managed.ExternalCreation{}, errors.Wrap(c.db.Exec(ctx, xsql.Query{String: b.String()}), errCreateSchema)
+	createSchemaQueries(cr.Spec.ForProvider, &queries, meta.GetExternalName(cr))
+
+	err := c.db.ExecTx(ctx, queries)
+	return managed.ExternalCreation{}, errors.Wrap(err, errCreateSchema)
 }
 
 func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { //nolint:gocyclo
@@ -189,13 +187,10 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 		return managed.ExternalUpdate{}, nil
 	}
 
-	var b strings.Builder
-	b.WriteString("ALTER SCHEMA ")
-	b.WriteString(pq.QuoteIdentifier(meta.GetExternalName(mg)))
-	b.WriteString(" OWNER TO ")
-	b.WriteString(pq.QuoteIdentifier(*cr.Spec.ForProvider.Role))
+	var queries []xsql.Query
+	updateSchemaQueries(cr.Spec.ForProvider, &queries, meta.GetExternalName(cr))
 
-	err := c.db.Exec(ctx, xsql.Query{String: b.String()})
+	err := c.db.ExecTx(ctx, queries)
 	return managed.ExternalUpdate{}, errors.Wrap(err, errAlterSchema)
 }
 
@@ -226,3 +221,46 @@ func lateInit(observed v1alpha1.SchemaParameters, desired *v1alpha1.SchemaParame
 
 	return li
 }
+
+func createSchemaQueries(sp v1alpha1.SchemaParameters, ql *[]xsql.Query, en string) { // nolint: gocyclo
+
+	var b strings.Builder
+	b.WriteString("CREATE SCHEMA IF NOT EXISTS ")
+	b.WriteString(pq.QuoteIdentifier(en))
+
+	if sp.Role != nil {
+		b.WriteString(" AUTHORIZATION ")
+		b.WriteString(pq.QuoteIdentifier(*sp.Role))
+		b.WriteString(";")
+	}
+
+	*ql = append(*ql,
+		xsql.Query{String: b.String()},
+	)
+
+	if sp.RevokePublicOnSchema != nil && *sp.RevokePublicOnSchema {
+		*ql = append(*ql,
+			xsql.Query{String: "REVOKE ALL ON SCHEMA PUBLIC FROM PUBLIC;"},
+		)
+	}
+
+}
+
+func updateSchemaQueries(sp v1alpha1.SchemaParameters, ql *[]xsql.Query, en string) { // nolint: gocyclo
+
+	var b strings.Builder
+	b.WriteString("ALTER SCHEMA ")
+	b.WriteString(pq.QuoteIdentifier(en))
+	b.WriteString(" OWNER TO ")
+	b.WriteString(pq.QuoteIdentifier(*sp.Role))
+
+	*ql = append(*ql,
+		xsql.Query{String: b.String()},
+	)
+
+	if sp.RevokePublicOnSchema != nil && *sp.RevokePublicOnSchema {
+		*ql = append(*ql,
+			xsql.Query{String: "REVOKE ALL ON SCHEMA PUBLIC FROM PUBLIC;"},
+		)
+	}
+}
diff --git a/pkg/controller/postgresql/schema/reconciler_test.go b/pkg/controller/postgresql/schema/reconciler_test.go
@@ -371,7 +371,7 @@ func TestCreate(t *testing.T) {
 			reason: "Any errors encountered while creating the schema should be returned",
 			fields: fields{
 				db: &mockDB{
-					MockExec: func(ctx context.Context, q xsql.Query) error { return errBoom },
+					MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return errBoom },
 				},
 			},
 			args: args{
@@ -387,7 +387,7 @@ func TestCreate(t *testing.T) {
 			reason: "No error should be returned when we successfully create a extension",
 			fields: fields{
 				db: &mockDB{
-					MockExec: func(ctx context.Context, q xsql.Query) error { return nil },
+					MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil },
 				},
 			},
 			args: args{
@@ -457,7 +457,7 @@ func TestUpdate(t *testing.T) {
 			reason: "No error should be returned when we successfully update a schema",
 			fields: fields{
 				db: &mockDB{
-					MockExec: func(ctx context.Context, q xsql.Query) error { return nil },
+					MockExecTx: func(ctx context.Context, ql []xsql.Query) error { return nil },
 				},
 			},
 			args: args{