Merge pull request #124 from muvaf/importmenow

external name: use function in the config directly instead of generating the call

pkg/config/common.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2021 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config
+
+// Common ExternalName configurations.
+var (
+	// NameAsIdentifier uses "name" field in the arguments as the identifier of
+	// the resource.
+	NameAsIdentifier = ExternalName{
+		SetIdentifierArgumentFn: func(base map[string]interface{}, name string) {
+			base["name"] = name
+		},
+		OmittedFields: []string{
+			"name",
+			"name_prefix",
+		},
+	}
+
+	// IdentifierFromProvider is used in resources whose identifier is assigned by
+	// the remote client, such as AWS VPC where it gets an identifier like
+	// vpc-2213das instead of letting user choose a name.
+	IdentifierFromProvider = ExternalName{
+		SetIdentifierArgumentFn: NopSetIdentifierArgument,
+		DisableNameInitializer:  true,
+	}
+)

pkg/config/resource.go

@@ -18,13 +18,12 @@ package config
 
 import "github.com/imdario/mergo"
 
-// ConfigureWithNameFn functions configure the base resource fields by using
-// given name value.
-type ConfigureWithNameFn func(base map[string]interface{}, name string)
+// SetIdentifierArgumentFn sets the name of the resource in Terraform attributes map.
+type SetIdentifierArgumentFn func(base map[string]interface{}, name string)
 
-// NopConfigureWithName does nothing. It's useful for cases where the external
+// NopSetIdentifierArgument does nothing. It's useful for cases where the external
 // name is calculated by provider and doesn't have any effect on spec fields.
-func NopConfigureWithName(_ map[string]interface{}, _ string) {}
+func NopSetIdentifierArgument(_ map[string]interface{}, _ string) {}
 
 // ResourceOption allows setting optional fields of a Resource object.
 type ResourceOption func(*Resource)
