-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Conversation
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.
90ac96f
to
d6e2c73
Compare
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 🏝️ |
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. |
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.) |
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? |
Number one is the maintenance cost on an ever-increasingly more-complex code base. As an example, 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 ( |
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. |
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::
, whereNAME
is the local name of the provider as declared in therequired_providers
block. For example, ifhashicorp/aws
had a hypotheticalparse_arn
function then it would appear like this under this implementation: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:
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:
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.