direct: new type-safe way of defining resources (#3532)

## Changes
- All resource methods now use concrete types related to this resource,
e.g. for job we define DoCreate(ctx, config *jobs.JobSettings).
- Resources no longer store config on the instance, it will be passed to
methods that need it.
- All resource-related code is now configured via resource methods in
one file (no longer have separate Settings struct). Each
tnresources/_resourcename_.go is self contained with exception of entry
point registration in all.go.
- The resource implementation is now a singleton, it does not have state
related to a particular resource but instead it's a stateless component
used for all resources. One consequence is that we cannot store waiters
there but need to reconstruct it ourselves. That is okay, because we
need to make "ready" part of the persisted state anyway and be able to
wait on the resource that was created by previous process.
- Internal renames: BundleDeployer -> DeploymentBundle, Deployer ->
DeploymentUnit.
- DeploymentUnits are now cached between plan and apply, allowing for
remote state cache to be propagated between these phases (needed in
#3531).
- New package libs/calladapt to facilitate validating and calling such
methods against interface.

## Why
This makes resource implementation type-safe, making it easier to
implement manually today or via LLM or generator in the future.

Especially useful when there are more types / structs involved, as in
remote state support #3531

Also, you get initial feedback quickly, by doing "go test
./bundle/terranova/tnresources'. That type checks all the interfaces and
runs your resource against testserver.

## Tests
There are a new set of unit tests that do a bunch of type checks across
methods, e.g. DoCreate and DoUpdate must accept the same config type.
There are also unit tests that run a few of resource methods against
testserver.

---------

Co-authored-by: Pieter Noordhuis <[email protected]>

Author: denik

.golangci.yaml

@@ -69,6 +69,9 @@ linters:
       - path-except: bundle/terranova/tnresources
         linters:
           - exhaustruct
+      - path: bundle/terranova/tnresources/all_test.go
+        linters:
+          - exhaustruct
 issues:
   max-issues-per-linter: 1000
   max-same-issues: 1000

bundle/bundle.go

@@ -129,8 +129,8 @@ type Bundle struct {
 	// If true, the plan is empty and applying it will not do anything
 	TerraformPlanIsEmpty bool
 
-	// (direct only) graph of dependencies between resources
-	BundleDeployer terranova.BundleDeployer
+	// (direct only) deployment implementation and state
+	DeploymentBundle terranova.DeploymentBundle
 
 	// if true, we skip approval checks for deploy, destroy resources and delete
 	// files
@@ -351,7 +351,7 @@ func (b *Bundle) OpenStateFile(ctx context.Context) error {
 		return err
 	}
 
-	err = b.BundleDeployer.OpenStateFile(statePath)
+	err = b.DeploymentBundle.OpenStateFile(statePath)
 	if err != nil {
 		return fmt.Errorf("failed to open/create state file at %s: %s", statePath, err)
 	}

bundle/phases/deploy.go

@@ -31,11 +31,11 @@ func getActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Action, err
 		if err != nil {
 			return nil, err
 		}
-		err = b.BundleDeployer.CalculatePlanForDeploy(ctx, b.WorkspaceClient(), &b.Config)
+		err = b.DeploymentBundle.CalculatePlanForDeploy(ctx, b.WorkspaceClient(), &b.Config)
 		if err != nil {
 			return nil, err
 		}
-		return b.BundleDeployer.GetActions(ctx), nil
+		return b.DeploymentBundle.GetActions(ctx), nil
 	} else {
 		tf := b.Terraform
 		if tf == nil {
@@ -118,7 +118,7 @@ func deployCore(ctx context.Context, b *bundle.Bundle) {
 	cmdio.LogString(ctx, "Deploying resources...")
 
 	if b.DirectDeployment {
-		b.BundleDeployer.Apply(ctx, b.WorkspaceClient(), &b.Config)
+		b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(), &b.Config)
 	} else {
 		bundle.ApplyContext(ctx, b, terraform.Apply())
 	}

bundle/phases/destroy.go

@@ -36,11 +36,11 @@ func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Actio
 		if err != nil {
 			return nil, err
 		}
-		err = b.BundleDeployer.CalculatePlanForDestroy(ctx)
+		err = b.DeploymentBundle.CalculatePlanForDestroy(ctx, b.WorkspaceClient())
 		if err != nil {
 			return nil, err
 		}
-		return b.BundleDeployer.GetActions(ctx), nil
+		return b.DeploymentBundle.GetActions(ctx), nil
 	}
 
 	tf := b.Terraform
@@ -119,7 +119,7 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) {
 
 func destroyCore(ctx context.Context, b *bundle.Bundle) {
 	if b.DirectDeployment {
-		b.BundleDeployer.Apply(ctx, b.WorkspaceClient(), &b.Config)
+		b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(), &b.Config)
 	} else {
 		// Core destructive mutators for destroy. These require informed user consent.
 		bundle.ApplyContext(ctx, b, terraform.Apply())

bundle/statemgmt/state_load.go

@@ -40,7 +40,7 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 		if err != nil {
 			return diag.FromErr(err)
 		}
-		state = b.BundleDeployer.StateDB.ExportState(ctx)
+		state = b.DeploymentBundle.StateDB.ExportState(ctx)
 	} else {
 		tf := b.Terraform
 		if tf == nil {

bundle/terranova/apply.go

@@ -11,49 +11,38 @@ import (
 	"github.com/databricks/cli/bundle/deployplan"
 	"github.com/databricks/cli/bundle/terranova/tnstate"
 	"github.com/databricks/cli/libs/log"
-	"github.com/databricks/databricks-sdk-go"
 )
 
-type Deployer struct {
-	client       *databricks.WorkspaceClient
-	db           *tnstate.TerranovaState
-	group        string
-	resourceName string
-	settings     ResourceSettings
-}
-
-func (d *Deployer) Destroy(ctx context.Context) error {
-	entry, hasEntry := d.db.GetResourceEntry(d.group, d.resourceName)
+func (d *DeploymentUnit) Destroy(ctx context.Context, db *tnstate.TerranovaState) error {
+	entry, hasEntry := db.GetResourceEntry(d.Group, d.Key)
 	if !hasEntry {
-		log.Infof(ctx, "Cannot delete %s.%s: missing from state", d.group, d.resourceName)
+		log.Infof(ctx, "Cannot delete %s.%s: missing from state", d.Group, d.Key)
 		return nil
 	}
 
 	if entry.ID == "" {
 		return errors.New("invalid state: empty id")
 	}
 
-	err := d.Delete(ctx, entry.ID)
+	err := d.Delete(ctx, db, entry.ID)
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (d *Deployer) Deploy(ctx context.Context, inputConfig any, actionType deployplan.ActionType) error {
-	resource, _, err := New(d.client, d.group, d.resourceName, inputConfig)
+func (d *DeploymentUnit) Deploy(ctx context.Context, db *tnstate.TerranovaState, inputConfig any, actionType deployplan.ActionType) error {
+	config, err := d.Adapter.PrepareConfig(inputConfig)
 	if err != nil {
 		return err
 	}
 
-	config := resource.Config()
-
 	if actionType == deployplan.ActionTypeCreate {
-		return d.Create(ctx, resource, config)
+		return d.Create(ctx, db, config)
 	}
 
-	entry, hasEntry := d.db.GetResourceEntry(d.group, d.resourceName)
+	entry, hasEntry := db.GetResourceEntry(d.Group, d.Key)
 	if !hasEntry {
 		return errors.New("state entry not found")
 	}
@@ -65,126 +54,104 @@ func (d *Deployer) Deploy(ctx context.Context, inputConfig any, actionType deplo
 
 	switch actionType {
 	case deployplan.ActionTypeRecreate:
-		return d.Recreate(ctx, resource, oldID, config)
+		return d.Recreate(ctx, db, oldID, config)
 	case deployplan.ActionTypeUpdate:
-		return d.Update(ctx, resource, oldID, config)
+		return d.Update(ctx, db, oldID, config)
 	case deployplan.ActionTypeUpdateWithID:
-		updater, hasUpdater := resource.(IResourceUpdatesID)
-		if !hasUpdater {
-			return errors.New("internal error: plan is update_with_id but resource does not implement UpdateWithID")
-		}
-		return d.UpdateWithID(ctx, resource, updater, oldID, config)
+		return d.UpdateWithID(ctx, db, oldID, config)
 	default:
 		return fmt.Errorf("internal error: unexpected actionType: %#v", actionType)
 	}
 }
 
-func (d *Deployer) Create(ctx context.Context, resource IResource, config any) error {
-	newID, err := resource.DoCreate(ctx)
+func (d *DeploymentUnit) Create(ctx context.Context, db *tnstate.TerranovaState, config any) error {
+	newID, err := d.Adapter.DoCreate(ctx, config)
 	if err != nil {
 		// No need to prefix error, there is no ambiguity (only one operation - DoCreate) and no additional context (like id)
 		return err
 	}
 
-	log.Infof(ctx, "Created %s.%s id=%#v", d.group, d.resourceName, newID)
+	log.Infof(ctx, "Created %s.%s id=%#v", d.Group, d.Key, newID)
 
-	err = d.db.SaveState(d.group, d.resourceName, newID, config)
+	err = db.SaveState(d.Group, d.Key, newID, config)
 	if err != nil {
 		return fmt.Errorf("saving state after creating id=%s: %w", newID, err)
 	}
 
-	err = resource.WaitAfterCreate(ctx)
+	err = d.Adapter.WaitAfterCreate(ctx, config)
 	if err != nil {
 		return fmt.Errorf("waiting after creating id=%s: %w", newID, err)
 	}
 
 	return nil
 }
 
-func (d *Deployer) Recreate(ctx context.Context, resource IResource, oldID string, config any) error {
-	err := DeleteResource(ctx, d.client, d.group, oldID)
+func (d *DeploymentUnit) Recreate(ctx context.Context, db *tnstate.TerranovaState, oldID string, config any) error {
+	err := d.Adapter.DoDelete(ctx, oldID)
 	if err != nil {
 		return fmt.Errorf("deleting old id=%s: %w", oldID, err)
 	}
 
-	err = d.db.SaveState(d.group, d.resourceName, "", nil)
+	err = db.SaveState(d.Group, d.Key, "", nil)
 	if err != nil {
 		return fmt.Errorf("deleting state: %w", err)
 	}
 
-	newID, err := resource.DoCreate(ctx)
-	if err != nil {
-		return fmt.Errorf("re-creating: %w", err)
-	}
-
-	// TODO: This should be at notice level (info < notice < warn) and it should be visible by default,
-	// but to match terraform output today, we hide it (and also we don't have notice level)
-	log.Infof(ctx, "Recreated %s.%s id=%#v (previously %#v)", d.group, d.resourceName, newID, oldID)
-	err = d.db.SaveState(d.group, d.resourceName, newID, config)
-	if err != nil {
-		return fmt.Errorf("saving state for id=%s: %w", newID, err)
-	}
-
-	err = resource.WaitAfterCreate(ctx)
-	if err != nil {
-		return fmt.Errorf("waiting after re-creating id=%s: %w", newID, err)
-	}
-
-	return nil
+	return d.Create(ctx, db, config)
 }
 
-func (d *Deployer) Update(ctx context.Context, resource IResource, id string, config any) error {
-	err := resource.DoUpdate(ctx, id)
+func (d *DeploymentUnit) Update(ctx context.Context, db *tnstate.TerranovaState, id string, config any) error {
+	err := d.Adapter.DoUpdate(ctx, id, config)
 	if err != nil {
 		return fmt.Errorf("updating id=%s: %w", id, err)
 	}
 
-	err = d.db.SaveState(d.group, d.resourceName, id, config)
+	err = db.SaveState(d.Group, d.Key, id, config)
 	if err != nil {
 		return fmt.Errorf("saving state id=%s: %w", id, err)
 	}
 
-	err = resource.WaitAfterUpdate(ctx)
+	err = d.Adapter.WaitAfterUpdate(ctx, config)
 	if err != nil {
 		return fmt.Errorf("waiting after updating id=%s: %w", id, err)
 	}
 
 	return nil
 }
 
-func (d *Deployer) UpdateWithID(ctx context.Context, resource IResource, updater IResourceUpdatesID, oldID string, config any) error {
-	newID, err := updater.DoUpdateWithID(ctx, oldID)
+func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *tnstate.TerranovaState, oldID string, config any) error {
+	newID, err := d.Adapter.DoUpdateWithID(ctx, oldID, config)
 	if err != nil {
 		return fmt.Errorf("updating id=%s: %w", oldID, err)
 	}
 
 	if oldID != newID {
-		log.Infof(ctx, "Updated %s.%s id=%#v (previously %#v)", d.group, d.resourceName, newID, oldID)
+		log.Infof(ctx, "Updated %s.%s id=%#v (previously %#v)", d.Group, d.Key, newID, oldID)
 	} else {
-		log.Infof(ctx, "Updated %s.%s id=%#v", d.group, d.resourceName, newID)
+		log.Infof(ctx, "Updated %s.%s id=%#v", d.Group, d.Key, newID)
 	}
 
-	err = d.db.SaveState(d.group, d.resourceName, newID, config)
+	err = db.SaveState(d.Group, d.Key, newID, config)
 	if err != nil {
 		return fmt.Errorf("saving state id=%s: %w", oldID, err)
 	}
 
-	err = resource.WaitAfterUpdate(ctx)
+	err = d.Adapter.WaitAfterUpdate(ctx, config)
 	if err != nil {
 		return fmt.Errorf("waiting after updating id=%s: %w", newID, err)
 	}
 
 	return nil
 }
 
-func (d *Deployer) Delete(ctx context.Context, oldID string) error {
+func (d *DeploymentUnit) Delete(ctx context.Context, db *tnstate.TerranovaState, oldID string) error {
 	// TODO: recognize 404 and 403 as "deleted" and proceed to removing state
-	err := DeleteResource(ctx, d.client, d.group, oldID)
+	err := d.Adapter.DoDelete(ctx, oldID)
 	if err != nil {
 		return fmt.Errorf("deleting id=%s: %w", oldID, err)
 	}
 
-	err = d.db.DeleteState(d.group, d.resourceName)
+	err = db.DeleteState(d.Group, d.Key)
 	if err != nil {
 		return fmt.Errorf("deleting state id=%s: %w", oldID, err)
 	}

bundle/terranova/bundle_apply.go

@@ -10,51 +10,41 @@ import (
 	"github.com/databricks/databricks-sdk-go"
 )
 
-func (b *BundleDeployer) Apply(ctx context.Context, client *databricks.WorkspaceClient, configRoot *config.Root) {
+func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.WorkspaceClient, configRoot *config.Root) {
 	if b.Graph == nil {
 		panic("Planning is not done")
 	}
 
-	if len(b.PlannedActions) == 0 {
+	if len(b.DeploymentUnits) == 0 {
 		// Avoid creating state file if nothing to deploy
 		return
 	}
 
 	b.StateDB.AssertOpened()
 
 	b.Graph.Run(defaultParallelism, func(node deployplan.ResourceNode, failedDependency *deployplan.ResourceNode) bool {
-		actionType := b.PlannedActions[node]
-
-		errorPrefix := fmt.Sprintf("cannot %s %s.%s", actionType.String(), node.Group, node.Key)
+		d, exists := b.DeploymentUnits[node]
+		if !exists {
+			// Resource with actionType == noop are not added to DeploymentUnits.
+			// All references to it must have been resolved during planning.
+			return true
+		}
+		errorPrefix := fmt.Sprintf("cannot %s %s.%s", d.ActionType.String(), node.Group, node.Key)
 
 		// If a dependency failed, report and skip execution for this node by returning false
 		if failedDependency != nil {
 			logdiag.LogError(ctx, fmt.Errorf("%s: dependency failed: %s", errorPrefix, failedDependency.String()))
 			return false
 		}
 
-		settings, ok := SupportedResources[node.Group]
-		if !ok {
-			// Unexpected, this should be filtered at plan.
-			return false
-		}
-
 		// The way plan currently works, is that it does not add resources with Noop action, turning them into Unset.
 		// So we skip both, although at this point we will not see Noop here.
-		if actionType == deployplan.ActionTypeUnset || actionType == deployplan.ActionTypeNoop {
+		if d.ActionType == deployplan.ActionTypeUnset || d.ActionType == deployplan.ActionTypeNoop {
 			return true
 		}
 
-		d := Deployer{
-			client:       client,
-			db:           &b.StateDB,
-			group:        node.Group,
-			resourceName: node.Key,
-			settings:     settings,
-		}
-
-		if actionType == deployplan.ActionTypeDelete {
-			err := d.Destroy(ctx)
+		if d.ActionType == deployplan.ActionTypeDelete {
+			err := d.Destroy(ctx, &b.StateDB)
 			if err != nil {
 				logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
 				return false
@@ -84,15 +74,15 @@ func (b *BundleDeployer) Apply(ctx context.Context, client *databricks.Workspace
 
 		// TODO: redo calcDiff to downgrade planned action if possible (?)
 
-		err = d.Deploy(ctx, config, actionType)
+		err = d.Deploy(ctx, &b.StateDB, config, d.ActionType)
 		if err != nil {
 			logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err))
 			return false
 		}
 
 		// Update resources.id after successful deploy so that future ${resources...id} refs are replaced
 		if b.Graph.HasOutgoingEdges(node) {
-			err = resolveIDReference(ctx, d.db, configRoot, node.Group, node.Key)
+			err = resolveIDReference(ctx, &b.StateDB, configRoot, node.Group, node.Key)
 			if err != nil {
 				// not using errorPrefix because resource was deployed
 				logdiag.LogError(ctx, fmt.Errorf("failed to replace ref to resources.%s.%s.id: %w", node.Group, node.Key, err))