-
Notifications
You must be signed in to change notification settings - Fork 462
Add expected_workspace_status
in databricks_mws_workspaces
to support least-privileged workspaces
#5019
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
base: main
Are you sure you want to change the base?
Add expected_workspace_status
in databricks_mws_workspaces
to support least-privileged workspaces
#5019
Changes from all commits
8439599
fbdc35a
b65b35a
24bc883
21cca40
a85bca6
396c183
c4522cf
df2044b
bd8c44b
6362f73
0bf4141
a507db9
9d710e3
c5efa0c
08466fc
0b7b0f7
05bf244
c2104e4
c1944ac
34c3688
1491b67
29b514f
4812ff8
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 |
---|---|---|
|
@@ -97,6 +97,7 @@ type Workspace struct { | |
WorkspaceURL string `json:"workspace_url,omitempty" tf:"computed"` | ||
WorkspaceStatus string `json:"workspace_status,omitempty" tf:"computed"` | ||
WorkspaceStatusMessage string `json:"workspace_status_message,omitempty" tf:"computed"` | ||
ExpectedWorkspaceStatus string `json:"expected_workspace_status,omitempty"` | ||
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. We should also document this in docs/resources/mws_workspaces.md. 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. Is this true even if the feature is still in private preview? 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. Yes. Private preview services, methods and fields (and their documentation) are still part of the SDKs. Currently, private preview methods are not shown in the REST API documentation. For example: https://registry.terraform.io/providers/databricks/databricks/latest/docs/resources/feature_engineering_feature I would like us to add badges on individual fields if their stability level lags behind the resource level, but we haven't gotten to that yet. In the meantime, we can add a note in the documentation that this feature is in private preview. 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 documentation, let me know if the wording works |
||
CreationTime int64 `json:"creation_time,omitempty" tf:"computed"` | ||
ExternalCustomerInfo *externalCustomerInfo `json:"external_customer_info,omitempty"` | ||
CloudResourceBucket *CloudResourceContainer `json:"cloud_resource_container,omitempty"` | ||
|
@@ -147,6 +148,9 @@ func (w *Workspace) MarshalJSON() ([]byte, error) { | |
if w.ComputeMode != "" { | ||
workspaceCreationRequest["compute_mode"] = w.ComputeMode | ||
} | ||
if w.ExpectedWorkspaceStatus != "" { | ||
workspaceCreationRequest["expected_workspace_status"] = w.ExpectedWorkspaceStatus | ||
} | ||
return json.Marshal(workspaceCreationRequest) | ||
} | ||
|
||
|
@@ -161,7 +165,7 @@ func (a WorkspacesAPI) Create(ws *Workspace, timeout time.Duration) error { | |
if err != nil { | ||
return err | ||
} | ||
if err = a.WaitForRunning(*ws, timeout); err != nil { | ||
if err = a.WaitForExpectedStatus(*ws, ws.ExpectedWorkspaceStatus, timeout); err != nil { | ||
log.Printf("[ERROR] Deleting failed workspace: %s", err) | ||
if derr := a.Delete(ws.AccountID, fmt.Sprintf("%d", ws.WorkspaceID)); derr != nil { | ||
return fmt.Errorf("%s - %s", err, derr) | ||
|
@@ -239,22 +243,35 @@ func (a WorkspacesAPI) explainWorkspaceFailure(ws Workspace) error { | |
ws.WorkspaceStatusMessage, strBuffer.String()) | ||
} | ||
|
||
// WaitForRunning will wait until workspace is running, otherwise will try to explain why it failed | ||
func (a WorkspacesAPI) WaitForRunning(ws Workspace, timeout time.Duration) error { | ||
// If expected_workspace_status is specified, WaitForExpectedStatus will wait until workspace is in the expected status. | ||
// If not, it will wait until workspace is running, and otherwise will try to explain why it failed. | ||
func (a WorkspacesAPI) WaitForExpectedStatus(ws Workspace, expectedStatus string, timeout time.Duration) error { | ||
// If expected_status is empty, default to RUNNING | ||
if expectedStatus == "" { | ||
expectedStatus = WorkspaceStatusRunning | ||
log.Printf("[INFO] No expected_workspace_status specified, defaulting to %s", expectedStatus) | ||
} | ||
|
||
return resource.RetryContext(a.context, timeout, func() *resource.RetryError { | ||
|
||
workspace, err := a.Read(ws.AccountID, fmt.Sprintf("%d", ws.WorkspaceID)) | ||
if err != nil { | ||
return resource.NonRetryableError(err) | ||
} | ||
|
||
switch workspace.WorkspaceStatus { | ||
case WorkspaceStatusRunning: | ||
log.Printf("[INFO] Workspace is now running") | ||
if strings.Contains(ws.DeploymentName, "900150983cd24fb0") { | ||
// nobody would probably name workspace as 900150983cd24fb0, | ||
// so we'll use it as unit testing shim | ||
return nil | ||
case expectedStatus: | ||
log.Printf("[INFO] Workspace is now in expected status %s", expectedStatus) | ||
// only verify that workspace is reachable if expected status is RUNNING | ||
if expectedStatus == WorkspaceStatusRunning { | ||
if strings.Contains(ws.DeploymentName, "900150983cd24fb0") { | ||
// nobody would probably name workspace as 900150983cd24fb0, | ||
// so we'll use it as unit testing shim | ||
return nil | ||
} | ||
return a.verifyWorkspaceReachable(workspace) | ||
} | ||
return a.verifyWorkspaceReachable(workspace) | ||
return nil | ||
case WorkspaceStatusCanceled, WorkspaceStatusFailed: | ||
log.Printf("[ERROR] Cannot start workspace: %s", workspace.WorkspaceStatusMessage) | ||
err = a.explainWorkspaceFailure(workspace) | ||
|
@@ -267,7 +284,15 @@ func (a WorkspacesAPI) WaitForRunning(ws Workspace, timeout time.Duration) error | |
}) | ||
} | ||
|
||
var workspaceRunningUpdatesAllowed = []string{"credentials_id", "network_id", "storage_customer_managed_key_id", "private_access_settings_id", "managed_services_customer_managed_key_id", "custom_tags"} | ||
var workspaceRunningUpdatesAllowed = []string{ | ||
"credentials_id", | ||
"network_id", | ||
"storage_customer_managed_key_id", | ||
"private_access_settings_id", | ||
"managed_services_customer_managed_key_id", | ||
"custom_tags", | ||
"expected_workspace_status", | ||
} | ||
|
||
// UpdateRunning will update running workspace with couple of possible fields | ||
func (a WorkspacesAPI) UpdateRunning(ws Workspace, timeout time.Duration) error { | ||
|
@@ -301,6 +326,9 @@ func (a WorkspacesAPI) UpdateRunning(ws Workspace, timeout time.Duration) error | |
} | ||
request["custom_tags"] = ws.CustomTags | ||
} | ||
if ws.ExpectedWorkspaceStatus != "" { | ||
request["expected_workspace_status"] = ws.ExpectedWorkspaceStatus | ||
} | ||
|
||
if len(request) == 0 { | ||
return nil | ||
|
@@ -310,7 +338,7 @@ func (a WorkspacesAPI) UpdateRunning(ws Workspace, timeout time.Duration) error | |
if err != nil { | ||
return err | ||
} | ||
return a.WaitForRunning(ws, timeout) | ||
return a.WaitForExpectedStatus(ws, ws.ExpectedWorkspaceStatus, timeout) | ||
} | ||
|
||
// Read will return the mws workspace metadata and status of the workspace deployment | ||
|
@@ -551,6 +579,15 @@ func ResourceMwsWorkspaces() common.Resource { | |
Type: schema.TypeString, | ||
Computed: true, | ||
} | ||
// validate that expected_workspace_status is one of [PROVISIONING, RUNNING] | ||
if f, ok := s["expected_workspace_status"]; ok { | ||
f.ValidateDiagFunc = validation.ToDiagFunc( | ||
validation.StringInSlice([]string{ | ||
WorkspaceStatusProvisioning, | ||
WorkspaceStatusRunning, | ||
}, false), | ||
) | ||
} | ||
docOptions := docs.DocOptions{ | ||
Section: docs.Guides, | ||
Slug: "gcp-workspace", | ||
|
@@ -638,7 +675,13 @@ func ResourceMwsWorkspaces() common.Resource { | |
if err = common.StructToData(workspace, workspaceSchema, d); err != nil { | ||
return err | ||
} | ||
err = workspacesAPI.WaitForRunning(workspace, d.Timeout(schema.TimeoutRead)) | ||
// The expected_workspace_status field is input only. | ||
// Therefore, we need to read it from the original Terraform configuration. | ||
expectedStatus := d.Get("expected_workspace_status").(string) | ||
tinglin-db marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// PROVISIONING workspace import may fail because the "expected_workspace_status" is not included in the state during import, nor is it returned by the API. | ||
// As a result, the provider will wait for RUNNING state, which will never happen, and timeout. | ||
// TODO: fix this. | ||
err = workspacesAPI.WaitForExpectedStatus(workspace, expectedStatus, d.Timeout(schema.TimeoutRead)) | ||
if err != nil { | ||
return err | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.