@@ -43,6 +42,9 @@ func NewResource(version, kind, terraformResourceType string, opts ...ResourceOp
 		Kind:                  kind,
 		TerraformResourceType: terraformResourceType,
 		TerraformIDFieldName:  "id",
+		ExternalName: ExternalName{
+			SetIdentifierArgumentFn: NopSetIdentifierArgument,
+		},
 	}
 	for _, f := range opts {
 		f(c)
@@ -54,8 +56,9 @@ func NewResource(version, kind, terraformResourceType string, opts ...ResourceOp
 // such as removal of those fields from spec schema and calling Configure function
 // to fill attributes with information given in external name.
 type ExternalName struct {
-	// Configure name attributes of the given configuration using external name.
-	ConfigureFunctionPath string
+	// SetIdentifierArgumentFn sets the name of the resource in Terraform argument
+	// map.
+	SetIdentifierArgumentFn SetIdentifierArgumentFn
 
 	// OmittedFields are the ones you'd like to be removed from the schema since
 	// they are specified via external name. You can omit only the top level fields.

pkg/pipeline/templates/terraformed.go.tmpl

@@ -57,9 +57,6 @@ func (tr *{{ .CRD.Kind }}) GetParameters() (map[string]interface{}, error) {
 		return nil, err
 	}
 	base := map[string]interface{}{}
-	{{- if .CRD.ConfigureExternalName }}
-	{{ .CRD.ConfigureExternalName }}(base, meta.GetExternalName(tr))
-	{{- end }}
 	return base, json.TFParser.Unmarshal(p, &base)
 }
 

pkg/pipeline/terraformed.go

@@ -53,15 +53,10 @@ func (tg *TerraformedGenerator) Generate(c *config.Resource, sch *schema.Resourc
 		wrapper.WithGenStatement(GenStatement),
 		wrapper.WithHeaderPath("hack/boilerplate.go.txt"), // todo
 	)
-	cfgPath := ""
-	if c.ExternalName.ConfigureFunctionPath != "" {
-		cfgPath = trFile.Imports.UseType(c.ExternalName.ConfigureFunctionPath)
-	}
 	vars := map[string]interface{}{
 		"CRD": map[string]string{
-			"APIVersion":            c.Version,
-			"Kind":                  c.Kind,
-			"ConfigureExternalName": cfgPath,
+			"APIVersion": c.Version,
+			"Kind":       c.Kind,
 		},
 		"Terraform": map[string]interface{}{
 			// TODO(hasan): This identifier is used to generate external name.

pkg/terraform/files.go

@@ -22,11 +22,11 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/crossplane/crossplane-runtime/pkg/meta"
 	"github.com/pkg/errors"
 	"github.com/spf13/afero"
 
-	"github.com/crossplane/crossplane-runtime/pkg/meta"
-
+	"github.com/crossplane-contrib/terrajet/pkg/config"
 	"github.com/crossplane-contrib/terrajet/pkg/resource"
 	"github.com/crossplane-contrib/terrajet/pkg/resource/json"
 )
@@ -48,15 +48,36 @@ func WithFileSystem(fs afero.Fs) FileProducerOption {
 	}
 }
 
+// WithConfig sets the resource configuration to be used.
+func WithConfig(cfg config.Resource) FileProducerOption {
+	return func(fp *FileProducer) {
+		fp.Config = cfg
+	}
+}
+
 // NewFileProducer returns a new FileProducer.
 func NewFileProducer(ctx context.Context, client resource.SecretClient, dir string, tr resource.Terraformed, ts Setup, opts ...FileProducerOption) (*FileProducer, error) {
+	fp := &FileProducer{
+		Resource: tr,
+		Setup:    ts,
+		Dir:      dir,
+		fs:       afero.Afero{Fs: afero.NewOsFs()},
+	}
+	for _, f := range opts {
+		f(fp)
+	}
 	params, err := tr.GetParameters()
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot get parameters")
 	}
 	if err = resource.GetSensitiveParameters(ctx, client, tr, params, tr.GetConnectionDetailsMapping()); err != nil {
 		return nil, errors.Wrap(err, "cannot get sensitive parameters")
 	}
+	// TODO(muvaf): Once we have automatic defaulting, remove this if check.
+	if fp.Config.ExternalName.SetIdentifierArgumentFn != nil {
+		fp.Config.ExternalName.SetIdentifierArgumentFn(params, meta.GetExternalName(tr))
+	}
+	fp.parameters = params
 
 	obs, err := tr.GetObservation()
 	if err != nil {
@@ -65,18 +86,8 @@ func NewFileProducer(ctx context.Context, client resource.SecretClient, dir stri
 	if err = resource.GetSensitiveObservation(ctx, client, tr.GetWriteConnectionSecretToReference(), obs); err != nil {
 		return nil, errors.Wrap(err, "cannot get sensitive observation")
 	}
+	fp.observation = obs
 
-	fp := &FileProducer{
-		Resource:    tr,
-		Setup:       ts,
-		Dir:         dir,
-		parameters:  params,
-		observation: obs,
-		fs:          afero.Afero{Fs: afero.NewOsFs()},
-	}
-	for _, f := range opts {
-		f(fp)
-	}
 	return fp, nil
 }
 
@@ -86,6 +97,7 @@ type FileProducer struct {
 	Resource resource.Terraformed
 	Setup    Setup
 	Dir      string
+	Config   config.Resource
 
 	parameters  map[string]interface{}
 	observation map[string]interface{}

pkg/terraform/store.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/crossplane-contrib/terrajet/pkg/config"
 	"github.com/crossplane-contrib/terrajet/pkg/resource"
 )
 
@@ -66,12 +67,13 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption {
 }
 
 // NewWorkspaceStore returns a new WorkspaceStore.
-func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore {
+func NewWorkspaceStore(configs *config.Provider, l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore {
 	ws := &WorkspaceStore{
-		store:  map[types.UID]*Workspace{},
-		logger: l,
-		mu:     sync.Mutex{},
-		fs:     afero.Afero{Fs: afero.NewOsFs()},
+		Configurations: configs,
+		store:          map[types.UID]*Workspace{},
+		logger:         l,
+		mu:             sync.Mutex{},
+		fs:             afero.Afero{Fs: afero.NewOsFs()},
 	}
 	for _, f := range opts {
 		f(ws)
@@ -81,6 +83,8 @@ func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *Workspac
 
 // WorkspaceStore allows you to manage multiple Terraform workspaces.
 type WorkspaceStore struct {
+	Configurations *config.Provider
+
 	// store holds information about ongoing operations of given resource.
 	// Since there can be multiple calls that add/remove values from the map at
 	// the same time, it has to be safe for concurrency since those operations
@@ -100,7 +104,7 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient
 	if err := ws.fs.MkdirAll(dir, os.ModePerm); err != nil {
 		return nil, errors.Wrap(err, "cannot create directory for workspace")
 	}
-	fp, err := NewFileProducer(ctx, c, dir, tr, ts)
+	fp, err := NewFileProducer(ctx, c, dir, tr, ts, WithConfig(ws.Configurations.GetForResource(tr.GetTerraformResourceType())))
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot create a new file producer")
 	}