config.externalname: make external name overrides return error

Signed-off-by: Muvaffak Onus <me@muvaf.com>
crossplane · Nov 17, 2021 · 36c9793 · 36c9793
1 parent 924507c
commit 36c9793
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 19 deletions.
diff --git a/pkg/config/resource.go b/pkg/config/resource.go
@@ -18,6 +18,7 @@ package config
 
 import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/pkg/errors"
 )
 
 // SetIdentifierArgumentsFn sets the name of the resource in Terraform attributes map,
@@ -30,22 +31,22 @@ var NopSetIdentifierArgument SetIdentifierArgumentsFn = func(_ map[string]interf
 
 // GetIDFn returns the ID to be used in TF State file, i.e. "id" field in
 // terraform.tfstate.
-type GetIDFn func(externalName string, parameters map[string]interface{}, providerConfig map[string]interface{}) string
+type GetIDFn func(externalName string, parameters map[string]interface{}, providerConfig map[string]interface{}) (string, error)
 
 // ExternalNameAsID returns the name to be used as ID in TF State file.
-var ExternalNameAsID GetIDFn = func(externalName string, _ map[string]interface{}, _ map[string]interface{}) string {
-	return externalName
+var ExternalNameAsID GetIDFn = func(externalName string, _ map[string]interface{}, _ map[string]interface{}) (string, error) {
+	return externalName, nil
 }
 
 // GetExternalNameFn returns the external name extracted from the TF State.
-type GetExternalNameFn func(tfstate map[string]interface{}) string
+type GetExternalNameFn func(tfstate map[string]interface{}) (string, error)
 
 // IDAsExternalName returns the TF State ID as external name.
-var IDAsExternalName GetExternalNameFn = func(tfstate map[string]interface{}) string {
-	if id, ok := tfstate["id"].(string); ok {
-		return id
+var IDAsExternalName GetExternalNameFn = func(tfstate map[string]interface{}) (string, error) {
+	if id, ok := tfstate["id"].(string); ok && id != "" {
+		return id, nil
 	}
-	return ""
+	return "", errors.New("cannot find id in tfstate")
 }
 
 // AdditionalConnectionDetailsFn functions adds custom keys to connection details

diff --git a/pkg/controller/external.go b/pkg/controller/external.go
@@ -138,19 +138,19 @@ func (e *external) Observe(ctx context.Context, mg xpresource.Managed) (managed.
 
 	// No operation was in progress, our observation completed successfully, and
 	// we have an observation to consume.
-	attr := map[string]interface{}{}
-	if err := json.JSParser.Unmarshal(res.State.GetAttributes(), &attr); err != nil {
+	tfstate := map[string]interface{}{}
+	if err := json.JSParser.Unmarshal(res.State.GetAttributes(), &tfstate); err != nil {
 		return managed.ExternalObservation{}, errors.Wrap(err, "cannot unmarshal state attributes")
 	}
-	if err := tr.SetObservation(attr); err != nil {
+	if err := tr.SetObservation(tfstate); err != nil {
 		return managed.ExternalObservation{}, errors.Wrap(err, "cannot set observation")
 	}
 
-	lateInitedAnn, err := resource.LateInitializeAnnotations(tr, e.config.ExternalName.GetExternalNameFn(attr), string(res.State.GetPrivateRaw()))
+	lateInitedAnn, err := resource.LateInitializeAnnotations(tr, e.config, tfstate, string(res.State.GetPrivateRaw()))
 	if err != nil {
 		return managed.ExternalObservation{}, errors.Wrap(err, "cannot late initialize annotations")
 	}
-	conn, err := resource.GetConnectionDetails(attr, tr, e.config)
+	conn, err := resource.GetConnectionDetails(tfstate, tr, e.config)
 	if err != nil {
 		return managed.ExternalObservation{}, errors.Wrap(err, "cannot get connection details")
 	}
@@ -199,18 +199,18 @@ func (e *external) Create(ctx context.Context, mg xpresource.Managed) (managed.E
 	if err != nil {
 		return managed.ExternalCreation{}, errors.Wrap(err, errApply)
 	}
-	attr := map[string]interface{}{}
-	if err := json.JSParser.Unmarshal(res.State.GetAttributes(), &attr); err != nil {
+	tfstate := map[string]interface{}{}
+	if err := json.JSParser.Unmarshal(res.State.GetAttributes(), &tfstate); err != nil {
 		return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal state attributes")
 	}
 
-	conn, err := resource.GetConnectionDetails(attr, tr, e.config)
+	conn, err := resource.GetConnectionDetails(tfstate, tr, e.config)
 	if err != nil {
 		return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details")
 	}
 
 	// NOTE(muvaf): Only spec and metadata changes are saved after Create call.
-	_, err = resource.LateInitializeAnnotations(tr, e.config.ExternalName.GetExternalNameFn(attr), string(res.State.GetPrivateRaw()))
+	_, err = resource.LateInitializeAnnotations(tr, e.config, tfstate, string(res.State.GetPrivateRaw()))
 	return managed.ExternalCreation{ConnectionDetails: conn}, errors.Wrap(err, "cannot late initialize annotations")
 }
 

diff --git a/pkg/resource/lateinit.go b/pkg/resource/lateinit.go
@@ -25,6 +25,8 @@ import (
 	xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/crossplane-contrib/terrajet/pkg/config"
 )
 
 const (
@@ -54,7 +56,7 @@ type GenericLateInitializer struct {
 }
 
 // LateInitializeAnnotations late initializes annotations of the resource
-func LateInitializeAnnotations(tr metav1.Object, name string, privateRaw string) (bool, error) {
+func LateInitializeAnnotations(tr metav1.Object, cfg *config.Resource, tfstate map[string]interface{}, privateRaw string) (bool, error) {
 	if tr.GetAnnotations()[AnnotationKeyPrivateRawAttribute] == privateRaw &&
 		xpmeta.GetExternalName(tr) != "" {
 		return false, nil
@@ -65,6 +67,10 @@ func LateInitializeAnnotations(tr metav1.Object, name string, privateRaw string)
 	if xpmeta.GetExternalName(tr) != "" {
 		return true, nil
 	}
+	name, err := cfg.ExternalName.GetExternalNameFn(tfstate)
+	if err != nil {
+		return false, errors.Wrap(err, "cannot get external name")
+	}
 	xpmeta.SetExternalName(tr, name)
 	return true, nil
 }

diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go
@@ -101,7 +101,11 @@ func (fp *FileProducer) WriteTFState() error {
 	for k, v := range fp.observation {
 		base[k] = v
 	}
-	base["id"] = fp.Config.ExternalName.GetIDFn(meta.GetExternalName(fp.Resource), fp.parameters, fp.Setup.Configuration)
+	id, err := fp.Config.ExternalName.GetIDFn(meta.GetExternalName(fp.Resource), fp.parameters, fp.Setup.Configuration)
+	if err != nil {
+		return errors.Wrap(err, "cannot get id")
+	}
+	base["id"] = id
 	attr, err := json.JSParser.Marshal(base)
 	if err != nil {
 		return errors.Wrap(err, "cannot marshal produced state attributes")