From f8668e2d0f288849324ffef8db1d0424770774d4 Mon Sep 17 00:00:00 2001 From: Cem Mergenci Date: Thu, 28 Dec 2023 22:53:37 +0300 Subject: [PATCH 01/24] WIP: Break ground for Terraform Plugin Framework reconciliation. Signed-off-by: Cem Mergenci --- go.mod | 10 +- go.sum | 18 + pkg/config/common.go | 28 +- pkg/config/common_test.go | 12 +- pkg/config/provider.go | 54 ++- pkg/config/resource.go | 12 + .../external_terraform_plugin_framework.go | 423 ++++++++++++++++++ pkg/controller/external_test.go | 2 +- pkg/pipeline/controller.go | 13 +- pkg/pipeline/templates/controller.go.tmpl | 14 +- pkg/resource/sensitive_test.go | 8 +- pkg/terraform/files_test.go | 16 +- 12 files changed, 566 insertions(+), 44 deletions(-) create mode 100644 pkg/controller/external_terraform_plugin_framework.go diff --git a/go.mod b/go.mod index 65eaef97..01e61a2d 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,8 @@ require ( github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 github.com/hashicorp/hcl/v2 v2.19.1 github.com/hashicorp/terraform-json v0.17.1 + github.com/hashicorp/terraform-plugin-framework v1.2.0 + github.com/hashicorp/terraform-plugin-go v0.19.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.30.0 github.com/iancoleman/strcase v0.2.0 github.com/json-iterator/go v1.1.12 @@ -74,11 +76,14 @@ require ( github.com/google/uuid v1.3.0 // indirect github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect github.com/hashicorp/go-hclog v1.5.0 // indirect + github.com/hashicorp/go-plugin v1.5.1 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect github.com/hashicorp/go-version v1.6.0 // indirect github.com/hashicorp/logutils v1.0.0 // indirect - github.com/hashicorp/terraform-plugin-go v0.19.0 // indirect github.com/hashicorp/terraform-plugin-log v0.9.0 // indirect + github.com/hashicorp/terraform-registry-address v0.2.2 // indirect + github.com/hashicorp/terraform-svchost v0.1.1 // indirect + github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect github.com/imdario/mergo v0.3.16 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect @@ -94,6 +99,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/oklog/run v1.0.0 // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.44.0 // indirect @@ -116,6 +122,8 @@ require ( golang.org/x/time v0.3.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/appengine v1.6.7 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 // indirect + google.golang.org/grpc v1.58.3 // indirect google.golang.org/protobuf v1.31.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect k8s.io/apiextensions-apiserver v0.28.2 // indirect diff --git a/go.sum b/go.sum index 48362ec2..7978d0f9 100644 --- a/go.sum +++ b/go.sum @@ -63,6 +63,7 @@ github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmms github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/bufbuild/protocompile v0.6.0 h1:Uu7WiSQ6Yj9DbkdnOe7U4mNKp58y9WDMKDn28/ZlunY= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= @@ -210,6 +211,8 @@ github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 h1:1/D3zfFHttUK github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320/go.mod h1:EiZBMaudVLy8fmjf9Npq1dq9RalhveqZG5w/yz3mHWs= github.com/hashicorp/go-hclog v1.5.0 h1:bI2ocEMgcVlz55Oj1xZNBsVi900c7II+fWDyV9o+13c= github.com/hashicorp/go-hclog v1.5.0/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-plugin v1.5.1 h1:oGm7cWBaYIp3lJpx1RUEfLWophprE2EV/KUeqBYo+6k= +github.com/hashicorp/go-plugin v1.5.1/go.mod h1:w1sAEES3g3PuV/RzUrgow20W2uErMly84hhD3um1WL4= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= @@ -223,12 +226,20 @@ github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= github.com/hashicorp/terraform-json v0.17.1 h1:eMfvh/uWggKmY7Pmb3T85u86E2EQg6EQHgyRwf3RkyA= github.com/hashicorp/terraform-json v0.17.1/go.mod h1:Huy6zt6euxaY9knPAFKjUITn8QxUFIe9VuSzb4zn/0o= +github.com/hashicorp/terraform-plugin-framework v1.2.0 h1:MZjFFfULnFq8fh04FqrKPcJ/nGpHOvX4buIygT3MSNY= +github.com/hashicorp/terraform-plugin-framework v1.2.0/go.mod h1:nToI62JylqXDq84weLJ/U3umUsBhZAaTmU0HXIVUOcw= github.com/hashicorp/terraform-plugin-go v0.19.0 h1:BuZx/6Cp+lkmiG0cOBk6Zps0Cb2tmqQpDM3iAtnhDQU= github.com/hashicorp/terraform-plugin-go v0.19.0/go.mod h1:EhRSkEPNoylLQntYsk5KrDHTZJh9HQoumZXbOGOXmec= github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0= github.com/hashicorp/terraform-plugin-log v0.9.0/go.mod h1:rKL8egZQ/eXSyDqzLUuwUYLVdlYeamldAHSxjUFADow= github.com/hashicorp/terraform-plugin-sdk/v2 v2.30.0 h1:X7vB6vn5tON2b49ILa4W7mFAsndeqJ7bZFOGbVO+0Cc= github.com/hashicorp/terraform-plugin-sdk/v2 v2.30.0/go.mod h1:ydFcxbdj6klCqYEPkPvdvFKiNGKZLUs+896ODUXCyao= +github.com/hashicorp/terraform-registry-address v0.2.2 h1:lPQBg403El8PPicg/qONZJDC6YlgCVbWDtNmmZKtBno= +github.com/hashicorp/terraform-registry-address v0.2.2/go.mod h1:LtwNbCihUoUZ3RYriyS2wF/lGPB6gF9ICLRtuDk7hSo= +github.com/hashicorp/terraform-svchost v0.1.1 h1:EZZimZ1GxdqFRinZ1tpJwVxxt49xc/S52uzrw4x0jKQ= +github.com/hashicorp/terraform-svchost v0.1.1/go.mod h1:mNsjQfZyf/Jhz35v6/0LWcv26+X7JPS+buii2c9/ctc= +github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= +github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/iancoleman/strcase v0.2.0 h1:05I4QRnGpI0m37iZQRuskXh+w77mr6Z41lwQzuHLwW0= github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= @@ -236,6 +247,7 @@ github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1: github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= +github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -293,6 +305,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/muvaf/typewriter v0.0.0-20210910160850-80e49fe1eb32 h1:yBQlHXLeUJL3TWVmzup5uT3wG5FLxhiTAiTsmNVocys= github.com/muvaf/typewriter v0.0.0-20210910160850-80e49fe1eb32/go.mod h1:SAAdeMEiFXR8LcHffvIdiLI1w243DCH2DuHq7UrA5YQ= +github.com/oklog/run v1.0.0 h1:Ru7dDtJNOyC66gQ5dQmaCa0qIsAUFY3sFpK1Xk8igrw= +github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= github.com/onsi/ginkgo/v2 v2.11.0 h1:WgqUCUt/lT6yXoQ8Wef0fsNn5cAuMK7+KT9UFRz2tcU= github.com/onsi/ginkgo/v2 v2.11.0/go.mod h1:ZhrRA5XmEE3x3rhlzamx/JJvujdZoJ2uvgI7kR0iZvM= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= @@ -683,6 +697,8 @@ google.golang.org/genproto v0.0.0-20201210142538-e3217bee35cc/go.mod h1:FWY/as6D google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20210108203827-ffc7fda8c3d7/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20210226172003-ab064af71705/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= +google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 h1:bVf09lpb+OJbByTj913DRJioFFAjf/ZGxEz7MajTp2U= +google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98/go.mod h1:TUfxEVdsvPg18p6AslUXFoLdpED4oBnGwyqk3dV1XzM= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= @@ -699,6 +715,8 @@ google.golang.org/grpc v1.31.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= google.golang.org/grpc v1.34.0/go.mod h1:WotjhfgOW/POjDeRt8vscBtXq+2VjORFy659qA51WJ8= google.golang.org/grpc v1.35.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= +google.golang.org/grpc v1.58.3 h1:BjnpXut1btbtgN/6sp+brB2Kbm2LjNXnidYujAVbSoQ= +google.golang.org/grpc v1.58.3/go.mod h1:tgX3ZQDlNJGU96V6yHh1T/JeoBQ2TXdr43YbYSsCJk0= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/pkg/config/common.go b/pkg/config/common.go index eb5c0b8d..c34eee8c 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -7,6 +7,7 @@ package config import ( "strings" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/crossplane/upjet/pkg/registry" @@ -56,7 +57,7 @@ type ResourceOption func(*Resource) // DefaultResource keeps an initial default configuration for all resources of a // provider. -func DefaultResource(name string, terraformSchema *schema.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource { +func DefaultResource(name string, terraformSchema *schema.Resource, terraformPluginFrameworkResource *fwresource.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource { words := strings.Split(name, "_") // As group name we default to the second element if resource name // has at least 3 elements, otherwise, we took the first element as @@ -77,18 +78,19 @@ func DefaultResource(name string, terraformSchema *schema.Resource, terraformReg } r := &Resource{ - Name: name, - TerraformResource: terraformSchema, - MetaResource: terraformRegistry, - ShortGroup: group, - Kind: kind, - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: make(References), - Sensitive: NopSensitive, - UseAsync: true, - SchemaElementOptions: make(SchemaElementOptions), - ServerSideApplyMergeStrategies: make(ServerSideApplyMergeStrategies), + Name: name, + TerraformResource: terraformSchema, + TerraformPluginFrameworkResource: terraformPluginFrameworkResource, + MetaResource: terraformRegistry, + ShortGroup: group, + Kind: kind, + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: make(References), + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: make(SchemaElementOptions), + ServerSideApplyMergeStrategies: make(ServerSideApplyMergeStrategies), } for _, f := range opts { f(r) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index 4b8a7207..c97aa975 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/crossplane/upjet/pkg/registry" @@ -16,10 +17,11 @@ import ( func TestDefaultResource(t *testing.T) { type args struct { - name string - sch *schema.Resource - reg *registry.Resource - opts []ResourceOption + name string + sch *schema.Resource + frameworkResource *fwresource.Resource + reg *registry.Resource + opts []ResourceOption } cases := map[string]struct { @@ -129,7 +131,7 @@ func TestDefaultResource(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := DefaultResource(tc.args.name, tc.args.sch, tc.args.reg, tc.args.opts...) + r := DefaultResource(tc.args.name, tc.args.sch, tc.args.frameworkResource, tc.args.reg, tc.args.opts...) if diff := cmp.Diff(tc.want, r, ignoreUnexported...); diff != "" { t.Errorf("\n%s\nDefaultResource(...): -want, +got:\n%s", tc.reason, diff) } diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 0b31f3f3..b651d16f 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -5,10 +5,13 @@ package config import ( + "context" "fmt" "regexp" tfjson "github.com/hashicorp/terraform-json" + fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pkg/errors" @@ -124,6 +127,8 @@ type Provider struct { // Defaults to []string{".+"} which would include all resources. NoForkIncludeList []string + TerraformPluginFrameworkIncludeList []string + // Resources is a map holding resource configurations where key is Terraform // resource name. Resources map[string]*Resource @@ -131,6 +136,8 @@ type Provider struct { // TerraformProvider is the Terraform schema of the provider. TerraformProvider *schema.Provider + TerraformPluginFrameworkProvider *fwprovider.Provider + // refInjectors is an ordered list of `ReferenceInjector`s for // injecting references across this Provider's resources. refInjectors []ReferenceInjector @@ -177,6 +184,12 @@ func WithNoForkIncludeList(l []string) ProviderOption { } } +func WithTerraformPluginFrameworkIncludeList(l []string) ProviderOption { + return func(p *Provider) { + p.TerraformPluginFrameworkIncludeList = l + } +} + // WithTerraformProvider configures the TerraformProvider for this Provider. func WithTerraformProvider(tp *schema.Provider) ProviderOption { return func(p *Provider) { @@ -184,6 +197,12 @@ func WithTerraformProvider(tp *schema.Provider) ProviderOption { } } +func WithTerraformPluginFrameworkProvider(tp *fwprovider.Provider) ProviderOption { + return func(p *Provider) { + p.TerraformPluginFrameworkProvider = tp + } +} + // WithSkipList configures SkipList for this Provider. func WithSkipList(l []string) ProviderOption { return func(p *Provider) { @@ -231,7 +250,7 @@ func WithMainTemplate(template string) ProviderOption { // NewProvider builds and returns a new Provider from provider // tfjson schema, that is generated using Terraform CLI with: // `terraform providers schema --json` -func NewProvider(schema []byte, prefix string, modulePath string, metadata []byte, opts ...ProviderOption) *Provider { //nolint:gocyclo +func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath string, metadata []byte, opts ...ProviderOption) *Provider { //nolint:gocyclo ps := tfjson.ProviderSchemas{} if err := ps.UnmarshalJSON(schema); err != nil { panic(err) @@ -276,7 +295,8 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt } // if in both of the include lists, the new behavior prevails isNoFork := matches(name, p.NoForkIncludeList) - if len(terraformResource.Schema) == 0 || matches(name, p.SkipList) || (!matches(name, p.IncludeList) && !isNoFork) { + isPluginFrameworkResource := matches(name, p.TerraformPluginFrameworkIncludeList) + if len(terraformResource.Schema) == 0 || matches(name, p.SkipList) || (!matches(name, p.IncludeList) && !isNoFork && !isPluginFrameworkResource) { p.skippedResourceNames = append(p.skippedResourceNames, name) continue } @@ -295,8 +315,36 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt terraformResource.Schema = terraformResource.SchemaFunc() } } - p.Resources[name] = DefaultResource(name, terraformResource, providerMetadata.Resources[name], p.DefaultResourceOptions...) + + terraformPluginFrameworkResource := (*fwresource.Resource)(nil) + if isPluginFrameworkResource { + // TODO: Consider whether to replace the commented out conditional in the next line with an equivalent conditional for plugin framework. + if p.TerraformPluginFrameworkProvider == nil /*|| p.TerraformProvider.ResourcesMap[name] == nil */ { + panic(errors.Errorf("resource %q is configured to be reconciled without the Terraform CLI"+ + "but either config.Provider.TerraformProvider is not configured or the Go schema does not exist for the resource", name)) + } + + // TODO(cem): Consider creating a new context here, rather than getting one as input to this function. + // TODO(cem): Currently, terraformPluginFrameworkResourceFunctions is calculated for each plugin framework resource. Doing so is wasteful, because the result is independent of the resource. It should be called once, outside the loop. + terraformPluginFrameworkResourceFunctions := (*p.TerraformPluginFrameworkProvider).Resources(ctx) + for _, resourceFunc := range terraformPluginFrameworkResourceFunctions { + resource := resourceFunc() + + resourceTypeNameReq := fwresource.MetadataRequest{ + ProviderTypeName: name, + } + resourceTypeNameResp := fwresource.MetadataResponse{} + resource.Metadata(ctx, resourceTypeNameReq, &resourceTypeNameResp) + if resourceTypeNameResp.TypeName == name { + terraformPluginFrameworkResource = &resource + break + } + } + } + + p.Resources[name] = DefaultResource(name, terraformResource, terraformPluginFrameworkResource, providerMetadata.Resources[name], p.DefaultResourceOptions...) p.Resources[name].useNoForkClient = isNoFork + p.Resources[name].useTerraformPluginFrameworkClient = isPluginFrameworkResource } for i, refInjector := range p.refInjectors { if err := refInjector.InjectReferences(p.Resources); err != nil { diff --git a/pkg/config/resource.go b/pkg/config/resource.go index c405c698..93192d72 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -13,6 +13,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/pkg/errors" @@ -379,6 +380,8 @@ type Resource struct { // TerraformResource is the Terraform representation of the resource. TerraformResource *schema.Resource + TerraformPluginFrameworkResource *fwresource.Resource + // ShortGroup is the short name of the API group of this CRD. The full // CRD API group is calculated by adding the group suffix of the provider. // For example, ShortGroup could be `ec2` where group suffix of the @@ -453,6 +456,11 @@ type Resource struct { // be generated instead of the Terraform CLI-forking client. useNoForkClient bool + // useTerraformPluginFrameworkClient indicates that a Terraform + // Plugin Framework external client should be generated instead of + // the Terraform Plugin SDKv2 client. + useTerraformPluginFrameworkClient bool + // OverrideFieldNames allows to manually override the relevant field name to // avoid possible Go struct name conflicts that may occur after Multiversion // CRDs support. During field generation, there may be fields with the same @@ -484,6 +492,10 @@ func (r *Resource) ShouldUseNoForkClient() bool { return r.useNoForkClient } +func (r *Resource) ShouldUseTerraformPluginFrameworkClient() bool { + return r.useTerraformPluginFrameworkClient +} + // CustomDiff customizes the computed Terraform InstanceDiff. This can be used // in cases where, for example, changes in a certain argument should just be // dismissed. The new InstanceDiff is returned along with any errors. diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go new file mode 100644 index 00000000..d4c3b093 --- /dev/null +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -0,0 +1,423 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/crossplane-runtime/pkg/meta" + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/hashicorp/terraform-plugin-framework/providerserver" + fwresource "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource" + "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/metrics" + "github.com/crossplane/upjet/pkg/terraform" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type TerraformPluginFrameworkConnector struct { + getTerraformSetup terraform.SetupFn + kube client.Client + config *config.Resource + logger logging.Logger + metricRecorder *metrics.MetricRecorder + operationTrackerStore *OperationTrackerStore + isManagementPoliciesEnabled bool + terraformPluginFrameworkProvider *fwprovider.Provider +} + +// TerraformPluginFrameworkConnectorOption allows you to configure TerraformPluginFrameworkConnector. +type TerraformPluginFrameworkConnectorOption func(connector *TerraformPluginFrameworkConnector) + +// WithTerraformPluginFrameworkLogger configures a logger for the TerraformPluginFrameworkConnector. +func WithTerraformPluginFrameworkLogger(l logging.Logger) TerraformPluginFrameworkConnectorOption { + return func(c *TerraformPluginFrameworkConnector) { + c.logger = l + } +} + +// WithTerraformPluginFrameworkMetricRecorder configures a metrics.MetricRecorder for the +// TerraformPluginFrameworkConnectorOption. +func WithTerraformPluginFrameworkMetricRecorder(r *metrics.MetricRecorder) TerraformPluginFrameworkConnectorOption { + return func(c *TerraformPluginFrameworkConnector) { + c.metricRecorder = r + } +} + +// WithTerraformPluginFrameworkManagementPolicies configures whether the client should +// handle management policies. +func WithTerraformPluginFrameworkManagementPolicies(isManagementPoliciesEnabled bool) TerraformPluginFrameworkConnectorOption { + return func(c *TerraformPluginFrameworkConnector) { + c.isManagementPoliciesEnabled = isManagementPoliciesEnabled + } +} + +func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, terraformPluginFrameworkProvider *fwprovider.Provider, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { + connector := &TerraformPluginFrameworkConnector{ + getTerraformSetup: sf, + kube: kube, + config: cfg, + operationTrackerStore: ots, + terraformPluginFrameworkProvider: terraformPluginFrameworkProvider, + } + for _, f := range opts { + f(connector) + } + return connector +} + +type terraformPluginFrameworkExternalClient struct { + ts terraform.Setup + config *config.Resource + logger logging.Logger + metricRecorder *metrics.MetricRecorder + opTracker *AsyncTracker + resource *fwresource.Resource + server tfprotov5.ProviderServer + params map[string]any +} + +func processParamsWithStateFunc(schemaMap map[string]*schema.Schema, params map[string]any) map[string]any { + if params == nil { + return params + } + for key, param := range params { + if sc, ok := schemaMap[key]; ok { + params[key] = applyStateFuncToParam(sc, param) + } else { + params[key] = param + } + } + return params +} + +func applyStateFuncToParam(sc *schema.Schema, param any) any { //nolint:gocyclo + if param == nil { + return param + } + switch sc.Type { + case schema.TypeMap: + if sc.Elem == nil { + return param + } + pmap, okParam := param.(map[string]any) + // TypeMap only supports schema in Elem + if mapSchema, ok := sc.Elem.(*schema.Schema); ok && okParam { + for pk, pv := range pmap { + pmap[pk] = applyStateFuncToParam(mapSchema, pv) + } + return pmap + } + case schema.TypeSet, schema.TypeList: + if sc.Elem == nil { + return param + } + pArray, okParam := param.([]any) + if setSchema, ok := sc.Elem.(*schema.Schema); ok && okParam { + for i, p := range pArray { + pArray[i] = applyStateFuncToParam(setSchema, p) + } + return pArray + } else if setResource, ok := sc.Elem.(*schema.Resource); ok { + for i, p := range pArray { + if resParam, okRParam := p.(map[string]any); okRParam { + pArray[i] = processParamsWithStateFunc(setResource.Schema, resParam) + } + } + } + case schema.TypeString: + // For String types check if it is an HCL string and process + if isHCLSnippetPattern.MatchString(param.(string)) { + hclProccessedParam, err := processHCLParam(param.(string)) + if err != nil { + // c.logger.Debug("could not process param, returning original", "param", sc.GoString()) + } else { + param = hclProccessedParam + } + } + if sc.StateFunc != nil { + return sc.StateFunc(param) + } + return param + case schema.TypeBool, schema.TypeInt, schema.TypeFloat: + if sc.StateFunc != nil { + return sc.StateFunc(param) + } + return param + case schema.TypeInvalid: + return param + default: + return param + } + return param +} + +func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { + c.metricRecorder.ObserveReconcileDelay(mg.GetObjectKind().GroupVersionKind(), mg.GetName()) + logger := c.logger.WithValues("uid", mg.GetUID(), "name", mg.GetName(), "gvk", mg.GetObjectKind().GroupVersionKind().String()) + logger.Debug("Connecting to the service provider") + start := time.Now() + ts, err := c.getTerraformSetup(ctx, c.kube, mg) + metrics.ExternalAPITime.WithLabelValues("connect").Observe(time.Since(start).Seconds()) + if err != nil { + return nil, errors.Wrap(err, errGetTerraformSetup) + } + + // To Compute the ResourceDiff: n.resourceSchema.Diff(...) + tr := mg.(resource.Terraformed) + opTracker := c.operationTrackerStore.Tracker(tr) + externalName := meta.GetExternalName(tr) + params, err := getExtendedParameters(ctx, tr, externalName, c.config, ts, c.isManagementPoliciesEnabled, c.kube) + if err != nil { + return nil, errors.Wrapf(err, "failed to get the extended parameters for resource %q", mg.GetName()) + } + + // TODO(cem): Check if we need to perform an equivalent functionality for framework resources. + params = processParamsWithStateFunc(c.config.TerraformResource.Schema, params) + + schemaBlock := c.config.TerraformResource.CoreConfigSchema() + rawConfig, err := schema.JSONMapToStateValue(params, schemaBlock) + if err != nil { + return nil, errors.Wrap(err, "failed to convert params JSON map to cty.Value") + } + if !opTracker.HasState() { + logger.Debug("Instance state not found in cache, reconstructing...") + tfState, err := tr.GetObservation() + if err != nil { + return nil, errors.Wrap(err, "failed to get the observation") + } + copyParams := len(tfState) == 0 + if err = resource.GetSensitiveParameters(ctx, &APISecretClient{kube: c.kube}, tr, tfState, tr.GetConnectionDetailsMapping()); err != nil { + return nil, errors.Wrap(err, "cannot store sensitive parameters into tfState") + } + c.config.ExternalName.SetIdentifierArgumentFn(tfState, externalName) + tfState["id"] = params["id"] + if copyParams { + tfState = copyParameters(tfState, params) + } + + tfStateCtyValue, err := schema.JSONMapToStateValue(tfState, schemaBlock) + if err != nil { + return nil, errors.Wrap(err, "cannot convert JSON map to state cty.Value") + } + s, err := c.config.TerraformResource.ShimInstanceStateFromValue(tfStateCtyValue) + if err != nil { + return nil, errors.Wrap(err, "failed to convert cty.Value to terraform.InstanceState") + } + s.RawPlan = tfStateCtyValue + s.RawConfig = rawConfig + opTracker.SetTfState(s) + } + + return &terraformPluginFrameworkExternalClient{ + ts: ts, + config: c.config, + logger: logger, + metricRecorder: c.metricRecorder, + opTracker: opTracker, + resource: c.config.TerraformPluginFrameworkResource, + server: providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)(), + params: params, + // rawConfig: rawConfig, + }, nil +} + +func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg xpresource.Managed) (managed.ExternalObservation, error) { //nolint:gocyclo + n.logger.Debug("Observing the external resource") + + if meta.WasDeleted(mg) && n.opTracker.IsDeleted() { + return managed.ExternalObservation{ + ResourceExists: false, + }, nil + } + + jsonBytes, err := json.Marshal(n.params) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert params to json bytes") + } + + readRequest := &tfprotov5.ReadResourceRequest{ + TypeName: n.config.Name, + CurrentState: &tfprotov5.DynamicValue{ + JSON: jsonBytes, + }, + } + readResponse, err := n.server.ReadResource(ctx, readRequest) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot read resource") + } + + fmt.Printf("%v", readResponse) + + // start := time.Now() + // newState, diag := n.resourceSchema.RefreshWithoutUpgrade(ctx, n.opTracker.GetTfState(), n.ts.Meta) + // metrics.ExternalAPITime.WithLabelValues("read").Observe(time.Since(start).Seconds()) + // if diag != nil && diag.HasError() { + // return managed.ExternalObservation{}, errors.Errorf("failed to observe the resource: %v", diag) + // } + // n.opTracker.SetTfState(newState) // TODO: missing RawConfig & RawPlan here... + + // resourceExists := newState != nil && newState.ID != "" + // instanceDiff, err := n.getResourceDataDiff(mg.(resource.Terraformed), ctx, newState, resourceExists) + // if err != nil { + // return managed.ExternalObservation{}, errors.Wrap(err, "cannot compute the instance diff") + // } + // n.instanceDiff = instanceDiff + // noDiff := instanceDiff.Empty() + + // var connDetails managed.ConnectionDetails + // if !resourceExists && mg.GetDeletionTimestamp() != nil { + // gvk := mg.GetObjectKind().GroupVersionKind() + // metrics.DeletionTime.WithLabelValues(gvk.Group, gvk.Version, gvk.Kind).Observe(time.Since(mg.GetDeletionTimestamp().Time).Seconds()) + // } + // specUpdateRequired := false + // if resourceExists { + // if mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionUnknown || + // mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionFalse { + // addTTR(mg) + // } + // mg.SetConditions(xpv1.Available()) + // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) + // if err != nil { + // return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") + // } + + // buff, err := json.TFParser.Marshal(stateValueMap) + // if err != nil { + // return managed.ExternalObservation{}, errors.Wrap(err, "cannot marshal the attributes of the new state for late-initialization") + // } + // specUpdateRequired, err = mg.(resource.Terraformed).LateInitialize(buff) + // if err != nil { + // return managed.ExternalObservation{}, errors.Wrap(err, "cannot late-initialize the managed resource") + // } + + // err = mg.(resource.Terraformed).SetObservation(stateValueMap) + // if err != nil { + // return managed.ExternalObservation{}, errors.Errorf("could not set observation: %v", err) + // } + // connDetails, err = resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) + // if err != nil { + // return managed.ExternalObservation{}, errors.Wrap(err, "cannot get connection details") + // } + + // if noDiff { + // n.metricRecorder.SetReconcileTime(mg.GetName()) + // } + // if !specUpdateRequired { + // resource.SetUpToDateCondition(mg, noDiff) + // } + // // check for an external-name change + // if nameChanged, err := n.setExternalName(mg, newState); err != nil { + // return managed.ExternalObservation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during observe") + // } else { + // specUpdateRequired = specUpdateRequired || nameChanged + // } + // } + + return managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ConnectionDetails: managed.ConnectionDetails{}, + ResourceLateInitialized: false, + }, nil +} + +func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { + n.logger.Debug("Creating the external resource") + + // start := time.Now() + // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) + // metrics.ExternalAPITime.WithLabelValues("create").Observe(time.Since(start).Seconds()) + // // diag := n.resourceSchema.CreateWithoutTimeout(ctx, n.resourceData, n.ts.Meta) + // if diag != nil && diag.HasError() { + // return managed.ExternalCreation{}, errors.Errorf("failed to create the resource: %v", diag) + // } + + // if newState == nil || newState.ID == "" { + // return managed.ExternalCreation{}, errors.New("failed to read the ID of the new resource") + // } + // n.opTracker.SetTfState(newState) + + // if _, err := n.setExternalName(mg, newState); err != nil { + // return managed.ExternalCreation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during create") + // } + // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) + // if err != nil { + // return managed.ExternalCreation{}, err + // } + // err = mg.(resource.Terraformed).SetObservation(stateValueMap) + // if err != nil { + // return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) + // } + // conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) + // if err != nil { + // return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") + // } + + // TODO: Obviously, remove the following definition of conn, after it is defined correctly above. + conn := managed.ConnectionDetails{} + + return managed.ExternalCreation{ConnectionDetails: conn}, nil +} + +func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { + n.logger.Debug("Updating the external resource") + + // if err := n.assertNoForceNew(); err != nil { + // return managed.ExternalUpdate{}, errors.Wrap(err, "refuse to update the external resource") + // } + + // start := time.Now() + // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) + // metrics.ExternalAPITime.WithLabelValues("update").Observe(time.Since(start).Seconds()) + // if diag != nil && diag.HasError() { + // return managed.ExternalUpdate{}, errors.Errorf("failed to update the resource: %v", diag) + // } + // n.opTracker.SetTfState(newState) + + // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) + // if err != nil { + // return managed.ExternalUpdate{}, err + // } + + // err = mg.(resource.Terraformed).SetObservation(stateValueMap) + // if err != nil { + // return managed.ExternalUpdate{}, errors.Errorf("failed to set observation: %v", err) + // } + + return managed.ExternalUpdate{}, nil +} + +func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error { + n.logger.Debug("Deleting the external resource") + // if n.instanceDiff == nil { + // n.instanceDiff = tf.NewInstanceDiff() + // } + + // n.instanceDiff.Destroy = true + // start := time.Now() + // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) + // metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds()) + // if diag != nil && diag.HasError() { + // return errors.Errorf("failed to delete the resource: %v", diag) + // } + // n.opTracker.SetTfState(newState) + // // mark the resource as logically deleted if the TF call clears the state + // n.opTracker.SetDeleted(newState == nil) + + return nil +} diff --git a/pkg/controller/external_test.go b/pkg/controller/external_test.go index 68e06c16..4b919ddc 100644 --- a/pkg/controller/external_test.go +++ b/pkg/controller/external_test.go @@ -592,7 +592,7 @@ func TestObserve(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := &external{workspace: tc.w, config: config.DefaultResource("upjet_resource", nil, nil), kube: tc.args.client, logger: logging.NewNopLogger()} + e := &external{workspace: tc.w, config: config.DefaultResource("upjet_resource", nil, nil, nil), kube: tc.args.client, logger: logging.NewNopLogger()} observation, err := e.Observe(context.TODO(), tc.args.obj) if diff := cmp.Diff(tc.want.obs, observation); diff != "" { t.Errorf("\n%s\nObserve(...): -want observation, +got observation:\n%s", tc.reason, diff) diff --git a/pkg/pipeline/controller.go b/pkg/pipeline/controller.go index 40a7c2d0..cdd0339a 100644 --- a/pkg/pipeline/controller.go +++ b/pkg/pipeline/controller.go @@ -47,12 +47,13 @@ func (cg *ControllerGenerator) Generate(cfg *config.Resource, typesPkgPath strin "CRD": map[string]string{ "Kind": cfg.Kind, }, - "DisableNameInitializer": cfg.ExternalName.DisableNameInitializer, - "TypePackageAlias": ctrlFile.Imports.UsePackage(typesPkgPath), - "UseAsync": cfg.UseAsync, - "UseNoForkClient": cfg.ShouldUseNoForkClient(), - "ResourceType": cfg.Name, - "Initializers": cfg.InitializerFns, + "DisableNameInitializer": cfg.ExternalName.DisableNameInitializer, + "TypePackageAlias": ctrlFile.Imports.UsePackage(typesPkgPath), + "UseAsync": cfg.UseAsync, + "UseNoForkClient": cfg.ShouldUseNoForkClient(), + "UseTerraformPluginFrameworkClient": cfg.ShouldUseTerraformPluginFrameworkClient(), + "ResourceType": cfg.Name, + "Initializers": cfg.InitializerFns, } // If the provider has a features package, add it to the controller template. diff --git a/pkg/pipeline/templates/controller.go.tmpl b/pkg/pipeline/templates/controller.go.tmpl index 5b584dc2..7a01b880 100644 --- a/pkg/pipeline/templates/controller.go.tmpl +++ b/pkg/pipeline/templates/controller.go.tmpl @@ -42,7 +42,7 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { cps = append(cps, connection.NewDetailsManager(mgr.GetClient(), *o.SecretStoreConfigGVK, connection.WithTLSConfig(o.ESSOptions.TLSConfig))) } eventHandler := handler.NewEventHandler(handler.WithLogger(o.Logger.WithValues("gvk", {{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind))) - {{- if .UseAsync }} + {{- if and .UseAsync (not .UseTerraformPluginFrameworkClient) }} ac := tjcontroller.NewAPICallbacks(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind), tjcontroller.WithEventHandler(eventHandler){{ if .UseNoForkClient }}, tjcontroller.WithStatusUpdates(false){{ end }}) {{- end}} opts := []managed.ReconcilerOption{ @@ -67,12 +67,20 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { {{- end -}} ) {{- end -}} + {{- else if .UseTerraformPluginFrameworkClient -}} + tjcontroller.NewTerraformPluginFrameworkConnector(mgr.GetClient(), o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.OperationTrackerStore, o.Provider.TerraformPluginFrameworkProvider, + tjcontroller.WithTerraformPluginFrameworkLogger(o.Logger), + tjcontroller.WithTerraformPluginFrameworkMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), + {{if .FeaturesPackageAlias -}} + tjcontroller.WithTerraformPluginFrameworkManagementPolicies(o.Features.Enabled({{ .FeaturesPackageAlias }}EnableBetaManagementPolicies)) + {{- end -}} + ) {{- else -}} - tjcontroller.NewConnector(mgr.GetClient(), o.WorkspaceStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], tjcontroller.WithLogger(o.Logger), tjcontroller.WithConnectorEventHandler(eventHandler), + tjcontroller.NewConnector(mgr.GetClient(), o.WorkspaceStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], tjcontroller.WithLogger(o.Logger), tjcontroller.WithConnectorEventHandler(eventHandler), {{- if .UseAsync }} tjcontroller.WithCallbackProvider(ac), {{- end }} - ) + ) {{- end -}} ), managed.WithLogger(o.Logger.WithValues("controller", name)), diff --git a/pkg/resource/sensitive_test.go b/pkg/resource/sensitive_test.go index de2aabc8..faf72c27 100644 --- a/pkg/resource/sensitive_test.go +++ b/pkg/resource/sensitive_test.go @@ -110,7 +110,7 @@ func TestGetConnectionDetails(t *testing.T) { "NoConnectionDetails": { args: args{ tr: &fake.Terraformed{}, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), }, }, "OnlyDefaultConnectionDetails": { @@ -122,7 +122,7 @@ func TestGetConnectionDetails(t *testing.T) { }, }, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), data: map[string]any{ "top_level_secret": "sensitive-data-top-level-secret", }, @@ -142,7 +142,7 @@ func TestGetConnectionDetails(t *testing.T) { }, }, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), data: map[string]any{ "top_level_secrets": []any{ "val1", @@ -168,7 +168,7 @@ func TestGetConnectionDetails(t *testing.T) { }, }, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), data: map[string]any{ "top_level_secrets": map[string]any{ "key1": "val1", diff --git a/pkg/terraform/files_test.go b/pkg/terraform/files_test.go index 926ac1f2..ba86a300 100644 --- a/pkg/terraform/files_test.go +++ b/pkg/terraform/files_test.go @@ -67,7 +67,7 @@ func TestEnsureTFState(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), fs: func() afero.Afero { return afero.Afero{Fs: afero.NewMemMapFs()} }, @@ -95,7 +95,7 @@ func TestEnsureTFState(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil, func(r *config.Resource) { + cfg: config.DefaultResource("upjet_resource", nil, nil, nil, func(r *config.Resource) { r.OperationTimeouts.Read = 2 * time.Minute }), fs: func() afero.Afero { @@ -126,7 +126,7 @@ func TestEnsureTFState(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), fs: func() afero.Afero { fss := afero.Afero{Fs: afero.NewMemMapFs()} _ = fss.WriteFile(filepath.Join(dir, "terraform.tfstate"), []byte(empty), 0600) @@ -278,7 +278,7 @@ func TestIsStateEmpty(t *testing.T) { Parameterizable: fake.Parameterizable{Parameters: map[string]any{}}, }, Setup{}, - config.DefaultResource("upjet_resource", nil, nil), WithFileSystem(tc.args.fs()), + config.DefaultResource("upjet_resource", nil, nil, nil), WithFileSystem(tc.args.fs()), ) empty, err := fp.isStateEmpty() if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -326,7 +326,7 @@ func TestWriteMainTF(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil, func(r *config.Resource) { + cfg: config.DefaultResource("upjet_resource", nil, nil, nil, func(r *config.Resource) { r.OperationTimeouts = config.OperationTimeouts{ Read: 30 * time.Second, Update: 2 * time.Minute, @@ -363,7 +363,7 @@ func TestWriteMainTF(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), s: Setup{ Requirement: ProviderRequirement{ Source: "hashicorp/provider-test", @@ -395,7 +395,7 @@ func TestWriteMainTF(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), s: Setup{ Requirement: ProviderRequirement{ Source: "my-company/namespace/provider-test", @@ -449,7 +449,7 @@ func TestWriteMainTF(t *testing.T) { "obs": "obsval", }}, }, - cfg: config.DefaultResource("upjet_resource", nil, nil), + cfg: config.DefaultResource("upjet_resource", nil, nil, nil), s: Setup{ Requirement: ProviderRequirement{ Source: "hashicorp/provider-test", From 40adf4fd006e0a980cded09877fcdb52f46ddd07 Mon Sep 17 00:00:00 2001 From: Cem Mergenci Date: Tue, 2 Jan 2024 09:32:25 +0300 Subject: [PATCH 02/24] Configure provider server. Configuring provider server with a nil configuration request lets server retrieve already-configured provider's configuration. Signed-off-by: Cem Mergenci --- pkg/controller/external_terraform_plugin_framework.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index d4c3b093..39ca4d05 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -223,6 +223,12 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre opTracker.SetTfState(s) } + server := providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)() + _, err = server.ConfigureProvider(ctx, nil) + if err != nil { + return nil, errors.Wrap(err, "cannot configure provider server") + } + return &terraformPluginFrameworkExternalClient{ ts: ts, config: c.config, @@ -230,7 +236,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre metricRecorder: c.metricRecorder, opTracker: opTracker, resource: c.config.TerraformPluginFrameworkResource, - server: providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)(), + server: server, params: params, // rawConfig: rawConfig, }, nil From 7f9ab46b86ab7909999feddecb68bfcf6dbf20fd Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Thu, 4 Jan 2024 13:50:35 +0300 Subject: [PATCH 03/24] initial full working setup Signed-off-by: Erhan Cagirici --- pkg/config/provider.go | 3 +- .../external_terraform_plugin_framework.go | 638 +++++++++++++----- pkg/controller/nofork_store.go | 29 + 3 files changed, 512 insertions(+), 158 deletions(-) diff --git a/pkg/config/provider.go b/pkg/config/provider.go index b651d16f..8c5b3978 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -316,7 +316,8 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s } } - terraformPluginFrameworkResource := (*fwresource.Resource)(nil) + var terraformPluginFrameworkResource *fwresource.Resource + if isPluginFrameworkResource { // TODO: Consider whether to replace the commented out conditional in the next line with an equivalent conditional for plugin framework. if p.TerraformPluginFrameworkProvider == nil /*|| p.TerraformProvider.ResourcesMap[name] == nil */ { diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 39ca4d05..2e669962 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -8,20 +8,25 @@ import ( "context" "encoding/json" "fmt" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + corev1 "k8s.io/api/core/v1" + "math/big" "time" "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource" + upjson "github.com/crossplane/upjet/pkg/resource/json" fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/providerserver" fwresource "github.com/hashicorp/terraform-plugin-framework/resource" + resschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + _ "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tfprotov5" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource" + "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/pkg/errors" "github.com/crossplane/upjet/pkg/metrics" @@ -89,8 +94,11 @@ type terraformPluginFrameworkExternalClient struct { resource *fwresource.Resource server tfprotov5.ProviderServer params map[string]any + fwDiff *tfprotov5.DynamicValue + resourceSchema resschema.Schema } +/* func processParamsWithStateFunc(schemaMap map[string]*schema.Schema, params map[string]any) map[string]any { if params == nil { return params @@ -165,6 +173,7 @@ func applyStateFuncToParam(sc *schema.Schema, param any) any { //nolint:gocyclo } return param } +*/ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { c.metricRecorder.ObserveReconcileDelay(mg.GetObjectKind().GroupVersionKind(), mg.GetName()) @@ -187,14 +196,9 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre } // TODO(cem): Check if we need to perform an equivalent functionality for framework resources. - params = processParamsWithStateFunc(c.config.TerraformResource.Schema, params) + // params = processParamsWithStateFunc(c.config.TerraformResource.Schema, params) - schemaBlock := c.config.TerraformResource.CoreConfigSchema() - rawConfig, err := schema.JSONMapToStateValue(params, schemaBlock) - if err != nil { - return nil, errors.Wrap(err, "failed to convert params JSON map to cty.Value") - } - if !opTracker.HasState() { + if !opTracker.HasFwState() { logger.Debug("Instance state not found in cache, reconstructing...") tfState, err := tr.GetObservation() if err != nil { @@ -210,23 +214,25 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre tfState = copyParameters(tfState, params) } - tfStateCtyValue, err := schema.JSONMapToStateValue(tfState, schemaBlock) + tfStateJsonBytes, err := json.Marshal(tfState) if err != nil { - return nil, errors.Wrap(err, "cannot convert JSON map to state cty.Value") + return nil, errors.Wrap(err, "could not marshal TF state map") } - s, err := c.config.TerraformResource.ShimInstanceStateFromValue(tfStateCtyValue) - if err != nil { - return nil, errors.Wrap(err, "failed to convert cty.Value to terraform.InstanceState") - } - s.RawPlan = tfStateCtyValue - s.RawConfig = rawConfig - opTracker.SetTfState(s) + + opTracker.SetFwState(&tfprotov5.DynamicValue{ + JSON: tfStateJsonBytes, + }) + + } + + configuredProviderServer, err := c.configureProvider(ctx, ts) + if err != nil { + return nil, errors.Wrap(err, "could not configure provider server") } - server := providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)() - _, err = server.ConfigureProvider(ctx, nil) + resourceSchema, err := c.getResourceSchema(ctx) if err != nil { - return nil, errors.Wrap(err, "cannot configure provider server") + return nil, errors.Wrap(err, "could not retrieve resource schema") } return &terraformPluginFrameworkExternalClient{ @@ -236,12 +242,154 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre metricRecorder: c.metricRecorder, opTracker: opTracker, resource: c.config.TerraformPluginFrameworkResource, - server: server, + server: configuredProviderServer, params: params, - // rawConfig: rawConfig, + resourceSchema: resourceSchema, }, nil } +func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (resschema.Schema, error) { + res := *c.config.TerraformPluginFrameworkResource + schemaResp := &fwresource.SchemaResponse{} + res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) + if schemaResp.Diagnostics.HasError() { + return resschema.Schema{}, errors.Errorf("could not retrieve resource schema: %v", schemaResp.Diagnostics) + } + + return schemaResp.Schema, nil +} + +func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Context, ts terraform.Setup) (tfprotov5.ProviderServer, error) { + providerServer := providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)() + tsBytes, err := json.Marshal(ts.Configuration) + if err != nil { + return nil, errors.Wrap(err, "cannot marshal ts config") + } + configureProviderReq := &tfprotov5.ConfigureProviderRequest{ + TerraformVersion: "crossTF000", + Config: &tfprotov5.DynamicValue{ + JSON: tsBytes, + }, + } + providerResp, err := providerServer.ConfigureProvider(ctx, configureProviderReq) + fmt.Printf("%p", providerResp) + if err != nil { + return nil, err + } + // TODO(erhan): improve diag reporting + if hasFatalDiag := hasFatalDiagnostics(providerResp.Diagnostics); hasFatalDiag { + return nil, errors.Errorf("provider configure request returned fatal diagnostics") + } + return providerServer, nil +} + +func (n *terraformPluginFrameworkExternalClient) configureProviderOLD(ctx context.Context) error { + tsBytes, err := json.Marshal(n.ts.Configuration) + if err != nil { + return errors.Wrap(err, "cannot marshal ts config") + } + configureProviderReq := &tfprotov5.ConfigureProviderRequest{ + TerraformVersion: "crossTF000", + Config: &tfprotov5.DynamicValue{ + JSON: tsBytes, + }, + } + providerResp, err := n.server.ConfigureProvider(ctx, configureProviderReq) + fmt.Printf("%p", providerResp) + if err != nil { + return err + } + // TODO(erhan): improve diag reporting + if hasFatalDiag := hasFatalDiagnostics(providerResp.Diagnostics); hasFatalDiag { + return errors.Errorf("provider configure request returned fatal diagnostics") + } + return nil +} + +func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context, + tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { + + valueTerraformType := n.resourceSchema.Type().TerraformType(ctx) + paramBytes, err := json.Marshal(n.params) + if err != nil { + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot convert params to json bytes") + } + + tfConfigValue, err := tftypes.ValueFromJSONWithOpts(paramBytes, valueTerraformType, tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}) + if err != nil { + return &tfprotov5.DynamicValue{}, false, err + } + + tfConfig, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue) + if err != nil { + return &tfprotov5.DynamicValue{}, false, err + } + + tfPlannedState, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue.Copy()) + if err != nil { + return &tfprotov5.DynamicValue{}, false, err + } + + diffs, err := tfStateValue.Diff(tfConfigValue) + if err != nil { + return &tfprotov5.DynamicValue{}, false, err + } + // process diffs + processedDiffs := diffs[:0] + for _, diff := range diffs { + if !diff.Value2.IsNull() { + processedDiffs = append(processedDiffs, diff) + } + } + + if len(processedDiffs) == 0 { + return nil, false, nil + } + + var proposedNewState tfprotov5.DynamicValue + + // calculate proposed state when we have a prior state + if !tfStateValue.IsNull() { + proposedNewStateValue := tfStateValue.Copy() + for _, pdiff := range processedDiffs { + proposedNewStateValue, err = tftypes.Transform(proposedNewStateValue, func(path *tftypes.AttributePath, value tftypes.Value) (tftypes.Value, error) { + if path.Equal(pdiff.Path) { + return *pdiff.Value2, nil + } + return value, nil + }) + if err != nil { + return nil, false, err + } + } + if proposedNewState, err = tfprotov5.NewDynamicValue(valueTerraformType, proposedNewStateValue); err != nil { + return nil, false, err + } + } else { + // resource does not exist - config becomes the proposed state + if proposedNewState, err = tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue.Copy()); err != nil { + return nil, false, err + } + } + fmt.Printf("%p\n", proposedNewState) + prcr := &tfprotov5.PlanResourceChangeRequest{ + TypeName: n.config.Name, + Config: &tfConfig, + ProposedNewState: &tfPlannedState, + } + planResponse, err := n.server.PlanResourceChange(ctx, prcr) + if err != nil { + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot plan change") + } + // TODO: improve diag reporting + if isFatal := hasFatalDiagnostics(planResponse.Diagnostics); isFatal { + return &tfprotov5.DynamicValue{}, false, errors.New("plan resource change has fatal diags") + } + + return planResponse.PlannedState, len(processedDiffs) > 0, nil + +} + func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg xpresource.Managed) (managed.ExternalObservation, error) { //nolint:gocyclo n.logger.Debug("Observing the external resource") @@ -251,131 +399,170 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg }, nil } - jsonBytes, err := json.Marshal(n.params) - if err != nil { - return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert params to json bytes") - } - readRequest := &tfprotov5.ReadResourceRequest{ - TypeName: n.config.Name, - CurrentState: &tfprotov5.DynamicValue{ - JSON: jsonBytes, - }, + TypeName: n.config.Name, + CurrentState: n.opTracker.GetFwState(), } readResponse, err := n.server.ReadResource(ctx, readRequest) + if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot read resource") } - fmt.Printf("%v", readResponse) + // TODO(erhan): improve diag reporting + if shouldError := hasFatalDiagnostics(readResponse.Diagnostics); shouldError { + return managed.ExternalObservation{}, errors.New("read returned diags") + } - // start := time.Now() - // newState, diag := n.resourceSchema.RefreshWithoutUpgrade(ctx, n.opTracker.GetTfState(), n.ts.Meta) - // metrics.ExternalAPITime.WithLabelValues("read").Observe(time.Since(start).Seconds()) - // if diag != nil && diag.HasError() { - // return managed.ExternalObservation{}, errors.Errorf("failed to observe the resource: %v", diag) - // } - // n.opTracker.SetTfState(newState) // TODO: missing RawConfig & RawPlan here... + tfStateValue, err := readResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot unmarshal state value") + } - // resourceExists := newState != nil && newState.ID != "" - // instanceDiff, err := n.getResourceDataDiff(mg.(resource.Terraformed), ctx, newState, resourceExists) - // if err != nil { - // return managed.ExternalObservation{}, errors.Wrap(err, "cannot compute the instance diff") - // } - // n.instanceDiff = instanceDiff - // noDiff := instanceDiff.Empty() + n.opTracker.SetFwState(readResponse.NewState) + resourceExists := !tfStateValue.IsNull() - // var connDetails managed.ConnectionDetails - // if !resourceExists && mg.GetDeletionTimestamp() != nil { - // gvk := mg.GetObjectKind().GroupVersionKind() - // metrics.DeletionTime.WithLabelValues(gvk.Group, gvk.Version, gvk.Kind).Observe(time.Since(mg.GetDeletionTimestamp().Time).Seconds()) - // } - // specUpdateRequired := false - // if resourceExists { - // if mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionUnknown || - // mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionFalse { - // addTTR(mg) - // } - // mg.SetConditions(xpv1.Available()) - // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) - // if err != nil { - // return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") - // } - - // buff, err := json.TFParser.Marshal(stateValueMap) - // if err != nil { - // return managed.ExternalObservation{}, errors.Wrap(err, "cannot marshal the attributes of the new state for late-initialization") - // } - // specUpdateRequired, err = mg.(resource.Terraformed).LateInitialize(buff) - // if err != nil { - // return managed.ExternalObservation{}, errors.Wrap(err, "cannot late-initialize the managed resource") - // } - - // err = mg.(resource.Terraformed).SetObservation(stateValueMap) - // if err != nil { - // return managed.ExternalObservation{}, errors.Errorf("could not set observation: %v", err) - // } - // connDetails, err = resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) - // if err != nil { - // return managed.ExternalObservation{}, errors.Wrap(err, "cannot get connection details") - // } - - // if noDiff { - // n.metricRecorder.SetReconcileTime(mg.GetName()) - // } - // if !specUpdateRequired { - // resource.SetUpToDateCondition(mg, noDiff) - // } - // // check for an external-name change - // if nameChanged, err := n.setExternalName(mg, newState); err != nil { - // return managed.ExternalObservation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during observe") - // } else { - // specUpdateRequired = specUpdateRequired || nameChanged - // } - // } + var stateValueMap map[string]any + if resourceExists { + if conv, err := valueToGo(tfStateValue); err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") + } else { + stateValueMap = conv.(map[string]any) + } + } + + plannedState, hasDiff, err := n.getDiffPlan(ctx, tfStateValue) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot calculate diff") + } + + n.fwDiff = plannedState + + var connDetails managed.ConnectionDetails + if !resourceExists && mg.GetDeletionTimestamp() != nil { + gvk := mg.GetObjectKind().GroupVersionKind() + metrics.DeletionTime.WithLabelValues(gvk.Group, gvk.Version, gvk.Kind).Observe(time.Since(mg.GetDeletionTimestamp().Time).Seconds()) + } + + specUpdateRequired := false + if resourceExists { + if mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionUnknown || + mg.GetCondition(xpv1.TypeReady).Status == corev1.ConditionFalse { + addTTR(mg) + } + mg.SetConditions(xpv1.Available()) + buff, err := upjson.TFParser.Marshal(stateValueMap) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot marshal the attributes of the new state for late-initialization") + } + specUpdateRequired, err = mg.(resource.Terraformed).LateInitialize(buff) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot late-initialize the managed resource") + } + err = mg.(resource.Terraformed).SetObservation(stateValueMap) + if err != nil { + return managed.ExternalObservation{}, errors.Errorf("could not set observation: %v", err) + } + connDetails, err = resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot get connection details") + } + if !hasDiff { + n.metricRecorder.SetReconcileTime(mg.GetName()) + } + if !specUpdateRequired { + resource.SetUpToDateCondition(mg, !hasDiff) + } + if nameChanged, err := n.setExternalName(mg, stateValueMap); err != nil { + return managed.ExternalObservation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during observe") + } else { + specUpdateRequired = specUpdateRequired || nameChanged + } + } return managed.ExternalObservation{ - ResourceExists: true, - ResourceUpToDate: true, - ConnectionDetails: managed.ConnectionDetails{}, - ResourceLateInitialized: false, + ResourceExists: resourceExists, + ResourceUpToDate: !hasDiff, + ConnectionDetails: connDetails, + ResourceLateInitialized: specUpdateRequired, }, nil } func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { n.logger.Debug("Creating the external resource") - // start := time.Now() - // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) - // metrics.ExternalAPITime.WithLabelValues("create").Observe(time.Since(start).Seconds()) - // // diag := n.resourceSchema.CreateWithoutTimeout(ctx, n.resourceData, n.ts.Meta) - // if diag != nil && diag.HasError() { - // return managed.ExternalCreation{}, errors.Errorf("failed to create the resource: %v", diag) - // } + configJsonBytes, err := json.Marshal(n.params) + if err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, "cannot convert params to json bytes") + } - // if newState == nil || newState.ID == "" { - // return managed.ExternalCreation{}, errors.New("failed to read the ID of the new resource") - // } - // n.opTracker.SetTfState(newState) + applyRequest := &tfprotov5.ApplyResourceChangeRequest{ + TypeName: n.config.Name, + PriorState: n.opTracker.GetFwState(), + PlannedState: n.fwDiff, + Config: &tfprotov5.DynamicValue{ + JSON: configJsonBytes, + }, + } + start := time.Now() + applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) + metrics.ExternalAPITime.WithLabelValues("create").Observe(time.Since(start).Seconds()) + if err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, "cannot create resource") + } - // if _, err := n.setExternalName(mg, newState); err != nil { - // return managed.ExternalCreation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during create") - // } - // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) - // if err != nil { - // return managed.ExternalCreation{}, err - // } - // err = mg.(resource.Terraformed).SetObservation(stateValueMap) - // if err != nil { - // return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) - // } - // conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) - // if err != nil { - // return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") - // } + // TODO(erhan): check diags reporting + if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { + return managed.ExternalCreation{}, errors.Errorf("failed to create the resource:") + } + + // TODO(erhan): refactor schema + res := *n.resource + schemaResp := &fwresource.SchemaResponse{} + res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) + + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaResp.Schema.Type().TerraformType(ctx)) + if err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal planned state") + } + + // TODO(erhan): check if new state ID is available + if newStateAfterApplyVal.IsNull() { + return managed.ExternalCreation{}, errors.New("new state is empty after creation") + } + + var stateValueMap map[string]any + if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + return managed.ExternalCreation{}, errors.New("cannot convert native state to go map") + } else { + stateValueMap = goval.(map[string]any) + } + + /* + if tfValErr := newStateAfterApplyVal.As(stateValueMap); tfValErr != nil { + return managed.ExternalCreation{}, errors.Wrap(err, "cannot convert to state map") + } + */ - // TODO: Obviously, remove the following definition of conn, after it is defined correctly above. - conn := managed.ConnectionDetails{} + // TODO(erhan): set to opTracker + n.opTracker.SetFwState(applyResponse.NewState) + + if _, err := n.setExternalName(mg, stateValueMap); err != nil { + return managed.ExternalCreation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during create") + } + + err = mg.(resource.Terraformed).SetObservation(stateValueMap) + if err != nil { + return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) + } + + // TODO(cem): Obviously, remove the following definition of conn, after it is defined correctly above. + // conn := managed.ConnectionDetails{} + // TODO(erhan): check config.Sensitive + conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) + if err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") + } return managed.ExternalCreation{ConnectionDetails: conn}, nil } @@ -383,47 +570,184 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { n.logger.Debug("Updating the external resource") + // TODO: check framework equivalent for forcenew // if err := n.assertNoForceNew(); err != nil { // return managed.ExternalUpdate{}, errors.Wrap(err, "refuse to update the external resource") // } - // start := time.Now() - // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) - // metrics.ExternalAPITime.WithLabelValues("update").Observe(time.Since(start).Seconds()) - // if diag != nil && diag.HasError() { - // return managed.ExternalUpdate{}, errors.Errorf("failed to update the resource: %v", diag) - // } - // n.opTracker.SetTfState(newState) + configJsonBytes, err := json.Marshal(n.params) + if err != nil { + return managed.ExternalUpdate{}, errors.Wrap(err, "cannot convert params to json bytes") + } - // stateValueMap, err := n.fromInstanceStateToJSONMap(newState) - // if err != nil { - // return managed.ExternalUpdate{}, err - // } + applyRequest := &tfprotov5.ApplyResourceChangeRequest{ + TypeName: n.config.Name, + PriorState: n.opTracker.GetFwState(), + PlannedState: n.fwDiff, + Config: &tfprotov5.DynamicValue{ + JSON: configJsonBytes, + }, + } + start := time.Now() + applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) + metrics.ExternalAPITime.WithLabelValues("update").Observe(time.Since(start).Seconds()) + if err != nil { + return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update resource") + } + if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { + return managed.ExternalUpdate{}, errors.Errorf("failed to create the resource:") + } + n.opTracker.SetFwState(applyResponse.NewState) - // err = mg.(resource.Terraformed).SetObservation(stateValueMap) - // if err != nil { - // return managed.ExternalUpdate{}, errors.Errorf("failed to set observation: %v", err) - // } + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + if err != nil { + return managed.ExternalUpdate{}, errors.Wrap(err, "cannot unmarshal updated state") + } + + if newStateAfterApplyVal.IsNull() { + return managed.ExternalUpdate{}, errors.New("new state is empty after update") + } + + var stateValueMap map[string]any + if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + return managed.ExternalUpdate{}, errors.New("cannot convert native state to go map") + } else { + stateValueMap = goval.(map[string]any) + } + + err = mg.(resource.Terraformed).SetObservation(stateValueMap) + if err != nil { + return managed.ExternalUpdate{}, errors.Errorf("could not set observation: %v", err) + } return managed.ExternalUpdate{}, nil } func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error { n.logger.Debug("Deleting the external resource") - // if n.instanceDiff == nil { - // n.instanceDiff = tf.NewInstanceDiff() - // } + configJsonBytes, err := json.Marshal(n.params) + if err != nil { + return errors.Wrap(err, "cannot convert params to json bytes") + } - // n.instanceDiff.Destroy = true - // start := time.Now() - // newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta) - // metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds()) - // if diag != nil && diag.HasError() { - // return errors.Errorf("failed to delete the resource: %v", diag) - // } - // n.opTracker.SetTfState(newState) - // // mark the resource as logically deleted if the TF call clears the state - // n.opTracker.SetDeleted(newState == nil) + schemaType := n.resourceSchema.Type().TerraformType(ctx) + // set an empty planned state, this corresponds to deleting + plannedState, err := tfprotov5.NewDynamicValue(schemaType, tftypes.NewValue(schemaType, nil)) + if err != nil { + return errors.Wrap(err, "cannot set the planned state") + } + + applyRequest := &tfprotov5.ApplyResourceChangeRequest{ + TypeName: n.config.Name, + PriorState: n.opTracker.GetFwState(), + PlannedState: &plannedState, + Config: &tfprotov5.DynamicValue{ + JSON: configJsonBytes, + }, + } + start := time.Now() + applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) + metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds()) + if err != nil { + return errors.Wrap(err, "cannot delete resource") + } + // TODO(erhan): improve diagnostics reporting + if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { + return errors.Errorf("failed to delete the resource with diags") + } + n.opTracker.SetFwState(applyResponse.NewState) + + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaType) + if err != nil { + return errors.Wrap(err, "cannot unmarshal updated state") + } + n.opTracker.SetFwState(applyResponse.NewState) + // mark the resource as logically deleted if the TF call clears the state + n.opTracker.SetDeleted(newStateAfterApplyVal.IsNull()) return nil } + +func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.Managed, stateValueMap map[string]interface{}) (bool, error) { + id, ok := stateValueMap["id"] + if !ok || id.(string) == "" { + return false, nil + } + newName, err := n.config.ExternalName.GetExternalNameFn(stateValueMap) + if err != nil { + return false, errors.Wrapf(err, "failed to compute the external-name from the state map of the resource with the ID %s", id) + } + oldName := meta.GetExternalName(mg) + // we have to make sure the newly set external-name is recorded + meta.SetExternalName(mg, newName) + return oldName != newName, nil +} + +func valueToGo(input tftypes.Value) (any, error) { + if input.IsNull() { + return nil, nil + } + valType := input.Type() + if valType.Is(tftypes.Object{}) || valType.Is(tftypes.Map{}) { + destInterim := make(map[string]tftypes.Value) + dest := make(map[string]any) + if err := input.As(&destInterim); err != nil { + return nil, err + } + for k, v := range destInterim { + if res, err := valueToGo(v); err != nil { + return nil, err + } else { + dest[k] = res + } + } + return dest, nil + } else if valType.Is(tftypes.Set{}) || valType.Is(tftypes.List{}) || valType.Is(tftypes.Tuple{}) { + destInterim := make([]tftypes.Value, 0) + dest := make([]any, 0) + if err := input.As(&destInterim); err != nil { + return nil, err + } + for i, v := range destInterim { + if res, err := valueToGo(v); err != nil { + return nil, err + } else { + dest[i] = res + } + } + return dest, nil + } else if valType.Is(tftypes.Bool) { + var x bool + if err := input.As(&x); err != nil { + return nil, err + } + return x, nil + } else if valType.Is(tftypes.Number) { + var x big.Float + if err := input.As(&x); err != nil { + return nil, err + } + xf, _ := x.Float64() + return xf, nil + } else if valType.Is(tftypes.String) { + var x string + if err := input.As(&x); err != nil { + return nil, err + } + return x, nil + } else if valType.Is(tftypes.DynamicPseudoType) { + return nil, nil + } else { + return nil, nil + } +} + +func hasFatalDiagnostics(diags []*tfprotov5.Diagnostic) bool { + shouldError := false + for _, tfdiag := range diags { + if tfdiag.Severity == tfprotov5.DiagnosticSeverityInvalid || tfdiag.Severity == tfprotov5.DiagnosticSeverityError { + shouldError = true + } + } + return shouldError +} diff --git a/pkg/controller/nofork_store.go b/pkg/controller/nofork_store.go index 4ec2e082..e83970f2 100644 --- a/pkg/controller/nofork_store.go +++ b/pkg/controller/nofork_store.go @@ -10,6 +10,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/logging" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" tfsdk "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "k8s.io/apimachinery/pkg/types" @@ -22,6 +23,7 @@ type AsyncTracker struct { logger logging.Logger mu *sync.Mutex tfState *tfsdk.InstanceState + fwState *tfprotov5.DynamicValue // lifecycle of certain external resources are bound to a parent resource's // lifecycle, and they cannot be deleted without actually deleting // the owning external resource (e.g., a database resource as the parent @@ -93,6 +95,33 @@ func (a *AsyncTracker) SetDeleted(deleted bool) { a.isDeleted.Store(deleted) } +func (a *AsyncTracker) GetFwState() *tfprotov5.DynamicValue { + a.mu.Lock() + defer a.mu.Unlock() + return a.fwState +} + +func (a *AsyncTracker) HasFwState() bool { + a.mu.Lock() + defer a.mu.Unlock() + return a.fwState != nil +} + +func (a *AsyncTracker) SetFwState(state *tfprotov5.DynamicValue) { + a.mu.Lock() + defer a.mu.Unlock() + a.fwState = state +} + +func (a *AsyncTracker) GetFwID() string { + a.mu.Lock() + defer a.mu.Unlock() + if a.fwState == nil { + return "" + } + return "TBD" +} + type OperationTrackerStore struct { store map[types.UID]*AsyncTracker logger logging.Logger From fde84728bb03359000a472cded90f17863508f03 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sat, 6 Jan 2024 00:55:00 +0300 Subject: [PATCH 04/24] refuse diffs with resource replacement & code cleanup refactor Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 216 ++++-------------- pkg/controller/nofork_store.go | 8 +- 2 files changed, 46 insertions(+), 178 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 2e669962..0a99f8db 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// SPDX-FileCopyrightText: 2024 The Crossplane Authors // // SPDX-License-Identifier: Apache-2.0 @@ -7,31 +7,32 @@ package controller import ( "context" "encoding/json" - "fmt" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - corev1 "k8s.io/api/core/v1" "math/big" + "strings" "time" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource" - upjson "github.com/crossplane/upjet/pkg/resource/json" + fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/providerserver" fwresource "github.com/hashicorp/terraform-plugin-framework/resource" - resschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" _ "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/metrics" + "github.com/crossplane/upjet/pkg/resource" + upjson "github.com/crossplane/upjet/pkg/resource/json" "github.com/crossplane/upjet/pkg/terraform" - "sigs.k8s.io/controller-runtime/pkg/client" ) type TerraformPluginFrameworkConnector struct { @@ -94,87 +95,10 @@ type terraformPluginFrameworkExternalClient struct { resource *fwresource.Resource server tfprotov5.ProviderServer params map[string]any - fwDiff *tfprotov5.DynamicValue - resourceSchema resschema.Schema + plannedState *tfprotov5.DynamicValue + resourceSchema rschema.Schema } -/* -func processParamsWithStateFunc(schemaMap map[string]*schema.Schema, params map[string]any) map[string]any { - if params == nil { - return params - } - for key, param := range params { - if sc, ok := schemaMap[key]; ok { - params[key] = applyStateFuncToParam(sc, param) - } else { - params[key] = param - } - } - return params -} - -func applyStateFuncToParam(sc *schema.Schema, param any) any { //nolint:gocyclo - if param == nil { - return param - } - switch sc.Type { - case schema.TypeMap: - if sc.Elem == nil { - return param - } - pmap, okParam := param.(map[string]any) - // TypeMap only supports schema in Elem - if mapSchema, ok := sc.Elem.(*schema.Schema); ok && okParam { - for pk, pv := range pmap { - pmap[pk] = applyStateFuncToParam(mapSchema, pv) - } - return pmap - } - case schema.TypeSet, schema.TypeList: - if sc.Elem == nil { - return param - } - pArray, okParam := param.([]any) - if setSchema, ok := sc.Elem.(*schema.Schema); ok && okParam { - for i, p := range pArray { - pArray[i] = applyStateFuncToParam(setSchema, p) - } - return pArray - } else if setResource, ok := sc.Elem.(*schema.Resource); ok { - for i, p := range pArray { - if resParam, okRParam := p.(map[string]any); okRParam { - pArray[i] = processParamsWithStateFunc(setResource.Schema, resParam) - } - } - } - case schema.TypeString: - // For String types check if it is an HCL string and process - if isHCLSnippetPattern.MatchString(param.(string)) { - hclProccessedParam, err := processHCLParam(param.(string)) - if err != nil { - // c.logger.Debug("could not process param, returning original", "param", sc.GoString()) - } else { - param = hclProccessedParam - } - } - if sc.StateFunc != nil { - return sc.StateFunc(param) - } - return param - case schema.TypeBool, schema.TypeInt, schema.TypeFloat: - if sc.StateFunc != nil { - return sc.StateFunc(param) - } - return param - case schema.TypeInvalid: - return param - default: - return param - } - return param -} -*/ - func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { c.metricRecorder.ObserveReconcileDelay(mg.GetObjectKind().GroupVersionKind(), mg.GetName()) logger := c.logger.WithValues("uid", mg.GetUID(), "name", mg.GetName(), "gvk", mg.GetObjectKind().GroupVersionKind().String()) @@ -186,7 +110,6 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre return nil, errors.Wrap(err, errGetTerraformSetup) } - // To Compute the ResourceDiff: n.resourceSchema.Diff(...) tr := mg.(resource.Terraformed) opTracker := c.operationTrackerStore.Tracker(tr) externalName := meta.GetExternalName(tr) @@ -195,10 +118,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre return nil, errors.Wrapf(err, "failed to get the extended parameters for resource %q", mg.GetName()) } - // TODO(cem): Check if we need to perform an equivalent functionality for framework resources. - // params = processParamsWithStateFunc(c.config.TerraformResource.Schema, params) - - if !opTracker.HasFwState() { + if !opTracker.HasFrameworkTFState() { logger.Debug("Instance state not found in cache, reconstructing...") tfState, err := tr.GetObservation() if err != nil { @@ -219,7 +139,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre return nil, errors.Wrap(err, "could not marshal TF state map") } - opTracker.SetFwState(&tfprotov5.DynamicValue{ + opTracker.SetFrameworkTFState(&tfprotov5.DynamicValue{ JSON: tfStateJsonBytes, }) @@ -248,12 +168,12 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre }, nil } -func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (resschema.Schema, error) { +func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (rschema.Schema, error) { res := *c.config.TerraformPluginFrameworkResource schemaResp := &fwresource.SchemaResponse{} res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) if schemaResp.Diagnostics.HasError() { - return resschema.Schema{}, errors.Errorf("could not retrieve resource schema: %v", schemaResp.Diagnostics) + return rschema.Schema{}, errors.Errorf("could not retrieve resource schema: %v", schemaResp.Diagnostics) } return schemaResp.Schema, nil @@ -272,7 +192,6 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex }, } providerResp, err := providerServer.ConfigureProvider(ctx, configureProviderReq) - fmt.Printf("%p", providerResp) if err != nil { return nil, err } @@ -283,29 +202,6 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex return providerServer, nil } -func (n *terraformPluginFrameworkExternalClient) configureProviderOLD(ctx context.Context) error { - tsBytes, err := json.Marshal(n.ts.Configuration) - if err != nil { - return errors.Wrap(err, "cannot marshal ts config") - } - configureProviderReq := &tfprotov5.ConfigureProviderRequest{ - TerraformVersion: "crossTF000", - Config: &tfprotov5.DynamicValue{ - JSON: tsBytes, - }, - } - providerResp, err := n.server.ConfigureProvider(ctx, configureProviderReq) - fmt.Printf("%p", providerResp) - if err != nil { - return err - } - // TODO(erhan): improve diag reporting - if hasFatalDiag := hasFatalDiagnostics(providerResp.Diagnostics); hasFatalDiag { - return errors.Errorf("provider configure request returned fatal diagnostics") - } - return nil -} - func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context, tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { @@ -334,6 +230,7 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context if err != nil { return &tfprotov5.DynamicValue{}, false, err } + // process diffs processedDiffs := diffs[:0] for _, diff := range diffs { @@ -346,38 +243,12 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context return nil, false, nil } - var proposedNewState tfprotov5.DynamicValue - - // calculate proposed state when we have a prior state - if !tfStateValue.IsNull() { - proposedNewStateValue := tfStateValue.Copy() - for _, pdiff := range processedDiffs { - proposedNewStateValue, err = tftypes.Transform(proposedNewStateValue, func(path *tftypes.AttributePath, value tftypes.Value) (tftypes.Value, error) { - if path.Equal(pdiff.Path) { - return *pdiff.Value2, nil - } - return value, nil - }) - if err != nil { - return nil, false, err - } - } - if proposedNewState, err = tfprotov5.NewDynamicValue(valueTerraformType, proposedNewStateValue); err != nil { - return nil, false, err - } - } else { - // resource does not exist - config becomes the proposed state - if proposedNewState, err = tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue.Copy()); err != nil { - return nil, false, err - } - } - fmt.Printf("%p\n", proposedNewState) - prcr := &tfprotov5.PlanResourceChangeRequest{ + prcReq := &tfprotov5.PlanResourceChangeRequest{ TypeName: n.config.Name, Config: &tfConfig, ProposedNewState: &tfPlannedState, } - planResponse, err := n.server.PlanResourceChange(ctx, prcr) + planResponse, err := n.server.PlanResourceChange(ctx, prcReq) if err != nil { return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot plan change") } @@ -386,6 +257,16 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context return &tfprotov5.DynamicValue{}, false, errors.New("plan resource change has fatal diags") } + if len(planResponse.RequiresReplace) > 0 { + var sb strings.Builder + sb.WriteString("diff contains fields that require resource replacement: ") + for _, attrPath := range planResponse.RequiresReplace { + sb.WriteString(attrPath.String()) + sb.WriteString(", ") + } + return nil, false, errors.New(sb.String()) + } + return planResponse.PlannedState, len(processedDiffs) > 0, nil } @@ -401,7 +282,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg readRequest := &tfprotov5.ReadResourceRequest{ TypeName: n.config.Name, - CurrentState: n.opTracker.GetFwState(), + CurrentState: n.opTracker.GetFrameworkTFState(), } readResponse, err := n.server.ReadResource(ctx, readRequest) @@ -419,7 +300,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg return managed.ExternalObservation{}, errors.Wrap(err, "cannot unmarshal state value") } - n.opTracker.SetFwState(readResponse.NewState) + n.opTracker.SetFrameworkTFState(readResponse.NewState) resourceExists := !tfStateValue.IsNull() var stateValueMap map[string]any @@ -436,7 +317,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg return managed.ExternalObservation{}, errors.Wrap(err, "cannot calculate diff") } - n.fwDiff = plannedState + n.plannedState = plannedState var connDetails managed.ConnectionDetails if !resourceExists && mg.GetDeletionTimestamp() != nil { @@ -498,8 +379,8 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, - PriorState: n.opTracker.GetFwState(), - PlannedState: n.fwDiff, + PriorState: n.opTracker.GetFrameworkTFState(), + PlannedState: n.plannedState, Config: &tfprotov5.DynamicValue{ JSON: configJsonBytes, }, @@ -538,14 +419,7 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg stateValueMap = goval.(map[string]any) } - /* - if tfValErr := newStateAfterApplyVal.As(stateValueMap); tfValErr != nil { - return managed.ExternalCreation{}, errors.Wrap(err, "cannot convert to state map") - } - */ - - // TODO(erhan): set to opTracker - n.opTracker.SetFwState(applyResponse.NewState) + n.opTracker.SetFrameworkTFState(applyResponse.NewState) if _, err := n.setExternalName(mg, stateValueMap); err != nil { return managed.ExternalCreation{}, errors.Wrapf(err, "failed to set the external-name of the managed resource during create") @@ -556,8 +430,6 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) } - // TODO(cem): Obviously, remove the following definition of conn, after it is defined correctly above. - // conn := managed.ConnectionDetails{} // TODO(erhan): check config.Sensitive conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) if err != nil { @@ -570,11 +442,6 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { n.logger.Debug("Updating the external resource") - // TODO: check framework equivalent for forcenew - // if err := n.assertNoForceNew(); err != nil { - // return managed.ExternalUpdate{}, errors.Wrap(err, "refuse to update the external resource") - // } - configJsonBytes, err := json.Marshal(n.params) if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot convert params to json bytes") @@ -582,8 +449,8 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, - PriorState: n.opTracker.GetFwState(), - PlannedState: n.fwDiff, + PriorState: n.opTracker.GetFrameworkTFState(), + PlannedState: n.plannedState, Config: &tfprotov5.DynamicValue{ JSON: configJsonBytes, }, @@ -597,7 +464,7 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { return managed.ExternalUpdate{}, errors.Errorf("failed to create the resource:") } - n.opTracker.SetFwState(applyResponse.NewState) + n.opTracker.SetFrameworkTFState(applyResponse.NewState) newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) if err != nil { @@ -639,7 +506,7 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, - PriorState: n.opTracker.GetFwState(), + PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: &plannedState, Config: &tfprotov5.DynamicValue{ JSON: configJsonBytes, @@ -655,13 +522,13 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { return errors.Errorf("failed to delete the resource with diags") } - n.opTracker.SetFwState(applyResponse.NewState) + n.opTracker.SetFrameworkTFState(applyResponse.NewState) newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaType) if err != nil { return errors.Wrap(err, "cannot unmarshal updated state") } - n.opTracker.SetFwState(applyResponse.NewState) + n.opTracker.SetFrameworkTFState(applyResponse.NewState) // mark the resource as logically deleted if the TF call clears the state n.opTracker.SetDeleted(newStateAfterApplyVal.IsNull()) @@ -736,6 +603,7 @@ func valueToGo(input tftypes.Value) (any, error) { } return x, nil } else if valType.Is(tftypes.DynamicPseudoType) { + // TODO: check if we need to handle DynamicPseudoType return nil, nil } else { return nil, nil diff --git a/pkg/controller/nofork_store.go b/pkg/controller/nofork_store.go index e83970f2..ad3d1bbb 100644 --- a/pkg/controller/nofork_store.go +++ b/pkg/controller/nofork_store.go @@ -95,25 +95,25 @@ func (a *AsyncTracker) SetDeleted(deleted bool) { a.isDeleted.Store(deleted) } -func (a *AsyncTracker) GetFwState() *tfprotov5.DynamicValue { +func (a *AsyncTracker) GetFrameworkTFState() *tfprotov5.DynamicValue { a.mu.Lock() defer a.mu.Unlock() return a.fwState } -func (a *AsyncTracker) HasFwState() bool { +func (a *AsyncTracker) HasFrameworkTFState() bool { a.mu.Lock() defer a.mu.Unlock() return a.fwState != nil } -func (a *AsyncTracker) SetFwState(state *tfprotov5.DynamicValue) { +func (a *AsyncTracker) SetFrameworkTFState(state *tfprotov5.DynamicValue) { a.mu.Lock() defer a.mu.Unlock() a.fwState = state } -func (a *AsyncTracker) GetFwID() string { +func (a *AsyncTracker) GetFrameworkTFID() string { a.mu.Lock() defer a.mu.Unlock() if a.fwState == nil { From c45556a5df0047b9c4370246b7ec4acc317b55f8 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sat, 6 Jan 2024 00:56:01 +0300 Subject: [PATCH 05/24] plugin framework async external client Signed-off-by: Erhan Cagirici --- ...ternal_async_terraform_plugin_framework.go | 201 ++++++++++++++++++ pkg/pipeline/templates/controller.go.tmpl | 16 +- 2 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 pkg/controller/external_async_terraform_plugin_framework.go diff --git a/pkg/controller/external_async_terraform_plugin_framework.go b/pkg/controller/external_async_terraform_plugin_framework.go new file mode 100644 index 00000000..eb6c2643 --- /dev/null +++ b/pkg/controller/external_async_terraform_plugin_framework.go @@ -0,0 +1,201 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/crossplane-runtime/pkg/meta" + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/controller/handler" + "github.com/crossplane/upjet/pkg/metrics" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/terraform" + tferrors "github.com/crossplane/upjet/pkg/terraform/errors" +) + +type TerraformPluginFrameworkAsyncConnector struct { + *TerraformPluginFrameworkConnector + callback CallbackProvider + eventHandler *handler.EventHandler +} + +type TerraformPluginFrameworkAsyncOption func(connector *TerraformPluginFrameworkAsyncConnector) + +func NewTerraformPluginFrameworkAsyncConnector(kube client.Client, + ots *OperationTrackerStore, + sf terraform.SetupFn, + cfg *config.Resource, + provider *provider.Provider, + opts ...TerraformPluginFrameworkAsyncOption) *TerraformPluginFrameworkAsyncConnector { + nfac := &TerraformPluginFrameworkAsyncConnector{ + TerraformPluginFrameworkConnector: NewTerraformPluginFrameworkConnector(kube, sf, cfg, ots, provider), + } + for _, f := range opts { + f(nfac) + } + return nfac +} + +func (c *TerraformPluginFrameworkAsyncConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { + ec, err := c.TerraformPluginFrameworkConnector.Connect(ctx, mg) + if err != nil { + return nil, errors.Wrap(err, "cannot initialize the no-fork async external client") + } + + return &terraformPluginFrameworkAsyncExternalClient{ + terraformPluginFrameworkExternalClient: ec.(*terraformPluginFrameworkExternalClient), + callback: c.callback, + eventHandler: c.eventHandler, + }, nil +} + +// WithTerraformPluginFrameworkAsyncConnectorEventHandler configures the EventHandler so that +// the no-fork external clients can requeue reconciliation requests. +func WithTerraformPluginFrameworkAsyncConnectorEventHandler(e *handler.EventHandler) TerraformPluginFrameworkAsyncOption { + return func(c *TerraformPluginFrameworkAsyncConnector) { + c.eventHandler = e + } +} + +// WithTerraformPluginFrameworkAsyncCallbackProvider configures the controller to use async variant of the functions +// of the Terraform client and run given callbacks once those operations are +// completed. +func WithTerraformPluginFrameworkAsyncCallbackProvider(ac CallbackProvider) TerraformPluginFrameworkAsyncOption { + return func(c *TerraformPluginFrameworkAsyncConnector) { + c.callback = ac + } +} + +// WithTerraformPluginFrameworkAsyncLogger configures a logger for the TerraformPluginFrameworkAsyncConnector. +func WithTerraformPluginFrameworkAsyncLogger(l logging.Logger) TerraformPluginFrameworkAsyncOption { + return func(c *TerraformPluginFrameworkAsyncConnector) { + c.logger = l + } +} + +// WithTerraformPluginFrameworkAsyncMetricRecorder configures a metrics.MetricRecorder for the +// TerraformPluginFrameworkAsyncConnector. +func WithTerraformPluginFrameworkAsyncMetricRecorder(r *metrics.MetricRecorder) TerraformPluginFrameworkAsyncOption { + return func(c *TerraformPluginFrameworkAsyncConnector) { + c.metricRecorder = r + } +} + +// WithTerraformPluginFrameworkAsyncManagementPolicies configures whether the client should +// handle management policies. +func WithTerraformPluginFrameworkAsyncManagementPolicies(isManagementPoliciesEnabled bool) TerraformPluginFrameworkAsyncOption { + return func(c *TerraformPluginFrameworkAsyncConnector) { + c.isManagementPoliciesEnabled = isManagementPoliciesEnabled + } +} + +type terraformPluginFrameworkAsyncExternalClient struct { + *terraformPluginFrameworkExternalClient + callback CallbackProvider + eventHandler *handler.EventHandler +} + +func (n *terraformPluginFrameworkAsyncExternalClient) Observe(ctx context.Context, mg xpresource.Managed) (managed.ExternalObservation, error) { + if n.opTracker.LastOperation.IsRunning() { + n.logger.WithValues("opType", n.opTracker.LastOperation.Type).Debug("ongoing async operation") + return managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + }, nil + } + n.opTracker.LastOperation.Flush() + + o, err := n.terraformPluginFrameworkExternalClient.Observe(ctx, mg) + // clear any previously reported LastAsyncOperation error condition here, + // because there are no pending updates on the existing resource and it's + // not scheduled to be deleted. + if err == nil && o.ResourceExists && o.ResourceUpToDate && !meta.WasDeleted(mg) { + mg.(resource.Terraformed).SetConditions(resource.LastAsyncOperationCondition(nil)) + } + return o, err +} + +func (n *terraformPluginFrameworkAsyncExternalClient) Create(_ context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { + if !n.opTracker.LastOperation.MarkStart("create") { + return managed.ExternalCreation{}, errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String()) + } + + ctx, cancel := context.WithDeadline(context.Background(), n.opTracker.LastOperation.StartTime().Add(defaultAsyncTimeout)) + go func() { + defer cancel() + + n.opTracker.logger.Debug("Async create starting...", "tfID", n.opTracker.GetFrameworkTFID()) + _, err := n.terraformPluginFrameworkExternalClient.Create(ctx, mg) + err = tferrors.NewAsyncCreateFailed(err) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async create ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + + n.opTracker.LastOperation.MarkEnd() + if cErr := n.callback.Create(mg.GetName())(err, ctx); cErr != nil { + n.opTracker.logger.Info("Async create callback failed", "error", cErr.Error()) + } + }() + + return managed.ExternalCreation{}, nil +} + +func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { + if !n.opTracker.LastOperation.MarkStart("update") { + return managed.ExternalUpdate{}, errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String()) + } + + ctx, cancel := context.WithDeadline(context.Background(), n.opTracker.LastOperation.StartTime().Add(defaultAsyncTimeout)) + go func() { + defer cancel() + + n.opTracker.logger.Debug("Async update starting...", "tfID", n.opTracker.GetFrameworkTFID()) + _, err := n.terraformPluginFrameworkExternalClient.Update(ctx, mg) + err = tferrors.NewAsyncUpdateFailed(err) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async update ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + + n.opTracker.LastOperation.MarkEnd() + if cErr := n.callback.Update(mg.GetName())(err, ctx); cErr != nil { + n.opTracker.logger.Info("Async update callback failed", "error", cErr.Error()) + } + }() + + return managed.ExternalUpdate{}, nil +} + +func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, mg xpresource.Managed) error { + switch { + case n.opTracker.LastOperation.Type == "delete": + n.opTracker.logger.Debug("The previous delete operation is still ongoing", "tfID", n.opTracker.GetFrameworkTFID()) + return nil + case !n.opTracker.LastOperation.MarkStart("delete"): + return errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String()) + } + + ctx, cancel := context.WithDeadline(context.Background(), n.opTracker.LastOperation.StartTime().Add(defaultAsyncTimeout)) + go func() { + defer cancel() + + n.opTracker.logger.Debug("Async delete starting...", "tfID", n.opTracker.GetFrameworkTFID()) + err := tferrors.NewAsyncDeleteFailed(n.terraformPluginFrameworkExternalClient.Delete(ctx, mg)) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async delete ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + + n.opTracker.LastOperation.MarkEnd() + if cErr := n.callback.Destroy(mg.GetName())(err, ctx); cErr != nil { + n.opTracker.logger.Info("Async delete callback failed", "error", cErr.Error()) + } + }() + + return nil +} diff --git a/pkg/pipeline/templates/controller.go.tmpl b/pkg/pipeline/templates/controller.go.tmpl index 7a01b880..e84d5ac6 100644 --- a/pkg/pipeline/templates/controller.go.tmpl +++ b/pkg/pipeline/templates/controller.go.tmpl @@ -42,8 +42,8 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { cps = append(cps, connection.NewDetailsManager(mgr.GetClient(), *o.SecretStoreConfigGVK, connection.WithTLSConfig(o.ESSOptions.TLSConfig))) } eventHandler := handler.NewEventHandler(handler.WithLogger(o.Logger.WithValues("gvk", {{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind))) - {{- if and .UseAsync (not .UseTerraformPluginFrameworkClient) }} - ac := tjcontroller.NewAPICallbacks(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind), tjcontroller.WithEventHandler(eventHandler){{ if .UseNoForkClient }}, tjcontroller.WithStatusUpdates(false){{ end }}) + {{- if .UseAsync }} + ac := tjcontroller.NewAPICallbacks(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind), tjcontroller.WithEventHandler(eventHandler){{ if or .UseNoForkClient .UseTerraformPluginFrameworkClient }}, tjcontroller.WithStatusUpdates(false){{ end }}) {{- end}} opts := []managed.ReconcilerOption{ managed.WithExternalConnecter( @@ -68,6 +68,17 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { ) {{- end -}} {{- else if .UseTerraformPluginFrameworkClient -}} + {{- if .UseAsync }} + tjcontroller.NewTerraformPluginFrameworkAsyncConnector(mgr.GetClient(), o.OperationTrackerStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.Provider.TerraformPluginFrameworkProvider, + tjcontroller.WithTerraformPluginFrameworkAsyncLogger(o.Logger), + tjcontroller.WithTerraformPluginFrameworkAsyncConnectorEventHandler(eventHandler), + tjcontroller.WithTerraformPluginFrameworkAsyncCallbackProvider(ac), + tjcontroller.WithTerraformPluginFrameworkAsyncMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), + {{if .FeaturesPackageAlias -}} + tjcontroller.WithTerraformPluginFrameworkAsyncManagementPolicies(o.Features.Enabled({{ .FeaturesPackageAlias }}EnableBetaManagementPolicies)) + {{- end -}} + ) + {{- else }} tjcontroller.NewTerraformPluginFrameworkConnector(mgr.GetClient(), o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.OperationTrackerStore, o.Provider.TerraformPluginFrameworkProvider, tjcontroller.WithTerraformPluginFrameworkLogger(o.Logger), tjcontroller.WithTerraformPluginFrameworkMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), @@ -75,6 +86,7 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { tjcontroller.WithTerraformPluginFrameworkManagementPolicies(o.Features.Enabled({{ .FeaturesPackageAlias }}EnableBetaManagementPolicies)) {{- end -}} ) + {{- end }} {{- else -}} tjcontroller.NewConnector(mgr.GetClient(), o.WorkspaceStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], tjcontroller.WithLogger(o.Logger), tjcontroller.WithConnectorEventHandler(eventHandler), {{- if .UseAsync }} From 9a712e85ff78cb8796883ddc2e3cd6778c41bfe1 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Mon, 8 Jan 2024 12:27:35 +0300 Subject: [PATCH 06/24] logging improvements & refactor Signed-off-by: Erhan Cagirici --- .../external_async_terraform_plugin_framework.go | 14 +++++++------- .../external_terraform_plugin_framework.go | 1 - pkg/controller/nofork_store.go | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/controller/external_async_terraform_plugin_framework.go b/pkg/controller/external_async_terraform_plugin_framework.go index eb6c2643..b61f97ba 100644 --- a/pkg/controller/external_async_terraform_plugin_framework.go +++ b/pkg/controller/external_async_terraform_plugin_framework.go @@ -134,11 +134,11 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Create(_ context.Context, go func() { defer cancel() - n.opTracker.logger.Debug("Async create starting...", "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async create starting...") _, err := n.terraformPluginFrameworkExternalClient.Create(ctx, mg) err = tferrors.NewAsyncCreateFailed(err) n.opTracker.LastOperation.SetError(err) - n.opTracker.logger.Debug("Async create ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async create ended.", "error", err) n.opTracker.LastOperation.MarkEnd() if cErr := n.callback.Create(mg.GetName())(err, ctx); cErr != nil { @@ -158,11 +158,11 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context, go func() { defer cancel() - n.opTracker.logger.Debug("Async update starting...", "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async update starting...") _, err := n.terraformPluginFrameworkExternalClient.Update(ctx, mg) err = tferrors.NewAsyncUpdateFailed(err) n.opTracker.LastOperation.SetError(err) - n.opTracker.logger.Debug("Async update ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async update ended.", "error", err) n.opTracker.LastOperation.MarkEnd() if cErr := n.callback.Update(mg.GetName())(err, ctx); cErr != nil { @@ -176,7 +176,7 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context, func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, mg xpresource.Managed) error { switch { case n.opTracker.LastOperation.Type == "delete": - n.opTracker.logger.Debug("The previous delete operation is still ongoing", "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("The previous delete operation is still ongoing") return nil case !n.opTracker.LastOperation.MarkStart("delete"): return errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String()) @@ -186,10 +186,10 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, go func() { defer cancel() - n.opTracker.logger.Debug("Async delete starting...", "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async delete starting...") err := tferrors.NewAsyncDeleteFailed(n.terraformPluginFrameworkExternalClient.Delete(ctx, mg)) n.opTracker.LastOperation.SetError(err) - n.opTracker.logger.Debug("Async delete ended.", "error", err, "tfID", n.opTracker.GetFrameworkTFID()) + n.opTracker.logger.Debug("Async delete ended.", "error", err) n.opTracker.LastOperation.MarkEnd() if cErr := n.callback.Destroy(mg.GetName())(err, ctx); cErr != nil { diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 0a99f8db..fbc51f41 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -528,7 +528,6 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x if err != nil { return errors.Wrap(err, "cannot unmarshal updated state") } - n.opTracker.SetFrameworkTFState(applyResponse.NewState) // mark the resource as logically deleted if the TF call clears the state n.opTracker.SetDeleted(newStateAfterApplyVal.IsNull()) diff --git a/pkg/controller/nofork_store.go b/pkg/controller/nofork_store.go index ad3d1bbb..1dfdf071 100644 --- a/pkg/controller/nofork_store.go +++ b/pkg/controller/nofork_store.go @@ -143,7 +143,7 @@ func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker defer ops.mu.Unlock() tracker, ok := ops.store[tr.GetUID()] if !ok { - l := ops.logger.WithValues("trackerUID", tr.GetUID(), "resourceName", tr.GetName()) + l := ops.logger.WithValues("trackerUID", tr.GetUID(), "resourceName", tr.GetName(), "gvk", tr.GetObjectKind().GroupVersionKind().String()) ops.store[tr.GetUID()] = NewAsyncTracker(WithAsyncTrackerLogger(l)) tracker = ops.store[tr.GetUID()] } From b9c6910fedb2d490ce9a72dcb9d6f7e0561a8d04 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 9 Jan 2024 14:53:17 +0300 Subject: [PATCH 07/24] dynamically configure provider of the framework server Signed-off-by: Erhan Cagirici --- pkg/config/provider.go | 6 ++--- ...ternal_async_terraform_plugin_framework.go | 2 +- .../external_terraform_plugin_framework.go | 24 ++++++++++++------- pkg/terraform/store.go | 3 +++ 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 8c5b3978..271c3ae5 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -136,7 +136,7 @@ type Provider struct { // TerraformProvider is the Terraform schema of the provider. TerraformProvider *schema.Provider - TerraformPluginFrameworkProvider *fwprovider.Provider + TerraformPluginFrameworkProvider fwprovider.Provider // refInjectors is an ordered list of `ReferenceInjector`s for // injecting references across this Provider's resources. @@ -197,7 +197,7 @@ func WithTerraformProvider(tp *schema.Provider) ProviderOption { } } -func WithTerraformPluginFrameworkProvider(tp *fwprovider.Provider) ProviderOption { +func WithTerraformPluginFrameworkProvider(tp fwprovider.Provider) ProviderOption { return func(p *Provider) { p.TerraformPluginFrameworkProvider = tp } @@ -327,7 +327,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s // TODO(cem): Consider creating a new context here, rather than getting one as input to this function. // TODO(cem): Currently, terraformPluginFrameworkResourceFunctions is calculated for each plugin framework resource. Doing so is wasteful, because the result is independent of the resource. It should be called once, outside the loop. - terraformPluginFrameworkResourceFunctions := (*p.TerraformPluginFrameworkProvider).Resources(ctx) + terraformPluginFrameworkResourceFunctions := p.TerraformPluginFrameworkProvider.Resources(ctx) for _, resourceFunc := range terraformPluginFrameworkResourceFunctions { resource := resourceFunc() diff --git a/pkg/controller/external_async_terraform_plugin_framework.go b/pkg/controller/external_async_terraform_plugin_framework.go index b61f97ba..e638230b 100644 --- a/pkg/controller/external_async_terraform_plugin_framework.go +++ b/pkg/controller/external_async_terraform_plugin_framework.go @@ -35,7 +35,7 @@ func NewTerraformPluginFrameworkAsyncConnector(kube client.Client, ots *OperationTrackerStore, sf terraform.SetupFn, cfg *config.Resource, - provider *provider.Provider, + provider provider.Provider, opts ...TerraformPluginFrameworkAsyncOption) *TerraformPluginFrameworkAsyncConnector { nfac := &TerraformPluginFrameworkAsyncConnector{ TerraformPluginFrameworkConnector: NewTerraformPluginFrameworkConnector(kube, sf, cfg, ots, provider), diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index fbc51f41..575acbdf 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -43,7 +43,7 @@ type TerraformPluginFrameworkConnector struct { metricRecorder *metrics.MetricRecorder operationTrackerStore *OperationTrackerStore isManagementPoliciesEnabled bool - terraformPluginFrameworkProvider *fwprovider.Provider + terraformPluginFrameworkProvider fwprovider.Provider } // TerraformPluginFrameworkConnectorOption allows you to configure TerraformPluginFrameworkConnector. @@ -72,7 +72,7 @@ func WithTerraformPluginFrameworkManagementPolicies(isManagementPoliciesEnabled } } -func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, terraformPluginFrameworkProvider *fwprovider.Provider, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { +func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, terraformPluginFrameworkProvider fwprovider.Provider, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { connector := &TerraformPluginFrameworkConnector{ getTerraformSetup: sf, kube: kube, @@ -118,7 +118,17 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre return nil, errors.Wrapf(err, "failed to get the extended parameters for resource %q", mg.GetName()) } - if !opTracker.HasFrameworkTFState() { + resourceSchema, err := c.getResourceSchema(ctx) + if err != nil { + return nil, errors.Wrap(err, "could not retrieve resource schema") + } + hasState := false + if opTracker.HasFrameworkTFState() { + tfStateValue, err := opTracker.GetFrameworkTFState().Unmarshal(resourceSchema.Type().TerraformType(ctx)) + hasState = err == nil && !tfStateValue.IsNull() + } + + if !hasState { logger.Debug("Instance state not found in cache, reconstructing...") tfState, err := tr.GetObservation() if err != nil { @@ -150,11 +160,6 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre return nil, errors.Wrap(err, "could not configure provider server") } - resourceSchema, err := c.getResourceSchema(ctx) - if err != nil { - return nil, errors.Wrap(err, "could not retrieve resource schema") - } - return &terraformPluginFrameworkExternalClient{ ts: ts, config: c.config, @@ -180,7 +185,7 @@ func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Contex } func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Context, ts terraform.Setup) (tfprotov5.ProviderServer, error) { - providerServer := providerserver.NewProtocol5(*c.terraformPluginFrameworkProvider)() + providerServer := providerserver.NewProtocol5(ts.FrameworkProvider)() tsBytes, err := json.Marshal(ts.Configuration) if err != nil { return nil, errors.Wrap(err, "cannot marshal ts config") @@ -245,6 +250,7 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context prcReq := &tfprotov5.PlanResourceChangeRequest{ TypeName: n.config.Name, + PriorState: n.opTracker.GetFrameworkTFState(), Config: &tfConfig, ProposedNewState: &tfPlannedState, } diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 862dab7c..09ffae94 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -19,6 +19,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/meta" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/mitchellh/go-ps" "github.com/pkg/errors" "github.com/spf13/afero" @@ -122,6 +123,8 @@ type Setup struct { Scheduler ProviderScheduler Meta any + + FrameworkProvider fwprovider.Provider } // Map returns the Setup object in map form. The initial reason was so that From 48630aeddc356fd301a5dc52c17010cf7cf24a06 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 9 Jan 2024 17:01:59 +0300 Subject: [PATCH 08/24] refactor obsolete plugin framework provider variables Signed-off-by: Erhan Cagirici --- pkg/config/provider.go | 4 +--- .../external_async_terraform_plugin_framework.go | 4 +--- pkg/controller/external_terraform_plugin_framework.go | 11 +++++------ pkg/pipeline/templates/controller.go.tmpl | 4 ++-- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 271c3ae5..47a27247 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -288,6 +288,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s } p.skippedResourceNames = make([]string, 0, len(resourceMap)) + terraformPluginFrameworkResourceFunctions := p.TerraformPluginFrameworkProvider.Resources(ctx) for name, terraformResource := range resourceMap { if len(terraformResource.Schema) == 0 { // There are resources with no schema, that we will address later. @@ -325,9 +326,6 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s "but either config.Provider.TerraformProvider is not configured or the Go schema does not exist for the resource", name)) } - // TODO(cem): Consider creating a new context here, rather than getting one as input to this function. - // TODO(cem): Currently, terraformPluginFrameworkResourceFunctions is calculated for each plugin framework resource. Doing so is wasteful, because the result is independent of the resource. It should be called once, outside the loop. - terraformPluginFrameworkResourceFunctions := p.TerraformPluginFrameworkProvider.Resources(ctx) for _, resourceFunc := range terraformPluginFrameworkResourceFunctions { resource := resourceFunc() diff --git a/pkg/controller/external_async_terraform_plugin_framework.go b/pkg/controller/external_async_terraform_plugin_framework.go index e638230b..1bc6a991 100644 --- a/pkg/controller/external_async_terraform_plugin_framework.go +++ b/pkg/controller/external_async_terraform_plugin_framework.go @@ -11,7 +11,6 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" - "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/pkg/errors" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,10 +34,9 @@ func NewTerraformPluginFrameworkAsyncConnector(kube client.Client, ots *OperationTrackerStore, sf terraform.SetupFn, cfg *config.Resource, - provider provider.Provider, opts ...TerraformPluginFrameworkAsyncOption) *TerraformPluginFrameworkAsyncConnector { nfac := &TerraformPluginFrameworkAsyncConnector{ - TerraformPluginFrameworkConnector: NewTerraformPluginFrameworkConnector(kube, sf, cfg, ots, provider), + TerraformPluginFrameworkConnector: NewTerraformPluginFrameworkConnector(kube, sf, cfg, ots), } for _, f := range opts { f(nfac) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 575acbdf..da69adb2 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -72,13 +72,12 @@ func WithTerraformPluginFrameworkManagementPolicies(isManagementPoliciesEnabled } } -func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, terraformPluginFrameworkProvider fwprovider.Provider, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { +func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { connector := &TerraformPluginFrameworkConnector{ - getTerraformSetup: sf, - kube: kube, - config: cfg, - operationTrackerStore: ots, - terraformPluginFrameworkProvider: terraformPluginFrameworkProvider, + getTerraformSetup: sf, + kube: kube, + config: cfg, + operationTrackerStore: ots, } for _, f := range opts { f(connector) diff --git a/pkg/pipeline/templates/controller.go.tmpl b/pkg/pipeline/templates/controller.go.tmpl index e84d5ac6..dfa651e2 100644 --- a/pkg/pipeline/templates/controller.go.tmpl +++ b/pkg/pipeline/templates/controller.go.tmpl @@ -69,7 +69,7 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { {{- end -}} {{- else if .UseTerraformPluginFrameworkClient -}} {{- if .UseAsync }} - tjcontroller.NewTerraformPluginFrameworkAsyncConnector(mgr.GetClient(), o.OperationTrackerStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.Provider.TerraformPluginFrameworkProvider, + tjcontroller.NewTerraformPluginFrameworkAsyncConnector(mgr.GetClient(), o.OperationTrackerStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], tjcontroller.WithTerraformPluginFrameworkAsyncLogger(o.Logger), tjcontroller.WithTerraformPluginFrameworkAsyncConnectorEventHandler(eventHandler), tjcontroller.WithTerraformPluginFrameworkAsyncCallbackProvider(ac), @@ -79,7 +79,7 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { {{- end -}} ) {{- else }} - tjcontroller.NewTerraformPluginFrameworkConnector(mgr.GetClient(), o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.OperationTrackerStore, o.Provider.TerraformPluginFrameworkProvider, + tjcontroller.NewTerraformPluginFrameworkConnector(mgr.GetClient(), o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.OperationTrackerStore, tjcontroller.WithTerraformPluginFrameworkLogger(o.Logger), tjcontroller.WithTerraformPluginFrameworkMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), {{if .FeaturesPackageAlias -}} From 50593c67ea57d2714e96a3f30221bd4960791c0b Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sun, 14 Jan 2024 01:05:33 +0300 Subject: [PATCH 09/24] bump terraform-plugin-framework to 1.4.1 Signed-off-by: Erhan Cagirici --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 01e61a2d..608b5b40 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 github.com/hashicorp/hcl/v2 v2.19.1 github.com/hashicorp/terraform-json v0.17.1 - github.com/hashicorp/terraform-plugin-framework v1.2.0 + github.com/hashicorp/terraform-plugin-framework v1.4.1 github.com/hashicorp/terraform-plugin-go v0.19.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.30.0 github.com/iancoleman/strcase v0.2.0 diff --git a/go.sum b/go.sum index 7978d0f9..10f2aece 100644 --- a/go.sum +++ b/go.sum @@ -226,8 +226,8 @@ github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= github.com/hashicorp/terraform-json v0.17.1 h1:eMfvh/uWggKmY7Pmb3T85u86E2EQg6EQHgyRwf3RkyA= github.com/hashicorp/terraform-json v0.17.1/go.mod h1:Huy6zt6euxaY9knPAFKjUITn8QxUFIe9VuSzb4zn/0o= -github.com/hashicorp/terraform-plugin-framework v1.2.0 h1:MZjFFfULnFq8fh04FqrKPcJ/nGpHOvX4buIygT3MSNY= -github.com/hashicorp/terraform-plugin-framework v1.2.0/go.mod h1:nToI62JylqXDq84weLJ/U3umUsBhZAaTmU0HXIVUOcw= +github.com/hashicorp/terraform-plugin-framework v1.4.1 h1:ZC29MoB3Nbov6axHdgPbMz7799pT5H8kIrM8YAsaVrs= +github.com/hashicorp/terraform-plugin-framework v1.4.1/go.mod h1:XC0hPcQbBvlbxwmjxuV/8sn8SbZRg4XwGMs22f+kqV0= github.com/hashicorp/terraform-plugin-go v0.19.0 h1:BuZx/6Cp+lkmiG0cOBk6Zps0Cb2tmqQpDM3iAtnhDQU= github.com/hashicorp/terraform-plugin-go v0.19.0/go.mod h1:EhRSkEPNoylLQntYsk5KrDHTZJh9HQoumZXbOGOXmec= github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0= From 0339b392a511872f5d551adb93614b65133911f6 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sun, 14 Jan 2024 01:07:20 +0300 Subject: [PATCH 10/24] diff detection with plan and state comparison & refactor Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 240 ++++++++++-------- 1 file changed, 130 insertions(+), 110 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index da69adb2..48d09934 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -7,6 +7,8 @@ package controller import ( "context" "encoding/json" + "fmt" + "math" "math/big" "strings" "time" @@ -143,15 +145,11 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre tfState = copyParameters(tfState, params) } - tfStateJsonBytes, err := json.Marshal(tfState) + tfStateDynamicValue, err := protov5DynamicValueFromMap(tfState, resourceSchema.Type().TerraformType(ctx)) if err != nil { - return nil, errors.Wrap(err, "could not marshal TF state map") + return nil, errors.Wrap(err, "cannot construct dynamic value for TF state") } - - opTracker.SetFrameworkTFState(&tfprotov5.DynamicValue{ - JSON: tfStateJsonBytes, - }) - + opTracker.SetFrameworkTFState(tfStateDynamicValue) } configuredProviderServer, err := c.configureProvider(ctx, ts) @@ -184,24 +182,32 @@ func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Contex } func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Context, ts terraform.Setup) (tfprotov5.ProviderServer, error) { + var schemaResp fwprovider.SchemaResponse + ts.FrameworkProvider.Schema(ctx, fwprovider.SchemaRequest{}, &schemaResp) + if schemaResp.Diagnostics.HasError() { + var diagErrors []string + for _, tfdiag := range schemaResp.Diagnostics.Errors() { + diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) + } + return nil, fmt.Errorf("cannot retrieve provider schema") + } providerServer := providerserver.NewProtocol5(ts.FrameworkProvider)() - tsBytes, err := json.Marshal(ts.Configuration) + + providerConfigDynamicVal, err := protov5DynamicValueFromMap(ts.Configuration, schemaResp.Schema.Type().TerraformType(ctx)) if err != nil { - return nil, errors.Wrap(err, "cannot marshal ts config") + return nil, errors.Wrap(err, "cannot construct dynamic value for TF provider config") } + configureProviderReq := &tfprotov5.ConfigureProviderRequest{ TerraformVersion: "crossTF000", - Config: &tfprotov5.DynamicValue{ - JSON: tsBytes, - }, + Config: providerConfigDynamicVal, } providerResp, err := providerServer.ConfigureProvider(ctx, configureProviderReq) if err != nil { return nil, err } - // TODO(erhan): improve diag reporting - if hasFatalDiag := hasFatalDiagnostics(providerResp.Diagnostics); hasFatalDiag { - return nil, errors.Errorf("provider configure request returned fatal diagnostics") + if fatalDiags := getFatalDiagnostics(providerResp.Diagnostics); fatalDiags != nil { + return nil, errors.Wrap(fatalDiags, "provider configure request failed") } return providerServer, nil } @@ -210,56 +216,30 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { valueTerraformType := n.resourceSchema.Type().TerraformType(ctx) - paramBytes, err := json.Marshal(n.params) - if err != nil { - return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot convert params to json bytes") - } - - tfConfigValue, err := tftypes.ValueFromJSONWithOpts(paramBytes, valueTerraformType, tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}) - if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - - tfConfig, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue) - if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - tfPlannedState, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue.Copy()) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) if err != nil { - return &tfprotov5.DynamicValue{}, false, err + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Config") } - diffs, err := tfStateValue.Diff(tfConfigValue) + // + tfPlannedStateDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - - // process diffs - processedDiffs := diffs[:0] - for _, diff := range diffs { - if !diff.Value2.IsNull() { - processedDiffs = append(processedDiffs, diff) - } - } - - if len(processedDiffs) == 0 { - return nil, false, nil + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Planned State") } prcReq := &tfprotov5.PlanResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), - Config: &tfConfig, - ProposedNewState: &tfPlannedState, + Config: tfConfigDynamicVal, + ProposedNewState: tfPlannedStateDynamicVal, } planResponse, err := n.server.PlanResourceChange(ctx, prcReq) if err != nil { return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot plan change") } - // TODO: improve diag reporting - if isFatal := hasFatalDiagnostics(planResponse.Diagnostics); isFatal { - return &tfprotov5.DynamicValue{}, false, errors.New("plan resource change has fatal diags") + if fatalDiags := getFatalDiagnostics(planResponse.Diagnostics); fatalDiags != nil { + return &tfprotov5.DynamicValue{}, false, errors.Wrap(fatalDiags, "plan resource change request failed") } if len(planResponse.RequiresReplace) > 0 { @@ -272,7 +252,17 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context return nil, false, errors.New(sb.String()) } - return planResponse.PlannedState, len(processedDiffs) > 0, nil + plannedStateValue, err := planResponse.PlannedState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + if err != nil { + return nil, false, errors.Wrap(err, "cannot unmarshal planned state") + } + + diffso, err := plannedStateValue.Diff(tfStateValue) + if err != nil { + return nil, false, errors.Wrap(err, "cannot compare prior state and plan") + } + + return planResponse.PlannedState, len(diffso) > 0, nil } @@ -295,9 +285,8 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg return managed.ExternalObservation{}, errors.Wrap(err, "cannot read resource") } - // TODO(erhan): improve diag reporting - if shouldError := hasFatalDiagnostics(readResponse.Diagnostics); shouldError { - return managed.ExternalObservation{}, errors.New("read returned diags") + if fatalDiags := getFatalDiagnostics(readResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalObservation{}, errors.Wrap(fatalDiags, "read resource request failed") } tfStateValue, err := readResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) @@ -310,7 +299,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg var stateValueMap map[string]any if resourceExists { - if conv, err := valueToGo(tfStateValue); err != nil { + if conv, err := tfValueToMap(tfStateValue); err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") } else { stateValueMap = conv.(map[string]any) @@ -377,18 +366,16 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { n.logger.Debug("Creating the external resource") - configJsonBytes, err := json.Marshal(n.params) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return managed.ExternalCreation{}, errors.Wrap(err, "cannot convert params to json bytes") + return managed.ExternalCreation{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: n.plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -396,29 +383,21 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot create resource") } - - // TODO(erhan): check diags reporting - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return managed.ExternalCreation{}, errors.Errorf("failed to create the resource:") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalCreation{}, errors.Wrap(fatalDiags, "resource creation failed with diags") } - // TODO(erhan): refactor schema - res := *n.resource - schemaResp := &fwresource.SchemaResponse{} - res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) - - newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaResp.Schema.Type().TerraformType(ctx)) + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal planned state") } - // TODO(erhan): check if new state ID is available if newStateAfterApplyVal.IsNull() { return managed.ExternalCreation{}, errors.New("new state is empty after creation") } var stateValueMap map[string]any - if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { return managed.ExternalCreation{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -435,7 +414,6 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) } - // TODO(erhan): check config.Sensitive conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") @@ -447,18 +425,16 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { n.logger.Debug("Updating the external resource") - configJsonBytes, err := json.Marshal(n.params) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, "cannot convert params to json bytes") + return managed.ExternalUpdate{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: n.plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -466,8 +442,8 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update resource") } - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return managed.ExternalUpdate{}, errors.Errorf("failed to create the resource:") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalUpdate{}, errors.Errorf("resource update failed") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) @@ -481,7 +457,7 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg } var stateValueMap map[string]any - if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { return managed.ExternalUpdate{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -497,9 +473,10 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error { n.logger.Debug("Deleting the external resource") - configJsonBytes, err := json.Marshal(n.params) + + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return errors.Wrap(err, "cannot convert params to json bytes") + return errors.Wrap(err, "cannot construct dynamic value for TF Config") } schemaType := n.resourceSchema.Type().TerraformType(ctx) @@ -513,9 +490,7 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: &plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -523,9 +498,8 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x if err != nil { return errors.Wrap(err, "cannot delete resource") } - // TODO(erhan): improve diagnostics reporting - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return errors.Errorf("failed to delete the resource with diags") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return errors.Wrap(fatalDiags, "resource deletion failed with diags") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) @@ -554,72 +528,118 @@ func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.M return oldName != newName, nil } -func valueToGo(input tftypes.Value) (any, error) { +func tfValueToMap(input tftypes.Value) (any, error) { + if !input.IsKnown() { + return nil, fmt.Errorf("cannot convert unknown value") + } if input.IsNull() { return nil, nil } valType := input.Type() - if valType.Is(tftypes.Object{}) || valType.Is(tftypes.Map{}) { + switch { + case valType.Is(tftypes.Object{}), valType.Is(tftypes.Map{}): destInterim := make(map[string]tftypes.Value) dest := make(map[string]any) if err := input.As(&destInterim); err != nil { return nil, err } for k, v := range destInterim { - if res, err := valueToGo(v); err != nil { + if res, err := tfValueToMap(v); err != nil { return nil, err } else { dest[k] = res } } return dest, nil - } else if valType.Is(tftypes.Set{}) || valType.Is(tftypes.List{}) || valType.Is(tftypes.Tuple{}) { + case valType.Is(tftypes.Set{}), valType.Is(tftypes.List{}), valType.Is(tftypes.Tuple{}): destInterim := make([]tftypes.Value, 0) dest := make([]any, 0) if err := input.As(&destInterim); err != nil { return nil, err } for i, v := range destInterim { - if res, err := valueToGo(v); err != nil { + if res, err := tfValueToMap(v); err != nil { return nil, err } else { dest[i] = res } } return dest, nil - } else if valType.Is(tftypes.Bool) { + case valType.Is(tftypes.Bool): var x bool if err := input.As(&x); err != nil { return nil, err } return x, nil - } else if valType.Is(tftypes.Number) { - var x big.Float - if err := input.As(&x); err != nil { + case valType.Is(tftypes.Number): + var valBigF big.Float + if err := input.As(&valBigF); err != nil { return nil, err } - xf, _ := x.Float64() - return xf, nil - } else if valType.Is(tftypes.String) { + // try to parse as integer + if valBigF.IsInt() { + intVal, accuracy := valBigF.Int64() + if accuracy != 0 { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit integer", valBigF) + } + return intVal, nil + } else { + xf, accuracy := valBigF.Float64() + // Underflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if xf == 0 && accuracy != big.Exact { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + } + + // Overflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if math.IsInf(xf, 0) { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + } + return xf, nil + } + case valType.Is(tftypes.String): var x string if err := input.As(&x); err != nil { return nil, err } return x, nil - } else if valType.Is(tftypes.DynamicPseudoType) { - // TODO: check if we need to handle DynamicPseudoType - return nil, nil - } else { - return nil, nil + case valType.Is(tftypes.DynamicPseudoType): + return nil, errors.New("DynamicPseudoType conversion is not supported") + default: + return nil, fmt.Errorf("input value has unknown type: %s", valType.String()) } } -func hasFatalDiagnostics(diags []*tfprotov5.Diagnostic) bool { - shouldError := false +func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { + var errs error + var diagErrors []string for _, tfdiag := range diags { if tfdiag.Severity == tfprotov5.DiagnosticSeverityInvalid || tfdiag.Severity == tfprotov5.DiagnosticSeverityError { - shouldError = true + diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary, tfdiag.Detail)) } } - return shouldError + if len(diagErrors) > 0 { + errs = errors.New(strings.Join(diagErrors, "\n")) + } + return errs +} + +func protov5DynamicValueFromMap(data map[string]any, terraformType tftypes.Type) (*tfprotov5.DynamicValue, error) { + jsonBytes, err := json.Marshal(data) + if err != nil { + return nil, errors.Wrap(err, "cannot marshal json") + } + + tfValue, err := tftypes.ValueFromJSONWithOpts(jsonBytes, terraformType, tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}) + if err != nil { + return nil, errors.Wrap(err, "cannot construct tf value from json") + } + + dynamicValue, err := tfprotov5.NewDynamicValue(terraformType, tfValue) + if err != nil { + return nil, errors.Wrap(err, "cannot construct dynamic value from tf value") + } + + return &dynamicValue, nil } From 1cabce63d1a839fd5bb6b6255225e03dfebec9ee Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 16 Jan 2024 10:49:52 +0300 Subject: [PATCH 11/24] fix linter issues Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 48d09934..d20cf47b 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -18,12 +18,10 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" - fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/providerserver" fwresource "github.com/hashicorp/terraform-plugin-framework/resource" rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" - _ "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/pkg/errors" @@ -38,14 +36,13 @@ import ( ) type TerraformPluginFrameworkConnector struct { - getTerraformSetup terraform.SetupFn - kube client.Client - config *config.Resource - logger logging.Logger - metricRecorder *metrics.MetricRecorder - operationTrackerStore *OperationTrackerStore - isManagementPoliciesEnabled bool - terraformPluginFrameworkProvider fwprovider.Provider + getTerraformSetup terraform.SetupFn + kube client.Client + config *config.Resource + logger logging.Logger + metricRecorder *metrics.MetricRecorder + operationTrackerStore *OperationTrackerStore + isManagementPoliciesEnabled bool } // TerraformPluginFrameworkConnectorOption allows you to configure TerraformPluginFrameworkConnector. @@ -100,7 +97,7 @@ type terraformPluginFrameworkExternalClient struct { resourceSchema rschema.Schema } -func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { +func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { //nolint:gocyclo c.metricRecorder.ObserveReconcileDelay(mg.GetObjectKind().GroupVersionKind(), mg.GetName()) logger := c.logger.WithValues("uid", mg.GetUID(), "name", mg.GetName(), "gvk", mg.GetObjectKind().GroupVersionKind().String()) logger.Debug("Connecting to the service provider") @@ -189,7 +186,7 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex for _, tfdiag := range schemaResp.Diagnostics.Errors() { diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) } - return nil, fmt.Errorf("cannot retrieve provider schema") + return nil, fmt.Errorf("cannot retrieve provider schema: %s", strings.Join(diagErrors, "\n")) } providerServer := providerserver.NewProtocol5(ts.FrameworkProvider)() @@ -528,7 +525,7 @@ func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.M return oldName != newName, nil } -func tfValueToMap(input tftypes.Value) (any, error) { +func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo if !input.IsKnown() { return nil, fmt.Errorf("cannot convert unknown value") } @@ -580,7 +577,7 @@ func tfValueToMap(input tftypes.Value) (any, error) { if valBigF.IsInt() { intVal, accuracy := valBigF.Int64() if accuracy != 0 { - return nil, fmt.Errorf("value %s cannot be represented as a 64-bit integer", valBigF) + return nil, fmt.Errorf("value %v cannot be represented as a 64-bit integer", valBigF) } return intVal, nil } else { @@ -588,13 +585,13 @@ func tfValueToMap(input tftypes.Value) (any, error) { // Underflow // Reference: https://pkg.go.dev/math/big#Float.Float64 if xf == 0 && accuracy != big.Exact { - return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) } // Overflow // Reference: https://pkg.go.dev/math/big#Float.Float64 if math.IsInf(xf, 0) { - return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) } return xf, nil } From 31e377d8634945294a0a5789a28bfd8bdc4c0240 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 16 Jan 2024 12:25:38 +0300 Subject: [PATCH 12/24] minor log message and naming changes Signed-off-by: Erhan Cagirici --- pkg/controller/external_async_terraform_plugin_framework.go | 4 ++-- pkg/controller/external_terraform_plugin_framework.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/external_async_terraform_plugin_framework.go b/pkg/controller/external_async_terraform_plugin_framework.go index 1bc6a991..bd3fb9de 100644 --- a/pkg/controller/external_async_terraform_plugin_framework.go +++ b/pkg/controller/external_async_terraform_plugin_framework.go @@ -47,7 +47,7 @@ func NewTerraformPluginFrameworkAsyncConnector(kube client.Client, func (c *TerraformPluginFrameworkAsyncConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { ec, err := c.TerraformPluginFrameworkConnector.Connect(ctx, mg) if err != nil { - return nil, errors.Wrap(err, "cannot initialize the no-fork async external client") + return nil, errors.Wrap(err, "cannot initialize the Terraform Plugin Framework async external client") } return &terraformPluginFrameworkAsyncExternalClient{ @@ -58,7 +58,7 @@ func (c *TerraformPluginFrameworkAsyncConnector) Connect(ctx context.Context, mg } // WithTerraformPluginFrameworkAsyncConnectorEventHandler configures the EventHandler so that -// the no-fork external clients can requeue reconciliation requests. +// the Terraform Plugin Framework external clients can requeue reconciliation requests. func WithTerraformPluginFrameworkAsyncConnectorEventHandler(e *handler.EventHandler) TerraformPluginFrameworkAsyncOption { return func(c *TerraformPluginFrameworkAsyncConnector) { c.eventHandler = e diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index d20cf47b..f633f5d2 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -440,7 +440,7 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update resource") } if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { - return managed.ExternalUpdate{}, errors.Errorf("resource update failed") + return managed.ExternalUpdate{}, errors.Wrap(fatalDiags, "resource update failed") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) @@ -480,7 +480,7 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x // set an empty planned state, this corresponds to deleting plannedState, err := tfprotov5.NewDynamicValue(schemaType, tftypes.NewValue(schemaType, nil)) if err != nil { - return errors.Wrap(err, "cannot set the planned state") + return errors.Wrap(err, "cannot set the planned state for deletion") } applyRequest := &tfprotov5.ApplyResourceChangeRequest{ @@ -502,7 +502,7 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaType) if err != nil { - return errors.Wrap(err, "cannot unmarshal updated state") + return errors.Wrap(err, "cannot unmarshal state after deletion") } // mark the resource as logically deleted if the TF call clears the state n.opTracker.SetDeleted(newStateAfterApplyVal.IsNull()) From 68bf3f177f9f6cbc4174c59307c321cec9b75513 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 16 Jan 2024 16:01:19 +0300 Subject: [PATCH 13/24] fix unit tests Signed-off-by: Erhan Cagirici --- pkg/config/common_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index c97aa975..bd0ff6a9 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -127,6 +127,7 @@ func TestDefaultResource(t *testing.T) { cmpopts.IgnoreFields(LateInitializer{}, "ignoredCanonicalFieldPaths"), cmpopts.IgnoreFields(ExternalName{}, "SetIdentifierArgumentFn", "GetExternalNameFn", "GetIDFn"), cmpopts.IgnoreFields(Resource{}, "useNoForkClient"), + cmpopts.IgnoreFields(Resource{}, "useTerraformPluginFrameworkClient"), } for name, tc := range cases { From 615d6e781723dae88f64d165e731902e90216384 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 16 Jan 2024 23:22:14 +0300 Subject: [PATCH 14/24] fix TF list conversions Signed-off-by: Erhan Cagirici --- pkg/controller/external_terraform_plugin_framework.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index f633f5d2..57563dc0 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -550,10 +550,10 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo return dest, nil case valType.Is(tftypes.Set{}), valType.Is(tftypes.List{}), valType.Is(tftypes.Tuple{}): destInterim := make([]tftypes.Value, 0) - dest := make([]any, 0) if err := input.As(&destInterim); err != nil { return nil, err } + dest := make([]any, len(destInterim)) for i, v := range destInterim { if res, err := tfValueToMap(v); err != nil { return nil, err From fe03f89abddf1f7d0f4a99df114ae73cd8944d98 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sun, 28 Jan 2024 14:25:52 +0300 Subject: [PATCH 15/24] refactor unnecessary iface pointers & add go doc comments Signed-off-by: Erhan Cagirici --- pkg/config/common.go | 2 +- pkg/config/common_test.go | 2 +- pkg/config/provider.go | 30 ++++++-- pkg/config/resource.go | 13 +++- .../external_terraform_plugin_framework.go | 4 +- pkg/controller/nofork_store.go | 70 ++++++++++++++++--- 6 files changed, 97 insertions(+), 24 deletions(-) diff --git a/pkg/config/common.go b/pkg/config/common.go index c34eee8c..651a4c88 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -57,7 +57,7 @@ type ResourceOption func(*Resource) // DefaultResource keeps an initial default configuration for all resources of a // provider. -func DefaultResource(name string, terraformSchema *schema.Resource, terraformPluginFrameworkResource *fwresource.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource { +func DefaultResource(name string, terraformSchema *schema.Resource, terraformPluginFrameworkResource fwresource.Resource, terraformRegistry *registry.Resource, opts ...ResourceOption) *Resource { words := strings.Split(name, "_") // As group name we default to the second element if resource name // has at least 3 elements, otherwise, we took the first element as diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index bd0ff6a9..19578e13 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -19,7 +19,7 @@ func TestDefaultResource(t *testing.T) { type args struct { name string sch *schema.Resource - frameworkResource *fwresource.Resource + frameworkResource fwresource.Resource reg *registry.Resource opts []ResourceOption } diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 47a27247..409a4466 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -118,24 +118,34 @@ type Provider struct { // Defaults to []string{".+"} which would include all resources. IncludeList []string - // NoForkIncludeList is a list of regex for the Terraform resources to be - // included and reconciled in the no-fork architecture (without the - // Terraform CLI). + // NoForkIncludeList is a list of regex for the Terraform resources + // implemented with Terraform Plugin SDKv2 to be included and reconciled + // in the no-fork architecture (without the Terraform CLI). // For example, to include "aws_shield_protection_group" into // the generated resources, one can add "aws_shield_protection_group$". // To include whole aws waf group, one can add "aws_waf.*" to the list. // Defaults to []string{".+"} which would include all resources. NoForkIncludeList []string + // TerraformPluginFrameworkIncludeList is a list of regex for the Terraform + // resources implemented with Terraform Plugin Framework to be included and + // reconciled in the no-fork architecture (without the Terraform CLI). + // For example, to include "aws_shield_protection_group" into + // the generated resources, one can add "aws_shield_protection_group$". + // To include whole aws waf group, one can add "aws_waf.*" to the list. + // Defaults to []string{".+"} which would include all resources. TerraformPluginFrameworkIncludeList []string // Resources is a map holding resource configurations where key is Terraform // resource name. Resources map[string]*Resource - // TerraformProvider is the Terraform schema of the provider. + // TerraformProvider is the Terraform provider in Terraform Plugin SDKv2 + // compatible format TerraformProvider *schema.Provider + // TerraformPluginFrameworkProvider is the Terraform provider reference + // in Terraform Plugin Framework compatible format TerraformPluginFrameworkProvider fwprovider.Provider // refInjectors is an ordered list of `ReferenceInjector`s for @@ -177,13 +187,17 @@ func WithIncludeList(l []string) ProviderOption { } } -// WithNoForkIncludeList configures IncludeList for this Provider. +// WithNoForkIncludeList configures the NoForkIncludeList for this Provider, +// with the given Terraform Plugin SDKv2-based resource name list func WithNoForkIncludeList(l []string) ProviderOption { return func(p *Provider) { p.NoForkIncludeList = l } } +// WithTerraformPluginFrameworkIncludeList configures the +// TerraformPluginFrameworkIncludeList for this Provider, with the given +// Terraform Plugin Framework-based resource name list func WithTerraformPluginFrameworkIncludeList(l []string) ProviderOption { return func(p *Provider) { p.TerraformPluginFrameworkIncludeList = l @@ -197,6 +211,8 @@ func WithTerraformProvider(tp *schema.Provider) ProviderOption { } } +// WithTerraformPluginFrameworkProvider configures the +// TerraformPluginFrameworkProvider for this Provider. func WithTerraformPluginFrameworkProvider(tp fwprovider.Provider) ProviderOption { return func(p *Provider) { p.TerraformPluginFrameworkProvider = tp @@ -317,7 +333,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s } } - var terraformPluginFrameworkResource *fwresource.Resource + var terraformPluginFrameworkResource fwresource.Resource if isPluginFrameworkResource { // TODO: Consider whether to replace the commented out conditional in the next line with an equivalent conditional for plugin framework. @@ -335,7 +351,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s resourceTypeNameResp := fwresource.MetadataResponse{} resource.Metadata(ctx, resourceTypeNameReq, &resourceTypeNameResp) if resourceTypeNameResp.TypeName == name { - terraformPluginFrameworkResource = &resource + terraformPluginFrameworkResource = resource break } } diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 93192d72..155210ca 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -377,10 +377,13 @@ type Resource struct { // e.g. aws_rds_cluster. Name string - // TerraformResource is the Terraform representation of the resource. + // TerraformResource is the Terraform representation of the + // Terraform Plugin SDKv2 based resource. TerraformResource *schema.Resource - TerraformPluginFrameworkResource *fwresource.Resource + // TerraformPluginFrameworkResource is the Terraform representation + // of the TF Plugin Framework based resource + TerraformPluginFrameworkResource fwresource.Resource // ShortGroup is the short name of the API group of this CRD. The full // CRD API group is calculated by adding the group suffix of the provider. @@ -488,10 +491,16 @@ type Resource struct { OverrideFieldNames map[string]string } +// ShouldUseNoForkClient returns whether to generate a SDKv2-based no-fork +// external client for this Resource, instead of the Terraform CLI-forking +// external client func (r *Resource) ShouldUseNoForkClient() bool { return r.useNoForkClient } +// ShouldUseTerraformPluginFrameworkClient returns whether to generate a +// Terraform Plugin Framework-based no-fork external client for this Resource +// instead of a Terraform Plugin SDKv2-based external client func (r *Resource) ShouldUseTerraformPluginFrameworkClient() bool { return r.useTerraformPluginFrameworkClient } diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 57563dc0..e1680274 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -90,7 +90,7 @@ type terraformPluginFrameworkExternalClient struct { logger logging.Logger metricRecorder *metrics.MetricRecorder opTracker *AsyncTracker - resource *fwresource.Resource + resource fwresource.Resource server tfprotov5.ProviderServer params map[string]any plannedState *tfprotov5.DynamicValue @@ -168,7 +168,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre } func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (rschema.Schema, error) { - res := *c.config.TerraformPluginFrameworkResource + res := c.config.TerraformPluginFrameworkResource schemaResp := &fwresource.SchemaResponse{} res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) if schemaResp.Diagnostics.HasError() { diff --git a/pkg/controller/nofork_store.go b/pkg/controller/nofork_store.go index 1dfdf071..7be3807f 100644 --- a/pkg/controller/nofork_store.go +++ b/pkg/controller/nofork_store.go @@ -18,12 +18,36 @@ import ( "github.com/crossplane/upjet/pkg/terraform" ) +// AsyncTracker holds information for a managed resource to track the +// async Terraform operations and the +// Terraform state (TF SDKv2 or TF Plugin Framework) of the external resource +// +// The typical usage is to instantiate an AsyncTracker for a managed resource, +// and store in a global OperationTrackerStore, to carry information between +// reconciliation scopes. +// +// When an asynchronous Terraform operation is started for the resource +// in a reconciliation (e.g. with a goroutine), consumers can mark an operation start +// on the LastOperation field, then access the operation status in the +// forthcoming reconciliation cycles, and act upon +// (e.g. hold further actions if there is an ongoing operation, mark the end +// when underlying Terraform operation is completed, save the resulting +// terraform state etc.) +// +// When utilized without the LastOperation usage, it can act as a Terraform +// state cache for synchronous reconciliations type AsyncTracker struct { + // LastOperation holds information about the most recent operation. + // Consumers are responsible for managing the last operation by starting, + // ending and flushing it when done with processing the results. + // Designed to allow only one ongoing operation at a given time. LastOperation *terraform.Operation logger logging.Logger mu *sync.Mutex - tfState *tfsdk.InstanceState - fwState *tfprotov5.DynamicValue + // TF Plugin SDKv2 instance state for TF Plugin SDKv2-based resources + tfState *tfsdk.InstanceState + // TF Plugin Framework instance state for TF Plugin Framework-based resources + fwState *tfprotov5.DynamicValue // lifecycle of certain external resources are bound to a parent resource's // lifecycle, and they cannot be deleted without actually deleting // the owning external resource (e.g., a database resource as the parent @@ -44,6 +68,8 @@ func WithAsyncTrackerLogger(l logging.Logger) AsyncTrackerOption { w.logger = l } } + +// NewAsyncTracker initializes an AsyncTracker with given options func NewAsyncTracker(opts ...AsyncTrackerOption) *AsyncTracker { w := &AsyncTracker{ LastOperation: &terraform.Operation{}, @@ -56,24 +82,35 @@ func NewAsyncTracker(opts ...AsyncTrackerOption) *AsyncTracker { return w } +// GetTfState returns the stored Terraform Plugin SDKv2 InstanceState for +// SDKv2 Terraform resources +// MUST be only used for SDKv2 resources. func (a *AsyncTracker) GetTfState() *tfsdk.InstanceState { a.mu.Lock() defer a.mu.Unlock() return a.tfState } +// HasState returns whether the AsyncTracker has a SDKv2 state stored. +// MUST be only used for SDKv2 resources. func (a *AsyncTracker) HasState() bool { a.mu.Lock() defer a.mu.Unlock() return a.tfState != nil && a.tfState.ID != "" } +// SetTfState stores the given SDKv2 Terraform InstanceState into +// the AsyncTracker +// MUST be only used for SDKv2 resources. func (a *AsyncTracker) SetTfState(state *tfsdk.InstanceState) { a.mu.Lock() defer a.mu.Unlock() a.tfState = state } +// GetTfID returns the Terraform ID of the external resource currently +// stored in this AsyncTracker's SDKv2 instance state. +// MUST be only used for SDKv2 resources. func (a *AsyncTracker) GetTfID() string { a.mu.Lock() defer a.mu.Unlock() @@ -95,39 +132,42 @@ func (a *AsyncTracker) SetDeleted(deleted bool) { a.isDeleted.Store(deleted) } +// GetFrameworkTFState returns the stored Terraform Plugin Framework external +// resource state in this AsyncTracker as *tfprotov5.DynamicValue +// MUST be used only for Terraform Plugin Framework resources func (a *AsyncTracker) GetFrameworkTFState() *tfprotov5.DynamicValue { a.mu.Lock() defer a.mu.Unlock() return a.fwState } +// HasFrameworkTFState returns whether this AsyncTracker has a +// Terraform Plugin Framework state stored. +// MUST be used only for Terraform Plugin Framework resources func (a *AsyncTracker) HasFrameworkTFState() bool { a.mu.Lock() defer a.mu.Unlock() return a.fwState != nil } +// SetFrameworkTFState stores the given *tfprotov5.DynamicValue Terraform Plugin Framework external +// resource state into this AsyncTracker's fwstate +// MUST be used only for Terraform Plugin Framework resources func (a *AsyncTracker) SetFrameworkTFState(state *tfprotov5.DynamicValue) { a.mu.Lock() defer a.mu.Unlock() a.fwState = state } -func (a *AsyncTracker) GetFrameworkTFID() string { - a.mu.Lock() - defer a.mu.Unlock() - if a.fwState == nil { - return "" - } - return "TBD" -} - +// OperationTrackerStore stores the AsyncTracker instances associated with the +// managed resource instance. type OperationTrackerStore struct { store map[types.UID]*AsyncTracker logger logging.Logger mu *sync.Mutex } +// NewOperationStore returns a new OperationTrackerStore instance func NewOperationStore(l logging.Logger) *OperationTrackerStore { ops := &OperationTrackerStore{ store: map[types.UID]*AsyncTracker{}, @@ -138,6 +178,12 @@ func NewOperationStore(l logging.Logger) *OperationTrackerStore { return ops } +// Tracker returns the associated *AsyncTracker stored in this +// OperationTrackerStore for the given managed resource. +// If there is no tracker stored previously, a new AsyncTracker is created and +// stored for the specified managed resource. Subsequent calls with the same managed +// resource will return the previously instantiated and stored AsyncTracker +// for that managed resource func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker { ops.mu.Lock() defer ops.mu.Unlock() @@ -150,6 +196,8 @@ func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker return tracker } +// RemoveTracker will remove the stored AsyncTracker of the given managed +// resource from this OperationTrackerStore. func (ops *OperationTrackerStore) RemoveTracker(obj xpresource.Object) error { ops.mu.Lock() defer ops.mu.Unlock() From 7444fba2d761ea8c0c152123fe7e4d107cf8dfd7 Mon Sep 17 00:00:00 2001 From: Cem Mergenci Date: Tue, 30 Jan 2024 09:05:45 +0300 Subject: [PATCH 16/24] Improve provider generation performance. 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 --- pkg/config/provider.go | 54 ++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 409a4466..6d0b71fc 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -266,7 +266,7 @@ func WithMainTemplate(template string) ProviderOption { // NewProvider builds and returns a new Provider from provider // tfjson schema, that is generated using Terraform CLI with: // `terraform providers schema --json` -func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath string, metadata []byte, opts ...ProviderOption) *Provider { //nolint:gocyclo +func NewProvider(schema []byte, prefix string, modulePath string, metadata []byte, opts ...ProviderOption) *Provider { //nolint:gocyclo ps := tfjson.ProviderSchemas{} if err := ps.UnmarshalJSON(schema); err != nil { panic(err) @@ -304,7 +304,7 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s } p.skippedResourceNames = make([]string, 0, len(resourceMap)) - terraformPluginFrameworkResourceFunctions := p.TerraformPluginFrameworkProvider.Resources(ctx) + terraformPluginFrameworkResourceFunctionsMap := terraformPluginFrameworkResourceFunctionsMap(p.TerraformPluginFrameworkProvider) for name, terraformResource := range resourceMap { if len(terraformResource.Schema) == 0 { // There are resources with no schema, that we will address later. @@ -334,27 +334,14 @@ func NewProvider(ctx context.Context, schema []byte, prefix string, modulePath s } var terraformPluginFrameworkResource fwresource.Resource - if isPluginFrameworkResource { - // TODO: Consider whether to replace the commented out conditional in the next line with an equivalent conditional for plugin framework. - if p.TerraformPluginFrameworkProvider == nil /*|| p.TerraformProvider.ResourcesMap[name] == nil */ { - panic(errors.Errorf("resource %q is configured to be reconciled without the Terraform CLI"+ - "but either config.Provider.TerraformProvider is not configured or the Go schema does not exist for the resource", name)) + resourceFunc := terraformPluginFrameworkResourceFunctionsMap[name] + if p.TerraformPluginFrameworkProvider == nil || resourceFunc == nil { + panic(errors.Errorf("resource %q is configured to be reconciled with Terraform Plugin Framework"+ + "but either config.Provider.TerraformPluginFrameworkProvider is not configured or the provider doesn't have the resource.", name)) } - for _, resourceFunc := range terraformPluginFrameworkResourceFunctions { - resource := resourceFunc() - - resourceTypeNameReq := fwresource.MetadataRequest{ - ProviderTypeName: name, - } - resourceTypeNameResp := fwresource.MetadataResponse{} - resource.Metadata(ctx, resourceTypeNameReq, &resourceTypeNameResp) - if resourceTypeNameResp.TypeName == name { - terraformPluginFrameworkResource = resource - break - } - } + terraformPluginFrameworkResource = resourceFunc() } p.Resources[name] = DefaultResource(name, terraformResource, terraformPluginFrameworkResource, providerMetadata.Resources[name], p.DefaultResourceOptions...) @@ -414,3 +401,30 @@ func matches(name string, regexList []string) bool { } return false } + +func terraformPluginFrameworkResourceFunctionsMap(provider fwprovider.Provider) map[string]func() fwresource.Resource { + if provider == nil { + return make(map[string]func() fwresource.Resource, 0) + } + + ctx := context.TODO() + resourceFunctions := provider.Resources(ctx) + resourceFunctionsMap := make(map[string]func() fwresource.Resource, len(resourceFunctions)) + + providerMetadata := fwprovider.MetadataResponse{} + provider.Metadata(ctx, fwprovider.MetadataRequest{}, &providerMetadata) + + for _, resourceFunction := range resourceFunctions { + resource := resourceFunction() + + resourceTypeNameReq := fwresource.MetadataRequest{ + ProviderTypeName: providerMetadata.TypeName, + } + resourceTypeNameResp := fwresource.MetadataResponse{} + resource.Metadata(ctx, resourceTypeNameReq, &resourceTypeNameResp) + + resourceFunctionsMap[resourceTypeNameResp.TypeName] = resourceFunction + } + + return resourceFunctionsMap +} From 9b9149c1a5089c096c7cf56fe544ea25edb14926 Mon Sep 17 00:00:00 2001 From: Cem Mergenci Date: Tue, 30 Jan 2024 09:43:54 +0300 Subject: [PATCH 17/24] Add doc comments. Signed-off-by: Cem Mergenci --- pkg/controller/external_terraform_plugin_framework.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index e1680274..7042a2e0 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -35,6 +35,9 @@ import ( "github.com/crossplane/upjet/pkg/terraform" ) +// TerraformPluginFrameworkConnector is an external client, with credentials and +// other configuration parameters, for Terraform Plugin Framework resources. You +// can use NewTerraformPluginFrameworkConnector to construct. type TerraformPluginFrameworkConnector struct { getTerraformSetup terraform.SetupFn kube client.Client @@ -71,6 +74,8 @@ func WithTerraformPluginFrameworkManagementPolicies(isManagementPoliciesEnabled } } +// NewTerraformPluginFrameworkConnector creates a new +// TerraformPluginFrameworkConnector with given options. func NewTerraformPluginFrameworkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, opts ...TerraformPluginFrameworkConnectorOption) *TerraformPluginFrameworkConnector { connector := &TerraformPluginFrameworkConnector{ getTerraformSetup: sf, @@ -97,6 +102,8 @@ type terraformPluginFrameworkExternalClient struct { resourceSchema rschema.Schema } +// Connect makes sure the underlying client is ready to issue requests to the +// provider API. func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { //nolint:gocyclo c.metricRecorder.ObserveReconcileDelay(mg.GetObjectKind().GroupVersionKind(), mg.GetName()) logger := c.logger.WithValues("uid", mg.GetUID(), "name", mg.GetName(), "gvk", mg.GetObjectKind().GroupVersionKind().String()) From e7b633e14b240c759f9b4d4e242bb2a5cc26cf20 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 30 Jan 2024 12:33:50 +0300 Subject: [PATCH 18/24] refactor diag to string, terraform value type, error messages Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 140 +++++++++--------- 1 file changed, 72 insertions(+), 68 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index 7042a2e0..fa742c9b 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -18,6 +18,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + fwdiag "github.com/hashicorp/terraform-plugin-framework/diag" fwprovider "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/providerserver" fwresource "github.com/hashicorp/terraform-plugin-framework/resource" @@ -100,6 +101,8 @@ type terraformPluginFrameworkExternalClient struct { params map[string]any plannedState *tfprotov5.DynamicValue resourceSchema rschema.Schema + // the terraform value type associated with the resource schema + resourceValueTerraformType tftypes.Type } // Connect makes sure the underlying client is ready to issue requests to the @@ -127,9 +130,13 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre if err != nil { return nil, errors.Wrap(err, "could not retrieve resource schema") } + resourceTfValueType := resourceSchema.Type().TerraformType(ctx) hasState := false if opTracker.HasFrameworkTFState() { - tfStateValue, err := opTracker.GetFrameworkTFState().Unmarshal(resourceSchema.Type().TerraformType(ctx)) + tfStateValue, err := opTracker.GetFrameworkTFState().Unmarshal(resourceTfValueType) + if err != nil { + return nil, errors.Wrap(err, "cannot unmarshal TF state dynamic value during state existence check") + } hasState = err == nil && !tfStateValue.IsNull() } @@ -149,7 +156,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre tfState = copyParameters(tfState, params) } - tfStateDynamicValue, err := protov5DynamicValueFromMap(tfState, resourceSchema.Type().TerraformType(ctx)) + tfStateDynamicValue, err := protov5DynamicValueFromMap(tfState, resourceTfValueType) if err != nil { return nil, errors.Wrap(err, "cannot construct dynamic value for TF state") } @@ -162,15 +169,16 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre } return &terraformPluginFrameworkExternalClient{ - ts: ts, - config: c.config, - logger: logger, - metricRecorder: c.metricRecorder, - opTracker: opTracker, - resource: c.config.TerraformPluginFrameworkResource, - server: configuredProviderServer, - params: params, - resourceSchema: resourceSchema, + ts: ts, + config: c.config, + logger: logger, + metricRecorder: c.metricRecorder, + opTracker: opTracker, + resource: c.config.TerraformPluginFrameworkResource, + server: configuredProviderServer, + params: params, + resourceSchema: resourceSchema, + resourceValueTerraformType: resourceTfValueType, }, nil } @@ -179,7 +187,8 @@ func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Contex schemaResp := &fwresource.SchemaResponse{} res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) if schemaResp.Diagnostics.HasError() { - return rschema.Schema{}, errors.Errorf("could not retrieve resource schema: %v", schemaResp.Diagnostics) + fwErrors := frameworkDiagnosticsToString(schemaResp.Diagnostics) + return rschema.Schema{}, errors.Errorf("could not retrieve resource schema: %s", fwErrors) } return schemaResp.Schema, nil @@ -189,11 +198,8 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex var schemaResp fwprovider.SchemaResponse ts.FrameworkProvider.Schema(ctx, fwprovider.SchemaRequest{}, &schemaResp) if schemaResp.Diagnostics.HasError() { - var diagErrors []string - for _, tfdiag := range schemaResp.Diagnostics.Errors() { - diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) - } - return nil, fmt.Errorf("cannot retrieve provider schema: %s", strings.Join(diagErrors, "\n")) + fwDiags := frameworkDiagnosticsToString(schemaResp.Diagnostics) + return nil, fmt.Errorf("cannot retrieve provider schema: %s", fwDiags) } providerServer := providerserver.NewProtocol5(ts.FrameworkProvider)() @@ -208,7 +214,7 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex } providerResp, err := providerServer.ConfigureProvider(ctx, configureProviderReq) if err != nil { - return nil, err + return nil, errors.Wrap(err, "cannot configure framework provider") } if fatalDiags := getFatalDiagnostics(providerResp.Diagnostics); fatalDiags != nil { return nil, errors.Wrap(fatalDiags, "provider configure request failed") @@ -218,16 +224,13 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context, tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { - - valueTerraformType := n.resourceSchema.Type().TerraformType(ctx) - - tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) if err != nil { return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Config") } // - tfPlannedStateDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) + tfPlannedStateDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) if err != nil { return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Planned State") } @@ -256,7 +259,7 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context return nil, false, errors.New(sb.String()) } - plannedStateValue, err := planResponse.PlannedState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + plannedStateValue, err := planResponse.PlannedState.Unmarshal(n.resourceValueTerraformType) if err != nil { return nil, false, errors.Wrap(err, "cannot unmarshal planned state") } @@ -267,7 +270,6 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context } return planResponse.PlannedState, len(diffso) > 0, nil - } func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg xpresource.Managed) (managed.ExternalObservation, error) { //nolint:gocyclo @@ -293,7 +295,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg return managed.ExternalObservation{}, errors.Wrap(fatalDiags, "read resource request failed") } - tfStateValue, err := readResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + tfStateValue, err := readResponse.NewState.Unmarshal(n.resourceValueTerraformType) if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot unmarshal state value") } @@ -370,7 +372,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { n.logger.Debug("Creating the external resource") - tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } @@ -383,15 +385,15 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) - metrics.ExternalAPITime.WithLabelValues("create").Observe(time.Since(start).Seconds()) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot create resource") } + metrics.ExternalAPITime.WithLabelValues("create").Observe(time.Since(start).Seconds()) if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { - return managed.ExternalCreation{}, errors.Wrap(fatalDiags, "resource creation failed with diags") + return managed.ExternalCreation{}, errors.Wrap(fatalDiags, "resource creation call returned error diags") } - newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceValueTerraformType) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal planned state") } @@ -429,7 +431,7 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { n.logger.Debug("Updating the external resource") - tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } @@ -442,16 +444,16 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) - metrics.ExternalAPITime.WithLabelValues("update").Observe(time.Since(start).Seconds()) if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update resource") } + metrics.ExternalAPITime.WithLabelValues("update").Observe(time.Since(start).Seconds()) if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { - return managed.ExternalUpdate{}, errors.Wrap(fatalDiags, "resource update failed") + return managed.ExternalUpdate{}, errors.Wrap(fatalDiags, "resource update call returned error diags") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) - newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceValueTerraformType) if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot unmarshal updated state") } @@ -478,14 +480,12 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error { n.logger.Debug("Deleting the external resource") - tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) if err != nil { return errors.Wrap(err, "cannot construct dynamic value for TF Config") } - - schemaType := n.resourceSchema.Type().TerraformType(ctx) // set an empty planned state, this corresponds to deleting - plannedState, err := tfprotov5.NewDynamicValue(schemaType, tftypes.NewValue(schemaType, nil)) + plannedState, err := tfprotov5.NewDynamicValue(n.resourceValueTerraformType, tftypes.NewValue(n.resourceValueTerraformType, nil)) if err != nil { return errors.Wrap(err, "cannot set the planned state for deletion") } @@ -498,16 +498,16 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) - metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds()) if err != nil { return errors.Wrap(err, "cannot delete resource") } + metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds()) if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { - return errors.Wrap(fatalDiags, "resource deletion failed with diags") + return errors.Wrap(fatalDiags, "resource deletion call returned error diags") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) - newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaType) + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceValueTerraformType) if err != nil { return errors.Wrap(err, "cannot unmarshal state after deletion") } @@ -548,11 +548,12 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo return nil, err } for k, v := range destInterim { - if res, err := tfValueToMap(v); err != nil { + res, err := tfValueToMap(v) + if err != nil { return nil, err - } else { - dest[k] = res } + dest[k] = res + } return dest, nil case valType.Is(tftypes.Set{}), valType.Is(tftypes.List{}), valType.Is(tftypes.Tuple{}): @@ -562,19 +563,16 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo } dest := make([]any, len(destInterim)) for i, v := range destInterim { - if res, err := tfValueToMap(v); err != nil { + res, err := tfValueToMap(v) + if err != nil { return nil, err - } else { - dest[i] = res } + dest[i] = res } return dest, nil case valType.Is(tftypes.Bool): var x bool - if err := input.As(&x); err != nil { - return nil, err - } - return x, nil + return x, input.As(&x) case valType.Is(tftypes.Number): var valBigF big.Float if err := input.As(&valBigF); err != nil { @@ -587,27 +585,25 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo return nil, fmt.Errorf("value %v cannot be represented as a 64-bit integer", valBigF) } return intVal, nil - } else { - xf, accuracy := valBigF.Float64() - // Underflow - // Reference: https://pkg.go.dev/math/big#Float.Float64 - if xf == 0 && accuracy != big.Exact { - return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) - } + } + // try to parse as float64 + xf, accuracy := valBigF.Float64() + // Underflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if xf == 0 && accuracy != big.Exact { + return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) + } - // Overflow - // Reference: https://pkg.go.dev/math/big#Float.Float64 - if math.IsInf(xf, 0) { - return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) - } - return xf, nil + // Overflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if math.IsInf(xf, 0) { + return nil, fmt.Errorf("value %v cannot be represented as a 64-bit floating point", valBigF) } + return xf, nil + case valType.Is(tftypes.String): var x string - if err := input.As(&x); err != nil { - return nil, err - } - return x, nil + return x, input.As(&x) case valType.Is(tftypes.DynamicPseudoType): return nil, errors.New("DynamicPseudoType conversion is not supported") default: @@ -629,6 +625,14 @@ func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { return errs } +func frameworkDiagnosticsToString(fwdiags fwdiag.Diagnostics) string { + var diagErrors []string + for _, tfdiag := range fwdiags.Errors() { + diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) + } + return strings.Join(diagErrors, "\n") +} + func protov5DynamicValueFromMap(data map[string]any, terraformType tftypes.Type) (*tfprotov5.DynamicValue, error) { jsonBytes, err := json.Marshal(data) if err != nil { From 340b66fb892db1e11fe2c4f409c09ec068899d3d Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 30 Jan 2024 14:34:49 +0300 Subject: [PATCH 19/24] pre-allocate diag error accumulator slice Signed-off-by: Erhan Cagirici --- pkg/controller/external_terraform_plugin_framework.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index fa742c9b..c7a26b24 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -626,8 +626,9 @@ func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { } func frameworkDiagnosticsToString(fwdiags fwdiag.Diagnostics) string { - var diagErrors []string - for _, tfdiag := range fwdiags.Errors() { + frameworkErrorDiags := fwdiags.Errors() + diagErrors := make([]string, 0, len(frameworkErrorDiags)) + for _, tfdiag := range frameworkErrorDiags { diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) } return strings.Join(diagErrors, "\n") From 2b2812fe6161cab7fa5a50e14eaf8098179469d9 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 30 Jan 2024 15:19:10 +0300 Subject: [PATCH 20/24] add doc comments for internal funcs Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index c7a26b24..edf44e6f 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -182,6 +182,7 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre }, nil } +// getResourceSchema returns the Terraform Plugin Framework-style resource schema for the configured framework resource on the connector func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Context) (rschema.Schema, error) { res := c.config.TerraformPluginFrameworkResource schemaResp := &fwresource.SchemaResponse{} @@ -194,6 +195,11 @@ func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Contex return schemaResp.Schema, nil } +// configureProvider returns a configured Terraform protocol v5 provider server +// with the preconfigured provider instance in the terraform setup. +// The provider instance used should be already preconfigured +// at the terraform setup layer with the relevant provider meta if needed +// by the provider implementation. func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Context, ts terraform.Setup) (tfprotov5.ProviderServer, error) { var schemaResp fwprovider.SchemaResponse ts.FrameworkProvider.Schema(ctx, fwprovider.SchemaRequest{}, &schemaResp) @@ -222,6 +228,11 @@ func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Contex return providerServer, nil } +// getDiffPlan calls the underlying native TF provider's PlanResourceChange RPC, +// and returns the planned state and whether a diff exists. +// If plan response contains non-empty RequiresReplace (i.e. the resource needs +// to be recreated) an error is returned as Crossplane Resource Model (XRM) +// prohibits resource re-creations and rejects this plan. func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context, tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType) @@ -305,7 +316,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg var stateValueMap map[string]any if resourceExists { - if conv, err := tfValueToMap(tfStateValue); err != nil { + if conv, err := tfValueToGoValue(tfStateValue); err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") } else { stateValueMap = conv.(map[string]any) @@ -403,7 +414,7 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg } var stateValueMap map[string]any - if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToGoValue(newStateAfterApplyVal); err != nil { return managed.ExternalCreation{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -463,7 +474,7 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg } var stateValueMap map[string]any - if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToGoValue(newStateAfterApplyVal); err != nil { return managed.ExternalUpdate{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -532,7 +543,18 @@ func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.M return oldName != newName, nil } -func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo +// tfValueToGoValue converts a given tftypes.Value to Go-native any type. +// Useful for converting terraform values of state to JSON or for setting +// observations at the MR. +// Nested values are recursively converted. +// Supported conversions: +// tftypes.Object, tftypes.Map => map[string]any +// tftypes.Set, tftypes.List, tftypes.Tuple => []string +// tftypes.Bool => bool +// tftypes.Number => int64, float64 +// tftypes.String => string +// tftypes.DynamicPseudoType => conversion not supported and returns an error +func tfValueToGoValue(input tftypes.Value) (any, error) { //nolint:gocyclo if !input.IsKnown() { return nil, fmt.Errorf("cannot convert unknown value") } @@ -548,7 +570,7 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo return nil, err } for k, v := range destInterim { - res, err := tfValueToMap(v) + res, err := tfValueToGoValue(v) if err != nil { return nil, err } @@ -563,7 +585,7 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo } dest := make([]any, len(destInterim)) for i, v := range destInterim { - res, err := tfValueToMap(v) + res, err := tfValueToGoValue(v) if err != nil { return nil, err } @@ -611,6 +633,8 @@ func tfValueToMap(input tftypes.Value) (any, error) { //nolint:gocyclo } } +// getFatalDiagnostics traverses the given Terraform protov5 diagnostics type +// and constructs a Go error. If the provided diag slice is empty, returns nil. func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { var errs error var diagErrors []string @@ -625,6 +649,9 @@ func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { return errs } +// frameworkDiagnosticsToString constructs an error string from the provided +// Plugin Framework diagnostics instance. Only Error severity diagnostics are +// included. func frameworkDiagnosticsToString(fwdiags fwdiag.Diagnostics) string { frameworkErrorDiags := fwdiags.Errors() diagErrors := make([]string, 0, len(frameworkErrorDiags)) @@ -634,6 +661,8 @@ func frameworkDiagnosticsToString(fwdiags fwdiag.Diagnostics) string { return strings.Join(diagErrors, "\n") } +// protov5DynamicValueFromMap constructs a protov5 DynamicValue given the +// map[string]any using the terraform type as reference. func protov5DynamicValueFromMap(data map[string]any, terraformType tftypes.Type) (*tfprotov5.DynamicValue, error) { jsonBytes, err := json.Marshal(data) if err != nil { From 67c38e9c47f961d5cb5b856d4369d105fe57b279 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Tue, 30 Jan 2024 21:25:13 +0300 Subject: [PATCH 21/24] go mod tidy Signed-off-by: Erhan Cagirici --- go.sum | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.sum b/go.sum index 10f2aece..6915c4e9 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,7 @@ github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZx github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bufbuild/protocompile v0.6.0 h1:Uu7WiSQ6Yj9DbkdnOe7U4mNKp58y9WDMKDn28/ZlunY= +github.com/bufbuild/protocompile v0.6.0/go.mod h1:YNP35qEYoYGme7QMtz5SBCoN4kL4g12jTtjuzRNdjpE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= @@ -248,6 +249,7 @@ github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c= +github.com/jhump/protoreflect v1.15.1/go.mod h1:jD/2GMKKE6OqX8qTjhADU1e6DShO+gavG9e0Q693nKo= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= From bad163c4c499cc67f7d84a7a3e5974f01aa075b6 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Wed, 31 Jan 2024 14:41:02 +0300 Subject: [PATCH 22/24] terraform resource nil check for SDKv2 Signed-off-by: Erhan Cagirici --- pkg/controller/external_nofork.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/external_nofork.go b/pkg/controller/external_nofork.go index 7e8cfab7..1d753c66 100644 --- a/pkg/controller/external_nofork.go +++ b/pkg/controller/external_nofork.go @@ -146,8 +146,10 @@ func getExtendedParameters(ctx context.Context, tr resource.Terraformed, externa // not all providers may have this attribute // TODO: tags-tags_all implementation is AWS specific. // Consider making this logic independent of provider. - if _, ok := config.TerraformResource.CoreConfigSchema().Attributes["tags_all"]; ok { - params["tags_all"] = params["tags"] + if config.TerraformResource != nil { + if _, ok := config.TerraformResource.CoreConfigSchema().Attributes["tags_all"]; ok { + params["tags_all"] = params["tags"] + } } return params, nil } From b7edc58a70857d73aaed62211a16750e0c5c5012 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Wed, 31 Jan 2024 14:41:40 +0300 Subject: [PATCH 23/24] add unit tests for terraform plugin framework external client Signed-off-by: Erhan Cagirici --- ...xternal_terraform_plugin_framework_test.go | 697 ++++++++++++++++++ 1 file changed, 697 insertions(+) create mode 100644 pkg/controller/external_terraform_plugin_framework_test.go diff --git a/pkg/controller/external_terraform_plugin_framework_test.go b/pkg/controller/external_terraform_plugin_framework_test.go new file mode 100644 index 00000000..136d040c --- /dev/null +++ b/pkg/controller/external_terraform_plugin_framework_test.go @@ -0,0 +1,697 @@ +// SPDX-FileCopyrightText: 2024 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "testing" + + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/hashicorp/terraform-plugin-framework/resource" + rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource/fake" + "github.com/crossplane/upjet/pkg/terraform" +) + +var baseSchema = rschema.Schema{ + Attributes: map[string]rschema.Attribute{ + "name": rschema.StringAttribute{ + Required: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "id": rschema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "map": rschema.MapAttribute{ + Required: true, + ElementType: types.StringType, + }, + "list": rschema.ListAttribute{ + Required: true, + ElementType: types.StringType, + }, + }, +} +var baseResource = &mockTPFResource{ + SchemaMethod: func(ctx context.Context, request resource.SchemaRequest, response *resource.SchemaResponse) { + response.Schema = baseSchema + }, + ReadMethod: func(ctx context.Context, request resource.ReadRequest, response *resource.ReadResponse) { + response.State = tfsdk.State{ + Raw: tftypes.Value{}, + Schema: nil, + } + }, +} +var baseConfig = &config.Resource{ + TerraformPluginFrameworkResource: baseResource, + ExternalName: config.IdentifierFromProvider, + Sensitive: config.Sensitive{AdditionalConnectionDetailsFn: func(attr map[string]any) (map[string][]byte, error) { + return nil, nil + }}, +} + +type testConfiguration struct { + r resource.Resource + cfg *config.Resource + obj xpresource.Managed + params map[string]any + currentStateMap map[string]any + plannedStateMap map[string]any + newStateMap map[string]any + + readErr error + readDiags []*tfprotov5.Diagnostic + + applyErr error + applyDiags []*tfprotov5.Diagnostic + + planErr error + planDiags []*tfprotov5.Diagnostic +} + +func prepareTPFExternalWithTestConfig(testConfig testConfiguration) *terraformPluginFrameworkExternalClient { + testConfig.cfg.TerraformPluginFrameworkResource = testConfig.r + schemaResp := &resource.SchemaResponse{} + testConfig.r.Schema(context.TODO(), resource.SchemaRequest{}, schemaResp) + tfValueType := schemaResp.Schema.Type().TerraformType(context.TODO()) + + currentStateVal, err := protov5DynamicValueFromMap(testConfig.currentStateMap, tfValueType) + if err != nil { + panic("cannot prepare TPF") + } + plannedStateVal, err := protov5DynamicValueFromMap(testConfig.plannedStateMap, tfValueType) + if err != nil { + panic("cannot prepare TPF") + } + newStateAfterApplyVal, err := protov5DynamicValueFromMap(testConfig.newStateMap, tfValueType) + if err != nil { + panic("cannot prepare TPF") + } + return &terraformPluginFrameworkExternalClient{ + ts: terraform.Setup{ + FrameworkProvider: &mockTPFProvider{}, + }, + config: cfg, + logger: logTest, + // metricRecorder: nil, + opTracker: NewAsyncTracker(), + resource: testConfig.r, + server: &mockTPFProviderServer{ + ReadResourceFn: func(ctx context.Context, request *tfprotov5.ReadResourceRequest) (*tfprotov5.ReadResourceResponse, error) { + return &tfprotov5.ReadResourceResponse{ + NewState: currentStateVal, + Diagnostics: testConfig.readDiags, + }, testConfig.readErr + }, + PlanResourceChangeFn: func(ctx context.Context, request *tfprotov5.PlanResourceChangeRequest) (*tfprotov5.PlanResourceChangeResponse, error) { + return &tfprotov5.PlanResourceChangeResponse{ + PlannedState: plannedStateVal, + Diagnostics: testConfig.planDiags, + }, testConfig.planErr + }, + ApplyResourceChangeFn: func(ctx context.Context, request *tfprotov5.ApplyResourceChangeRequest) (*tfprotov5.ApplyResourceChangeResponse, error) { + return &tfprotov5.ApplyResourceChangeResponse{ + NewState: newStateAfterApplyVal, + Diagnostics: testConfig.applyDiags, + }, testConfig.applyErr + }, + }, + params: testConfig.params, + plannedState: plannedStateVal, + resourceSchema: schemaResp.Schema, + resourceValueTerraformType: tfValueType, + } +} + +func TestTPFConnect(t *testing.T) { + type args struct { + setupFn terraform.SetupFn + cfg *config.Resource + ots *OperationTrackerStore + obj xpresource.Managed + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + setupFn: func(_ context.Context, _ client.Client, _ xpresource.Managed) (terraform.Setup, error) { + return terraform.Setup{ + FrameworkProvider: &mockTPFProvider{}, + }, nil + }, + cfg: baseConfig, + obj: obj, + ots: ots, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := NewTerraformPluginFrameworkConnector(nil, tc.args.setupFn, tc.args.cfg, tc.args.ots, WithTerraformPluginFrameworkLogger(logTest)) + _, err := c.Connect(context.TODO(), tc.args.obj) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestTPFObserve(t *testing.T) { + type want struct { + obs managed.ExternalObservation + err error + } + cases := map[string]struct { + testConfiguration + want + }{ + "NotExists": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: nil, + plannedStateMap: map[string]any{ + "name": "example", + }, + params: map[string]any{ + "name": "example", + }, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: false, + ResourceUpToDate: false, + ResourceLateInitialized: false, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + + "UpToDate": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + params: map[string]any{ + "id": "example-id", + "name": "example", + }, + currentStateMap: map[string]any{ + "id": "example-id", + "name": "example", + }, + plannedStateMap: map[string]any{ + "id": "example-id", + "name": "example", + }, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + + "LateInitialize": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: &fake.Terraformed{ + Parameterizable: fake.Parameterizable{ + Parameters: map[string]any{ + "name": "example", + "map": map[string]any{ + "key": "value", + }, + "list": []any{"elem1", "elem2"}, + }, + InitParameters: map[string]any{ + "list": []any{"elem1", "elem2", "elem3"}, + }, + }, + Observable: fake.Observable{ + Observation: map[string]any{}, + }, + }, + params: map[string]any{ + "id": "example-id", + }, + currentStateMap: map[string]any{ + "id": "example-id", + "name": "example2", + }, + plannedStateMap: map[string]any{ + "id": "example-id", + "name": "example2", + }, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + tpfExternal := prepareTPFExternalWithTestConfig(tc.testConfiguration) + observation, err := tpfExternal.Observe(context.TODO(), tc.testConfiguration.obj) + if diff := cmp.Diff(tc.want.obs, observation); diff != "" { + t.Errorf("\n%s\nObserve(...): -want observation, +got observation:\n", diff) + } + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestTPFCreate(t *testing.T) { + type want struct { + err error + } + cases := map[string]struct { + testConfiguration + want + }{ + "Successful": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: nil, + plannedStateMap: map[string]any{ + "name": "example", + }, + params: map[string]any{ + "name": "example", + }, + newStateMap: map[string]any{ + "name": "example", + "id": "example-id", + }, + }, + }, + "EmptyStateAfterCreation": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: nil, + plannedStateMap: map[string]any{ + "name": "example", + }, + params: map[string]any{ + "name": "example", + }, + newStateMap: nil, + }, + want: want{ + err: errors.New("new state is empty after creation"), + }, + }, + "ApplyWithError": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: nil, + plannedStateMap: map[string]any{ + "name": "example", + }, + params: map[string]any{ + "name": "example", + }, + newStateMap: nil, + applyErr: errors.New("foo error"), + }, + want: want{ + err: errors.Wrap(errors.New("foo error"), "cannot create resource"), + }, + }, + "ApplyWithDiags": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: nil, + plannedStateMap: map[string]any{ + "name": "example", + }, + params: map[string]any{ + "name": "example", + }, + newStateMap: nil, + applyDiags: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "foo summary", + Detail: "foo detail", + }, + }, + }, + want: want{ + err: errors.Wrap(errors.New("foo summary: foo detail"), "resource creation call returned error diags"), + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + tpfExternal := prepareTPFExternalWithTestConfig(tc.testConfiguration) + _, err := tpfExternal.Create(context.TODO(), tc.testConfiguration.obj) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestTPFUpdate(t *testing.T) { + type want struct { + err error + } + cases := map[string]struct { + testConfiguration + want + }{ + "Successful": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: map[string]any{ + "name": "example", + "id": "example-id", + }, + plannedStateMap: map[string]any{ + "name": "example-updated", + "id": "example-id", + }, + params: map[string]any{ + "name": "example-updated", + }, + newStateMap: map[string]any{ + "name": "example-updated", + "id": "example-id", + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + tpfExternal := prepareTPFExternalWithTestConfig(tc.testConfiguration) + _, err := tpfExternal.Update(context.TODO(), tc.testConfiguration.obj) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestTPFDelete(t *testing.T) { + + type want struct { + err error + } + cases := map[string]struct { + testConfiguration + want + }{ + "Successful": { + testConfiguration: testConfiguration{ + r: baseResource, + cfg: baseConfig, + obj: obj, + currentStateMap: map[string]any{ + "name": "example", + "id": "example-id", + }, + plannedStateMap: nil, + params: map[string]any{ + "name": "example", + }, + newStateMap: nil, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + tpfExternal := prepareTPFExternalWithTestConfig(tc.testConfiguration) + err := tpfExternal.Delete(context.TODO(), tc.testConfiguration.obj) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +// Mocks + +var _ resource.Resource = &mockTPFResource{} +var _ tfprotov5.ProviderServer = &mockTPFProviderServer{} +var _ provider.Provider = &mockTPFProvider{} + +type mockTPFProviderServer struct { + GetMetadataFn func(ctx context.Context, request *tfprotov5.GetMetadataRequest) (*tfprotov5.GetMetadataResponse, error) + GetProviderSchemaFn func(ctx context.Context, request *tfprotov5.GetProviderSchemaRequest) (*tfprotov5.GetProviderSchemaResponse, error) + PrepareProviderConfigFn func(ctx context.Context, request *tfprotov5.PrepareProviderConfigRequest) (*tfprotov5.PrepareProviderConfigResponse, error) + ConfigureProviderFn func(ctx context.Context, request *tfprotov5.ConfigureProviderRequest) (*tfprotov5.ConfigureProviderResponse, error) + StopProviderFn func(ctx context.Context, request *tfprotov5.StopProviderRequest) (*tfprotov5.StopProviderResponse, error) + ValidateResourceTypeConfigFn func(ctx context.Context, request *tfprotov5.ValidateResourceTypeConfigRequest) (*tfprotov5.ValidateResourceTypeConfigResponse, error) + UpgradeResourceStateFn func(ctx context.Context, request *tfprotov5.UpgradeResourceStateRequest) (*tfprotov5.UpgradeResourceStateResponse, error) + ReadResourceFn func(ctx context.Context, request *tfprotov5.ReadResourceRequest) (*tfprotov5.ReadResourceResponse, error) + PlanResourceChangeFn func(ctx context.Context, request *tfprotov5.PlanResourceChangeRequest) (*tfprotov5.PlanResourceChangeResponse, error) + ApplyResourceChangeFn func(ctx context.Context, request *tfprotov5.ApplyResourceChangeRequest) (*tfprotov5.ApplyResourceChangeResponse, error) + ImportResourceStateFn func(ctx context.Context, request *tfprotov5.ImportResourceStateRequest) (*tfprotov5.ImportResourceStateResponse, error) + ValidateDataSourceConfigFn func(ctx context.Context, request *tfprotov5.ValidateDataSourceConfigRequest) (*tfprotov5.ValidateDataSourceConfigResponse, error) + ReadDataSourceFn func(ctx context.Context, request *tfprotov5.ReadDataSourceRequest) (*tfprotov5.ReadDataSourceResponse, error) +} + +func (m *mockTPFProviderServer) GetMetadata(ctx context.Context, request *tfprotov5.GetMetadataRequest) (*tfprotov5.GetMetadataResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) GetProviderSchema(ctx context.Context, request *tfprotov5.GetProviderSchemaRequest) (*tfprotov5.GetProviderSchemaResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) PrepareProviderConfig(ctx context.Context, request *tfprotov5.PrepareProviderConfigRequest) (*tfprotov5.PrepareProviderConfigResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) ConfigureProvider(ctx context.Context, request *tfprotov5.ConfigureProviderRequest) (*tfprotov5.ConfigureProviderResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) StopProvider(ctx context.Context, request *tfprotov5.StopProviderRequest) (*tfprotov5.StopProviderResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) ValidateResourceTypeConfig(ctx context.Context, request *tfprotov5.ValidateResourceTypeConfigRequest) (*tfprotov5.ValidateResourceTypeConfigResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) UpgradeResourceState(ctx context.Context, request *tfprotov5.UpgradeResourceStateRequest) (*tfprotov5.UpgradeResourceStateResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) ReadResource(ctx context.Context, request *tfprotov5.ReadResourceRequest) (*tfprotov5.ReadResourceResponse, error) { + if m.ReadResourceFn == nil { + return nil, nil + } + return m.ReadResourceFn(ctx, request) +} + +func (m *mockTPFProviderServer) PlanResourceChange(ctx context.Context, request *tfprotov5.PlanResourceChangeRequest) (*tfprotov5.PlanResourceChangeResponse, error) { + if m.PlanResourceChangeFn == nil { + return nil, nil + } + return m.PlanResourceChangeFn(ctx, request) +} + +func (m *mockTPFProviderServer) ApplyResourceChange(ctx context.Context, request *tfprotov5.ApplyResourceChangeRequest) (*tfprotov5.ApplyResourceChangeResponse, error) { + if m.ApplyResourceChangeFn == nil { + return nil, nil + } + return m.ApplyResourceChangeFn(ctx, request) +} + +func (m *mockTPFProviderServer) ImportResourceState(ctx context.Context, request *tfprotov5.ImportResourceStateRequest) (*tfprotov5.ImportResourceStateResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) ValidateDataSourceConfig(ctx context.Context, request *tfprotov5.ValidateDataSourceConfigRequest) (*tfprotov5.ValidateDataSourceConfigResponse, error) { + //TODO implement me + panic("implement me") +} + +func (m *mockTPFProviderServer) ReadDataSource(ctx context.Context, request *tfprotov5.ReadDataSourceRequest) (*tfprotov5.ReadDataSourceResponse, error) { + //TODO implement me + panic("implement me") +} + +type mockTPFProvider struct { + // Provider interface methods + MetadataMethod func(context.Context, provider.MetadataRequest, *provider.MetadataResponse) + ConfigureMethod func(context.Context, provider.ConfigureRequest, *provider.ConfigureResponse) + SchemaMethod func(context.Context, provider.SchemaRequest, *provider.SchemaResponse) + DataSourcesMethod func(context.Context) []func() datasource.DataSource + ResourcesMethod func(context.Context) []func() resource.Resource +} + +// Configure satisfies the provider.Provider interface. +func (p *mockTPFProvider) Configure(ctx context.Context, req provider.ConfigureRequest, resp *provider.ConfigureResponse) { + if p == nil || p.ConfigureMethod == nil { + return + } + + p.ConfigureMethod(ctx, req, resp) +} + +// DataSources satisfies the provider.Provider interface. +func (p *mockTPFProvider) DataSources(ctx context.Context) []func() datasource.DataSource { + if p == nil || p.DataSourcesMethod == nil { + return nil + } + + return p.DataSourcesMethod(ctx) +} + +// Metadata satisfies the provider.Provider interface. +func (p *mockTPFProvider) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) { + if p == nil || p.MetadataMethod == nil { + return + } + + p.MetadataMethod(ctx, req, resp) +} + +// Schema satisfies the provider.Provider interface. +func (p *mockTPFProvider) Schema(ctx context.Context, req provider.SchemaRequest, resp *provider.SchemaResponse) { + if p == nil || p.SchemaMethod == nil { + return + } + + p.SchemaMethod(ctx, req, resp) +} + +// Resources satisfies the provider.Provider interface. +func (p *mockTPFProvider) Resources(ctx context.Context) []func() resource.Resource { + if p == nil || p.ResourcesMethod == nil { + return nil + } + + return p.ResourcesMethod(ctx) +} + +type mockTPFResource struct { + // Resource interface methods + MetadataMethod func(context.Context, resource.MetadataRequest, *resource.MetadataResponse) + SchemaMethod func(context.Context, resource.SchemaRequest, *resource.SchemaResponse) + CreateMethod func(context.Context, resource.CreateRequest, *resource.CreateResponse) + DeleteMethod func(context.Context, resource.DeleteRequest, *resource.DeleteResponse) + ReadMethod func(context.Context, resource.ReadRequest, *resource.ReadResponse) + UpdateMethod func(context.Context, resource.UpdateRequest, *resource.UpdateResponse) +} + +// Metadata satisfies the resource.Resource interface. +func (r *mockTPFResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + if r.MetadataMethod == nil { + return + } + + r.MetadataMethod(ctx, req, resp) +} + +// Schema satisfies the resource.Resource interface. +func (r *mockTPFResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { + if r.SchemaMethod == nil { + return + } + + r.SchemaMethod(ctx, req, resp) +} + +// Create satisfies the resource.Resource interface. +func (r *mockTPFResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + if r.CreateMethod == nil { + return + } + + r.CreateMethod(ctx, req, resp) +} + +// Delete satisfies the resource.Resource interface. +func (r *mockTPFResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + if r.DeleteMethod == nil { + return + } + + r.DeleteMethod(ctx, req, resp) +} + +// Read satisfies the resource.Resource interface. +func (r *mockTPFResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + if r.ReadMethod == nil { + return + } + + r.ReadMethod(ctx, req, resp) +} + +// Update satisfies the resource.Resource interface. +func (r *mockTPFResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + if r.UpdateMethod == nil { + return + } + + r.UpdateMethod(ctx, req, resp) +} From 5dff491aeac2a940acea2ffb3fb333e73ace7b32 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Wed, 31 Jan 2024 14:58:20 +0300 Subject: [PATCH 24/24] refactor unit tests Signed-off-by: Erhan Cagirici --- ...xternal_terraform_plugin_framework_test.go | 161 ++++++++++-------- 1 file changed, 93 insertions(+), 68 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework_test.go b/pkg/controller/external_terraform_plugin_framework_test.go index 136d040c..4465a2bf 100644 --- a/pkg/controller/external_terraform_plugin_framework_test.go +++ b/pkg/controller/external_terraform_plugin_framework_test.go @@ -30,47 +30,72 @@ import ( "github.com/crossplane/upjet/pkg/terraform" ) -var baseSchema = rschema.Schema{ - Attributes: map[string]rschema.Attribute{ - "name": rschema.StringAttribute{ - Required: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), +func newBaseObject() *fake.Terraformed { + return &fake.Terraformed{ + Parameterizable: fake.Parameterizable{ + Parameters: map[string]any{ + "name": "example", + "map": map[string]any{ + "key": "value", + }, + "list": []any{"elem1", "elem2"}, }, }, - "id": rschema.StringAttribute{ - Computed: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), + Observable: fake.Observable{ + Observation: map[string]any{}, + }, + } +} + +func newBaseSchema() rschema.Schema { + return rschema.Schema{ + Attributes: map[string]rschema.Attribute{ + "name": rschema.StringAttribute{ + Required: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "id": rschema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "map": rschema.MapAttribute{ + Required: true, + ElementType: types.StringType, + }, + "list": rschema.ListAttribute{ + Required: true, + ElementType: types.StringType, }, }, - "map": rschema.MapAttribute{ - Required: true, - ElementType: types.StringType, + } +} + +func newMockBaseTPFResource() *mockTPFResource { + return &mockTPFResource{ + SchemaMethod: func(ctx context.Context, request resource.SchemaRequest, response *resource.SchemaResponse) { + response.Schema = newBaseSchema() }, - "list": rschema.ListAttribute{ - Required: true, - ElementType: types.StringType, + ReadMethod: func(ctx context.Context, request resource.ReadRequest, response *resource.ReadResponse) { + response.State = tfsdk.State{ + Raw: tftypes.Value{}, + Schema: nil, + } }, - }, -} -var baseResource = &mockTPFResource{ - SchemaMethod: func(ctx context.Context, request resource.SchemaRequest, response *resource.SchemaResponse) { - response.Schema = baseSchema - }, - ReadMethod: func(ctx context.Context, request resource.ReadRequest, response *resource.ReadResponse) { - response.State = tfsdk.State{ - Raw: tftypes.Value{}, - Schema: nil, - } - }, -} -var baseConfig = &config.Resource{ - TerraformPluginFrameworkResource: baseResource, - ExternalName: config.IdentifierFromProvider, - Sensitive: config.Sensitive{AdditionalConnectionDetailsFn: func(attr map[string]any) (map[string][]byte, error) { - return nil, nil - }}, + } +} + +func newBaseUpjetConfig() *config.Resource { + return &config.Resource{ + TerraformPluginFrameworkResource: newMockBaseTPFResource(), + ExternalName: config.IdentifierFromProvider, + Sensitive: config.Sensitive{AdditionalConnectionDetailsFn: func(attr map[string]any) (map[string][]byte, error) { + return nil, nil + }}, + } } type testConfiguration struct { @@ -167,8 +192,8 @@ func TestTPFConnect(t *testing.T) { FrameworkProvider: &mockTPFProvider{}, }, nil }, - cfg: baseConfig, - obj: obj, + cfg: newBaseUpjetConfig(), + obj: newBaseObject(), ots: ots, }, }, @@ -196,8 +221,8 @@ func TestTPFObserve(t *testing.T) { }{ "NotExists": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: obj, currentStateMap: nil, plannedStateMap: map[string]any{ @@ -220,9 +245,9 @@ func TestTPFObserve(t *testing.T) { "UpToDate": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, - obj: obj, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), + obj: newBaseObject(), params: map[string]any{ "id": "example-id", "name": "example", @@ -249,8 +274,8 @@ func TestTPFObserve(t *testing.T) { "LateInitialize": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: &fake.Terraformed{ Parameterizable: fake.Parameterizable{ Parameters: map[string]any{ @@ -316,8 +341,8 @@ func TestTPFCreate(t *testing.T) { }{ "Successful": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: obj, currentStateMap: nil, plannedStateMap: map[string]any{ @@ -334,8 +359,8 @@ func TestTPFCreate(t *testing.T) { }, "EmptyStateAfterCreation": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: obj, currentStateMap: nil, plannedStateMap: map[string]any{ @@ -352,8 +377,8 @@ func TestTPFCreate(t *testing.T) { }, "ApplyWithError": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: obj, currentStateMap: nil, plannedStateMap: map[string]any{ @@ -371,8 +396,8 @@ func TestTPFCreate(t *testing.T) { }, "ApplyWithDiags": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), obj: obj, currentStateMap: nil, plannedStateMap: map[string]any{ @@ -416,9 +441,9 @@ func TestTPFUpdate(t *testing.T) { }{ "Successful": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, - obj: obj, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), + obj: newBaseObject(), currentStateMap: map[string]any{ "name": "example", "id": "example-id", @@ -459,9 +484,9 @@ func TestTPFDelete(t *testing.T) { }{ "Successful": { testConfiguration: testConfiguration{ - r: baseResource, - cfg: baseConfig, - obj: obj, + r: newMockBaseTPFResource(), + cfg: newBaseUpjetConfig(), + obj: newBaseObject(), currentStateMap: map[string]any{ "name": "example", "id": "example-id", @@ -508,37 +533,37 @@ type mockTPFProviderServer struct { } func (m *mockTPFProviderServer) GetMetadata(ctx context.Context, request *tfprotov5.GetMetadataRequest) (*tfprotov5.GetMetadataResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) GetProviderSchema(ctx context.Context, request *tfprotov5.GetProviderSchemaRequest) (*tfprotov5.GetProviderSchemaResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) PrepareProviderConfig(ctx context.Context, request *tfprotov5.PrepareProviderConfigRequest) (*tfprotov5.PrepareProviderConfigResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) ConfigureProvider(ctx context.Context, request *tfprotov5.ConfigureProviderRequest) (*tfprotov5.ConfigureProviderResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) StopProvider(ctx context.Context, request *tfprotov5.StopProviderRequest) (*tfprotov5.StopProviderResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) ValidateResourceTypeConfig(ctx context.Context, request *tfprotov5.ValidateResourceTypeConfigRequest) (*tfprotov5.ValidateResourceTypeConfigResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) UpgradeResourceState(ctx context.Context, request *tfprotov5.UpgradeResourceStateRequest) (*tfprotov5.UpgradeResourceStateResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } @@ -564,17 +589,17 @@ func (m *mockTPFProviderServer) ApplyResourceChange(ctx context.Context, request } func (m *mockTPFProviderServer) ImportResourceState(ctx context.Context, request *tfprotov5.ImportResourceStateRequest) (*tfprotov5.ImportResourceStateResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) ValidateDataSourceConfig(ctx context.Context, request *tfprotov5.ValidateDataSourceConfigRequest) (*tfprotov5.ValidateDataSourceConfigResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") } func (m *mockTPFProviderServer) ReadDataSource(ctx context.Context, request *tfprotov5.ReadDataSourceRequest) (*tfprotov5.ReadDataSourceResponse, error) { - //TODO implement me + // TODO implement me panic("implement me") }