refactor unnecessary iface pointers & add go doc comments

Signed-off-by: Erhan Cagirici <[email protected]>

pkg/config/common.go

@@ -57,7 +57,7 @@ type ResourceOption func(*Resource)
 
 // DefaultResource keeps an initial default configuration for all resources of a
 // provider.
-func DefaultResource(name string, terraformSchema *schema.Resource, terraformPluginFrameworkResource *fwresource.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource {
+func DefaultResource(name string, terraformSchema *schema.Resource, terraformPluginFrameworkResource fwresource.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource {
 	words := strings.Split(name, "_")
 	// As group name we default to the second element if resource name
 	// has at least 3 elements, otherwise, we took the first element as

pkg/config/common_test.go

@@ -19,7 +19,7 @@ func TestDefaultResource(t *testing.T) {
 	type args struct {
 		name              string
 		sch               *schema.Resource
-		frameworkResource *fwresource.Resource
+		frameworkResource fwresource.Resource
 		reg               *registry.Resource
 		opts              []ResourceOption
 	}

pkg/config/provider.go

@@ -118,24 +118,34 @@ type Provider struct {
 	// Defaults to []string{".+"} which would include all resources.
 	IncludeList []string
 
-	// NoForkIncludeList  is a list of regex for the Terraform resources to be
-	// included and reconciled in the no-fork architecture (without the
-	// Terraform CLI).
+	// NoForkIncludeList is a list of regex for the Terraform resources
+	// implemented with Terraform Plugin SDKv2 to be included and reconciled
+	// in the no-fork architecture (without the Terraform CLI).
 	// For example, to include "aws_shield_protection_group" into
 	// the generated resources, one can add "aws_shield_protection_group$".
 	// To include whole aws waf group, one can add "aws_waf.*" to the list.
 	// Defaults to []string{".+"} which would include all resources.
 	NoForkIncludeList []string
 
+	// TerraformPluginFrameworkIncludeList is a list of regex for the Terraform
+	// resources implemented with Terraform Plugin Framework  to be included and
+	// reconciled in the no-fork architecture (without the Terraform CLI).
+	// For example, to include "aws_shield_protection_group" into
+	// the generated resources, one can add "aws_shield_protection_group$".
+	// To include whole aws waf group, one can add "aws_waf.*" to the list.
+	// Defaults to []string{".+"} which would include all resources.
 	TerraformPluginFrameworkIncludeList []string
 
 	// Resources is a map holding resource configurations where key is Terraform
 	// resource name.
 	Resources map[string]*Resource
 
-	// TerraformProvider is the Terraform schema of the provider.
+	// TerraformProvider is the Terraform provider in Terraform Plugin SDKv2
+	// compatible format
 	TerraformProvider *schema.Provider
 
+	// TerraformPluginFrameworkProvider is the Terraform provider reference
+	// in Terraform Plugin Framework compatible format
 	TerraformPluginFrameworkProvider fwprovider.Provider
 
 	// refInjectors is an ordered list of `ReferenceInjector`s for
@@ -177,13 +187,17 @@ func WithIncludeList(l []string) ProviderOption {
 	}
 }
 
-// WithNoForkIncludeList configures IncludeList for this Provider.
+// WithNoForkIncludeList configures the NoForkIncludeList for this Provider,
+// with the given Terraform Plugin SDKv2-based resource name list
 func WithNoForkIncludeList(l []string) ProviderOption {
 	return func(p *Provider) {
 		p.NoForkIncludeList = l
 	}
 }
 
+// WithTerraformPluginFrameworkIncludeList configures the
+// TerraformPluginFrameworkIncludeList for this Provider, with the given
+// Terraform Plugin Framework-based resource name list
 func WithTerraformPluginFrameworkIncludeList(l []string) ProviderOption {
 	return func(p *Provider) {
 		p.TerraformPluginFrameworkIncludeList = l
@@ -197,6 +211,8 @@ func WithTerraformProvider(tp *schema.Provider) ProviderOption {
 	}
 }
 
+// WithTerraformPluginFrameworkProvider configures the
+// TerraformPluginFrameworkProvider for this Provider.
 func WithTerraformPluginFrameworkProvider(tp fwprovider.Provider) ProviderOption {
 	return func(p *Provider) {
 		p.TerraformPluginFrameworkProvider = tp
@@ -317,7 +333,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s
 			}
 		}
 
-		var terraformPluginFrameworkResource *fwresource.Resource
+		var terraformPluginFrameworkResource fwresource.Resource
 
 		if isPluginFrameworkResource {
 			// TODO: Consider whether to replace the commented out conditional in the next line with an equivalent conditional for plugin framework.
@@ -335,7 +351,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s
 				resourceTypeNameResp := fwresource.MetadataResponse{}
 				resource.Metadata(ctx, resourceTypeNameReq, &resourceTypeNameResp)
 				if resourceTypeNameResp.TypeName == name {
-					terraformPluginFrameworkResource = &resource
+					terraformPluginFrameworkResource = resource
 					break
 				}
 			}

pkg/config/resource.go

@@ -376,10 +376,13 @@ type Resource struct {
 	// e.g. aws_rds_cluster.
 	Name string
 
-	// TerraformResource is the Terraform representation of the resource.
+	// TerraformResource is the Terraform representation of the
+	// Terraform Plugin SDKv2 based resource.
 	TerraformResource *schema.Resource
 
-	TerraformPluginFrameworkResource *fwresource.Resource
+	// TerraformPluginFrameworkResource is the Terraform representation
+	// of the TF Plugin Framework based resource
+	TerraformPluginFrameworkResource fwresource.Resource
 
 	// ShortGroup is the short name of the API group of this CRD. The full
 	// CRD API group is calculated by adding the group suffix of the provider.
@@ -459,10 +462,16 @@ type Resource struct {
 	useTerraformPluginFrameworkClient bool
 }
 
+// ShouldUseNoForkClient returns whether to generate a SDKv2-based no-fork
+// external client for this Resource, instead of the Terraform CLI-forking
+// external client
 func (r *Resource) ShouldUseNoForkClient() bool {
 	return r.useNoForkClient
 }
 
+// ShouldUseTerraformPluginFrameworkClient returns whether to generate a
+// Terraform Plugin Framework-based no-fork external client for this Resource
+// instead of a Terraform Plugin SDKv2-based external client
 func (r *Resource) ShouldUseTerraformPluginFrameworkClient() bool {
 	return r.useTerraformPluginFrameworkClient
 }

pkg/controller/external_terraform_plugin_framework.go

@@ -90,7 +90,7 @@ type terraformPluginFrameworkExternalClient struct {
 	logger         logging.Logger
 	metricRecorder *metrics.MetricRecorder
 	opTracker      *AsyncTracker
-	resource       *fwresource.Resource
+	resource       fwresource.Resource
 	server         tfprotov5.ProviderServer
 	params         map[string]any
 	plannedState   *tfprotov5.DynamicValue
@@ -168,7 +168,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre
 }
 
 func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (rschema.Schema, error) {
-	res := *c.config.TerraformPluginFrameworkResource
+	res := c.config.TerraformPluginFrameworkResource
 	schemaResp := &fwresource.SchemaResponse{}
 	res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp)
 	if schemaResp.Diagnostics.HasError() {

pkg/controller/nofork_store.go

@@ -18,12 +18,36 @@ import (
 	"github.com/crossplane/upjet/pkg/terraform"
 )
 
+// AsyncTracker holds information for a managed resource to track the
+// async Terraform operations and the
+// Terraform state (TF SDKv2 or TF Plugin Framework) of the external resource
+//
+// The typical usage is to instantiate an AsyncTracker for a managed resource,
+// and store in a global OperationTrackerStore, to carry information between
+// reconciliation scopes.
+//
+// When an asynchronous Terraform operation is started for the resource
+// in a reconciliation (e.g. with a goroutine), consumers can mark an operation start
+// on the LastOperation field, then access the operation status in the
+// forthcoming reconciliation cycles, and act upon
+// (e.g. hold further actions if there is an ongoing operation, mark the end
+// when underlying Terraform operation is completed, save the resulting
+// terraform state etc.)
+//
+// When utilized without the LastOperation usage, it can act as a Terraform
+// state cache for synchronous reconciliations
 type AsyncTracker struct {
+	// LastOperation holds information about the most recent operation.
+	// Consumers are responsible for managing the last operation by starting,
+	// ending and flushing it when done with processing the results.
+	// Designed to allow only one ongoing operation at a given time.
 	LastOperation *terraform.Operation
 	logger        logging.Logger
 	mu            *sync.Mutex
-	tfState       *tfsdk.InstanceState
-	fwState       *tfprotov5.DynamicValue
+	// TF Plugin SDKv2 instance state for TF Plugin SDKv2-based resources
+	tfState *tfsdk.InstanceState
+	// TF Plugin Framework instance state for TF Plugin Framework-based resources
+	fwState *tfprotov5.DynamicValue
 	// lifecycle of certain external resources are bound to a parent resource's
 	// lifecycle, and they cannot be deleted without actually deleting
 	// the owning external resource (e.g.,  a database resource as the parent
@@ -44,6 +68,8 @@ func WithAsyncTrackerLogger(l logging.Logger) AsyncTrackerOption {
 		w.logger = l
 	}
 }
+
+// NewAsyncTracker initializes an AsyncTracker with given options
 func NewAsyncTracker(opts ...AsyncTrackerOption) *AsyncTracker {
 	w := &AsyncTracker{
 		LastOperation: &terraform.Operation{},
@@ -56,24 +82,35 @@ func NewAsyncTracker(opts ...AsyncTrackerOption) *AsyncTracker {
 	return w
 }
 
+// GetTfState returns the stored Terraform Plugin SDKv2 InstanceState for
+// SDKv2 Terraform resources
+// MUST be only used for SDKv2 resources.
 func (a *AsyncTracker) GetTfState() *tfsdk.InstanceState {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	return a.tfState
 }
 
+// HasState returns whether the AsyncTracker has a SDKv2 state stored.
+// MUST be only used for SDKv2 resources.
 func (a *AsyncTracker) HasState() bool {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	return a.tfState != nil && a.tfState.ID != ""
 }
 
+// SetTfState stores the given SDKv2 Terraform InstanceState into
+// the AsyncTracker
+// MUST be only used for SDKv2 resources.
 func (a *AsyncTracker) SetTfState(state *tfsdk.InstanceState) {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	a.tfState = state
 }
 
+// GetTfID returns the Terraform ID of the external resource currently
+// stored in this AsyncTracker's SDKv2 instance state.
+// MUST be only used for SDKv2 resources.
 func (a *AsyncTracker) GetTfID() string {
 	a.mu.Lock()
 	defer a.mu.Unlock()
@@ -95,39 +132,42 @@ func (a *AsyncTracker) SetDeleted(deleted bool) {
 	a.isDeleted.Store(deleted)
 }
 
+// GetFrameworkTFState returns the stored Terraform Plugin Framework external
+// resource state in this AsyncTracker as *tfprotov5.DynamicValue
+// MUST be used only for Terraform Plugin Framework resources
 func (a *AsyncTracker) GetFrameworkTFState() *tfprotov5.DynamicValue {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	return a.fwState
 }
 
+// HasFrameworkTFState returns whether this AsyncTracker has a
+// Terraform Plugin Framework state stored.
+// MUST be used only for Terraform Plugin Framework resources
 func (a *AsyncTracker) HasFrameworkTFState() bool {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	return a.fwState != nil
 }
 
+// SetFrameworkTFState stores the given *tfprotov5.DynamicValue Terraform Plugin Framework external
+// resource state into this AsyncTracker's fwstate
+// MUST be used only for Terraform Plugin Framework resources
 func (a *AsyncTracker) SetFrameworkTFState(state *tfprotov5.DynamicValue) {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	a.fwState = state
 }
 
-func (a *AsyncTracker) GetFrameworkTFID() string {
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	if a.fwState == nil {
-		return ""
-	}
-	return "TBD"
-}
-
+// OperationTrackerStore stores the AsyncTracker instances associated with the
+// managed resource instance.
 type OperationTrackerStore struct {
 	store  map[types.UID]*AsyncTracker
 	logger logging.Logger
 	mu     *sync.Mutex
 }
 
+// NewOperationStore returns a new OperationTrackerStore instance
 func NewOperationStore(l logging.Logger) *OperationTrackerStore {
 	ops := &OperationTrackerStore{
 		store:  map[types.UID]*AsyncTracker{},
@@ -138,6 +178,12 @@ func NewOperationStore(l logging.Logger) *OperationTrackerStore {
 	return ops
 }
 
+// Tracker returns the associated *AsyncTracker stored in this
+// OperationTrackerStore for the given managed resource.
+// If there is no tracker stored previously, a new AsyncTracker is created and
+// stored for the specified managed resource. Subsequent calls with the same managed
+// resource will return the previously instantiated and stored AsyncTracker
+// for that managed resource
 func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker {
 	ops.mu.Lock()
 	defer ops.mu.Unlock()
@@ -150,6 +196,8 @@ func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker
 	return tracker
 }
 
+// RemoveTracker will remove the stored AsyncTracker of the given managed
+// resource from this OperationTrackerStore.
 func (ops *OperationTrackerStore) RemoveTracker(obj xpresource.Object) error {
 	ops.mu.Lock()
 	defer ops.mu.Unlock()