Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New resource: mssql_database_permissions #70

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

favoretti
Copy link

This PR introduces a new resource mssql_database_permissions that allows to manage permissions of a single user on the server.

This PR introduces a new resource `mssql_database_permissions` that
allows to manage permissions of a single user on the server.
@cj-dalley
Copy link

Godsend! Yes, please!

@favoretti
Copy link
Author

@magne ping? I know it's holiday season, but... :) Thanks for looking!

@magne
Copy link
Contributor

magne commented Dec 29, 2023

I will have another look at this if you include acceptance tests. Also, the update and import operations should be working.

Other than that, it looks OK (haven't tested it yet). Thanks for your work.

@favoretti
Copy link
Author

Makes sense. I'll add tests in the next couple of days. I tested functionality itself - it works, but I got a bit lost in your test suite, need to dive in.

Thank you!

@favoretti
Copy link
Author

@magne Looking at your comment on update and import operations. Re: update - I don't really wanna compare the lists of permissions, and I don't see a lot of harm in dropping them and creating them again (i.e. we can opt in to skip update op altogether and just leave all fields as ForceNew) - what's your take on this?

@magne
Copy link
Contributor

magne commented Jan 5, 2024

@favoretti, I'm not really sure. It's been some time since I did real terraform provider coding, and I don't remember how it behaved without the update. Why don't you add import and the tests, and I'll take it for a spin and see how it behaves. (Does it recreate the resource on every apply?)

@favoretti
Copy link
Author

Without update it just assumes replacement on every change. It does not recreate them every apply, only if any of the arguments change. I'll add tests and import then first. Thank you!

@favoretti
Copy link
Author

@magne for some reason when I'm trying to run local acceptance tests it fails to connect to docker image..

=== RUN   TestAccUser_Local_Update_Roles
    resource_user_test.go:320: Step 1/4 error: Error running apply: exit status 1

        Error: unable to create login [user_update]: db connection failed after 30s timeout

          with mssql_login.update,
          on terraform_plugin_test.tf line 2, in resource "mssql_login" "update":
           2:            resource "mssql_login" "update" {

Any pointers on what might be happening here?

@favoretti
Copy link
Author

Ahh, I see now. I'm on a M2 Mac, so mssql just doesn't want to start up... Could use some help running/finalizing actual tests... Anyone please? :)

Copy link
Contributor

@magne magne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests fail:

=== RUN   TestAccDatabasePermissions_Azure_Basic
    resource_database_permissions_test.go:14: Step 1/1 error: Error running apply: exit status 1
        
        Error: unable to create database permissions [[EXECUTE UPDATE]] on database [test] for principal [2]: mssql: login error: Login failed for user '<token-identified principal>'.
        
          with mssql_database_permissions.test,
          on terraform_plugin_test.tf line 1, in resource "mssql_database_permissions" "test":
           1: resource "mssql_database_permissions" "test" {
        
--- FAIL: TestAccDatabasePermissions_Azure_Basic (2.73s)
=== RUN   TestAccDatabasePermissions_Local_Basic
    resource_database_permissions_test.go:33: Step 1/1 error: Error running apply: exit status 1
        
        Error: unable to create database permissions [[EXECUTE UPDATE]] on database [test] for principal [2]: db connection failed after 30s timeout
        
          with mssql_database_permissions.test,
          on terraform_plugin_test.tf line 1, in resource "mssql_database_permissions" "test":
           1: resource "mssql_database_permissions" "test" {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants