From 12db0f08577bd6933b916ceff3b72bb5cb087836 Mon Sep 17 00:00:00 2001 From: bakito Date: Fri, 20 Sep 2024 11:31:09 +0200 Subject: [PATCH] add support for mssql schema in grants Signed-off-by: bakito --- apis/mssql/v1alpha1/grant_types.go | 6 + apis/mssql/v1alpha1/zz_generated.deepcopy.go | 5 + .../crds/mssql.sql.crossplane.io_grants.yaml | 6 + pkg/controller/mssql/grant/reconciler.go | 65 +++++--- pkg/controller/mssql/grant/reconciler_test.go | 144 +++++++++++++++++- 5 files changed, 207 insertions(+), 19 deletions(-) diff --git a/apis/mssql/v1alpha1/grant_types.go b/apis/mssql/v1alpha1/grant_types.go index 7e2aa4e5..d717c2ae 100644 --- a/apis/mssql/v1alpha1/grant_types.go +++ b/apis/mssql/v1alpha1/grant_types.go @@ -55,6 +55,11 @@ type GrantParameters struct { // for available privileges. Permissions GrantPermissions `json:"permissions"` + // Schema for the permissions to be granted for. + // +immutable + // +optional + Schema *string `json:"schema,omitempty"` + // User this grant is for. // +optional // +crossplane:generate:reference:type=User @@ -100,6 +105,7 @@ type GrantStatus struct { // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" // +kubebuilder:printcolumn:name="ROLE",type="string",JSONPath=".spec.forProvider.user" // +kubebuilder:printcolumn:name="DATABASE",type="string",JSONPath=".spec.forProvider.database" +// +kubebuilder:printcolumn:name="SCHEMA",type="string",JSONPath=".spec.forProvider.schema" // +kubebuilder:printcolumn:name="PERMISSIONS",type="string",JSONPath=".spec.forProvider.permissions" // +kubebuilder:resource:scope=Cluster,categories={crossplane,managed,sql} type Grant struct { diff --git a/apis/mssql/v1alpha1/zz_generated.deepcopy.go b/apis/mssql/v1alpha1/zz_generated.deepcopy.go index 729bad36..d2998458 100644 --- a/apis/mssql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/mssql/v1alpha1/zz_generated.deepcopy.go @@ -183,6 +183,11 @@ func (in *GrantParameters) DeepCopyInto(out *GrantParameters) { *out = make(GrantPermissions, len(*in)) copy(*out, *in) } + if in.Schema != nil { + in, out := &in.Schema, &out.Schema + *out = new(string) + **out = **in + } if in.User != nil { in, out := &in.User, &out.User *out = new(string) diff --git a/package/crds/mssql.sql.crossplane.io_grants.yaml b/package/crds/mssql.sql.crossplane.io_grants.yaml index 342e74d2..2c70d591 100644 --- a/package/crds/mssql.sql.crossplane.io_grants.yaml +++ b/package/crds/mssql.sql.crossplane.io_grants.yaml @@ -34,6 +34,9 @@ spec: - jsonPath: .spec.forProvider.database name: DATABASE type: string + - jsonPath: .spec.forProvider.schema + name: SCHEMA + type: string - jsonPath: .spec.forProvider.permissions name: PERMISSIONS type: string @@ -170,6 +173,9 @@ spec: type: string minItems: 1 type: array + schema: + description: Schema for the permissions to be granted for. + type: string user: description: User this grant is for. type: string diff --git a/pkg/controller/mssql/grant/reconciler.go b/pkg/controller/mssql/grant/reconciler.go index 35620a70..7566446d 100644 --- a/pkg/controller/mssql/grant/reconciler.go +++ b/pkg/controller/mssql/grant/reconciler.go @@ -130,7 +130,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.New(errNotGrant) } - permissions, err := c.getPermissions(ctx, *cr.Spec.ForProvider.User) + permissions, err := c.getPermissions(ctx, cr) if err != nil { return managed.ExternalObservation{}, err } @@ -156,7 +156,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext username := *cr.Spec.ForProvider.User permissions := strings.Join(cr.Spec.ForProvider.Permissions.ToStringSlice(), ", ") - query := fmt.Sprintf("GRANT %s TO %s", permissions, mssql.QuoteIdentifier(username)) + query := fmt.Sprintf("GRANT %s %s TO %s", permissions, onSchemaQuery(cr), mssql.QuoteIdentifier(username)) return managed.ExternalCreation{}, errors.Wrap(c.db.Exec(ctx, xsql.Query{String: query}), errGrant) } @@ -166,7 +166,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, errors.New(errNotGrant) } - observed, err := c.getPermissions(ctx, *cr.Spec.ForProvider.User) + observed, err := c.getPermissions(ctx, cr) if err != nil { return managed.ExternalUpdate{}, err } @@ -175,16 +175,16 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext if len(toRevoke) > 0 { sort.Strings(toRevoke) - query := fmt.Sprintf("REVOKE %s FROM %s", - strings.Join(toRevoke, ", "), mssql.QuoteIdentifier(*cr.Spec.ForProvider.User)) + query := fmt.Sprintf("REVOKE %s %s FROM %s", + strings.Join(toRevoke, ", "), onSchemaQuery(cr), mssql.QuoteIdentifier(*cr.Spec.ForProvider.User)) if err = c.db.Exec(ctx, xsql.Query{String: query}); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errRevoke) } } if len(toGrant) > 0 { sort.Strings(toGrant) - query := fmt.Sprintf("GRANT %s TO %s", - strings.Join(toGrant, ", "), mssql.QuoteIdentifier(*cr.Spec.ForProvider.User)) + query := fmt.Sprintf("GRANT %s %s TO %s", + strings.Join(toGrant, ", "), onSchemaQuery(cr), mssql.QuoteIdentifier(*cr.Spec.ForProvider.User)) if err = c.db.Exec(ctx, xsql.Query{String: query}); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errGrant) } @@ -200,23 +200,47 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error { username := *cr.Spec.ForProvider.User - query := fmt.Sprintf("REVOKE %s FROM %s", + query := fmt.Sprintf("REVOKE %s %s FROM %s", strings.Join(cr.Spec.ForProvider.Permissions.ToStringSlice(), ", "), + onSchemaQuery(cr), mssql.QuoteIdentifier(username), ) return errors.Wrap(c.db.Exec(ctx, xsql.Query{String: query}), errRevoke) } -func (c *external) getPermissions(ctx context.Context, username string) ([]string, error) { - // TODO(turkenh/ulucinar): Possible performance improvement. We first - // calculate the Cartesian product, and then filter. It would be more - // efficient to first filter principals by name, and then join. - query := fmt.Sprintf(`SELECT pe.permission_name - FROM sys.database_principals AS pr - JOIN sys.database_permissions AS pe - ON pe.grantee_principal_id = pr.principal_id +// TODO(turkenh/ulucinar): Possible performance improvement. We first +// +// calculate the Cartesian product, and then filter. It would be more +// efficient to first filter principals by name, and then join. +const queryPermissionDefault = `SELECT pe.permission_name + FROM sys.database_principals AS pr + JOIN sys.database_permissions AS pe + ON pe.grantee_principal_id = pr.principal_id WHERE - pr.name = %s`, mssql.QuoteValue(username)) + pe.class = 0 /* DATABASE (default) */ + AND pr.name = %s` + +const queryPermissionSchema = `SELECT pe.permission_name + FROM sys.database_principals AS pr + JOIN sys.database_permissions AS pe + ON pe.grantee_principal_id = pr.principal_id + JOIN sys.schemas AS s + ON s.schema_id = pe.major_id + WHERE + pe.class = 3 /* SCHEMA */ + AND s.name = %s + AND pr.name = %s` + +func (c *external) getPermissions(ctx context.Context, cr *v1alpha1.Grant) ([]string, error) { + var query string + if cr.Spec.ForProvider.Schema == nil { + query = fmt.Sprintf(queryPermissionDefault, mssql.QuoteValue(*cr.Spec.ForProvider.User)) + } else { + query = fmt.Sprintf(queryPermissionSchema, + mssql.QuoteValue(*cr.Spec.ForProvider.Schema), + mssql.QuoteValue(*cr.Spec.ForProvider.User), + ) + } rows, err := c.db.Query(ctx, xsql.Query{String: query}) if err != nil { return nil, errors.Wrap(err, errCannotGetGrants) @@ -237,6 +261,13 @@ func (c *external) getPermissions(ctx context.Context, username string) ([]strin return permissions, nil } +func onSchemaQuery(cr *v1alpha1.Grant) (schema string) { + if cr.Spec.ForProvider.Schema != nil { + schema = fmt.Sprintf("ON SCHEMA::%s", *cr.Spec.ForProvider.Schema) + } + return +} + func diffPermissions(desired, observed []string) ([]string, []string) { md := make(map[string]struct{}, len(desired)) mo := make(map[string]struct{}, len(observed)) diff --git a/pkg/controller/mssql/grant/reconciler_test.go b/pkg/controller/mssql/grant/reconciler_test.go index 7227f100..65aba6f1 100644 --- a/pkg/controller/mssql/grant/reconciler_test.go +++ b/pkg/controller/mssql/grant/reconciler_test.go @@ -266,6 +266,9 @@ func TestObserve(t *testing.T) { fields: fields{ db: mockDB{ MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) { + if strings.Contains(q.String, "sys.schemas") { + return nil, errBoom + } return mockRowsToSQLRows( sqlmock.NewRows( []string{"Grants"}, @@ -293,6 +296,42 @@ func TestObserve(t *testing.T) { err: nil, }, }, + "SuccessSchema": { + reason: "We should return no error if we can successfully get our permissions", + fields: fields{ + db: mockDB{ + MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) { + if !strings.Contains(q.String, "sys.schemas") { + return nil, errBoom + } + return mockRowsToSQLRows( + sqlmock.NewRows( + []string{"Grants"}, + ).AddRow("ALTER"), + ), nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("success-db"), + User: ptr.To("success-user"), + Schema: ptr.To("success-schema"), + Permissions: v1alpha1.GrantPermissions{"ALTER"}, + }, + }, + }, + }, + want: want{ + o: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, + err: nil, + }, + }, "SuccessDiffPermissions": { reason: "We should return no error if different permissions exist", fields: fields{ @@ -430,7 +469,12 @@ func TestCreate(t *testing.T) { reason: "No error should be returned when we successfully create a grant", fields: fields{ db: &mockDB{ - MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + MockExec: func(ctx context.Context, q xsql.Query) error { + if strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } + return nil + }, }, }, args: args{ @@ -448,6 +492,34 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "SuccessSchema": { + reason: "No error should be returned when we successfully create a grant", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if !strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + User: ptr.To("test-example"), + Schema: ptr.To("success-schema"), + Permissions: v1alpha1.GrantPermissions{"ALTER"}, + }, + }, + }, + }, + want: want{ + err: nil, + }, + }, } for name, tc := range cases { @@ -529,6 +601,9 @@ func TestUpdate(t *testing.T) { return mockRowsToSQLRows(sqlmock.NewRows([]string{})), nil }, MockExec: func(ctx context.Context, q xsql.Query) error { + if strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } if strings.Contains(q.String, "CREATE, DELETE") { return nil } @@ -552,6 +627,41 @@ func TestUpdate(t *testing.T) { c: managed.ExternalUpdate{}, }, }, + "SuccessSchema": { + reason: "No error should be returned when we update a grant", + fields: fields{ + db: &mockDB{ + MockQuery: func(ctx context.Context, q xsql.Query) (*sql.Rows, error) { + return mockRowsToSQLRows(sqlmock.NewRows([]string{})), nil + }, + MockExec: func(ctx context.Context, q xsql.Query) error { + if !strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } + if strings.Contains(q.String, "ALTER") { + return nil + } + return errBoom + }, + }, + }, + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + User: ptr.To("test-example"), + Schema: ptr.To("success-schema"), + Permissions: v1alpha1.GrantPermissions{"ALTER"}, + }, + }, + }, + }, + want: want{ + err: nil, + c: managed.ExternalUpdate{}, + }, + }, } for name, tc := range cases { @@ -630,7 +740,37 @@ func TestDelete(t *testing.T) { }, fields: fields{ db: &mockDB{ - MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + MockExec: func(ctx context.Context, q xsql.Query) error { + if strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } + return nil + }, + }, + }, + want: nil, + }, + "SuccessSchema": { + reason: "No error should be returned if the grant was revoked", + args: args{ + mg: &v1alpha1.Grant{ + Spec: v1alpha1.GrantSpec{ + ForProvider: v1alpha1.GrantParameters{ + Database: ptr.To("test-example"), + User: ptr.To("test-example"), + Schema: ptr.To("success-schema"), + }, + }, + }, + }, + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if !strings.Contains(q.String, "ON SCHEMA::") { + return errBoom + } + return nil + }, }, }, want: nil,