-
Notifications
You must be signed in to change notification settings - Fork 510
workspace_id support of SDKv2 resources using Go SDK #5492
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?
Changes from 12 commits
e52006d
456b19f
a52eab2
aa55a8a
cba7b53
bdec6fc
3ce9a13
85d60da
d93748a
ee64a9d
4ac2e20
6d4ef97
7116c4b
3f42734
3f1ac9d
603bc63
e4135ad
f86123b
104c332
ac77de3
b4c9f75
33b2076
514cc7c
e5043dd
e57c5ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,10 +4,12 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "regexp" | ||
| "strconv" | ||
|
|
||
| "github.com/databricks/databricks-sdk-go" | ||
| "github.com/databricks/databricks-sdk-go/client" | ||
| "github.com/databricks/databricks-sdk-go/config" | ||
| "github.com/hashicorp/go-cty/cty" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" | ||
| ) | ||
|
|
@@ -48,6 +50,7 @@ func AddNamespaceInSchema(m map[string]*schema.Schema) map[string]*schema.Schema | |
| m["provider_config"] = &schema.Schema{ | ||
| Type: schema.TypeList, | ||
| Optional: true, | ||
| Computed: true, | ||
| MaxItems: 1, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
|
|
@@ -64,6 +67,7 @@ func AddNamespaceInSchema(m map[string]*schema.Schema) map[string]*schema.Schema | |
| // NamespaceCustomizeSchema is used to customize the schema for the provider configuration | ||
| // for a single schema. | ||
| func NamespaceCustomizeSchema(s *CustomizableSchema) { | ||
| s.SchemaPath("provider_config").SetComputed() | ||
| s.SchemaPath("provider_config", "workspace_id").SetValidateFunc(workspaceIDValidateFunc()) | ||
| } | ||
|
|
||
|
|
@@ -74,6 +78,7 @@ func NamespaceCustomizeSchemaMap(m map[string]*schema.Schema) map[string]*schema | |
| if !ok { | ||
| panic("provider_config not found in schema") | ||
| } | ||
| providerConfig.Computed = true | ||
| elem, ok := providerConfig.Elem.(*schema.Resource) | ||
| if !ok { | ||
| panic("provider_config.Elem is not a *schema.Resource") | ||
|
|
@@ -84,11 +89,63 @@ func NamespaceCustomizeSchemaMap(m map[string]*schema.Schema) map[string]*schema | |
| return m | ||
| } | ||
|
|
||
| // namespaceForceNew marks the workspace_id field as ForceNew if it changed. | ||
| func namespaceForceNew(d *schema.ResourceDiff) error { | ||
| oldWorkspaceID, newWorkspaceID := d.GetChange(workspaceIDSchemaKey) | ||
| if oldWorkspaceID != "" && newWorkspaceID != "" && oldWorkspaceID != newWorkspaceID { | ||
| if err := d.ForceNew(workspaceIDSchemaKey); err != nil { | ||
| // namespaceForceNew is used to customize the diff for the provider configuration | ||
| // in a resource diff. It resolves effective workspace IDs (accounting for | ||
| // workspace_id fallback) and triggers ForceNew when the effective | ||
| // workspace changes. | ||
| // | ||
| // With provider_config marked as Optional+Computed, Terraform preserves the state | ||
| // value when the config doesn't specify provider_config. This means d.GetChange() | ||
| // shows no change in that case. We use GetRawConfigAt to inspect the actual user | ||
| // config and determine the true effective new workspace ID. | ||
| func namespaceForceNew(ctx context.Context, d *schema.ResourceDiff, c *DatabricksClient) error { | ||
| workspaceIDKey := workspaceIDSchemaKey | ||
|
|
||
| // Get the old (state) workspace ID. | ||
| oldWsID, _ := d.GetChange(workspaceIDKey) | ||
| oldEffective, _ := oldWsID.(string) | ||
|
|
||
| // Determine the new effective workspace ID by inspecting the raw config. | ||
| // With Optional+Computed, d.GetChange may return the preserved state value | ||
| // rather than reflecting the actual config, so we check the raw config directly. | ||
| configWsID, configHasProviderConfig := workspaceIDFromRawDiffConfig(d) | ||
|
|
||
| var newEffective string | ||
| if configHasProviderConfig { | ||
| // Config explicitly sets provider_config.workspace_id | ||
| newEffective = configWsID | ||
| } else { | ||
| // Config does not have provider_config; use workspace_id | ||
| newEffective = c.Config.WorkspaceID | ||
| } | ||
| // Fallback to cachedWorkspaceID (resolved from provider host during init). | ||
| // This handles workspace host changes: if the user switches from | ||
| // host=ws-A to host=ws-B, the cached ID reflects the new workspace. | ||
| if newEffective == "" && c.cachedWorkspaceID != 0 { | ||
| newEffective = strconv.FormatInt(c.cachedWorkspaceID, 10) | ||
| } | ||
|
|
||
| // If a resource has a workspace ID in state but the new effective | ||
| // workspace ID is empty (user removed workspace_id and there's no | ||
| // explicit provider_config), error out. This prevents silently continuing | ||
| // to operate against a workspace the user thought they disconnected from. | ||
| if oldEffective != "" && newEffective == "" && c.Config.HostType() != config.WorkspaceHost { | ||
| return fmt.Errorf("resource has provider_config.workspace_id = %q in state, "+ | ||
| "but managing workspace-level resources requires a workspace_id and "+ | ||
| "none was found in provider_config or the provider configuration", oldEffective) | ||
| } | ||
|
|
||
| if oldEffective != "" && newEffective != "" && oldEffective != newEffective { | ||
| // When Optional+Computed preserves the old state value (config removes | ||
| // provider_config), the SDK sees no diff for workspace_id. ForceNew | ||
| // requires HasChange, so we SetNew on the top-level provider_config key | ||
| // to create the diff entry before calling ForceNew. | ||
| if !d.HasChange(workspaceIDKey) { | ||
| if err := d.SetNew("provider_config", []map[string]interface{}{{"workspace_id": newEffective}}); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| if err := d.ForceNew(workspaceIDKey); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
@@ -127,10 +184,29 @@ func NamespaceValidateWorkspaceID(ctx context.Context, d *schema.ResourceDiff, c | |
| return nil | ||
| } | ||
|
|
||
| // workspaceIDFromRawDiffConfig extracts the workspace ID from the raw config | ||
| // of a ResourceDiff. Returns the workspace ID string and whether provider_config | ||
| // is present in the config. | ||
| func workspaceIDFromRawDiffConfig(d *schema.ResourceDiff) (string, bool) { | ||
| path := cty.Path{ | ||
| cty.GetAttrStep{Name: "provider_config"}, | ||
| cty.IndexStep{Key: cty.NumberIntVal(0)}, | ||
| cty.GetAttrStep{Name: "workspace_id"}, | ||
| } | ||
| rawValue, diags := d.GetRawConfigAt(path) | ||
| if diags.HasError() || rawValue.IsNull() || !rawValue.IsKnown() { | ||
| return "", false | ||
| } | ||
| if rawValue.Type() == cty.String { | ||
| return rawValue.AsString(), true | ||
| } | ||
| return "", false | ||
| } | ||
|
|
||
| // NamespaceCustomizeDiff is used to customize the diff for the provider configuration | ||
| // in a resource diff. | ||
| func NamespaceCustomizeDiff(ctx context.Context, d *schema.ResourceDiff, c *DatabricksClient) error { | ||
| if err := namespaceForceNew(d); err != nil { | ||
| if err := namespaceForceNew(ctx, d, c); err != nil { | ||
| return err | ||
| } | ||
| return NamespaceValidateWorkspaceID(ctx, d, c) | ||
|
|
@@ -168,9 +244,15 @@ func (c *DatabricksClient) DatabricksClientForUnifiedProvider(ctx context.Contex | |
| if !ok { | ||
| return nil, fmt.Errorf("workspace_id must be a string") | ||
| } | ||
| // If the workspace_id is not passed in the resource configuration, we don't need to create a new client | ||
| // and can return the current client. | ||
| // If the workspace_id is not passed in the resource configuration, | ||
| // fall back to workspace_id for account-level providers. | ||
| // We don't error here when no workspace_id is found because the caller may | ||
| // use AccountOrWorkspaceRequest to route to account APIs, which doesn't need | ||
| // a workspace-scoped client. | ||
| if workspaceID == "" { | ||
| if c.DatabricksClient != nil && c.Config.HostType() == config.AccountHost && c.Config.WorkspaceID != "" { | ||
| return c.getDatabricksClientForUnifiedProvider(ctx, c.Config.WorkspaceID) | ||
| } | ||
|
||
| return c, nil | ||
| } | ||
| return c.getDatabricksClientForUnifiedProvider(ctx, workspaceID) | ||
|
|
@@ -209,6 +291,22 @@ func (c *DatabricksClient) getDatabricksClientForUnifiedProvider(ctx context.Con | |
| }, nil | ||
| } | ||
|
|
||
| func workspaceIDFromRawConfig(d *schema.ResourceData) (string, bool) { | ||
| path := cty.Path{ | ||
| cty.GetAttrStep{Name: "provider_config"}, | ||
| cty.IndexStep{Key: cty.NumberIntVal(0)}, | ||
| cty.GetAttrStep{Name: "workspace_id"}, | ||
| } | ||
| rawValue, diags := d.GetRawConfigAt(path) | ||
| if diags.HasError() || rawValue.IsNull() || !rawValue.IsKnown() { | ||
| return "", false | ||
| } | ||
| if rawValue.Type() == cty.String { | ||
| return rawValue.AsString(), true | ||
| } | ||
| return "", false | ||
| } | ||
|
|
||
| // setCachedDatabricksClient sets the cached Databricks Client. | ||
| func (c *DatabricksClient) setCachedDatabricksClient(ctx context.Context, workspaceID string) error { | ||
| // Acquire the lock to avoid race conditions. | ||
|
|
||
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.
Would current users not using provider_config see any diff when they upgrade their terraform version during terraform plan?
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.
Also should check
make diff-schemasince we are making schema changesThere 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.
It does not show in plan as it is suppressed by our customised diff for this field.
make diff-schema return "no changes to schema"