Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Provider-contributed Functions #31225

Closed
wants to merge 5 commits into from

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Jun 11, 2022

This is the beginnings of work to evaluate the feasibility of and one possible language design for allowing normal functions to be contributed by providers.

This implementation exposes provider functions under the function namespace provider::NAME::, where NAME is the local name of the provider as declared in the required_providers block. For example, if hashicorp/aws had a hypothetical parse_arn function then it would appear like this under this implementation:

terraform {
  required_providers {
    aws = {
      source = "hashicorp/aws"
    }
  }
}

locals {
  arn_parts = provider::aws::parse_arn("arn:aws:s3:::examplebucket/developers/design_info.doc")
}

output "arn_service" {
  value = local.arn_parts.service  
}

With what I have here so far, the basic mechanism works: providers can declare that they have certain functions available as part of their schema, and then Terraform will call a new CallFunction RPC each time on an unconfigured instance of the provider each it needs to evaluate one of the declared functions.

However, there's a fair amount more to do here before this could be shipped, including:

  • It spins up an entirely new instance of the contributing provider for each individual function call, which is likely to be pretty slow and memory intensive for real plugins that run as child processes. We'll need to find some way to reuse these for multiple calls and then clean them up when we're finished.
  • It doesn't make any attempts to guarantee that the provider-contributed functions correctly honor the contracts such as behaving as pure functions. Properly checking this is important because if a function doesn't uphold Terraform's expectations then it will cause confusing errors reported downstream, incorrectly blaming other components for the inconsistency.

There are also many external dependencies that will require some broader consensus on this path before we could possibly move forward to shipping, including but certainly not limited to:

  • The plugin framework will need some reasonable way for a provider developer to implement and register functions, integrated with the same type system implementation it uses for other purposes.
  • The language server will need awareness of provider-contributed functions in general and some way to learn about which specific functions are in scope for a particular module in order to offer code completion and hover tips.
  • Terraform Registry will need some way to display documentation for provider-contributed functions as part of the documentation for a provider.

I'm sure there are other shortcomings that I've not thought of yet, too. I've also only wrote some minimal additional tests just to show the main case working, so it'll need considerably more automated tests before it could be shippable.

This essentially doubles up the registrations for all of the built-in
functions to have both non-namespaced and core::-namespaced versions of
each function.

This is in preparation for later commits introducing other namespaces,
such as a provider:: namespace which could hold functions that were
contributed by providers that are currently in scope.
This is some initial work in the direction of allowing providers to
contribute additional functions for use in modules that make use of those
providers.

The milestone here is establishing the protocol elements for providers to
declare functions as part of their schema information and then to handle
calls to those functions via the new CallFunction RPC.

Terraform Core doesn't yet know how to look up these functions and wire
them into the HCL evaluation context so that they'll actually be callable,
so implementing and declaring functions would not actually be useful as
of this commit, but we'll use this starting point to wire these into the
Terraform language in subsequent commits.
This is intended as a bridge to functions defined outside of the core
language, which will live in other namespaces in order to avoid ambiguity
with any built-in functions.

For now we only accept provider-contributed external functions. The lang
package doesn't really know anything special about providers, so its only
responsibility here is to enforce the naming scheme that any
provider-contributed functions always live under the namespace prefix
"provider::NAME::", where NAME is a name decided by whatever codepath
instantiated the state as a unique identifier for a particular provider.

This commit also includes updates to the core package to make it possible
to pass in ExternalFuncs in principle, although in practice we don't yet
have any codepaths which do so, and so this is effectively a no-op commit.
This establishes all of the wiring necessary for a provider's declared
functions to appear in the hcl.EvalContext when we evaluate expressions
inside a module that depends on a provider that contributes functions.

Although this includes the minimum required machinery to make it work,
it is still lacking in a few different ways:
- It spins up an entirely new instance of the contributing provider for
  each individual function call, which is likely to be pretty slow and
  memory intensive for real plugins that run as child processes. We'll
  need to find some way to reuse these for multiple calls and then
  clean them up when we're finished.
