diff --git a/pkg/resources/grant_privileges_to_database_role.go b/pkg/resources/grant_privileges_to_database_role.go index dcfea1edc9a..0fd162d41d2 100644 --- a/pkg/resources/grant_privileges_to_database_role.go +++ b/pkg/resources/grant_privileges_to_database_role.go @@ -400,8 +400,6 @@ func ImportGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource } func CreateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - db := meta.(*sql.DB) client := sdk.NewClientFromDB(db) @@ -416,11 +414,13 @@ func CreateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource }, ) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "An error occurred when granting privileges to database role", - Detail: fmt.Sprintf("Id: %s\nError: %s", id.DatabaseRoleName, err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "An error occurred when granting privileges to database role", + Detail: fmt.Sprintf("Id: %s\nError: %s", id.DatabaseRoleName, err.Error()), + }, + } } d.SetId(id.String()) @@ -429,17 +429,17 @@ func CreateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource } func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - db := meta.(*sql.DB) client := sdk.NewClientFromDB(db) id, err := ParseGrantPrivilegesToDatabaseRoleId(d.Id()) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to parse internal identifier", - Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to parse internal identifier", + Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), + }, + } } if d.HasChange("privileges") { @@ -478,11 +478,13 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource new(sdk.GrantPrivilegesToDatabaseRoleOptions), ) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to grant added privileges", - Detail: fmt.Sprintf("Id: %s\nPrivileges to add: %v\nError: %s", d.Id(), privilegesToAdd, err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to grant added privileges", + Detail: fmt.Sprintf("Id: %s\nPrivileges to add: %v\nError: %s", d.Id(), privilegesToAdd, err.Error()), + }, + } } } @@ -501,11 +503,13 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource new(sdk.RevokePrivilegesFromDatabaseRoleOptions), ) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to revoke removed privileges", - Detail: fmt.Sprintf("Id: %s\nPrivileges to remove: %v\nError: %s", d.Id(), privilegesToRemove, err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to revoke removed privileges", + Detail: fmt.Sprintf("Id: %s\nPrivileges to remove: %v\nError: %s", d.Id(), privilegesToRemove, err.Error()), + }, + } } } @@ -527,11 +531,13 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource }, ) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Always apply. An error occurred when granting privileges to database role", - Detail: fmt.Sprintf("Id: %s\nDatabase role name: %s\nError: %s", d.Id(), id.DatabaseRoleName, err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Always apply. An error occurred when granting privileges to database role", + Detail: fmt.Sprintf("Id: %s\nDatabase role name: %s\nError: %s", d.Id(), id.DatabaseRoleName, err.Error()), + }, + } } } @@ -541,17 +547,17 @@ func UpdateGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource } func DeleteGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - db := meta.(*sql.DB) client := sdk.NewClientFromDB(db) id, err := ParseGrantPrivilegesToDatabaseRoleId(d.Id()) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to parse internal identifier", - Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to parse internal identifier", + Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), + }, + } } err = client.Grants.RevokePrivilegesFromDatabaseRole( @@ -562,62 +568,70 @@ func DeleteGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.Resource &sdk.RevokePrivilegesFromDatabaseRoleOptions{}, ) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "An error occurred when revoking privileges from database role", - Detail: fmt.Sprintf("Id: %s\nDatabase role name: %s\nError: %s", d.Id(), id.DatabaseRoleName, err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "An error occurred when revoking privileges from database role", + Detail: fmt.Sprintf("Id: %s\nDatabase role name: %s\nError: %s", d.Id(), id.DatabaseRoleName, err.Error()), + }, + } } d.SetId("") - return diags + return nil } func ReadGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - id, err := ParseGrantPrivilegesToDatabaseRoleId(d.Id()) if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to parse internal identifier", - Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to parse internal identifier", + Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), + }, + } } if id.AlwaysApply { triggerId, err := uuid.GenerateUUID() if err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Failed to generate UUID", - Detail: fmt.Sprintf("Original error: %s", err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to generate UUID", + Detail: fmt.Sprintf("Original error: %s", err.Error()), + }, + } } // Change the value of always_apply_trigger to produce a plan if err := d.Set("always_apply_trigger", triggerId); err != nil { - return append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "Error setting always_apply_trigger for database role", - Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Error setting always_apply_trigger for database role", + Detail: fmt.Sprintf("Id: %s\nError: %s", d.Id(), err.Error()), + }, + } } } if id.AllPrivileges { - return append(diags, diag.Diagnostic{ - Severity: diag.Warning, - Summary: "Show with all_privileges option is skipped.", - // TODO: link to the design decisions doc - Detail: "See our document on design decisions for grants: ", - }) + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Show with all_privileges option is skipped.", + // TODO: link to the design decisions doc + Detail: "See our document on design decisions for grants: ", + }, + } } - opts, grantedOn, diagnostics := prepareShowGrantsRequest(id) - if len(diagnostics) != 0 { - return append(diags, diagnostics...) + opts, grantedOn, diags := prepareShowGrantsRequest(id) + if diags != nil && len(diags) != 0 { + return diags } db := meta.(*sql.DB) @@ -671,7 +685,6 @@ func ReadGrantPrivilegesToDatabaseRole(ctx context.Context, d *schema.ResourceDa func prepareShowGrantsRequest(id GrantPrivilegesToDatabaseRoleId) (*sdk.ShowGrantOptions, sdk.ObjectType, diag.Diagnostics) { opts := new(sdk.ShowGrantOptions) var grantedOn sdk.ObjectType - var diags diag.Diagnostics switch id.Kind { case OnDatabaseDatabaseRoleGrantKind: @@ -696,12 +709,14 @@ func prepareShowGrantsRequest(id GrantPrivilegesToDatabaseRoleId) (*sdk.ShowGran }, } case OnAllSchemasInDatabaseSchemaGrantKind: - return nil, "", append(diags, diag.Diagnostic{ - Severity: diag.Warning, - Summary: "Show with OnAllSchemasInDatabase option is skipped.", - // TODO: link to the design decisions doc - Detail: "See our document on design decisions for grants: ", - }) + return nil, "", diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Show with OnAllSchemasInDatabase option is skipped.", + // TODO: link to the design decisions doc + Detail: "See our document on design decisions for grants: ", + }, + } case OnFutureSchemasInDatabaseSchemaGrantKind: opts.Future = sdk.Bool(true) opts.In = &sdk.ShowGrantsIn{ @@ -718,12 +733,14 @@ func prepareShowGrantsRequest(id GrantPrivilegesToDatabaseRoleId) (*sdk.ShowGran Object: data.Object, } case OnAllSchemaObjectGrantKind: - return nil, "", append(diags, diag.Diagnostic{ - Severity: diag.Warning, - Summary: "Show with OnAll option is skipped.", - // TODO: link to the design decisions doc - Detail: "See our document on design decisions for grants: ", - }) + return nil, "", diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Show with OnAll option is skipped.", + // TODO: link to the design decisions doc + Detail: "See our document on design decisions for grants: ", + }, + } case OnFutureSchemaObjectGrantKind: grantedOn = data.OnAllOrFuture.ObjectNamePlural.Singular() opts.Future = sdk.Bool(true) @@ -741,7 +758,7 @@ func prepareShowGrantsRequest(id GrantPrivilegesToDatabaseRoleId) (*sdk.ShowGran } } - return opts, grantedOn, diags + return opts, grantedOn, nil } func getDatabaseRolePrivilegesFromSchema(d *schema.ResourceData) *sdk.DatabaseRoleGrantPrivileges { diff --git a/pkg/resources/grant_privileges_to_database_role_identifier_test.go b/pkg/resources/grant_privileges_to_database_role_identifier_test.go index f9478d812fd..ce364b438e8 100644 --- a/pkg/resources/grant_privileges_to_database_role_identifier_test.go +++ b/pkg/resources/grant_privileges_to_database_role_identifier_test.go @@ -57,7 +57,7 @@ func TestParseGrantPrivilegesToDatabaseRoleId(t *testing.T) { }, { Name: "grant database role on schema with schema name", - Identifier: `"database-name"."database-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, // TODO: OnSchema OnSchema x2 + Identifier: `"database-name"."database-role"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, Expected: GrantPrivilegesToDatabaseRoleId{ DatabaseRoleName: sdk.NewDatabaseObjectIdentifier("database-name", "database-role"), WithGrantOption: false, @@ -292,11 +292,12 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { WithGrantOption: true, AllPrivileges: true, Kind: OnDatabaseDatabaseRoleGrantKind, + AlwaysApply: true, Data: &OnDatabaseGrantData{ DatabaseName: sdk.NewAccountObjectIdentifier("database-name"), }, }, - Expected: `"database-name"."role-name"|true|ALL|OnDatabase|"database-name"`, + Expected: `"database-name"."role-name"|true|true|ALL|OnDatabase|"database-name"`, }, { Name: "grant database role on schema on schema", @@ -310,12 +311,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { SchemaName: sdk.Pointer(sdk.NewDatabaseObjectIdentifier("database-name", "schema-name")), }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, - // TODO: Could be - // OnSchema|schema-name - // OnAllSchemasInDatabase|database-name - // OnFutureSchemasInDatabase|database-name - // instead of repeating OnSchema x2 + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnSchema|"database-name"."schema-name"`, }, { Name: "grant database role on all schemas in database", @@ -329,7 +325,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { DatabaseName: sdk.Pointer(sdk.NewAccountObjectIdentifier("database-name")), }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnAllSchemasInDatabase|"database-name"`, + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnAllSchemasInDatabase|"database-name"`, }, { Name: "grant database role on future schemas in database", @@ -343,7 +339,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { DatabaseName: sdk.Pointer(sdk.NewAccountObjectIdentifier("database-name")), }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnFutureSchemasInDatabase|"database-name"`, + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchema|OnFutureSchemasInDatabase|"database-name"`, }, { Name: "grant database role on schema object on object", @@ -360,23 +356,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { }, }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnObject|TABLE|"database-name"."schema-name"."table-name"`, - }, - { - Name: "grant database role on schema object on all tables", - Identifier: GrantPrivilegesToDatabaseRoleId{ - DatabaseRoleName: sdk.NewDatabaseObjectIdentifier("database-name", "role-name"), - WithGrantOption: false, - Privileges: []string{"CREATE SCHEMA", "USAGE", "MONITOR"}, - Kind: OnSchemaObjectDatabaseRoleGrantKind, - Data: &OnSchemaObjectGrantData{ - Kind: OnAllSchemaObjectGrantKind, - OnAllOrFuture: &BulkOperationGrantData{ - ObjectNamePlural: sdk.PluralObjectTypeTables, - }, - }, - }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES`, + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnObject|TABLE|"database-name"."schema-name"."table-name"`, }, { Name: "grant database role on schema object on all tables in database", @@ -394,7 +374,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { }, }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES|InDatabase|"database-name"`, + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES|InDatabase|"database-name"`, }, { Name: "grant database role on schema object on all tables in schema", @@ -412,7 +392,7 @@ func TestGrantPrivilegesToDatabaseRoleIdString(t *testing.T) { }, }, }, - Expected: `"database-name"."role-name"|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES|InSchema|"database-name"."schema-name"`, + Expected: `"database-name"."role-name"|false|false|CREATE SCHEMA,USAGE,MONITOR|OnSchemaObject|OnAll|TABLES|InSchema|"database-name"."schema-name"`, }, } diff --git a/pkg/resources/helpers.go b/pkg/resources/helpers.go index 1a066322632..08890b354fd 100644 --- a/pkg/resources/helpers.go +++ b/pkg/resources/helpers.go @@ -125,17 +125,3 @@ func GetPropertyAsPointer[T any](d *schema.ResourceData, property string) *T { } return &typedValue } - -func GetPropertyOrDefault[T any](d *schema.ResourceData, property string) (T, bool) { - value, ok := d.GetOk(property) - if !ok { - var defaultValue T - return defaultValue, false - } - typedValue, ok := value.(T) - if !ok { - var defaultValue T - return defaultValue, false - } - return typedValue, true -} diff --git a/pkg/sdk/grants_test.go b/pkg/sdk/grants_test.go index 6c0e081af86..260c6cca7e1 100644 --- a/pkg/sdk/grants_test.go +++ b/pkg/sdk/grants_test.go @@ -351,7 +351,7 @@ func TestGrants_GrantPrivilegesToDatabaseRole(t *testing.T) { t.Run("validation: no privileges set", func(t *testing.T) { opts := defaultGrantsForDb() opts.privileges = &DatabaseRoleGrantPrivileges{} - assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges")) + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges", "AllPrivileges")) }) t.Run("validation: too many privileges set", func(t *testing.T) { @@ -360,7 +360,7 @@ func TestGrants_GrantPrivilegesToDatabaseRole(t *testing.T) { DatabasePrivileges: []AccountObjectPrivilege{AccountObjectPrivilegeCreateSchema}, SchemaPrivileges: []SchemaPrivilege{SchemaPrivilegeCreateAlert}, } - assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges")) + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges", "AllPrivileges")) }) t.Run("validation: no on set", func(t *testing.T) { @@ -545,7 +545,7 @@ func TestGrants_RevokePrivilegesFromDatabaseRoleRole(t *testing.T) { t.Run("validation: no privileges set", func(t *testing.T) { opts := defaultGrantsForDb() opts.privileges = &DatabaseRoleGrantPrivileges{} - assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges")) + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges", "AllPrivileges")) }) t.Run("validation: too many privileges set", func(t *testing.T) { @@ -554,7 +554,7 @@ func TestGrants_RevokePrivilegesFromDatabaseRoleRole(t *testing.T) { DatabasePrivileges: []AccountObjectPrivilege{AccountObjectPrivilegeCreateSchema}, SchemaPrivileges: []SchemaPrivilege{SchemaPrivilegeCreateAlert}, } - assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges")) + assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("DatabaseRoleGrantPrivileges", "DatabasePrivileges", "SchemaPrivileges", "SchemaObjectPrivileges", "AllPrivileges")) }) t.Run("validation: nil on set", func(t *testing.T) { diff --git a/pkg/sdk/validators.go b/pkg/sdk/validators.go deleted file mode 100644 index 0919fd1b065..00000000000 --- a/pkg/sdk/validators.go +++ /dev/null @@ -1 +0,0 @@ -package sdk