-
Notifications
You must be signed in to change notification settings - Fork 233
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
Make provider configuration available prior to GetSchema call #281
Comments
Hi @UnquietCode
Generally speaking predictability and the ability to validate configuration early on is an important value of Terraform. Giving plugin the ability to dynamically modify parts of schema on-the-fly is likely to go against that goal and for that reason I imagine we'd be hesitant to make any changes in that area. I may be misinterpreting your suggestions here though and/or what you are trying to achieve may be achievable without having to modify the provider schema.
Do you mind sharing some details about the API design here? e.g. what makes API premium? Is it entirely different endpoint (hostname)? Are users likely to use both regular and premium APIs at the same time (in the same configuration), or would they more likely use one of these? Is there any functional overlap between these APIs?
This sounds like an important argument. Do you mind sharing more details about this? e.g. how many resources are you dealing with and how many would you expect to get "disabled" under what circumstances? Also could you share any data in regards to performance, if you have measured it in any way? I did notice that our biggest official provider - AWS - tends to have a small delay before actually beginning to calculate the plan, apply, or most other plugin-dependent operations, which may have a couple of reasons, number of resources (539) and data sources (154) being one. Our stance in the past was that most users of that particular plugin are more likely to wait for remote APIs and that waiting is experienced many times repeatedly during any So when we put the delay into context with other delays user experiences, we concluded that optimizing this was not a priority. That doesn't mean we can't or shouldn't optimize it, just that it doesn't seem the effort would match the value majority of users would gain from such optimization. Having more data and knowing more details about your use case may help us better understand it and perhaps re-evaluate the importance of mentioned optimizations or other changes in that area. |
Hey, thank you for the detailed response! I know that takes time and focus on your part. Your point about consistency and predictability, I had expected some push back here, and even find myself agreeing with that as a goal. I can talk a little more about the API layout, but let me just answer these quickly first:
Mainly there are services we offer at a premium that a normal user would have no use for, so the thought was not even loading them could be a better experience for the average user. It actually isn't hard to just imagine these as two separate plugins with a common codebase, however there doesn't seem like a great way to share provider configurations right now (correct me if I'm wrong), and we were hoping to try and keep the configuration burden rather low for new users. The performance we've clocked is about 10 seconds for each plugin invocation, of which there are a few. Compared to the AWS provider, which actually does a few network calls each time, it's about 10x slower. However we also have about 10x more resources and datasources combined, about 10,000 total, so there is much schema to chew through. I think I saw most of the time being spent in messagepack serialization, so maybe some kind of caching or pre-rendering of the schema messages could help us as well. When I think solely about the loading time problem, some other possibilities come to mind, some of which have their own feature requests already:
As I mentioned, the premium API problem is related but is more addressable. We would prefer to not have to break up our other provider into smaller units. I will give a hint towards why our providers are so big, and that is because they are built with code generators. Obviously this is a smaller use case, but increasingly there are some great Terraform plugin projects which are doing just that. So then, if there is a path towards small configuration or optimization changes that would start to open up some breathing room for large providers, maybe we could contribute that on our own, but with a little steering towards what would be an acceptable solution and performance goal. |
+1 My plugin needs this feature, the ability to get the configuration, so that it can generate the Schema. Currently I depend on the default kubeconfig. |
Thank you for describing the API more closely - this is useful! It sounds like you do have a valid reason to keep the logic within one provider based on these details.
It is not something providers often do today (at least I'm not aware of many examples in the wild), but it's possible. I wouldn't call it an anti-pattern nor bad idea - at least not today with the information I have available. The only downside to be aware of is that interpolation doesn't work between the provider blocks themselves at this point, unless you are just interpolating data sources and variables. See hashicorp/terraform#4149 which describes this problem in more detail. It may not be a blocker though if each provider connects to the API independently and they only interpolate on resource level.
For better or worse your provider seems to be on the other end (more extreme) of our diverse user base and I'm not too surprised that you're experiencing these delays given the number of resources and data sources you have there. I don't know about any provider that would be anywhere near these figures.
I would say #224 is aiming to solve a more logical problem where user mistakenly defines a duplicate resource and Terraform attempts to manage it in two or more places. I don't think it aims to solve any performance related problem.
Potentially yes - I suppose there is some space for improvement, but from my experience caching usually doesn't come "for free", so I'd be personally tempted to try everything else before building a cache mechanism.
This seems the most appealing way to tackle this for me 👍 It will require some changes in the protocol and that most likely won't happen until we have deprecated the old protocol 4 (Terraform 0.11). Feel free to file a separate issue for that though - we can discuss this path in more detail there.
Not sure I follow. What do you mean by "already created configs"? Only relevant parts of parsed config are sent to the relevant plugin and only made available to the relevant resource: terraform-plugin-sdk/internal/tfplugin5/tfplugin5.proto Lines 188 to 196 in 22ad314
Unless I'm mistaken it sounds like you're proposing what I proposed back in 2015 here hashicorp/terraform#1199 For the last two point though I have doubts that any optimization related to interpolation is going to make a significant performance difference in this context, because the whole config will eventually be interpolated and individually handed over to plugins for consumption. This is mostly by design as it allows us to parallelise and decouple operations and decoupling the memory makes it easier to reason about these abstractions.
Sharing functionality is possible, but providers were never designed to be importable and their Go code shouldn't represent public API because that is likely to cause headaches as providers need to comply with semantic versioning from end-user's perspective. For that reason we recommend decoupling shared logic into separate repos (Go modules) outside of the provider codebase. We do have some plans to allow sharing of resources within a provider, but the motivation there wasn't originally performance but improvement of the user experience: #57 Perhaps we can discuss expanding that outside of a single provider.
Generated providers is certainly something we do want to support in the future. I have built a custom generator of provider code myself which was used to generate the initial version of the K8S provider. Google also developed their solution. In that context I understand some reasons and challenges. I think though this is a parallel discussion as generating provider in itself doesn't necessarily imply its size, although it makes it easier to create and maintain bigger providers. It seems we do not have a GitHub issue for this yet - feel free to create one, I'd be happy to contribute with some more thoughts. 10k resources really does sound like a lot. It's hard to judge if it's the right design though without knowing the purpose of the API and what these individual resources represent though. All I can suggest is think about resources from user's perspective and avoid just blindly translating each API endpoint to a resource or a data source. Terraform is meant to help users reducing the complexity and not just act like a proxy between an API and a human. For example AWS EC2 has plenty of capabilities, but we don't map each to a resource. In fact the |
SDK version
Use-cases
When working with large or more complicated providers, it would be very helpful to have the provider configuration attributes available during the schema gathering phase. Being able to make some slight adjustments to the schema based on the config opens up some possibilities that I am currently implementing through other means. For example, enabling premium vs. regular API's, or turning off resources that aren't desired to improve loading time. (The loading time in particular is a struggle because a plugin is invoked repeatedly during a normal run.)
Attempted Solutions
An available workaround is to use environment variables to provide configuration options to the provider, which involves duplicating the options in the terraform configuration and also means validation of the options has to be done separately.
Proposal
While it would be nice to have the provider configured in full prior to the initial calls, this would have to be optional since many providers expect to do slower initialization work during that process. Instead it would be great to just have the configuration attributes as specified in the terraform configuration made available ahead of time, so that they can be actionable in both the GetSchema and provider configuration phases.
References
I wasn't able to find an existing request of this nature.
The text was updated successfully, but these errors were encountered: