Skip to content

Commit

Permalink
Added external_id & force options for service principals (#1293)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexott authored May 3, 2022
1 parent e97ff62 commit bb242ed
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Version changelog

## 0.5.7

* Added `external_id` and `force` attributes to `databricks_service_principal` resource ([#1293](https://github.com/databrickslabs/terraform-provider-databricks/pull/1293))

## 0.5.6

* Added `databricks_views` data resource, making `databricks_tables` return only managed or external tables in Unity Catalog ([#1274](https://github.com/databrickslabs/terraform-provider-databricks/issues/1274)).
Expand Down
2 changes: 2 additions & 0 deletions docs/resources/service_principal.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ The following arguments are available:

* `application_id` - This is the application id of the given service principal and will be their form of access and identity. On other clouds than Azure this value is auto-generated.
* `display_name` - (Required) This is an alias for the service principal and can be the full name of the service principal.
* `external_id` - (Optional) ID of the service principal in an external identity provider.
* `allow_cluster_create` - (Optional) Allow the service principal to have [cluster](cluster.md) create privileges. Defaults to false. More fine grained permissions could be assigned with [databricks_permissions](permissions.md#Cluster-usage) and `cluster_id` argument. Everyone without `allow_cluster_create` argument set, but with [permission to use](permissions.md#Cluster-Policy-usage) Cluster Policy would be able to create clusters, but within the boundaries of that specific policy.
* `allow_instance_pool_create` - (Optional) Allow the service principal to have [instance pool](instance_pool.md) create privileges. Defaults to false. More fine grained permissions could be assigned with [databricks_permissions](permissions.md#Instance-Pool-usage) and [instance_pool_id](permissions.md#instance_pool_id) argument.
* `databricks_sql_access` - (Optional) This is a field to allow the group to have access to [Databricks SQL](https://databricks.com/product/databricks-sql) feature through [databricks_sql_endpoint](sql_endpoint.md).
* `workspace_access` - (Optional) This is a field to allow the group to have access to Databricks Workspace.
* `active` - (Optional) Either service principal is active or not. True by default, but can be set to false in case of service principal deactivation with preserving service principal assets.
* `force` - (Optional) Ignore `cannot create service principal: Service principal with application ID X already exists` errors and implicitly import the specific service principal into Terraform state, enforcing entitlements defined in the instance of resource. _This functionality is experimental_ and is designed to simplify corner cases, like Azure Active Directory synchronisation.

## Attribute Reference

Expand Down
50 changes: 48 additions & 2 deletions scim/resource_service_principal.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package scim
import (
"context"
"fmt"
"net/http"
"strings"

"github.com/databrickslabs/terraform-provider-databricks/common"

Expand Down Expand Up @@ -35,6 +37,20 @@ func (a ServicePrincipalsAPI) read(servicePrincipalID string) (sp User, err erro
return
}

func (a ServicePrincipalsAPI) filter(filter string) (u []User, err error) {
var sps UserList
req := map[string]string{}
if filter != "" {
req["filter"] = filter
}
err = a.client.Scim(a.context, http.MethodGet, "/preview/scim/v2/ServicePrincipals", req, &sps)
if err != nil {
return
}
u = sps.Resources
return
}

// Update replaces resource-friendly-entity
func (a ServicePrincipalsAPI) Update(servicePrincipalID string, updateRequest User) error {
servicePrincipal, err := a.read(servicePrincipalID)
Expand Down Expand Up @@ -62,11 +78,16 @@ func ResourceServicePrincipal() *schema.Resource {
ApplicationID string `json:"application_id,omitempty" tf:"computed,force_new"`
DisplayName string `json:"display_name,omitempty" tf:"computed"`
Active bool `json:"active,omitempty"`
ExternalID string `json:"external_id,omitempty" tf:"suppress_diff"`
}
servicePrincipalSchema := common.StructToSchema(entity{},
func(m map[string]*schema.Schema) map[string]*schema.Schema {
addEntitlementsToSchema(&m)
m["active"].Default = true
m["force"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
}
return m
})
spFromData := func(d *schema.ResourceData) User {
Expand All @@ -77,15 +98,17 @@ func ResourceServicePrincipal() *schema.Resource {
DisplayName: u.DisplayName,
Active: u.Active,
Entitlements: readEntitlementsFromData(d),
ExternalID: u.ExternalID,
}
}
return common.Resource{
Schema: servicePrincipalSchema,
Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
sp := spFromData(d)
servicePrincipal, err := NewServicePrincipalsAPI(ctx, c).Create(sp)
spAPI := NewServicePrincipalsAPI(ctx, c)
servicePrincipal, err := spAPI.Create(sp)
if err != nil {
return err
return createForceOverridesManuallyAddedServicePrincipal(err, d, spAPI, sp)
}
d.SetId(servicePrincipal.ID)
return nil
Expand All @@ -106,10 +129,33 @@ func ResourceServicePrincipal() *schema.Resource {
DisplayName: d.Get("display_name").(string),
Active: d.Get("active").(bool),
Entitlements: readEntitlementsFromData(d),
ExternalID: d.Get("external_id").(string),
})
},
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
return NewServicePrincipalsAPI(ctx, c).Delete(d.Id())
},
}.ToResource()
}

func createForceOverridesManuallyAddedServicePrincipal(err error, d *schema.ResourceData, spAPI ServicePrincipalsAPI, u User) error {
forceCreate := d.Get("force").(bool)
if !forceCreate {
return err
}
// corner-case for overriding manually provisioned service principals
force := fmt.Sprintf("Service principal with application ID %s already exists.", u.ApplicationID)
if err.Error() != force {
return err
}
spList, err := spAPI.filter(fmt.Sprintf("applicationId eq '%s'", strings.ReplaceAll(u.ApplicationID, "'", "")))
if err != nil {
return err
}
if len(spList) == 0 {
return fmt.Errorf("cannot find SP with ID %s for force import", u.ApplicationID)
}
sp := spList[0]
d.SetId(sp.ID)
return spAPI.Update(d.Id(), u)
}
98 changes: 98 additions & 0 deletions scim/resource_service_principal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package scim

import (
"context"
"fmt"
"os"
"testing"

Expand Down Expand Up @@ -372,3 +373,100 @@ func TestResourceServicePrincipalDelete_Error(t *testing.T) {
}.Apply(t)
require.Error(t, err, err)
}

func TestCreateForceOverridesManuallyAddedServicePrincipalErrorNotMatched(t *testing.T) {
d := ResourceUser().TestResourceData()
d.Set("force", true)
rerr := createForceOverridesManuallyAddedServicePrincipal(
fmt.Errorf("nonsense"), d,
NewServicePrincipalsAPI(context.Background(), &common.DatabricksClient{}), User{})
assert.EqualError(t, rerr, "nonsense")
}

func TestCreateForceOverwriteCannotListServicePrincipals(t *testing.T) {
appID := "12344ca0-e1d7-45d1-951e-f4b93592f123"
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
{
Method: "GET",
Resource: fmt.Sprintf("/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%%20eq%%20%%27%s%%27", appID),
Status: 417,
Response: common.APIError{
Message: "cannot find service principal",
},
},
}, func(ctx context.Context, client *common.DatabricksClient) {
d := ResourceUser().TestResourceData()
d.Set("force", true)
err := createForceOverridesManuallyAddedServicePrincipal(
fmt.Errorf("Service principal with application ID %s already exists.", appID),
d, NewServicePrincipalsAPI(ctx, client), User{
ApplicationID: appID,
})
assert.EqualError(t, err, "cannot find service principal")
})
}

func TestCreateForceOverwriteCannotListAccServicePrincipals(t *testing.T) {
appID := "12344ca0-e1d7-45d1-951e-f4b93592f123"
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
{
Method: "GET",
Resource: fmt.Sprintf("/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%%20eq%%20%%27%s%%27", appID),
Response: UserList{
TotalResults: 0,
},
},
}, func(ctx context.Context, client *common.DatabricksClient) {
d := ResourceUser().TestResourceData()
d.Set("force", true)
err := createForceOverridesManuallyAddedServicePrincipal(
fmt.Errorf("Service principal with application ID %s already exists.", appID),
d, NewServicePrincipalsAPI(ctx, client), User{
ApplicationID: appID,
})
assert.EqualError(t, err, fmt.Sprintf("cannot find SP with ID %s for force import", appID))
})
}

func TestCreateForceOverwriteFindsAndSetsServicePrincipalID(t *testing.T) {
appID := "12344ca0-e1d7-45d1-951e-f4b93592f123"
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
{
Method: "GET",
Resource: fmt.Sprintf("/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%%20eq%%20%%27%s%%27", appID),
Response: UserList{
Resources: []User{
{
ID: "abc",
},
},
},
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals/abc",
Response: User{
ID: "abc",
},
},
{
Method: "PUT",
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals/abc",
ExpectedRequest: User{
Schemas: []URN{ServicePrincipalSchema},
ApplicationID: appID,
},
},
}, func(ctx context.Context, client *common.DatabricksClient) {
d := ResourceUser().TestResourceData()
d.Set("force", true)
d.Set("application_id", appID)
err := createForceOverridesManuallyAddedServicePrincipal(
fmt.Errorf("Service principal with application ID %s already exists.", appID),
d, NewServicePrincipalsAPI(ctx, client), User{
ApplicationID: appID,
})
assert.NoError(t, err)
assert.Equal(t, "abc", d.Id())
})
}

0 comments on commit bb242ed

Please sign in to comment.