Add new 'mssql_role' resource to manipulate role of a SQL Server. #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
December 13, 2022 19:19
Added newline at end of fedauth main.tf
paulbrittain
changed the title
alxy
reviewed
Dec 14, 2022
docs/resources/role.md
Outdated
| The following arguments are supported: | ||
|
|
||
| * `server` - (Required) Server and login details for the SQL Server. The attributes supported in the `server` block is detailed below. | ||
| * `role_name` - (Required) The name of the roler. Changing this resource property modifies the existing resource. |
There was a problem hiding this comment.
Suggested change
| * `role_name` - (Required) The name of the roler. Changing this resource property modifies the existing resource. | |
| * `role_name` - (Required) The name of the role. Changing this resource property modifies the existing resource. |
alxy
reviewed
Dec 14, 2022
alxy
reviewed
Dec 14, 2022
|
|
||
| func (c *Connector) CreateRole(ctx context.Context, database, roleName string) error { | ||
| cmd := `DECLARE @sql nvarchar(max); | ||
| SET @sql = 'CREATE ROLE ' + QuoteName(@roleName); |
There was a problem hiding this comment.
I think one potential improvement here could be to also support authorization. But that can probably be added in another PR:
CREATE ROLE role_name [ AUTHORIZATION owner_name ]
alxy
reviewed
Dec 14, 2022
| @@ -24,3 +24,12 @@ variable "local_ip_addresses" { | |||
| description = "The external IP addresses of the machines running the acceptance tests. This is necessary to allow access to the Azure SQL Server resource." | |||
| type = list(string) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This is odd, you need this variable only to run the local tests, where the sql server is started in the Docker container. Is there no better fix for this?
|
Hi - Is there any chance this will be merged and pushed to the TF registry? We are having a need for this functionality at work as well. :-) Thanks! |
magne
requested changes
Mar 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, I hope this PR finds you well.
The objective of this PR is primarily to add a new resource mssql_role for manipulating SQL Server Roles.
Other changes include
Note: I've tried to run the acceptance tests against Azure AD with no success. Further attempts are ongoing but the issue is likely related to Azure Firewall rules. Acceptance tests pass locally. I can provide an update here if progress is made.
If none is possible, can the acceptance tests be ran + verified by an existing pipeline or by a maintainer?
PS. The changelog needs to be updated with the diff link for the proposed minor verson bump when it's available.
Many thanks