terrajet icon indicating copy to clipboard operation
terrajet copied to clipboard

Can't create cross resource reference if the terraform field type is Number

Open ytsarev opened this issue 2 years ago • 5 comments

What happened?

Context:

  • https://github.com/crossplane-contrib/provider-jet-datadog

  • Attempt to create cross resource reference for ServiceLevelObjective resource pointing to Monitors

    p.AddResourceConfigurator("datadog_service_level_objective", func(r *tjconfig.Resource) {
        r.ExternalName = tjconfig.IdentifierFromProvider
        r.Kind = "ServiceLevelObjective"
        r.ShortGroup = "monitor"
        r.References["monitor_ids"] = config.Reference{
            Type:      "Monitor",
        }
    })

It fails with type mismatch error

ERRO Running error: buildir: analysis skipped: errors in package: [/Users/xnull/upstream/provider-jet-datadog/apis/monitor/v1alpha1/zz_generated.resolvers.go:36:42: cannot use mg.Spec.ForProvider.MonitorIds (variable of type []*float64) as []*string value in argument to reference.FromPtrValues /User
s/xnull/upstream/provider-jet-datadog/apis/monitor/v1alpha1/zz_generated.resolvers.go:48:35: cannot use reference.ToPtrValues(mrsp.ResolvedValues) (vadiff --git a/apis/monitor/v1alpha1/zz_generated.deepcopy.go b/apis/monitor/v1alpha1/zz_generated.deepcopy.go

I've tried to override the field type ( thanks @ezgidemirel ) with

r.TerraformResource.Schema["monitor_ids"].Type = schema.TypeString

but it failed runtime with

    Message:               observe failed: cannot run refresh: refresh failed: Incorrect attribute value type: Inappropriate value for attribute "monitor_ids": set of number required.: File name: main.tf.json

reporting the underlying terraform type mismatch

I've also tried to play with Extractor function but it looks like it always has to return ExtractValueFn of crossplane-runtime but it also eventually return string.

CC: @turkenh

How can we reproduce it?

git clone https://github.com/crossplane-contrib/provider-jet-datadog.git
cd provider-jet-datadog
vi config/monitor/config.go

Add the reference configuration to config/monitor/config.go

+               r.References["monitor_ids"] = config.Reference{
+                       Type: "Monitor",
+               }
make reviewable

to reproduce the type mismatch failure

ytsarev avatar Apr 22 '22 10:04 ytsarev

I think this is a missing feature in reference resolver in crossplane-runtime affecting all providers. It assumes that the value could either be string or []string. There are some helpers for pointers, maybe we could get away with adding helpers for float/int etc?

muvaf avatar Apr 22 '22 10:04 muvaf

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 13 '22 00:08 stale[bot]

I guess this would block building a hcloud provider as well, since it has integer id's.

For example: https://registry.terraform.io/providers/hetznercloud/hcloud/latest/docs/resources/network#attributes-reference

systemstart avatar Sep 06 '22 10:09 systemstart

@muvaf can we revive this?

systemstart avatar Sep 07 '22 07:09 systemstart

I just wanted to say that I've started working on implementing support for cross-resource references for other primitives than string. So far I've added support for float64 but will add support for all basic types.

From my understanding, the best way to solve this is by updating the crossplane-runtime and add methods that support converting resolved references to other primitives than strings and also to update the crossplane-tools code generation to make sure the correct methods are used to convert the resolved values.

The initial commit for supporting this can be found in my fork of crossplane-runtime and crossplane-tools

E.g. in the case where the the source is of type []*float64, crossplane-tools will now generate the following code instead:

    for i3 := 0; i3 < len(mg.Spec.ForProvider.Reviewers); i3++ {
    mrsp, err = r.ResolveMultiple(ctx, reference.MultiResolutionRequest{
--       CurrentValues: reference.FromPtrValues(mg.Spec.ForProvider.Reviewers[i3].Teams),
++       CurrentValues: reference.FromFloatPtrValues(mg.Spec.ForProvider.Reviewers[i3].Teams),
	    Extract:       reference.ExternalName(),
	    References:    mg.Spec.ForProvider.Reviewers[i3].TeamsRefs,
	    Selector:      mg.Spec.ForProvider.Reviewers[i3].TeamsSelector,
	    To: reference.To{
		    List:    &v1alpha1.TeamList{},
		    Managed: &v1alpha1.Team{},
	    },
    })
    if err != nil {
	    return errors.Wrap(err, "mg.Spec.ForProvider.Reviewers[i3].Teams")
    }
--  mg.Spec.ForProvider.Reviewers[i3].Teams = reference.ToPtrValues(mrsp.ResolvedValues)
++  mg.Spec.ForProvider.Reviewers[i3].Teams = reference.ToFloatPtrValues(mrsp.ResolvedValues)
     mg.Spec.ForProvider.Reviewers[i3].TeamsRefs = mrsp.ResolvedReferences
    }

joakimhew avatar Oct 25 '22 21:10 joakimhew