-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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: azurerm_sql_active_directory_administrator
#765
Changes from 5 commits
12f372d
bf8645b
fd5b00a
230b15b
831238b
4504706
b3870ae
dc1a2da
a02d35a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package azurerm | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/acctest" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
) | ||
|
||
func TestAccAzureRMSqlAdministrator_importBasic(t *testing.T) { | ||
resourceName := "azurerm_sql_active_directory_administrator.test" | ||
|
||
ri := acctest.RandInt() | ||
config := testAccAzureRMSqlAdministrator_basic(ri, testLocation()) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testCheckAzureRMSqlAdministratorDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
}, | ||
{ | ||
ResourceName: resourceName, | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
ImportStateVerifyIgnore: []string{}, | ||
}, | ||
}, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,143 @@ | ||
package azurerm | ||
|
||
import ( | ||
"fmt" | ||
"log" | ||
|
||
"github.com/Azure/azure-sdk-for-go/services/sql/mgmt/2015-05-01-preview/sql" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
uuid "github.com/satori/go.uuid" | ||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" | ||
) | ||
|
||
func resourceArmSqlAdministrator() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceArmSqlActiveDirectoryAdministratorCreateUpdate, | ||
Read: resourceArmSqlActiveDirectoryAdministratorRead, | ||
Update: resourceArmSqlActiveDirectoryAdministratorCreateUpdate, | ||
Delete: resourceArmSqlActiveDirectoryAdministratorDelete, | ||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given we've supported import - can we add an import test for this resource? here's an example of how we do that https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/import_arm_mysql_server_test.go There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I've implemented this. i'm getting a "filename too long" error, which makes it tricky to test this is working at all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no worries, I can kick off the tests and take a look. Out of interest (I'm assuming you're running on Windows) - do you have the unlimited filename length Windows feature enabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hadn't & I have now, but it seems VS Code still complains. |
||
|
||
Schema: map[string]*schema.Schema{ | ||
"server_name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"resource_group_name": resourceGroupNameSchema(), | ||
|
||
"login": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would this be better as username? also, can this value be updated? if not, this would be better as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. login matches up with the Azure API. They don't tend to use username very much, as this could also be a group name. I'm not precious about it either way though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 that's fair - let's leave this as-is |
||
Type: schema.TypeString, | ||
Required: true, | ||
}, | ||
|
||
"object_id": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ValidateFunc: validateUUID, | ||
}, | ||
|
||
"tenant_id": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ValidateFunc: validateUUID, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add some validation for this to ensure it's a valid UUID? we can use:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added suggested validator. This value can be updated. |
||
}, | ||
} | ||
} | ||
|
||
func resourceArmSqlActiveDirectoryAdministratorCreateUpdate(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).sqlServerAzureADAdministratorsClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
serverName := d.Get("server_name").(string) | ||
resGroup := d.Get("resource_group_name").(string) | ||
administratorName := "activeDirectory" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note when #825 is resolved we shouldn't need this field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. presuming I don't need to do anything here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indeed - we'll take care of those changes once it's merged in (it's more a future TODO for us 😄) |
||
login := d.Get("login").(string) | ||
objectId := uuid.FromStringOrNil(d.Get("object_id").(string)) | ||
tenantId := uuid.FromStringOrNil(d.Get("tenant_id").(string)) | ||
parameters := sql.ServerAzureADAdministrator{ | ||
ServerAdministratorProperties: &sql.ServerAdministratorProperties{ | ||
AdministratorType: utils.String("ActiveDirectory"), | ||
Login: utils.String(login), | ||
Sid: &objectId, | ||
TenantID: &tenantId, | ||
}, | ||
} | ||
|
||
future, err := client.CreateOrUpdate(ctx, resGroup, serverName, administratorName, parameters) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = future.WaitForCompletion(ctx, client.Client) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
resp, err := client.Get(ctx, resGroup, serverName, administratorName) | ||
if err != nil { | ||
return err | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor can we change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated to match conventions |
||
|
||
d.SetId(*resp.ID) | ||
|
||
return nil | ||
} | ||
|
||
func resourceArmSqlActiveDirectoryAdministratorRead(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).sqlServerAzureADAdministratorsClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
resourceGroup := id.ResourceGroup | ||
serverName := id.Path["servers"] | ||
administratorName := id.Path["administrators"] | ||
|
||
resp, err := client.Get(ctx, resourceGroup, serverName, administratorName) | ||
if err != nil { | ||
if utils.ResponseWasNotFound(resp.Response) { | ||
log.Printf("[INFO] Error reading SQL AD administrator %q - removing from state", d.Id()) | ||
d.SetId("") | ||
return nil | ||
} | ||
|
||
return fmt.Errorf("Error reading SQL AD administrator: %+v", err) | ||
} | ||
|
||
d.Set("resource_group_name", resourceGroup) | ||
d.Set("server_name", serverName) | ||
d.Set("login", resp.Login) | ||
d.Set("object_id", resp.Sid.String()) | ||
d.Set("tenant_id", resp.TenantID.String()) | ||
|
||
return nil | ||
} | ||
|
||
func resourceArmSqlActiveDirectoryAdministratorDelete(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).sqlServerAzureADAdministratorsClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
resourceGroup := id.ResourceGroup | ||
serverName := id.Path["servers"] | ||
administratorName := id.Path["administrators"] | ||
|
||
_, err = client.Delete(ctx, resourceGroup, serverName, administratorName) | ||
if err != nil { | ||
return fmt.Errorf("Error deleting SQL AD administrator: %+v", err) | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor we can actually remove this, since it's empty