terraform plugin framework external client & connectors #329

erhancagirici · 2024-01-16T07:37:17Z

Description of your changes

Introduces new external clients for terraform plugin framework resources

I have:

Read and followed Upjet's [contribution process].
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Tested with crossplane-contrib/provider-upjet-aws#1086.

jeanduplessis · 2024-01-16T08:29:38Z

pkg/config/provider.go

+			}
+		}
+
+		p.Resources[name] = DefaultResource(name, terraformResource, terraformPluginFrameworkResource, providerMetadata.Resources[name], p.DefaultResourceOptions...)
 		p.Resources[name].useNoForkClient = isNoFork


Now that we are introducing TerraformPluginFramework terminology, could we please refactor "noFork" terminology to TerraformPluginSDK throughout the code base for uniformity and clearer understanding?

Let's keep renaming the no-fork external client out of scope for this PR and we will follow-up with a new PR after we merge this one. Thank you.

ulucinar

Thank you @erhancagirici & @mergenci for attacking the remaining issue of reconciling the Terraform plugin framework resources natively without the TF CLI. Outstanding work here. Left some comments for you to consider. Also planning a second pass.

@erhancagirici, could you please also update the description of the PR and briefly discuss the architecture and the newly introduced APIs (tf plugin fw external connecter/client, extensions to the async tracker, etc.) with this PR?

pkg/config/provider.go

pkg/config/common.go

pkg/config/common_test.go

pkg/config/provider.go

pkg/config/resource.go

ulucinar · 2024-01-26T17:37:18Z

pkg/config/provider.go

