Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

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

Open
ytsarev opened this issue Apr 22, 2022 · 5 comments
Open
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@ytsarev
Copy link
Member

ytsarev commented Apr 22, 2022

What happened?

Context:

    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 ytsarev added the bug Something isn't working label Apr 22, 2022
@muvaf
Copy link
Member

muvaf commented Apr 22, 2022

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?

@stale
Copy link

stale bot commented Aug 13, 2022

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 stale bot added the wontfix This will not be worked on label Aug 13, 2022
@systemstart
Copy link

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
Copy link

@muvaf can we revive this?

@joakimhew
Copy link

joakimhew commented Oct 25, 2022

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
    }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants