Skip to content

Commit

Permalink
work
Browse files Browse the repository at this point in the history
  • Loading branch information
mgyucht committed Jul 11, 2024
1 parent 790bccc commit 3d1b501
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 9 deletions.
42 changes: 36 additions & 6 deletions mws/resource_mws_workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ import (
"strings"
"time"

"golang.org/x/oauth2"

"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/terraform-provider-databricks/common"
"github.com/databricks/terraform-provider-databricks/tokens"

"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -383,6 +386,11 @@ func CreateTokenIfNeeded(workspacesAPI WorkspacesAPI,
tokensAPI := tokens.NewTokensAPI(workspacesAPI.context, client)
lifetime := time.Duration(wsToken.Token.LifetimeSeconds) * time.Second
token, err := tokensAPI.Create(lifetime, wsToken.Token.Comment)
if isInvalidClient(err) {
return fmt.Errorf("cannot create token: the principal used by Databricks (client ID %s) is not authorized to create a token in this workspace. "+
"If this is a UC-enabled workspace, add this client to the workspace, either using databricks_mws_permission_assignment or manually (see https://docs.databricks.com/en/admin/users-groups/service-principals.html#assign-a-service-principal-to-a-workspace-using-the-account-console for instructions). "+
"If this is not a UC-enabled workspace, remove the token block from this configuration and create a workspace-level service principal to configure resources in the workspace (see https://docs.databricks.com/en/admin/users-groups/service-principals.html#add-a-service-principal-to-a-workspace-using-the-workspace-admin-settings for instructions)", client.Config.ClientID)
}
if err != nil {
return fmt.Errorf("cannot create token: %w", err)
}
Expand All @@ -391,6 +399,18 @@ func CreateTokenIfNeeded(workspacesAPI WorkspacesAPI,
return common.StructToData(wsToken, workspaceSchema, d)
}

// isInvalidClient checks whether the API request failed due to the client being invalid.
// This can happen if the provided client does not belong to the workspace. For UC workspaces,
// it is possible for an admin to add the user/service principal used by Terraform to the
// workspace so that Terraform is able to create a token. For non-UC workspaces, it is not
// possible to add account-level users/service principals to the workspace, so customers
// need to manually create a workspace-level service principal and use it to authenticate to
// the workspace.
func isInvalidClient(err error) bool {
var retrieveError *oauth2.RetrieveError
return errors.As(err, &retrieveError) && retrieveError.ErrorCode == "invalid_client"
}

func EnsureTokenExistsIfNeeded(a WorkspacesAPI,
workspaceSchema map[string]*schema.Schema, d *schema.ResourceData) error {
var wsToken WorkspaceToken
Expand All @@ -404,6 +424,13 @@ func EnsureTokenExistsIfNeeded(a WorkspacesAPI,
}
tokensAPI := tokens.NewTokensAPI(a.context, client)
_, err = tokensAPI.Read(wsToken.Token.TokenID)
// If we cannot authenticate to the workspace and we're using an in-house OAuth principal,
// log a warning but do not fail. This can happen if the provider is authenticated with a
// different principal than was used to create the workspace.
if isInvalidClient(err) {
tflog.Debug(a.context, fmt.Sprintf("unable to fetch token with ID %s from workspace using the provided service principal, continuing", wsToken.Token.TokenID))
return nil
}
if apierr.IsMissing(err) {
return CreateTokenIfNeeded(a, workspaceSchema, d)
}
Expand All @@ -413,14 +440,17 @@ func EnsureTokenExistsIfNeeded(a WorkspacesAPI,
return nil
}

func removeTokenIfNeeded(a WorkspacesAPI,
workspaceSchema map[string]*schema.Schema, tokenID string, d *schema.ResourceData) error {
func removeTokenIfNeeded(a WorkspacesAPI, tokenID string, d *schema.ResourceData) error {
client, err := a.client.ClientForHost(a.context, d.Get("workspace_url").(string))
if err != nil {
return err
}
tokensAPI := tokens.NewTokensAPI(a.context, client)
err = tokensAPI.Delete(tokenID)
if isInvalidClient(err) {
tflog.Debug(a.context, fmt.Sprintf("unable to delete token with ID %s from workspace using the provided service principal, continuing", tokenID))
return nil
}
if err != nil {
return fmt.Errorf("cannot remove token: %w", err)
}
Expand All @@ -437,12 +467,12 @@ func UpdateTokenIfNeeded(workspacesAPI WorkspacesAPI,
return CreateTokenIfNeeded(workspacesAPI, workspaceSchema, d)
case len(old) > 0 && len(new) == 0: // delete
raw := old[0].(map[string]any)
return removeTokenIfNeeded(workspacesAPI, workspaceSchema,
raw["token_id"].(string), d)
id := raw["token_id"].(string)
return removeTokenIfNeeded(workspacesAPI, id, d)
case len(old) > 0 && len(new) > 0: // delete & create
rawOld := old[0].(map[string]any)
err := removeTokenIfNeeded(workspacesAPI, workspaceSchema,
rawOld["token_id"].(string), d)
id := rawOld["token_id"].(string)
err := removeTokenIfNeeded(workspacesAPI, id, d)
if err != nil {
return err
}
Expand Down
25 changes: 22 additions & 3 deletions mws/resource_mws_workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,25 @@ func TestUpdateWorkspace_DeleteToken(t *testing.T) {
}, ``)
}

func TestUpdateWorkspace_DeleteTokenIgnoreOAuthFailure(t *testing.T) {
updateWorkspaceScimFixture(t, []qa.HTTPFixture{
{
Method: "POST",
Resource: "/api/2.0/token/delete",
ExpectedRequest: map[string]any{
"token_id": "abcdef",
},
Status: 400,
},
}, map[string]string{
"token.#": "1",
"token.0.comment": "Terraform PAT",
"token.0.lifetime_seconds": "2592000",
"token.0.token_id": "abcdef",
"token.0.token_value": "sensitive",
}, ``)
}

func TestUpdateWorkspace_ReplaceTokenAndChangeNetworkId(t *testing.T) {
updateWorkspaceScimFixtureWithPatch(t, []qa.HTTPFixture{
{
Expand Down Expand Up @@ -1503,7 +1522,7 @@ func TestWorkspaceTokenWrongAuthCornerCase(t *testing.T) {

assert.EqualError(t, CreateTokenIfNeeded(wsApi, r.Schema, d), noAuth, "create")
assert.EqualError(t, EnsureTokenExistsIfNeeded(wsApi, r.Schema, d), noAuth, "ensure")
assert.EqualError(t, removeTokenIfNeeded(wsApi, r.Schema, "x", d), noAuth, "remove")
assert.EqualError(t, removeTokenIfNeeded(wsApi, "x", d), noAuth, "remove")
}

func TestWorkspaceTokenHttpCornerCases(t *testing.T) {
Expand Down Expand Up @@ -1533,7 +1552,7 @@ func TestWorkspaceTokenHttpCornerCases(t *testing.T) {
for msg, err := range map[string]error{
"cannot create token: i'm a teapot": CreateTokenIfNeeded(wsApi, r.Schema, d),
"cannot read token: i'm a teapot": EnsureTokenExistsIfNeeded(wsApi, r.Schema, d),
"cannot remove token: i'm a teapot": removeTokenIfNeeded(wsApi, r.Schema, "x", d),
"cannot remove token: i'm a teapot": removeTokenIfNeeded(wsApi, "x", d),
} {
assert.EqualError(t, err, msg)
}
Expand Down Expand Up @@ -1769,4 +1788,4 @@ func TestSensitiveDataInLogs(t *testing.T) {
assert.NotContains(t, fmt.Sprintf("%v", tk), "sensitive")
assert.NotContains(t, fmt.Sprintf("%#v", tk), "sensitive")
assert.NotContains(t, fmt.Sprintf("%+v", tk), "sensitive")
}
}

0 comments on commit 3d1b501

Please sign in to comment.