+					"but either config.Provider.TerraformProvider is not configured or the Go schema does not exist for the resource", name))
+			}
+
+			for _, resourceFunc := range terraformPluginFrameworkResourceFunctions {


Suggested change

for _, resourceFunc := range terraformPluginFrameworkResourceFunctions {

for _, resourceFunc := range p.TerraformPluginFrameworkProvider.Resources(ctx) {

as mentioned in #329 (comment), we prefer to call p.TerraformPluginFrameworkProvider.Resources(ctx) one time out of the main loop.

For loop here is now removed, as part of a further performance improved. See the comment above.

pkg/config/resource.go

pkg/controller/nofork_store.go

ulucinar · 2024-01-26T18:04:36Z

pkg/terraform/store.go

@@ -122,6 +123,8 @@ type Setup struct {
 	Scheduler ProviderScheduler

 	Meta any
+
+	FrameworkProvider fwprovider.Provider


Leaving a comment here as a reminder for what we had discussed previously: Because we initialize the framework server and the framework provider at each reconciliation, we may try to extend the provider configuration framework and have the provider directly initialized at each reconciliation in the ExternalConnecter.Connect instead of the provider's terraform.SetupFn so that we don't have to carry it via the terraform.Setup. Let's keep it out of scope for this PR though. We may attempt this "optimization" in a follow-up PR.

ulucinar

Thank you @erhancagirici & @mergenci. We will also need to implement the unit tests for these changes.

pkg/controller/external_terraform_plugin_framework.go

Signed-off-by: Cem Mergenci <[email protected]>

Configuring provider server with a nil configuration request lets server retrieve already-configured provider's configuration. Signed-off-by: Cem Mergenci <[email protected]>

Signed-off-by: Erhan Cagirici <[email protected]>

Speedup in upbound/provider-aws, obtained via non-rigorous measurement techniques, is around 1.1x. Performance benefits would be higher, if there were more Terraform Plugin Framework resources configured. Signed-off-by: Cem Mergenci <[email protected]>

Signed-off-by: Cem Mergenci <[email protected]>

Signed-off-by: Erhan Cagirici <[email protected]>

ulucinar

Thanks @erhancagirici & @mergenci for tackling with the framework resources. It's been a really fruitful journey with lots of learning and at the end we've been able to add support for framework resources natively. We will pursue increasing the unit test coverage in #336 in addition to the integration tests we've already performed. There are also two additional framework resources waiting for this PR and we will have validated the changes introduced here with those. In addition, changes from this PR are being tested as part crossplane-contrib/provider-upjet-aws#1086, which depends on this PR.

Thank you for the great work!

jeanduplessis reviewed Jan 16, 2024

View reviewed changes

ulucinar marked this pull request as ready for review January 26, 2024 16:54

ulucinar requested review from ulucinar and sergenyalcin as code owners January 26, 2024 16:54

ulucinar reviewed Jan 26, 2024

View reviewed changes

ulucinar reviewed Jan 29, 2024

View reviewed changes

mergenci force-pushed the no-fork-fw branch from f17bb6a to b348aaf Compare January 30, 2024 06:23

mergenci and others added 20 commits January 30, 2024 21:22

WIP: Break ground for Terraform Plugin Framework reconciliation.

f8668e2

Signed-off-by: Cem Mergenci <[email protected]>

Configure provider server.

40adf4f

Configuring provider server with a nil configuration request lets server retrieve already-configured provider's configuration. Signed-off-by: Cem Mergenci <[email protected]>

initial full working setup

7f9ab46

Signed-off-by: Erhan Cagirici <[email protected]>

refuse diffs with resource replacement & code cleanup refactor

fde8472

Signed-off-by: Erhan Cagirici <[email protected]>

plugin framework async external client

c45556a

Signed-off-by: Erhan Cagirici <[email protected]>

logging improvements & refactor

9a712e8

Signed-off-by: Erhan Cagirici <[email protected]>

dynamically configure provider of the framework server

b9c6910

Signed-off-by: Erhan Cagirici <[email protected]>

refactor obsolete plugin framework provider variables

48630ae

Signed-off-by: Erhan Cagirici <[email protected]>

bump terraform-plugin-framework to 1.4.1

50593c6

Signed-off-by: Erhan Cagirici <[email protected]>

diff detection with plan and state comparison & refactor

0339b39

Signed-off-by: Erhan Cagirici <[email protected]>

fix linter issues

1cabce6

Signed-off-by: Erhan Cagirici <[email protected]>

minor log message and naming changes

31e377d

Signed-off-by: Erhan Cagirici <[email protected]>

fix unit tests

68bf3f1

Signed-off-by: Erhan Cagirici <[email protected]>

fix TF list conversions

615d6e7

Signed-off-by: Erhan Cagirici <[email protected]>

refactor unnecessary iface pointers & add go doc comments

fe03f89

Signed-off-by: Erhan Cagirici <[email protected]>

Add doc comments.

9b9149c

Signed-off-by: Cem Mergenci <[email protected]>

refactor diag to string, terraform value type, error messages

e7b633e

Signed-off-by: Erhan Cagirici <[email protected]>

pre-allocate diag error accumulator slice

340b66f

Signed-off-by: Erhan Cagirici <[email protected]>

add doc comments for internal funcs

2b2812f

Signed-off-by: Erhan Cagirici <[email protected]>

erhancagirici force-pushed the no-fork-fw branch from 2c2b7b1 to 2b2812f Compare January 30, 2024 18:23

erhancagirici added 2 commits January 30, 2024 21:25

go mod tidy

67c38e9

Signed-off-by: Erhan Cagirici <[email protected]>

terraform resource nil check for SDKv2

bad163c

Signed-off-by: Erhan Cagirici <[email protected]>

erhancagirici added 2 commits January 31, 2024 14:41

add unit tests for terraform plugin framework external client

b7edc58

Signed-off-by: Erhan Cagirici <[email protected]>

refactor unit tests

5dff491

Signed-off-by: Erhan Cagirici <[email protected]>

erhancagirici mentioned this pull request Jan 31, 2024

increase unit test coverage for terraform plugin framework external client implementation #336

Open

ulucinar approved these changes Jan 31, 2024

View reviewed changes

ulucinar merged commit af203bd into crossplane:main Jan 31, 2024
6 of 7 checks passed

mergenci mentioned this pull request Feb 26, 2024

MEMBERSHIP: mergenci crossplane/org#73

Closed

6 tasks

sdischer-sap mentioned this pull request May 8, 2024

[FEATURE] Expose Terraform Plugin Framework Provider SAP/terraform-provider-btp#807

Closed

displague mentioned this pull request Jul 24, 2024

fix provider definition crossplane-contrib/provider-jet-equinix#59

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

terraform plugin framework external client & connectors #329

terraform plugin framework external client & connectors #329

erhancagirici commented Jan 16, 2024 •

edited by ulucinar

Loading

jeanduplessis Jan 16, 2024

ulucinar Jan 26, 2024

ulucinar left a comment •

edited

Loading

ulucinar Jan 26, 2024

erhancagirici Jan 27, 2024

mergenci Jan 30, 2024

ulucinar Jan 26, 2024

ulucinar left a comment

ulucinar left a comment

	for _, resourceFunc := range terraformPluginFrameworkResourceFunctions {
	for _, resourceFunc := range p.TerraformPluginFrameworkProvider.Resources(ctx) {

terraform plugin framework external client & connectors #329

terraform plugin framework external client & connectors #329

Conversation

erhancagirici commented Jan 16, 2024 • edited by ulucinar Loading

Description of your changes

How has this code been tested

jeanduplessis Jan 16, 2024

Choose a reason for hiding this comment

ulucinar Jan 26, 2024

Choose a reason for hiding this comment

ulucinar left a comment • edited Loading

Choose a reason for hiding this comment

ulucinar Jan 26, 2024

Choose a reason for hiding this comment

erhancagirici Jan 27, 2024

Choose a reason for hiding this comment

mergenci Jan 30, 2024

Choose a reason for hiding this comment

ulucinar Jan 26, 2024

Choose a reason for hiding this comment

ulucinar left a comment

Choose a reason for hiding this comment

ulucinar left a comment

Choose a reason for hiding this comment

erhancagirici commented Jan 16, 2024 •

edited by ulucinar

Loading

ulucinar left a comment •

edited

Loading