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: azurerm_sql_active_directory_administrator #765

Merged
merged 9 commits into from
Mar 2, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions azurerm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,19 @@ type ArmClient struct {
vmClient compute.VirtualMachinesClient

// Databases
mysqlConfigurationsClient mysql.ConfigurationsClient
mysqlDatabasesClient mysql.DatabasesClient
mysqlFirewallRulesClient mysql.FirewallRulesClient
mysqlServersClient mysql.ServersClient
postgresqlConfigurationsClient postgresql.ConfigurationsClient
postgresqlDatabasesClient postgresql.DatabasesClient
postgresqlFirewallRulesClient postgresql.FirewallRulesClient
postgresqlServersClient postgresql.ServersClient
sqlDatabasesClient sql.DatabasesClient
sqlElasticPoolsClient sql.ElasticPoolsClient
sqlFirewallRulesClient sql.FirewallRulesClient
sqlServersClient sql.ServersClient
mysqlConfigurationsClient mysql.ConfigurationsClient
mysqlDatabasesClient mysql.DatabasesClient
mysqlFirewallRulesClient mysql.FirewallRulesClient
mysqlServersClient mysql.ServersClient
postgresqlConfigurationsClient postgresql.ConfigurationsClient
postgresqlDatabasesClient postgresql.DatabasesClient
postgresqlFirewallRulesClient postgresql.FirewallRulesClient
postgresqlServersClient postgresql.ServersClient
sqlDatabasesClient sql.DatabasesClient
sqlElasticPoolsClient sql.ElasticPoolsClient
sqlFirewallRulesClient sql.FirewallRulesClient
sqlServersClient sql.ServersClient
sqlServerAzureADAdministratorsClient sql.ServerAzureADAdministratorsClient

// KeyVault
keyVaultClient keyvault.VaultsClient
Expand Down Expand Up @@ -586,6 +587,13 @@ func (c *ArmClient) registerDatabases(endpoint, subscriptionId string, auth auto
sqlSrvClient.Sender = sender
sqlSrvClient.SkipResourceProviderRegistration = c.skipProviderRegistration
c.sqlServersClient = sqlSrvClient

sqlADClient := sql.NewServerAzureADAdministratorsClientWithBaseURI(endpoint, subscriptionId)
setUserAgent(&sqlADClient.Client)
sqlADClient.Authorizer = auth
sqlADClient.Sender = sender
sqlADClient.SkipResourceProviderRegistration = c.skipProviderRegistration
c.sqlServerAzureADAdministratorsClient = sqlADClient
}

func (c *ArmClient) registerDNSClients(endpoint, subscriptionId string, auth autorest.Authorizer, sender autorest.Sender) {
Expand Down
32 changes: 32 additions & 0 deletions azurerm/import_arm_sql_administrator_test.go
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{},
Copy link
Contributor

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

},
},
})
}
1 change: 1 addition & 0 deletions azurerm/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ func Provider() terraform.ResourceProvider {
"azurerm_sql_database": resourceArmSqlDatabase(),
"azurerm_sql_elasticpool": resourceArmSqlElasticPool(),
"azurerm_sql_firewall_rule": resourceArmSqlFirewallRule(),
"azurerm_sql_active_directory_administrator": resourceArmSqlAdministrator(),
"azurerm_sql_server": resourceArmSqlServer(),
"azurerm_storage_account": resourceArmStorageAccount(),
"azurerm_storage_blob": resourceArmStorageBlob(),
Expand Down
143 changes: 143 additions & 0 deletions azurerm/resource_arm_sql_administrator.go
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,
},
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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": {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 ForceNew: true

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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,
},
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

ValidateFunc: validateUUID,

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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"
Copy link
Contributor

Choose a reason for hiding this comment

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

note when #825 is resolved we shouldn't need this field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

presuming I don't need to do anything here?

Copy link
Contributor

@tombuildsstuff tombuildsstuff Feb 28, 2018

Choose a reason for hiding this comment

The 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
}
Copy link
Contributor

Choose a reason for hiding this comment

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

minor can we change error -> err to match the convention here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
}
Loading