crossplane / terrajet

Generate Crossplane Providers from any Terraform Provider
https://crossplane.io
Apache License 2.0
290 stars 38 forks source link

Call external name configurator in controller #123

Closed muvaf closed 2 years ago

muvaf commented 2 years ago

What problem are you facing?

Currently, the custom external name config function is called in apis/group/v1alpha1/zz_.._terraformed.go. So, the type package of the CRD depends on the package that the external name configuration lives.

We used to have that custom func in types package but we're moving towards having all custom code in a separate folder like top-level config. So, the problem we face now is that we cannot use any CRD types in config folder because it'd result in circular dependency.

How could Terrajet help solve your problem?

Instead of calling the function in types package, we can have the WorkspaceStore call it, i.e. controller, hence the dependency tree would look like controller -> config -> apis. This would enable us to:

The last point I believe we discussed with @turkenh a while ago. We can't make reference config use strong-typed functions, hence wanted the rest to use func path as well but there wasn't many config points back then. Now that we have more configuration interfaces, I think it's worth to keep ref resolver use path and the rest use strong-typed funcs.

turkenh commented 2 years ago

Sounds good @muvaf, I agree that directly providing function instead of their paths would be a nice improvement 👍

Use CRD structs in custom reference extractors, hence not have to marshal objects into unstructured in those functions.

Actually, this was helpful because we would use some generic functions across multiple resources instead of writing the same for all of them. For example, ARNExtractor() in provider-tf-aws was used around 15-20 times, and assuming we would otherwise need to write about 5-10 strong typed functions, I am not sure if the performance improvement we get by not marshaling is really significant that requires us to do so.

muvaf commented 2 years ago

Actually, this was helpful because we would use some generic functions across multiple resources instead of writing the same for all of them. For example, ARNExtractor() in provider-tf-aws was used around 15-20 times,

I think we can keep using that or have a single extractor that can handle multiple types. It's just that this feature would give us ability to choose.