- It doesn't make any attempts to guarantee that the provider-contributed
  functions correctly honor the contracts such as behaving as pure
  functions. Properly checking this is important because if a function
  doesn't uphold Terraform's expectations then it will cause confusing
  errors reported downstream, incorrectly blaming other components for
  the inconsistency.

With that said then, there's still plenty more work to do here before this
could be shipped but at least it demonstrates that provider-contributed
functions are viable and demonstrates one design for how they might appear
in the Terraform language.
@flynnhandley
Copy link

Hey @crw @apparentlymart 👋

It looks like there are a few other PR's waiting on this PR.

Based on your comments @crw, it seems this feature is likely to be implemented.

Is there any way I can help progress it?

Cheers 🏝️

@crw
Copy link
Collaborator

crw commented Dec 5, 2022

The main thing is upvotes on the main issue. I am building an argument that Functions are the #1 most requested feature if you consider all of the upvotes on all of the various functions requests. However it is always a challenge to prioritize against other projects that also have a lot of user interest. In any case, thanks for the bump on this draft PR.

@apparentlymart
Copy link
Contributor Author

This PR is essentially a feasibility prototype I built to see what seems solved already vs. what remains to be solved. I don't expect that this PR will actually ever be merged itself, and instead it's more likely that there will be another PR that achieves a similar effect but with different implementation details.

If you want to express interest in the general idea of provider-contributed functions I would suggest instead adding your upvotes to #2771, which is the issue representing the use-case as opposed to this PR which is representing only a prototype implementation of that use-case.

(No harm in voting here too if you want, but what I want to avoid is people only voting here and then those votes getting missed when I inevitably close out this PR without merging it.)

@A141864
Copy link

A141864 commented Dec 6, 2022

The main thing is upvotes on the main issue. I am building an argument that Functions are the #1 most requested feature if you consider all of the upvotes on all of the various functions requests. However it is always a challenge to prioritize against other projects that also have a lot of user interest. In any case, thanks for the bump on this draft PR.

That's a fair argument, however, I can see there are a lot of PRs for functions that are pending a feature that likely won't be delivered in the near future.

What are the tradeoffs of merging the existing PRs into terraform core?

@crw
Copy link
Collaborator

crw commented Dec 6, 2022

What are the tradeoffs of merging the existing PRs into terraform core?

Number one is the maintenance cost on an ever-increasingly more-complex code base. As an example, base36decode: once it is added, it cannot be removed, but this is a function that belongs in a bigger package of encoding/decoding functions. It will make it easier, in the long run, to hold the line now so that these functions can be more cleanly and clearly organized in the future.

The other problem that comes to mind are functions that do not have a clear one-size-fits-all implementation. As an example, deep merge. There are a couple of different deep merge proposals, it is clear different users have different expected behavior for a deep merge function. With external functions, you can choose the implementation that works best for your use case (or write your own).

We have accepted functions recently (startswith and endswith string functions), and we do have a process to champion functions we believe are core-enough to include. We are not taking an all-or-nothing approach, but are being fairly picky about what makes the cut for "core." Let me know if I can clarify any of the above. As always, this thinking is always in motion and changing, so please take this as a snapshot of the current thinking on this topic.

@apparentlymart
Copy link
Contributor Author

This PR was only ever intended as a prototype to see what technical challenges might arise if we tried to do this, and it's become incredibly merge-conflicty as development has continued in the main branch.

I'm going to close this now just because this PR has served its purpose and will not see any more development in its current form. Others on the team are working towards a non-prototype design and implementation for this, and so although this PR might still serve as a useful reference point for them the future development in this area will happen in other PRs. My attention is elsewhere, so I'll leave those folks to decide how best to proceed and how much of this PR is useful beyond just being a feasibility prototype.

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config core enhancement providers/protocol Potentially affecting the Providers Protocol and SDKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants