diff --git a/.golangci.yml b/.golangci.yml index a6cc3f56..fb24e160 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -44,8 +44,7 @@ linters-settings: sections: - standard - default - - prefix(github.com/crossplane/crossplane-runtime) - - prefix(github.com/crossplane/crossplane) + - prefix(github.com/crossplane/upjet) - blank - dot diff --git a/cmd/scraper/main.go b/cmd/scraper/main.go index 077c3170..692697e1 100644 --- a/cmd/scraper/main.go +++ b/cmd/scraper/main.go @@ -8,8 +8,9 @@ import ( "os" "path/filepath" - "github.com/crossplane/upjet/pkg/registry" "gopkg.in/alecthomas/kingpin.v2" + + "github.com/crossplane/upjet/pkg/registry" ) func main() { diff --git a/go.mod b/go.mod index f524c161..89b10cbf 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/fatih/camelcase v1.0.0 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.6.0 + github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 github.com/hashicorp/hcl/v2 v2.14.1 github.com/hashicorp/terraform-json v0.14.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.24.0 @@ -27,6 +28,7 @@ require ( github.com/tmccombs/hcl2json v0.3.3 github.com/yuin/goldmark v1.4.13 github.com/zclconf/go-cty v1.11.0 + github.com/zclconf/go-cty-yaml v1.0.3 golang.org/x/net v0.15.0 gopkg.in/alecthomas/kingpin.v2 v2.2.6 gopkg.in/yaml.v2 v2.4.0 @@ -57,6 +59,7 @@ require ( github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-logr/logr v1.2.4 // indirect + github.com/go-logr/zapr v1.2.4 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect @@ -70,7 +73,6 @@ require ( github.com/google/uuid v1.3.0 // indirect github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 // indirect github.com/hashicorp/go-hclog v1.2.1 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect @@ -103,6 +105,8 @@ require ( github.com/vmihailenco/tagparser v0.1.1 // indirect github.com/xlab/treeprint v1.2.0 // indirect go.starlark.net v0.0.0-20230525235612-a134d8f9ddca // indirect + go.uber.org/multierr v1.11.0 // indirect + go.uber.org/zap v1.26.0 // indirect golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect golang.org/x/mod v0.12.0 // indirect golang.org/x/oauth2 v0.10.0 // indirect diff --git a/go.sum b/go.sum index c8cf1cdf..0e81176e 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,7 @@ github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/ github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw= github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo= +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/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -108,6 +109,7 @@ github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/zapr v1.2.4 h1:QHVo+6stLbfJmYGkQ7uGHUCu5hnAFAj6mDe6Ea0SeOo= +github.com/go-logr/zapr v1.2.4/go.mod h1:FyHWQIzQORZ0QVE1BtVHv3cKtNLuXsbNLtpuhNapBOA= github.com/go-openapi/jsonpointer v0.19.6 h1:eCs3fxoIi3Wh6vtgmLTOjdhSpiqphQ+DaPn38N2ZdrE= github.com/go-openapi/jsonpointer v0.19.6/go.mod h1:osyAmYz/mB/C3I+WsTTSgw1ONzaLJoLCyoi6/zppojs= github.com/go-openapi/jsonreference v0.20.2 h1:3sVjiK66+uXK/6oQ8xgcRKcFgQ5KXa2KvnJRumpMGbE= @@ -362,6 +364,8 @@ github.com/zclconf/go-cty v1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uU github.com/zclconf/go-cty v1.11.0 h1:726SxLdi2SDnjY+BStqB9J1hNp4+2WlzyXLuimibIe0= github.com/zclconf/go-cty v1.11.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= +github.com/zclconf/go-cty-yaml v1.0.3 h1:og/eOQ7lvA/WWhHGFETVWNduJM7Rjsv2RRpx1sdFMLc= +github.com/zclconf/go-cty-yaml v1.0.3/go.mod h1:9YLUH4g7lOhVWqUbctnVlZ5KLpg7JAprQNgxSZ1Gyxs= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= @@ -370,9 +374,15 @@ go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk= go.starlark.net v0.0.0-20230525235612-a134d8f9ddca h1:VdD38733bfYv5tUZwEIskMM93VanwNIi5bIKnDrJdEY= go.starlark.net v0.0.0-20230525235612-a134d8f9ddca/go.mod h1:jxU+3+j+71eXOW14274+SmmuW82qJzl6iZSeqEtTGds= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= +go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= +go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= +go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -597,6 +607,7 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.13.0 h1:Iey4qkscZuv0VvIt8E0neZjtPVQFSc870HQ448QgEmQ= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/config/common.go b/pkg/config/common.go index ac21fd66..ee291e39 100644 --- a/pkg/config/common.go +++ b/pkg/config/common.go @@ -7,9 +7,10 @@ package config import ( "strings" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/crossplane/upjet/pkg/registry" tjname "github.com/crossplane/upjet/pkg/types/name" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) const ( @@ -76,16 +77,17 @@ 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: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: name, + TerraformResource: terraformSchema, + MetaResource: terraformRegistry, + ShortGroup: group, + Kind: kind, + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: make(map[string]*SchemaElementOption), } for _, f := range opts { f(r) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index db3fe5e5..3ad62cab 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -7,10 +7,11 @@ package config import ( "testing" - "github.com/crossplane/upjet/pkg/registry" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/crossplane/upjet/pkg/registry" ) func TestDefaultResource(t *testing.T) { @@ -32,14 +33,15 @@ func TestDefaultResource(t *testing.T) { name: "aws_ec2_instance", }, want: &Resource{ - Name: "aws_ec2_instance", - ShortGroup: "ec2", - Kind: "Instance", - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: "aws_ec2_instance", + ShortGroup: "ec2", + Kind: "Instance", + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: SchemaElementOptions{}, }, }, "TwoSectionsName": { @@ -48,14 +50,15 @@ func TestDefaultResource(t *testing.T) { name: "aws_instance", }, want: &Resource{ - Name: "aws_instance", - ShortGroup: "aws", - Kind: "Instance", - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: "aws_instance", + ShortGroup: "aws", + Kind: "Instance", + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: SchemaElementOptions{}, }, }, "NameWithPrefixAcronym": { @@ -64,14 +67,15 @@ func TestDefaultResource(t *testing.T) { name: "aws_db_sql_server", }, want: &Resource{ - Name: "aws_db_sql_server", - ShortGroup: "db", - Kind: "SQLServer", - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: "aws_db_sql_server", + ShortGroup: "db", + Kind: "SQLServer", + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: SchemaElementOptions{}, }, }, "NameWithSuffixAcronym": { @@ -80,14 +84,15 @@ func TestDefaultResource(t *testing.T) { name: "aws_db_server_id", }, want: &Resource{ - Name: "aws_db_server_id", - ShortGroup: "db", - Kind: "ServerID", - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: "aws_db_server_id", + ShortGroup: "db", + Kind: "ServerID", + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: SchemaElementOptions{}, }, }, "NameWithMultipleAcronyms": { @@ -96,14 +101,15 @@ func TestDefaultResource(t *testing.T) { name: "aws_db_sql_server_id", }, want: &Resource{ - Name: "aws_db_sql_server_id", - ShortGroup: "db", - Kind: "SQLServerID", - Version: "v1alpha1", - ExternalName: NameAsIdentifier, - References: map[string]Reference{}, - Sensitive: NopSensitive, - UseAsync: true, + Name: "aws_db_sql_server_id", + ShortGroup: "db", + Kind: "SQLServerID", + Version: "v1alpha1", + ExternalName: NameAsIdentifier, + References: map[string]Reference{}, + Sensitive: NopSensitive, + UseAsync: true, + SchemaElementOptions: SchemaElementOptions{}, }, }, } @@ -113,6 +119,7 @@ func TestDefaultResource(t *testing.T) { cmpopts.IgnoreFields(Sensitive{}, "fieldPaths", "AdditionalConnectionDetailsFn"), cmpopts.IgnoreFields(LateInitializer{}, "ignoredCanonicalFieldPaths"), cmpopts.IgnoreFields(ExternalName{}, "SetIdentifierArgumentFn", "GetExternalNameFn", "GetIDFn"), + cmpopts.IgnoreFields(Resource{}, "useNoForkClient"), } for name, tc := range cases { diff --git a/pkg/config/externalname_test.go b/pkg/config/externalname_test.go index c9f39b2a..f090a0f6 100644 --- a/pkg/config/externalname_test.go +++ b/pkg/config/externalname_test.go @@ -8,10 +8,9 @@ import ( "context" "testing" - "github.com/google/go-cmp/cmp" - "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" ) func TestGetExternalNameFromTemplated(t *testing.T) { diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 8c6e1328..29ae740a 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -8,10 +8,12 @@ import ( "fmt" "regexp" - "github.com/crossplane/upjet/pkg/registry" - conversiontfjson "github.com/crossplane/upjet/pkg/types/conversion/tfjson" tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/registry" + conversiontfjson "github.com/crossplane/upjet/pkg/types/conversion/tfjson" ) // ResourceConfiguratorFn is a function that implements the ResourceConfigurator @@ -106,16 +108,29 @@ type Provider struct { skippedResourceNames []string // IncludeList is a list of regex for the Terraform resources to be - // included. For example, to include "aws_shield_protection_group" into + // included and reconciled via 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. 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). + // 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 + // 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 *schema.Provider + // refInjectors is an ordered list of `ReferenceInjector`s for // injecting references across this Provider's resources. refInjectors []ReferenceInjector @@ -155,6 +170,20 @@ func WithIncludeList(l []string) ProviderOption { } } +// WithNoForkIncludeList configures IncludeList for this Provider. +func WithNoForkIncludeList(l []string) ProviderOption { + return func(p *Provider) { + p.NoForkIncludeList = l + } +} + +// WithTerraformProvider configures the TerraformProvider for this Provider. +func WithTerraformProvider(tp *schema.Provider) ProviderOption { + return func(p *Provider) { + p.TerraformProvider = tp + } +} + // WithSkipList configures SkipList for this Provider. func WithSkipList(l []string) ProviderOption { return func(p *Provider) { @@ -215,7 +244,6 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt rs = v.ResourceSchemas break } - resourceMap := conversiontfjson.GetV2ResourceMap(rs) providerMetadata, err := registry.NewProviderMetadataFromFile(metadata) if err != nil { @@ -246,11 +274,27 @@ func NewProvider(schema []byte, prefix string, modulePath string, metadata []byt // There are resources with no schema, that we will address later. fmt.Printf("Skipping resource %s because it has no schema\n", name) } - if len(terraformResource.Schema) == 0 || matches(name, p.SkipList) || !matches(name, p.IncludeList) { + // 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) { p.skippedResourceNames = append(p.skippedResourceNames, name) continue } + if isNoFork { + if p.TerraformProvider == 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)) + } + terraformResource = p.TerraformProvider.ResourcesMap[name] + // TODO: we will need to bump the terraform-plugin-sdk dependency to handle + // schema.Resource.SchemaFunc + if terraformResource.Schema == nil { + p.skippedResourceNames = append(p.skippedResourceNames, name) + continue + } + } p.Resources[name] = DefaultResource(name, terraformResource, providerMetadata.Resources[name], p.DefaultResourceOptions...) + p.Resources[name].useNoForkClient = isNoFork } 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 7e5aa036..876ec6f5 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -9,18 +9,19 @@ import ( "fmt" "time" - "github.com/crossplane/upjet/pkg/registry" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" - "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/upjet/pkg/registry" ) // SetIdentifierArgumentsFn sets the name of the resource in Terraform attributes map, @@ -293,6 +294,8 @@ type Resource struct { // databases. UseAsync bool + // InitializerFns specifies the initializer functions to be used + // for this Resource. InitializerFns []NewInitializerFn // OperationTimeouts allows configuring resource operation timeouts. @@ -318,4 +321,65 @@ type Resource struct { // the plural name of the generated CRD. Overriding this sets both the // path and the plural name for the generated CRD. Path string + + // SchemaElementOptions is a map from the schema element paths to + // SchemaElementOption for configuring options for schema elements. + SchemaElementOptions SchemaElementOptions + + // TerraformConfigurationInjector allows a managed resource to inject + // configuration values in the Terraform configuration map obtained by + // deserializing its `spec.forProvider` value. Managed resources can + // use this resource configuration option to inject Terraform + // configuration parameters into their deserialized configuration maps, + // if the deserialization skips certain fields. + TerraformConfigurationInjector ConfigurationInjector + + // TerraformCustomDiff allows a resource.Terraformed to customize how its + // Terraform InstanceDiff is computed during reconciliation. + TerraformCustomDiff CustomDiff + + // useNoForkClient indicates that a no-fork external client should + // be generated instead of the Terraform CLI-forking client. + useNoForkClient bool +} + +func (r *Resource) ShouldUseNoForkClient() bool { + return r.useNoForkClient +} + +// 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. +type CustomDiff func(diff *terraform.InstanceDiff, state *terraform.InstanceState, config *terraform.ResourceConfig) (*terraform.InstanceDiff, error) + +// ConfigurationInjector is a function that injects Terraform configuration +// values from the specified managed resource into the specified configuration +// map. jsonMap is the map obtained by converting the `spec.forProvider` using +// the JSON tags and tfMap is obtained by using the TF tags. +type ConfigurationInjector func(jsonMap map[string]any, tfMap map[string]any) + +// SchemaElementOptions represents schema element options for the +// schema elements of a Resource. +type SchemaElementOptions map[string]*SchemaElementOption + +// SetAddToObservation sets the AddToObservation for the specified key. +func (m SchemaElementOptions) SetAddToObservation(el string) { + if m[el] == nil { + m[el] = &SchemaElementOption{} + } + m[el].AddToObservation = true +} + +// AddToObservation returns true if the schema element at the specified path +// should be added to the CRD type's Observation type. +func (m SchemaElementOptions) AddToObservation(el string) bool { + return m[el] != nil && m[el].AddToObservation +} + +// SchemaElementOption represents configuration options on a schema element. +type SchemaElementOption struct { + // AddToObservation is set to true if the field represented by + // a schema element is to be added to the generated CRD type's + // Observation type. + AddToObservation bool } diff --git a/pkg/config/resource_test.go b/pkg/config/resource_test.go index 4663781b..a94366c7 100644 --- a/pkg/config/resource_test.go +++ b/pkg/config/resource_test.go @@ -9,14 +9,13 @@ import ( "fmt" "testing" - "github.com/google/go-cmp/cmp" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/resource/fake" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( diff --git a/pkg/controller/api.go b/pkg/controller/api.go index f5a5c5f5..f8efda9b 100644 --- a/pkg/controller/api.go +++ b/pkg/controller/api.go @@ -7,9 +7,8 @@ package controller import ( "context" - "github.com/crossplane/upjet/pkg/controller/handler" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/terraform" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -17,8 +16,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ctrl "sigs.k8s.io/controller-runtime/pkg/manager" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/upjet/pkg/controller/handler" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/terraform" ) const ( @@ -68,6 +68,17 @@ func WithEventHandler(e *handler.EventHandler) APICallbacksOption { } } +// WithStatusUpdates sets whether the LastAsyncOperation status condition +// is enabled. If set to false, APICallbacks will not use the +// LastAsyncOperation status condition for reporting ongoing async +// operations or errors. Error conditions will still be reported +// as usual in the `Synced` status condition. +func WithStatusUpdates(enabled bool) APICallbacksOption { + return func(callbacks *APICallbacks) { + callbacks.enableStatusUpdates = enabled + } +} + // NewAPICallbacks returns a new APICallbacks. func NewAPICallbacks(m ctrl.Manager, of xpresource.ManagedKind, opts ...APICallbacksOption) *APICallbacks { nt := func() resource.Terraformed { @@ -76,6 +87,9 @@ func NewAPICallbacks(m ctrl.Manager, of xpresource.ManagedKind, opts ...APICallb cb := &APICallbacks{ kube: m.GetClient(), newTerraformed: nt, + // the default behavior is to use the LastAsyncOperation + // status condition for backwards compatibility. + enableStatusUpdates: true, } for _, o := range opts { o(cb) @@ -87,31 +101,41 @@ func NewAPICallbacks(m ctrl.Manager, of xpresource.ManagedKind, opts ...APICallb type APICallbacks struct { eventHandler *handler.EventHandler - kube client.Client - newTerraformed func() resource.Terraformed + kube client.Client + newTerraformed func() resource.Terraformed + enableStatusUpdates bool } -func (ac *APICallbacks) callbackFn(name, op string, requeue bool) terraform.CallbackFn { +func (ac *APICallbacks) callbackFn(name, op string) terraform.CallbackFn { return func(err error, ctx context.Context) error { nn := types.NamespacedName{Name: name} tr := ac.newTerraformed() if kErr := ac.kube.Get(ctx, nn, tr); kErr != nil { return errors.Wrapf(kErr, errGetFmt, tr.GetObjectKind().GroupVersionKind().String(), name, op) } + // For the no-fork architecture, we will need to be able to report + // reconciliation errors. The proper place is the `Synced` + // status condition but we need changes in the managed reconciler + // to do so. So we keep the `LastAsyncOperation` condition. + // TODO: move this to the `Synced` condition. tr.SetConditions(resource.LastAsyncOperationCondition(err)) - tr.SetConditions(resource.AsyncOperationFinishedCondition()) + if ac.enableStatusUpdates { + tr.SetConditions(resource.AsyncOperationFinishedCondition()) + } uErr := errors.Wrapf(ac.kube.Status().Update(ctx, tr), errUpdateStatusFmt, tr.GetObjectKind().GroupVersionKind().String(), name, op) - if ac.eventHandler != nil && requeue { + if ac.eventHandler != nil { + rateLimiter := handler.NoRateLimiter switch { case err != nil: - // TODO: use the errors.Join from - // github.com/crossplane/crossplane-runtime. - if ok := ac.eventHandler.RequestReconcile(rateLimiterCallback, name, nil); !ok { - return errors.Errorf(errReconcileRequestFmt, tr.GetObjectKind().GroupVersionKind().String(), name, op) - } + rateLimiter = rateLimiterCallback default: ac.eventHandler.Forget(rateLimiterCallback, name) } + // TODO: use the errors.Join from + // github.com/crossplane/crossplane-runtime. + if ok := ac.eventHandler.RequestReconcile(rateLimiter, name, nil); !ok { + return errors.Errorf(errReconcileRequestFmt, tr.GetObjectKind().GroupVersionKind().String(), name, op) + } } return uErr } @@ -119,28 +143,26 @@ func (ac *APICallbacks) callbackFn(name, op string, requeue bool) terraform.Call // Create makes sure the error is saved in async operation condition. func (ac *APICallbacks) Create(name string) terraform.CallbackFn { - return func(err error, ctx context.Context) error { - // requeue is set to true although the managed reconciler already - // requeues with exponential back-off during the creation phase - // because the upjet external client returns ResourceExists & - // ResourceUpToDate both set to true, if an async operation is - // in-progress immediately following a Create call. This will - // delay a reobservation of the resource (while being created) - // for the poll period. - return ac.callbackFn(name, "create", true)(err, ctx) - } + // request will be requeued although the managed reconciler already + // requeues with exponential back-off during the creation phase + // because the upjet external client returns ResourceExists & + // ResourceUpToDate both set to true, if an async operation is + // in-progress immediately following a Create call. This will + // delay a reobservation of the resource (while being created) + // for the poll period. + return ac.callbackFn(name, "create") } // Update makes sure the error is saved in async operation condition. func (ac *APICallbacks) Update(name string) terraform.CallbackFn { - return func(err error, ctx context.Context) error { - return ac.callbackFn(name, "update", true)(err, ctx) - } + return ac.callbackFn(name, "update") } // Destroy makes sure the error is saved in async operation condition. func (ac *APICallbacks) Destroy(name string) terraform.CallbackFn { - // requeue is set to false because the managed reconciler already requeues - // with exponential back-off during the deletion phase. - return ac.callbackFn(name, "destroy", false) + // request will be requeued although the managed reconciler requeues + // with exponential back-off during the deletion phase because + // during the async deletion operation, external client's + // observe just returns success to the managed reconciler. + return ac.callbackFn(name, "destroy") } diff --git a/pkg/controller/api_test.go b/pkg/controller/api_test.go index b42e662c..7bd60b3c 100644 --- a/pkg/controller/api_test.go +++ b/pkg/controller/api_test.go @@ -8,17 +8,17 @@ import ( "context" "testing" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/resource/fake" - tjerrors "github.com/crossplane/upjet/pkg/terraform/errors" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "sigs.k8s.io/controller-runtime/pkg/client" ctrl "sigs.k8s.io/controller-runtime/pkg/manager" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" - xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/resource/fake" + tjerrors "github.com/crossplane/upjet/pkg/terraform/errors" ) func TestAPICallbacksCreate(t *testing.T) { diff --git a/pkg/controller/external.go b/pkg/controller/external.go index 62580935..4eff3812 100644 --- a/pkg/controller/external.go +++ b/pkg/controller/external.go @@ -8,6 +8,14 @@ import ( "context" "time" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/util/sets" + "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" @@ -15,14 +23,6 @@ import ( "github.com/crossplane/upjet/pkg/resource/json" "github.com/crossplane/upjet/pkg/terraform" tferrors "github.com/crossplane/upjet/pkg/terraform/errors" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/util/sets" - "sigs.k8s.io/controller-runtime/pkg/client" - - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" ) const ( diff --git a/pkg/controller/external_async_nofork.go b/pkg/controller/external_async_nofork.go new file mode 100644 index 00000000..6829f5d1 --- /dev/null +++ b/pkg/controller/external_async_nofork.go @@ -0,0 +1,200 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "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/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" +) + +var defaultAsyncTimeout = 1 * time.Hour + +type NoForkAsyncConnector struct { + *NoForkConnector + callback CallbackProvider + eventHandler *handler.EventHandler +} + +type NoForkAsyncOption func(connector *NoForkAsyncConnector) + +func NewNoForkAsyncConnector(kube client.Client, ots *OperationTrackerStore, sf terraform.SetupFn, cfg *config.Resource, opts ...NoForkAsyncOption) *NoForkAsyncConnector { + nfac := &NoForkAsyncConnector{ + NoForkConnector: NewNoForkConnector(kube, sf, cfg, ots), + } + for _, f := range opts { + f(nfac) + } + return nfac +} + +func (c *NoForkAsyncConnector) Connect(ctx context.Context, mg xpresource.Managed) (managed.ExternalClient, error) { + ec, err := c.NoForkConnector.Connect(ctx, mg) + if err != nil { + return nil, errors.Wrap(err, "cannot initialize the no-fork async external client") + } + + return &noForkAsyncExternal{ + noForkExternal: ec.(*noForkExternal), + callback: c.callback, + eventHandler: c.eventHandler, + }, nil +} + +// WithNoForkAsyncConnectorEventHandler configures the EventHandler so that +// the no-fork external clients can requeue reconciliation requests. +func WithNoForkAsyncConnectorEventHandler(e *handler.EventHandler) NoForkAsyncOption { + return func(c *NoForkAsyncConnector) { + c.eventHandler = e + } +} + +// WithNoForkAsyncCallbackProvider configures the controller to use async variant of the functions +// of the Terraform client and run given callbacks once those operations are +// completed. +func WithNoForkAsyncCallbackProvider(ac CallbackProvider) NoForkAsyncOption { + return func(c *NoForkAsyncConnector) { + c.callback = ac + } +} + +// WithNoForkAsyncLogger configures a logger for the NoForkAsyncConnector. +func WithNoForkAsyncLogger(l logging.Logger) NoForkAsyncOption { + return func(c *NoForkAsyncConnector) { + c.logger = l + } +} + +// WithNoForkAsyncMetricRecorder configures a metrics.MetricRecorder for the +// NoForkAsyncConnector. +func WithNoForkAsyncMetricRecorder(r *metrics.MetricRecorder) NoForkAsyncOption { + return func(c *NoForkAsyncConnector) { + c.metricRecorder = r + } +} + +// WithNoForkAsyncManagementPolicies configures whether the client should +// handle management policies. +func WithNoForkAsyncManagementPolicies(isManagementPoliciesEnabled bool) NoForkAsyncOption { + return func(c *NoForkAsyncConnector) { + c.isManagementPoliciesEnabled = isManagementPoliciesEnabled + } +} + +type noForkAsyncExternal struct { + *noForkExternal + callback CallbackProvider + eventHandler *handler.EventHandler +} + +type CallbackFn func(error, context.Context) error + +func (n *noForkAsyncExternal) 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.noForkExternal.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 *noForkAsyncExternal) 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.GetTfID()) + _, err := n.noForkExternal.Create(ctx, mg) + err = tferrors.NewAsyncCreateFailed(err) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async create ended.", "error", err, "tfID", n.opTracker.GetTfID()) + + 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 *noForkAsyncExternal) 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.GetTfID()) + _, err := n.noForkExternal.Update(ctx, mg) + err = tferrors.NewAsyncUpdateFailed(err) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async update ended.", "error", err, "tfID", n.opTracker.GetTfID()) + + 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 *noForkAsyncExternal) 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.GetTfID()) + 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.GetTfID()) + err := tferrors.NewAsyncDeleteFailed(n.noForkExternal.Delete(ctx, mg)) + n.opTracker.LastOperation.SetError(err) + n.opTracker.logger.Debug("Async delete ended.", "error", err, "tfID", n.opTracker.GetTfID()) + + 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/controller/external_async_nofork_test.go b/pkg/controller/external_async_nofork_test.go new file mode 100644 index 00000000..7acd0ae4 --- /dev/null +++ b/pkg/controller/external_async_nofork_test.go @@ -0,0 +1,336 @@ +// SPDX-FileCopyrightText: 2023 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-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + tf "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "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 ( + cfgAsync = &config.Resource{ + TerraformResource: &schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Create: &timeout, + Read: &timeout, + Update: &timeout, + Delete: &timeout, + }, + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + "id": { + Type: schema.TypeString, + Computed: true, + Required: false, + }, + "map": { + Type: schema.TypeMap, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "list": { + Type: schema.TypeList, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + ExternalName: config.IdentifierFromProvider, + Sensitive: config.Sensitive{AdditionalConnectionDetailsFn: func(attr map[string]any) (map[string][]byte, error) { + return nil, nil + }}, + } + objAsync = &fake.Terraformed{ + Parameterizable: fake.Parameterizable{ + Parameters: map[string]any{ + "name": "example", + "map": map[string]any{ + "key": "value", + }, + "list": []any{"elem1", "elem2"}, + }, + }, + Observable: fake.Observable{ + Observation: map[string]any{}, + }, + } +) + +func prepareNoForkAsyncExternal(r Resource, cfg *config.Resource, fns CallbackFns) *noForkAsyncExternal { + schemaBlock := cfg.TerraformResource.CoreConfigSchema() + rawConfig, err := schema.JSONMapToStateValue(map[string]any{"name": "example"}, schemaBlock) + if err != nil { + panic(err) + } + return &noForkAsyncExternal{ + noForkExternal: &noForkExternal{ + ts: terraform.Setup{}, + resourceSchema: r, + config: cfg, + params: map[string]any{ + "name": "example", + }, + rawConfig: rawConfig, + logger: logTest, + opTracker: NewAsyncTracker(), + }, + callback: fns, + } +} + +func TestAsyncNoForkConnect(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{}, nil + }, + cfg: cfgAsync, + obj: objAsync, + ots: ots, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := NewNoForkAsyncConnector(nil, tc.args.ots, tc.args.setupFn, tc.args.cfg, WithNoForkAsyncLogger(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 TestAsyncNoForkObserve(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + } + type want struct { + obs managed.ExternalObservation + err error + } + cases := map[string]struct { + args + want + }{ + "NotExists": { + args: args{ + r: mockResource{ + RefreshWithoutUpgradeFn: func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return nil, nil + }, + }, + cfg: cfgAsync, + obj: objAsync, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: false, + ResourceUpToDate: false, + ResourceLateInitialized: false, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + "UpToDate": { + args: args{ + r: mockResource{ + RefreshWithoutUpgradeFn: func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id", Attributes: map[string]string{"name": "example"}}, nil + }, + }, + cfg: cfgAsync, + obj: objAsync, + }, + 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) { + noForkAsyncExternal := prepareNoForkAsyncExternal(tc.args.r, tc.args.cfg, CallbackFns{}) + observation, err := noForkAsyncExternal.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", diff) + } + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestAsyncNoForkCreate(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + fns CallbackFns + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfgAsync, + obj: objAsync, + fns: CallbackFns{ + CreateFn: func(s string) terraform.CallbackFn { + return func(err error, ctx context.Context) error { + return nil + } + }, + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkAsyncExternal := prepareNoForkAsyncExternal(tc.args.r, tc.args.cfg, tc.args.fns) + _, err := noForkAsyncExternal.Create(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 TestAsyncNoForkUpdate(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + fns CallbackFns + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfgAsync, + obj: objAsync, + fns: CallbackFns{ + UpdateFn: func(s string) terraform.CallbackFn { + return func(err error, ctx context.Context) error { + return nil + } + }, + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkAsyncExternal := prepareNoForkAsyncExternal(tc.args.r, tc.args.cfg, tc.args.fns) + _, err := noForkAsyncExternal.Update(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 TestAsyncNoForkDelete(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + fns CallbackFns + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfgAsync, + obj: objAsync, + fns: CallbackFns{ + DestroyFn: func(s string) terraform.CallbackFn { + return func(err error, ctx context.Context) error { + return nil + } + }, + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkAsyncExternal := prepareNoForkAsyncExternal(tc.args.r, tc.args.cfg, tc.args.fns) + err := noForkAsyncExternal.Delete(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) + } + }) + } +} diff --git a/pkg/controller/external_nofork.go b/pkg/controller/external_nofork.go new file mode 100644 index 00000000..3bcd674b --- /dev/null +++ b/pkg/controller/external_nofork.go @@ -0,0 +1,662 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "fmt" + "strings" + "time" + + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "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/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + tf "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "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" + "github.com/crossplane/upjet/pkg/resource/json" + "github.com/crossplane/upjet/pkg/terraform" +) + +type NoForkConnector struct { + getTerraformSetup terraform.SetupFn + kube client.Client + config *config.Resource + logger logging.Logger + metricRecorder *metrics.MetricRecorder + operationTrackerStore *OperationTrackerStore + isManagementPoliciesEnabled bool +} + +// NoForkOption allows you to configure NoForkConnector. +type NoForkOption func(connector *NoForkConnector) + +// WithNoForkLogger configures a logger for the NoForkConnector. +func WithNoForkLogger(l logging.Logger) NoForkOption { + return func(c *NoForkConnector) { + c.logger = l + } +} + +// WithNoForkMetricRecorder configures a metrics.MetricRecorder for the +// NoForkConnector. +func WithNoForkMetricRecorder(r *metrics.MetricRecorder) NoForkOption { + return func(c *NoForkConnector) { + c.metricRecorder = r + } +} + +// WithNoForkManagementPolicies configures whether the client should +// handle management policies. +func WithNoForkManagementPolicies(isManagementPoliciesEnabled bool) NoForkOption { + return func(c *NoForkConnector) { + c.isManagementPoliciesEnabled = isManagementPoliciesEnabled + } +} + +func NewNoForkConnector(kube client.Client, sf terraform.SetupFn, cfg *config.Resource, ots *OperationTrackerStore, opts ...NoForkOption) *NoForkConnector { + nfc := &NoForkConnector{ + kube: kube, + getTerraformSetup: sf, + config: cfg, + operationTrackerStore: ots, + } + for _, f := range opts { + f(nfc) + } + return nfc +} + +func copyParameters(tfState, params map[string]any) map[string]any { + targetState := make(map[string]any, len(params)) + for k, v := range params { + targetState[k] = v + } + for k, v := range tfState { + targetState[k] = v + } + return targetState +} + +func getJSONMap(mg xpresource.Managed) (map[string]any, error) { + pv, err := fieldpath.PaveObject(mg) + if err != nil { + return nil, errors.Wrap(err, "cannot pave the managed resource") + } + v, err := pv.GetValue("spec.forProvider") + if err != nil { + return nil, errors.Wrap(err, "cannot get spec.forProvider value from paved object") + } + return v.(map[string]any), nil +} + +type Resource interface { + Apply(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) + RefreshWithoutUpgrade(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) +} + +type noForkExternal struct { + ts terraform.Setup + resourceSchema Resource + config *config.Resource + instanceDiff *tf.InstanceDiff + params map[string]any + rawConfig cty.Value + logger logging.Logger + metricRecorder *metrics.MetricRecorder + opTracker *AsyncTracker +} + +func getExtendedParameters(ctx context.Context, tr resource.Terraformed, externalName string, config *config.Resource, ts terraform.Setup, initParamsMerged bool, kube client.Client) (map[string]any, error) { + params, err := tr.GetMergedParameters(initParamsMerged) + if err != nil { + return nil, errors.Wrap(err, "cannot get merged parameters") + } + if err = resource.GetSensitiveParameters(ctx, &APISecretClient{kube: kube}, tr, params, tr.GetConnectionDetailsMapping()); err != nil { + return nil, errors.Wrap(err, "cannot store sensitive parameters into params") + } + config.ExternalName.SetIdentifierArgumentFn(params, externalName) + if config.TerraformConfigurationInjector != nil { + m, err := getJSONMap(tr) + if err != nil { + return nil, errors.Wrap(err, "cannot get JSON map for the managed resource's spec.forProvider value") + } + config.TerraformConfigurationInjector(m, params) + } + + tfID, err := config.ExternalName.GetIDFn(ctx, externalName, params, ts.Map()) + if err != nil { + return nil, errors.Wrap(err, "cannot get ID") + } + params["id"] = tfID + // we need to parameterize the following for a provider + // 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"] + } + return params, nil +} + +func (c *NoForkConnector) 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] = c.applyStateFuncToParam(sc, param) + } else { + params[key] = param + } + } + return params +} + +func (c *NoForkConnector) 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] = c.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] = c.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] = c.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 *NoForkConnector) 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()) + } + params = c.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 &noForkExternal{ + ts: ts, + resourceSchema: c.config.TerraformResource, + config: c.config, + params: params, + rawConfig: rawConfig, + logger: logger, + metricRecorder: c.metricRecorder, + opTracker: opTracker, + }, nil +} + +func filterInitExclusiveDiffs(tr resource.Terraformed, instanceDiff *tf.InstanceDiff) error { //nolint:gocyclo + if instanceDiff == nil || instanceDiff.Empty() { + return nil + } + + paramsForProvider, err := tr.GetParameters() + if err != nil { + return errors.Wrap(err, "cannot get spec.forProvider parameters") + } + paramsInitProvider, err := tr.GetInitParameters() + if err != nil { + return errors.Wrap(err, "cannot get spec.initProvider parameters") + } + + initProviderExclusiveParamKeys := getTerraformIgnoreChanges(paramsForProvider, paramsInitProvider) + for _, keyToIgnore := range initProviderExclusiveParamKeys { + for attributeKey := range instanceDiff.Attributes { + keyToIgnoreAsPrefix := fmt.Sprintf("%s.", keyToIgnore) + if keyToIgnore != attributeKey && !strings.HasPrefix(attributeKey, keyToIgnoreAsPrefix) { + continue + } + + delete(instanceDiff.Attributes, attributeKey) + + // TODO: tags-tags_all implementation is AWS specific. + // Consider making this logic independent of provider. + keyComponents := strings.Split(attributeKey, ".") + if keyComponents[0] != "tags" { + continue + } + keyComponents[0] = "tags_all" + tagsAllAttributeKey := strings.Join(keyComponents, ".") + delete(instanceDiff.Attributes, tagsAllAttributeKey) + } + } + + // Delete length keys, such as "tags.%" (schema.TypeMap) and + // "cidrBlocks.#" (schema.TypeSet), because of two reasons: + // + // 1. Diffs are applied successfully without them, except for + // schema.TypeList. + // + // 2. If only length keys remain in the diff, after ignored + // attributes are removed above, they cause diff to be considered + // non-empty, even though it is effectively empty, therefore causing + // an infinite update loop. + for _, keyToIgnore := range initProviderExclusiveParamKeys { + keyComponents := strings.Split(keyToIgnore, ".") + if len(keyComponents) < 2 { + continue + } + + // TODO: Consider locating the schema corresponding to keyToIgnore + // and checking whether it's a collection, before attempting to + // delete its length key. + for _, lengthSymbol := range []string{"%", "#"} { + keyComponents[len(keyComponents)-1] = lengthSymbol + lengthKey := strings.Join(keyComponents, ".") + delete(instanceDiff.Attributes, lengthKey) + } + + // TODO: tags-tags_all implementation is AWS specific. + // Consider making this logic independent of provider. + if keyComponents[0] == "tags" { + keyComponents[0] = "tags_all" + keyComponents[len(keyComponents)-1] = "%" + lengthKey := strings.Join(keyComponents, ".") + delete(instanceDiff.Attributes, lengthKey) + } + } + return nil +} + +// resource timeouts configuration +func getTimeoutParameters(config *config.Resource) map[string]any { //nolint:gocyclo + timeouts := make(map[string]any) + // first use the timeout overrides specified in + // the Terraform resource schema + if config.TerraformResource.Timeouts != nil { + if config.TerraformResource.Timeouts.Create != nil && *config.TerraformResource.Timeouts.Create != 0 { + timeouts[schema.TimeoutCreate] = config.TerraformResource.Timeouts.Create.Nanoseconds() + } + if config.TerraformResource.Timeouts.Update != nil && *config.TerraformResource.Timeouts.Update != 0 { + timeouts[schema.TimeoutUpdate] = config.TerraformResource.Timeouts.Update.Nanoseconds() + } + if config.TerraformResource.Timeouts.Delete != nil && *config.TerraformResource.Timeouts.Delete != 0 { + timeouts[schema.TimeoutDelete] = config.TerraformResource.Timeouts.Delete.Nanoseconds() + } + if config.TerraformResource.Timeouts.Read != nil && *config.TerraformResource.Timeouts.Read != 0 { + timeouts[schema.TimeoutRead] = config.TerraformResource.Timeouts.Read.Nanoseconds() + } + } + // then, override any Terraform defaults using any upjet + // resource configuration overrides + if config.OperationTimeouts.Create != 0 { + timeouts[schema.TimeoutCreate] = config.OperationTimeouts.Create.Nanoseconds() + } + if config.OperationTimeouts.Update != 0 { + timeouts[schema.TimeoutUpdate] = config.OperationTimeouts.Update.Nanoseconds() + } + if config.OperationTimeouts.Delete != 0 { + timeouts[schema.TimeoutDelete] = config.OperationTimeouts.Delete.Nanoseconds() + } + if config.OperationTimeouts.Read != 0 { + timeouts[schema.TimeoutRead] = config.OperationTimeouts.Read.Nanoseconds() + } + return timeouts +} + +func (n *noForkExternal) getResourceDataDiff(tr resource.Terraformed, ctx context.Context, s *tf.InstanceState, resourceExists bool) (*tf.InstanceDiff, error) { //nolint:gocyclo + resourceConfig := tf.NewResourceConfigRaw(n.params) + instanceDiff, err := schema.InternalMap(n.config.TerraformResource.Schema).Diff(ctx, s, resourceConfig, nil, n.ts.Meta, false) + if err != nil { + return nil, errors.Wrap(err, "failed to get *terraform.InstanceDiff") + } + if n.config.TerraformCustomDiff != nil { + instanceDiff, err = n.config.TerraformCustomDiff(instanceDiff, s, resourceConfig) + if err != nil { + return nil, errors.Wrap(err, "failed to compute the customized terraform.InstanceDiff") + } + } + + if resourceExists { + if err := filterInitExclusiveDiffs(tr, instanceDiff); err != nil { + return nil, errors.Wrap(err, "failed to filter the diffs exclusive to spec.initProvider in the terraform.InstanceDiff") + } + } + if instanceDiff != nil { + v := cty.EmptyObjectVal + v, err = instanceDiff.ApplyToValue(v, n.config.TerraformResource.CoreConfigSchema()) + if err != nil { + return nil, errors.Wrap(err, "cannot apply Terraform instance diff to an empty value") + } + instanceDiff.RawPlan = v + } + if instanceDiff != nil && !instanceDiff.Empty() { + n.logger.Debug("Diff detected", "instanceDiff", instanceDiff.GoString()) + // Assumption: Source of truth when applying diffs, for instance on updates, is instanceDiff.Attributes. + // Setting instanceDiff.RawConfig has no effect on diff application. + instanceDiff.RawConfig = n.rawConfig + } + timeouts := getTimeoutParameters(n.config) + if len(timeouts) > 0 { + if instanceDiff == nil { + instanceDiff = tf.NewInstanceDiff() + } + if instanceDiff.Meta == nil { + instanceDiff.Meta = make(map[string]interface{}) + } + instanceDiff.Meta[schema.TimeoutKey] = timeouts + } + return instanceDiff, nil +} + +func (n *noForkExternal) 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 + } + + 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: resourceExists, + ResourceUpToDate: noDiff, + ConnectionDetails: connDetails, + ResourceLateInitialized: specUpdateRequired, + }, nil +} + +// sets the external-name on the MR. Returns `true` +// if the external-name of the MR has changed. +func (n *noForkExternal) setExternalName(mg xpresource.Managed, newState *tf.InstanceState) (bool, error) { + if newState.ID == "" { + return false, nil + } + newName, err := n.config.ExternalName.GetExternalNameFn(map[string]any{ + "id": newState.ID, + }) + if err != nil { + return false, errors.Wrapf(err, "failed to get the external-name from ID: %s", newState.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 (n *noForkExternal) 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") + } + + return managed.ExternalCreation{ConnectionDetails: conn}, nil +} + +func (n *noForkExternal) assertNoForceNew() error { + if n.instanceDiff == nil { + return nil + } + for k, ad := range n.instanceDiff.Attributes { + if ad == nil { + continue + } + // TODO: use a multi-error implementation to report changes to + // all `ForceNew` arguments. + if ad.RequiresNew { + if ad.Sensitive { + return errors.Errorf("cannot change the value of the argument %q", k) + } + return errors.Errorf("cannot change the value of the argument %q from %q to %q", k, ad.Old, ad.New) + } + } + return nil +} + +func (n *noForkExternal) 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 *noForkExternal) 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 +} + +func (n *noForkExternal) fromInstanceStateToJSONMap(newState *tf.InstanceState) (map[string]interface{}, error) { + impliedType := n.config.TerraformResource.CoreConfigSchema().ImpliedType() + attrsAsCtyValue, err := newState.AttrsAsObjectValue(impliedType) + if err != nil { + return nil, errors.Wrap(err, "could not convert attrs to cty value") + } + stateValueMap, err := schema.StateValueToJSONMap(attrsAsCtyValue, impliedType) + if err != nil { + return nil, errors.Wrap(err, "could not convert instance state value to JSON") + } + return stateValueMap, nil +} diff --git a/pkg/controller/external_nofork_test.go b/pkg/controller/external_nofork_test.go new file mode 100644 index 00000000..3109f669 --- /dev/null +++ b/pkg/controller/external_nofork_test.go @@ -0,0 +1,405 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "testing" + "time" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + "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-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + tf "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource/fake" + "github.com/crossplane/upjet/pkg/terraform" +) + +var ( + zl = zap.New(zap.UseDevMode(true)) + logTest = logging.NewLogrLogger(zl.WithName("provider-aws")) + ots = NewOperationStore(logTest) + timeout = time.Duration(1200000000000) + cfg = &config.Resource{ + TerraformResource: &schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Create: &timeout, + Read: &timeout, + Update: &timeout, + Delete: &timeout, + }, + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + "id": { + Type: schema.TypeString, + Computed: true, + Required: false, + }, + "map": { + Type: schema.TypeMap, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "list": { + Type: schema.TypeList, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + ExternalName: config.IdentifierFromProvider, + Sensitive: config.Sensitive{AdditionalConnectionDetailsFn: func(attr map[string]any) (map[string][]byte, error) { + return nil, nil + }}, + } + obj = &fake.Terraformed{ + Parameterizable: fake.Parameterizable{ + Parameters: map[string]any{ + "name": "example", + "map": map[string]any{ + "key": "value", + }, + "list": []any{"elem1", "elem2"}, + }, + }, + Observable: fake.Observable{ + Observation: map[string]any{}, + }, + } +) + +func prepareNoForkExternal(r Resource, cfg *config.Resource) *noForkExternal { + schemaBlock := cfg.TerraformResource.CoreConfigSchema() + rawConfig, err := schema.JSONMapToStateValue(map[string]any{"name": "example"}, schemaBlock) + if err != nil { + panic(err) + } + return &noForkExternal{ + ts: terraform.Setup{}, + resourceSchema: r, + config: cfg, + params: map[string]any{ + "name": "example", + }, + rawConfig: rawConfig, + logger: logTest, + opTracker: NewAsyncTracker(), + } +} + +type mockResource struct { + ApplyFn func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) + RefreshWithoutUpgradeFn func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) +} + +func (m mockResource) Apply(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return m.ApplyFn(ctx, s, d, meta) +} + +func (m mockResource) RefreshWithoutUpgrade(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return m.RefreshWithoutUpgradeFn(ctx, s, meta) +} + +func TestNoForkConnect(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{}, nil + }, + cfg: cfg, + obj: obj, + ots: ots, + }, + }, + "HCL": { + args: args{ + setupFn: func(_ context.Context, _ client.Client, _ xpresource.Managed) (terraform.Setup, error) { + return terraform.Setup{}, nil + }, + cfg: cfg, + obj: &fake.Terraformed{ + Parameterizable: fake.Parameterizable{ + Parameters: map[string]any{ + "name": " ${jsonencode({\n type = \"object\"\n })}", + "map": map[string]any{ + "key": "value", + }, + "list": []any{"elem1", "elem2"}, + }, + }, + Observable: fake.Observable{ + Observation: map[string]any{}, + }, + }, + ots: ots, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := NewNoForkConnector(nil, tc.args.setupFn, tc.args.cfg, tc.args.ots, WithNoForkLogger(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 TestNoForkObserve(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + } + type want struct { + obs managed.ExternalObservation + err error + } + cases := map[string]struct { + args + want + }{ + "NotExists": { + args: args{ + r: mockResource{ + RefreshWithoutUpgradeFn: func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return nil, nil + }, + }, + cfg: cfg, + obj: obj, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: false, + ResourceUpToDate: false, + ResourceLateInitialized: false, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + "UpToDate": { + args: args{ + r: mockResource{ + RefreshWithoutUpgradeFn: func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id", Attributes: map[string]string{"name": "example"}}, nil + }, + }, + cfg: cfg, + obj: obj, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + "InitProvider": { + args: args{ + r: mockResource{ + RefreshWithoutUpgradeFn: func(ctx context.Context, s *tf.InstanceState, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id", Attributes: map[string]string{"name": "example2"}}, nil + }, + }, + cfg: cfg, + 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{}, + }, + }, + }, + want: want{ + obs: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + ConnectionDetails: nil, + Diff: "", + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkExternal := prepareNoForkExternal(tc.args.r, tc.args.cfg) + observation, err := noForkExternal.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", diff) + } + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff) + } + }) + } +} + +func TestNoForkCreate(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Unsuccessful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return nil, nil + }, + }, + cfg: cfg, + obj: obj, + }, + want: want{ + err: errors.New("failed to read the ID of the new resource"), + }, + }, + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfg, + obj: obj, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkExternal := prepareNoForkExternal(tc.args.r, tc.args.cfg) + _, err := noForkExternal.Create(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 TestNoForkUpdate(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfg, + obj: obj, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkExternal := prepareNoForkExternal(tc.args.r, tc.args.cfg) + _, err := noForkExternal.Update(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 TestNoForkDelete(t *testing.T) { + type args struct { + r Resource + cfg *config.Resource + obj xpresource.Managed + } + type want struct { + err error + } + cases := map[string]struct { + args + want + }{ + "Successful": { + args: args{ + r: mockResource{ + ApplyFn: func(ctx context.Context, s *tf.InstanceState, d *tf.InstanceDiff, meta interface{}) (*tf.InstanceState, diag.Diagnostics) { + return &tf.InstanceState{ID: "example-id"}, nil + }, + }, + cfg: cfg, + obj: obj, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + noForkExternal := prepareNoForkExternal(tc.args.r, tc.args.cfg) + err := noForkExternal.Delete(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) + } + }) + } +} diff --git a/pkg/controller/external_test.go b/pkg/controller/external_test.go index 7710e705..68e06c16 100644 --- a/pkg/controller/external_test.go +++ b/pkg/controller/external_test.go @@ -8,17 +8,6 @@ import ( "context" "testing" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/resource/fake" - "github.com/crossplane/upjet/pkg/resource/json" - "github.com/crossplane/upjet/pkg/terraform" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/logging" xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" @@ -26,6 +15,17 @@ import ( xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/resource/fake" + "github.com/crossplane/upjet/pkg/resource/json" + "github.com/crossplane/upjet/pkg/terraform" ) const ( diff --git a/pkg/controller/handler/eventhandler.go b/pkg/controller/handler/eventhandler.go index 734bc1b6..7032e3ca 100644 --- a/pkg/controller/handler/eventhandler.go +++ b/pkg/controller/handler/eventhandler.go @@ -7,16 +7,18 @@ package handler import ( "context" "sync" + "time" + "github.com/crossplane/crossplane-runtime/pkg/logging" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/crossplane/crossplane-runtime/pkg/logging" ) +const NoRateLimiter = "" + // EventHandler handles Kubernetes events by queueing reconcile requests for // objects and allows upjet components to queue reconcile requests. type EventHandler struct { @@ -64,16 +66,19 @@ func (e *EventHandler) RequestReconcile(rateLimiterName, name string, failureLim Name: name, }, } - rateLimiter := e.rateLimiterMap[rateLimiterName] - if rateLimiter == nil { - rateLimiter = workqueue.DefaultControllerRateLimiter() - e.rateLimiterMap[rateLimiterName] = rateLimiter - } - if failureLimit != nil && rateLimiter.NumRequeues(item) > *failureLimit { - logger.Info("Failure limit has been exceeded.", "failureLimit", *failureLimit, "numRequeues", rateLimiter.NumRequeues(item)) - return false + var when time.Duration = 0 + if rateLimiterName != NoRateLimiter { + rateLimiter := e.rateLimiterMap[rateLimiterName] + if rateLimiter == nil { + rateLimiter = workqueue.DefaultControllerRateLimiter() + e.rateLimiterMap[rateLimiterName] = rateLimiter + } + if failureLimit != nil && rateLimiter.NumRequeues(item) > *failureLimit { + logger.Info("Failure limit has been exceeded.", "failureLimit", *failureLimit, "numRequeues", rateLimiter.NumRequeues(item)) + return false + } + when = rateLimiter.When(item) } - when := rateLimiter.When(item) e.queue.AddAfter(item, when) logger.Debug("Reconcile request has been requeued.", "rateLimiterName", rateLimiterName, "when", when) return true diff --git a/pkg/controller/hcl.go b/pkg/controller/hcl.go new file mode 100644 index 00000000..63853ecc --- /dev/null +++ b/pkg/controller/hcl.go @@ -0,0 +1,163 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "encoding/base64" + "fmt" + "log" + "regexp" + "unicode/utf8" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclparse" + ctyyaml "github.com/zclconf/go-cty-yaml" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" + ctyfuncstdlib "github.com/zclconf/go-cty/cty/function/stdlib" +) + +var Base64DecodeFunc = function.New(&function.Spec{ + Params: []function.Parameter{ + { + Name: "str", + Type: cty.String, + AllowMarked: true, + }, + }, + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + str, strMarks := args[0].Unmark() + s := str.AsString() + sDec, err := base64.StdEncoding.DecodeString(s) + if err != nil { + return cty.UnknownVal(cty.String), fmt.Errorf("failed to decode base64 data %s", s) + } + if !utf8.Valid(sDec) { + log.Printf("[DEBUG] the result of decoding the provided string is not valid UTF-8: %s", s) + return cty.UnknownVal(cty.String), fmt.Errorf("the result of decoding the provided string is not valid UTF-8") + } + return cty.StringVal(string(sDec)).WithMarks(strMarks), nil + }, +}) + +var Base64EncodeFunc = function.New(&function.Spec{ + Params: []function.Parameter{ + { + Name: "str", + Type: cty.String, + }, + }, + Type: function.StaticReturnType(cty.String), + Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { + return cty.StringVal(base64.StdEncoding.EncodeToString([]byte(args[0].AsString()))), nil + }, +}) + +// evalCtx registers the known functions for HCL processing +// variable interpolation is not supported, as in our case they are irrelevant +var evalCtx = &hcl.EvalContext{ + Variables: map[string]cty.Value{}, + Functions: map[string]function.Function{ + "abs": ctyfuncstdlib.AbsoluteFunc, + "ceil": ctyfuncstdlib.CeilFunc, + "chomp": ctyfuncstdlib.ChompFunc, + "coalescelist": ctyfuncstdlib.CoalesceListFunc, + "compact": ctyfuncstdlib.CompactFunc, + "concat": ctyfuncstdlib.ConcatFunc, + "contains": ctyfuncstdlib.ContainsFunc, + "csvdecode": ctyfuncstdlib.CSVDecodeFunc, + "distinct": ctyfuncstdlib.DistinctFunc, + "element": ctyfuncstdlib.ElementFunc, + "chunklist": ctyfuncstdlib.ChunklistFunc, + "flatten": ctyfuncstdlib.FlattenFunc, + "floor": ctyfuncstdlib.FloorFunc, + "format": ctyfuncstdlib.FormatFunc, + "formatdate": ctyfuncstdlib.FormatDateFunc, + "formatlist": ctyfuncstdlib.FormatListFunc, + "indent": ctyfuncstdlib.IndentFunc, + "join": ctyfuncstdlib.JoinFunc, + "jsondecode": ctyfuncstdlib.JSONDecodeFunc, + "jsonencode": ctyfuncstdlib.JSONEncodeFunc, + "keys": ctyfuncstdlib.KeysFunc, + "log": ctyfuncstdlib.LogFunc, + "lower": ctyfuncstdlib.LowerFunc, + "max": ctyfuncstdlib.MaxFunc, + "merge": ctyfuncstdlib.MergeFunc, + "min": ctyfuncstdlib.MinFunc, + "parseint": ctyfuncstdlib.ParseIntFunc, + "pow": ctyfuncstdlib.PowFunc, + "range": ctyfuncstdlib.RangeFunc, + "regex": ctyfuncstdlib.RegexFunc, + "regexall": ctyfuncstdlib.RegexAllFunc, + "reverse": ctyfuncstdlib.ReverseListFunc, + "setintersection": ctyfuncstdlib.SetIntersectionFunc, + "setproduct": ctyfuncstdlib.SetProductFunc, + "setsubtract": ctyfuncstdlib.SetSubtractFunc, + "setunion": ctyfuncstdlib.SetUnionFunc, + "signum": ctyfuncstdlib.SignumFunc, + "slice": ctyfuncstdlib.SliceFunc, + "sort": ctyfuncstdlib.SortFunc, + "split": ctyfuncstdlib.SplitFunc, + "strrev": ctyfuncstdlib.ReverseFunc, + "substr": ctyfuncstdlib.SubstrFunc, + "timeadd": ctyfuncstdlib.TimeAddFunc, + "title": ctyfuncstdlib.TitleFunc, + "trim": ctyfuncstdlib.TrimFunc, + "trimprefix": ctyfuncstdlib.TrimPrefixFunc, + "trimspace": ctyfuncstdlib.TrimSpaceFunc, + "trimsuffix": ctyfuncstdlib.TrimSuffixFunc, + "upper": ctyfuncstdlib.UpperFunc, + "values": ctyfuncstdlib.ValuesFunc, + "zipmap": ctyfuncstdlib.ZipmapFunc, + "yamldecode": ctyyaml.YAMLDecodeFunc, + "yamlencode": ctyyaml.YAMLEncodeFunc, + "base64encode": Base64EncodeFunc, + "base64decode": Base64DecodeFunc, + }, +} + +// hclBlock is the target type for decoding the specially-crafted HCL document. +// interested in processing HCL snippets for a single parameter +type hclBlock struct { + Parameter string `hcl:"parameter"` +} + +// isHCLSnippetPattern is the regex pattern for determining whether +// the param is an HCL template +var isHCLSnippetPattern = regexp.MustCompile(`\$\{\w+\s*\([\S\s]*\}`) + +// processHCLParam processes the given string parameter +// with HCL format and including HCL functions, +// coming from the Managed Resource spec parameters. +// It prepares a tailored HCL snippet which consist of only a single attribute +// parameter = theGivenParameterValueInHCLSyntax +// It only operates on string parameters, and returns a string. +// caller should ensure that the given parameter is an HCL snippet +func processHCLParam(param string) (string, error) { + param = fmt.Sprintf("parameter = \"%s\"\n", param) + return processHCLParamBytes([]byte(param)) +} + +// processHCLParamBytes parses and decodes the HCL snippet +func processHCLParamBytes(paramValueBytes []byte) (string, error) { + hclParser := hclparse.NewParser() + // here the filename argument is not important, + // used by the hcl parser lib for tracking caching purposes + // it is just a name reference + hclFile, diag := hclParser.ParseHCL(paramValueBytes, "dummy.hcl") + if diag.HasErrors() { + return "", diag + } + + var paramWrapper hclBlock + diags := gohcl.DecodeBody(hclFile.Body, evalCtx, ¶mWrapper) + if diags.HasErrors() { + return "", diags + } + + return paramWrapper.Parameter, nil +} diff --git a/pkg/controller/ignored_nofork.go b/pkg/controller/ignored_nofork.go new file mode 100644 index 00000000..74230698 --- /dev/null +++ b/pkg/controller/ignored_nofork.go @@ -0,0 +1,62 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import "fmt" + +// getTerraformIgnoreChanges returns a sorted Terraform `ignore_changes` +// lifecycle meta-argument expression by looking for differences between +// the `initProvider` and `forProvider` maps. The ignored fields are the ones +// that are present in initProvider, but not in forProvider. +// TODO: This method is copy-pasted from `pkg/resource/ignored.go` and adapted. +// Consider merging this implementation with the original one. +func getTerraformIgnoreChanges(forProvider, initProvider map[string]any) []string { + ignored := getIgnoredFieldsMap("%s", forProvider, initProvider) + return ignored +} + +// TODO: This method is copy-pasted from `pkg/resource/ignored.go` and adapted. +// Consider merging this implementation with the original one. +func getIgnoredFieldsMap(format string, forProvider, initProvider map[string]any) []string { + ignored := []string{} + + for k := range initProvider { + if _, ok := forProvider[k]; !ok { + ignored = append(ignored, fmt.Sprintf(format, k)) + } else { + // both are the same type so we dont need to check for forProvider type + if _, ok = initProvider[k].(map[string]any); ok { + ignored = append(ignored, getIgnoredFieldsMap(fmt.Sprintf(format, k)+".%v", forProvider[k].(map[string]any), initProvider[k].(map[string]any))...) + } + // if its an array, we need to check if its an array of maps or not + if _, ok = initProvider[k].([]any); ok { + ignored = append(ignored, getIgnoredFieldsArray(fmt.Sprintf(format, k), forProvider[k].([]any), initProvider[k].([]any))...) + } + + } + } + return ignored +} + +// TODO: This method is copy-pasted from `pkg/resource/ignored.go` and adapted. +// Consider merging this implementation with the original one. +func getIgnoredFieldsArray(format string, forProvider, initProvider []any) []string { + ignored := []string{} + for i := range initProvider { + // Construct the full field path with array index and prefix. + fieldPath := fmt.Sprintf("%s.%d", format, i) + if i < len(forProvider) { + if _, ok := initProvider[i].(map[string]any); ok { + ignored = append(ignored, getIgnoredFieldsMap(fieldPath+".%s", forProvider[i].(map[string]any), initProvider[i].(map[string]any))...) + } + if _, ok := initProvider[i].([]any); ok { + ignored = append(ignored, getIgnoredFieldsArray(fieldPath, forProvider[i].([]any), initProvider[i].([]any))...) + } + } else { + ignored = append(ignored, fieldPath) + } + } + return ignored +} diff --git a/pkg/controller/nofork_finalizer.go b/pkg/controller/nofork_finalizer.go new file mode 100644 index 00000000..7476f454 --- /dev/null +++ b/pkg/controller/nofork_finalizer.go @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/pkg/errors" +) + +const ( + errRemoveTracker = "cannot remove tracker from the store" +) + +// TrackerCleaner is the interface that the no-fork finalizer needs to work with. +type TrackerCleaner interface { + RemoveTracker(obj xpresource.Object) error +} + +// NewNoForkFinalizer returns a new NoForkFinalizer. +func NewNoForkFinalizer(tc TrackerCleaner, af xpresource.Finalizer) *NoForkFinalizer { + return &NoForkFinalizer{ + Finalizer: af, + OperationStore: tc, + } +} + +// NoForkFinalizer removes the operation tracker from the workspace store and only +// then calls RemoveFinalizer of the underlying Finalizer. +type NoForkFinalizer struct { + xpresource.Finalizer + OperationStore TrackerCleaner +} + +// AddFinalizer to the supplied Managed resource. +func (nf *NoForkFinalizer) AddFinalizer(ctx context.Context, obj xpresource.Object) error { + return nf.Finalizer.AddFinalizer(ctx, obj) +} + +// RemoveFinalizer removes the workspace from workspace store before removing +// the finalizer. +func (nf *NoForkFinalizer) RemoveFinalizer(ctx context.Context, obj xpresource.Object) error { + if err := nf.OperationStore.RemoveTracker(obj); err != nil { + return errors.Wrap(err, errRemoveTracker) + } + return nf.Finalizer.RemoveFinalizer(ctx, obj) +} diff --git a/pkg/controller/nofork_store.go b/pkg/controller/nofork_store.go new file mode 100644 index 00000000..4ec2e082 --- /dev/null +++ b/pkg/controller/nofork_store.go @@ -0,0 +1,129 @@ +// SPDX-FileCopyrightText: 2023 The Crossplane Authors +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "sync" + "sync/atomic" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + tfsdk "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "k8s.io/apimachinery/pkg/types" + + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/terraform" +) + +type AsyncTracker struct { + LastOperation *terraform.Operation + logger logging.Logger + mu *sync.Mutex + tfState *tfsdk.InstanceState + // 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 + // resource and a database configuration resource whose lifecycle is bound + // to it. For such resources, Terraform still removes the state for them + // after a successful delete call either by resetting to some defaults in + // the parent resource, or by a noop. We logically delete such resources as + // deleted after a successful delete call so that the next observe can + // tell the managed reconciler that the resource no longer "exists". + isDeleted atomic.Bool +} + +type AsyncTrackerOption func(manager *AsyncTracker) + +// WithAsyncTrackerLogger sets the logger of AsyncTracker. +func WithAsyncTrackerLogger(l logging.Logger) AsyncTrackerOption { + return func(w *AsyncTracker) { + w.logger = l + } +} +func NewAsyncTracker(opts ...AsyncTrackerOption) *AsyncTracker { + w := &AsyncTracker{ + LastOperation: &terraform.Operation{}, + logger: logging.NewNopLogger(), + mu: &sync.Mutex{}, + } + for _, f := range opts { + f(w) + } + return w +} + +func (a *AsyncTracker) GetTfState() *tfsdk.InstanceState { + a.mu.Lock() + defer a.mu.Unlock() + return a.tfState +} + +func (a *AsyncTracker) HasState() bool { + a.mu.Lock() + defer a.mu.Unlock() + return a.tfState != nil && a.tfState.ID != "" +} + +func (a *AsyncTracker) SetTfState(state *tfsdk.InstanceState) { + a.mu.Lock() + defer a.mu.Unlock() + a.tfState = state +} + +func (a *AsyncTracker) GetTfID() string { + a.mu.Lock() + defer a.mu.Unlock() + if a.tfState == nil { + return "" + } + return a.tfState.ID +} + +// IsDeleted returns whether the associated external resource +// has logically been deleted. +func (a *AsyncTracker) IsDeleted() bool { + return a.isDeleted.Load() +} + +// SetDeleted sets the logical deletion status of +// the associated external resource. +func (a *AsyncTracker) SetDeleted(deleted bool) { + a.isDeleted.Store(deleted) +} + +type OperationTrackerStore struct { + store map[types.UID]*AsyncTracker + logger logging.Logger + mu *sync.Mutex +} + +func NewOperationStore(l logging.Logger) *OperationTrackerStore { + ops := &OperationTrackerStore{ + store: map[types.UID]*AsyncTracker{}, + logger: l, + mu: &sync.Mutex{}, + } + + return ops +} + +func (ops *OperationTrackerStore) Tracker(tr resource.Terraformed) *AsyncTracker { + ops.mu.Lock() + defer ops.mu.Unlock() + tracker, ok := ops.store[tr.GetUID()] + if !ok { + l := ops.logger.WithValues("trackerUID", tr.GetUID(), "resourceName", tr.GetName()) + ops.store[tr.GetUID()] = NewAsyncTracker(WithAsyncTrackerLogger(l)) + tracker = ops.store[tr.GetUID()] + } + return tracker +} + +func (ops *OperationTrackerStore) RemoveTracker(obj xpresource.Object) error { + ops.mu.Lock() + defer ops.mu.Unlock() + delete(ops.store, obj.GetUID()) + return nil +} diff --git a/pkg/controller/options.go b/pkg/controller/options.go index e9e54a95..353ef866 100644 --- a/pkg/controller/options.go +++ b/pkg/controller/options.go @@ -8,11 +8,11 @@ import ( "crypto/tls" "time" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/terraform" + "github.com/crossplane/crossplane-runtime/pkg/controller" "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/crossplane/crossplane-runtime/pkg/controller" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/terraform" ) // Options contains incriminating options for a given Upjet controller instance. @@ -28,6 +28,8 @@ type Options struct { // instance should use. WorkspaceStore *terraform.WorkspaceStore + OperationTrackerStore *OperationTrackerStore + // SetupFn contains the provider-specific initialization logic, such as // preparing the auth token for Terraform CLI. SetupFn terraform.SetupFn diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 4e71dc7d..762c80b6 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -5,7 +5,17 @@ package metrics import ( + "context" + "sync" + "time" + + "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/tools/cache" + "sigs.k8s.io/controller-runtime/pkg/cluster" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics" ) @@ -25,6 +35,38 @@ var ( Buckets: []float64{1.0, 3, 5, 10, 15, 30, 60, 120, 300}, }, []string{"subcommand", "mode"}) + // ExternalAPITime is the SDK processing times histogram. + ExternalAPITime = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: promNSUpjet, + Subsystem: promSysResource, + Name: "ext_api_duration", + Help: "Measures in seconds how long it takes a Cloud SDK call to complete", + Buckets: []float64{1, 5, 10, 15, 30, 60, 120, 300, 600, 1800, 3600}, + }, []string{"operation"}) + + // DeletionTime is the histogram metric for collecting statistics on the + // intervals between the deletion timestamp and the moment when + // the resource is observed to be missing (actually deleted). + DeletionTime = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: promNSUpjet, + Subsystem: promSysResource, + Name: "deletion_seconds", + Help: "Measures in seconds how long it takes for a resource to be deleted", + Buckets: []float64{1, 5, 10, 15, 30, 60, 120, 300, 600, 1800, 3600}, + }, []string{"group", "version", "kind"}) + + // ReconcileDelay is the histogram metric for collecting statistics on the + // delays between when the expected reconciles of an up-to-date resource + // should happen and when the resource is actually reconciled. Only + // delays from the expected reconcile times are considered. + ReconcileDelay = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: promNSUpjet, + Subsystem: promSysResource, + Name: "reconcile_delay_seconds", + Help: "Measures in seconds how long the reconciles for a resource have been delayed from the configured poll periods", + Buckets: []float64{1, 5, 10, 15, 30, 60, 120, 300, 600, 1800, 3600}, + }, []string{"group", "version", "kind"}) + // CLIExecutions are the active number of terraform CLI invocations. CLIExecutions = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: promNSUpjet, @@ -49,10 +91,88 @@ var ( Subsystem: promSysResource, Name: "ttr", Help: "Measures in seconds the time-to-readiness (TTR) for managed resources", - Buckets: []float64{10, 15, 30, 60, 120, 300, 600, 1800, 3600}, + Buckets: []float64{1, 5, 10, 15, 30, 60, 120, 300, 600, 1800, 3600}, }, []string{"group", "version", "kind"}) ) +var _ manager.Runnable = &MetricRecorder{} + +type MetricRecorder struct { + observations sync.Map + gvk schema.GroupVersionKind + cluster cluster.Cluster + + pollInterval time.Duration +} + +type Observations struct { + expectedReconcileTime *time.Time + observeReconcileDelay bool +} + +func NewMetricRecorder(gvk schema.GroupVersionKind, c cluster.Cluster, pollInterval time.Duration) *MetricRecorder { + return &MetricRecorder{ + gvk: gvk, + cluster: c, + pollInterval: pollInterval, + } +} + +func (r *MetricRecorder) SetReconcileTime(name string) { + if r == nil { + return + } + o, ok := r.observations.Load(name) + if !ok { + o = &Observations{} + r.observations.Store(name, o) + } + t := time.Now().Add(r.pollInterval) + o.(*Observations).expectedReconcileTime = &t + o.(*Observations).observeReconcileDelay = true +} + +func (r *MetricRecorder) ObserveReconcileDelay(gvk schema.GroupVersionKind, name string) { + if r == nil { + return + } + o, _ := r.observations.Load(name) + if o == nil || !o.(*Observations).observeReconcileDelay || o.(*Observations).expectedReconcileTime == nil { + return + } + d := time.Since(*o.(*Observations).expectedReconcileTime) + if d < 0 { + d = 0 + } + ReconcileDelay.WithLabelValues(gvk.Group, gvk.Version, gvk.Kind).Observe(d.Seconds()) + o.(*Observations).observeReconcileDelay = false +} + +func (r *MetricRecorder) Start(ctx context.Context) error { + inf, err := r.cluster.GetCache().GetInformerForKind(ctx, r.gvk) + if err != nil { + return errors.Wrapf(err, "cannot get informer for metric recorder for resource %s", r.gvk) + } + + registered, err := inf.AddEventHandler(cache.ResourceEventHandlerFuncs{ + DeleteFunc: func(obj interface{}) { + if final, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = final.Obj + } + managed := obj.(resource.Managed) + r.observations.Delete(managed.GetName()) + }, + }) + if err != nil { + return errors.Wrap(err, "cannot add delete event handler to informer for metric recorder") + } + defer inf.RemoveEventHandler(registered) //nolint:errcheck // this happens on destruction. We cannot do anything anyway. + + <-ctx.Done() + + return nil +} + func init() { - metrics.Registry.MustRegister(CLITime, CLIExecutions, TFProcesses, TTRMeasurements) + metrics.Registry.MustRegister(CLITime, CLIExecutions, TFProcesses, TTRMeasurements, ExternalAPITime, DeletionTime, ReconcileDelay) } diff --git a/pkg/migration/api_steps.go b/pkg/migration/api_steps.go index f077fc89..de1082cd 100644 --- a/pkg/migration/api_steps.go +++ b/pkg/migration/api_steps.go @@ -8,14 +8,13 @@ import ( "fmt" "strconv" - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim" "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( diff --git a/pkg/migration/configurationmetadata_steps.go b/pkg/migration/configurationmetadata_steps.go index ea18ff33..a28ad611 100644 --- a/pkg/migration/configurationmetadata_steps.go +++ b/pkg/migration/configurationmetadata_steps.go @@ -8,10 +8,9 @@ import ( "fmt" "strconv" - "github.com/pkg/errors" - xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" + "github.com/pkg/errors" ) const ( diff --git a/pkg/migration/configurationpackage_steps.go b/pkg/migration/configurationpackage_steps.go index d8103eaa..dbabd470 100644 --- a/pkg/migration/configurationpackage_steps.go +++ b/pkg/migration/configurationpackage_steps.go @@ -7,11 +7,10 @@ package migration import ( "fmt" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( diff --git a/pkg/migration/converter.go b/pkg/migration/converter.go index b6cbe41f..d137e21f 100644 --- a/pkg/migration/converter.go +++ b/pkg/migration/converter.go @@ -7,24 +7,22 @@ package migration import ( "fmt" - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/json" - k8sjson "sigs.k8s.io/json" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/resource" - xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" xppkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" xppkgv1beta1 "github.com/crossplane/crossplane/apis/pkg/v1beta1" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" + k8sjson "sigs.k8s.io/json" ) const ( diff --git a/pkg/migration/fake/objects.go b/pkg/migration/fake/objects.go index c9b1a59e..81c7e614 100644 --- a/pkg/migration/fake/objects.go +++ b/pkg/migration/fake/objects.go @@ -7,10 +7,10 @@ package fake import ( - "github.com/crossplane/upjet/pkg/migration/fake/mocks" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "k8s.io/apimachinery/pkg/runtime/schema" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/upjet/pkg/migration/fake/mocks" ) const ( diff --git a/pkg/migration/fork_executor.go b/pkg/migration/fork_executor.go index 61e46281..f756f622 100644 --- a/pkg/migration/fork_executor.go +++ b/pkg/migration/fork_executor.go @@ -7,10 +7,9 @@ package migration import ( "os" + "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/pkg/errors" "k8s.io/utils/exec" - - "github.com/crossplane/crossplane-runtime/pkg/logging" ) const ( diff --git a/pkg/migration/fork_executor_test.go b/pkg/migration/fork_executor_test.go index cc146156..14538ff4 100644 --- a/pkg/migration/fork_executor_test.go +++ b/pkg/migration/fork_executor_test.go @@ -7,12 +7,11 @@ package migration import ( "testing" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" k8sExec "k8s.io/utils/exec" testingexec "k8s.io/utils/exec/testing" - - "github.com/crossplane/crossplane-runtime/pkg/test" ) var ( diff --git a/pkg/migration/interfaces.go b/pkg/migration/interfaces.go index a8771588..466b4b05 100644 --- a/pkg/migration/interfaces.go +++ b/pkg/migration/interfaces.go @@ -6,7 +6,6 @@ package migration import ( "github.com/crossplane/crossplane-runtime/pkg/resource" - xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" diff --git a/pkg/migration/patches.go b/pkg/migration/patches.go index d5d84188..e09ea770 100644 --- a/pkg/migration/patches.go +++ b/pkg/migration/patches.go @@ -11,11 +11,10 @@ import ( "regexp" "strings" + xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - - xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" ) var ( diff --git a/pkg/migration/plan_generator.go b/pkg/migration/plan_generator.go index fcd8d4a6..70c73bfa 100644 --- a/pkg/migration/plan_generator.go +++ b/pkg/migration/plan_generator.go @@ -9,20 +9,18 @@ import ( "reflect" "time" - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/rand" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/resource" - xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" xppkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" xppkgv1beta1 "github.com/crossplane/crossplane/apis/pkg/v1beta1" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/rand" ) const ( diff --git a/pkg/migration/plan_generator_test.go b/pkg/migration/plan_generator_test.go index eb16bdff..791d7cbf 100644 --- a/pkg/migration/plan_generator_test.go +++ b/pkg/migration/plan_generator_test.go @@ -11,7 +11,13 @@ import ( "regexp" "testing" - "github.com/crossplane/upjet/pkg/migration/fake" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/crossplane-runtime/pkg/test" + v1 "github.com/crossplane/crossplane/apis/apiextensions/v1" + xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" + xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" + xppkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" + xppkgv1beta1 "github.com/crossplane/crossplane/apis/pkg/v1beta1" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -21,14 +27,7 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" k8syaml "sigs.k8s.io/yaml" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" - "github.com/crossplane/crossplane-runtime/pkg/test" - - v1 "github.com/crossplane/crossplane/apis/apiextensions/v1" - xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" - xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" - xppkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" - xppkgv1beta1 "github.com/crossplane/crossplane/apis/pkg/v1beta1" + "github.com/crossplane/upjet/pkg/migration/fake" ) func TestGeneratePlan(t *testing.T) { diff --git a/pkg/migration/provider_package_steps.go b/pkg/migration/provider_package_steps.go index 5064b086..5e1f160e 100644 --- a/pkg/migration/provider_package_steps.go +++ b/pkg/migration/provider_package_steps.go @@ -8,10 +8,9 @@ import ( "fmt" "strings" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" ) const ( diff --git a/pkg/migration/registry.go b/pkg/migration/registry.go index 617dfd7b..c4970f0f 100644 --- a/pkg/migration/registry.go +++ b/pkg/migration/registry.go @@ -7,17 +7,15 @@ package migration import ( "regexp" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/crossplane/crossplane-runtime/pkg/resource" - xpv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" xpmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" xpmetav1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1" xppkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" xppkgv1beta1 "github.com/crossplane/crossplane/apis/pkg/v1beta1" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ) var ( diff --git a/pkg/pipeline/controller.go b/pkg/pipeline/controller.go index e34f05aa..40a7c2d0 100644 --- a/pkg/pipeline/controller.go +++ b/pkg/pipeline/controller.go @@ -9,10 +9,11 @@ import ( "path/filepath" "strings" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/pipeline/templates" "github.com/muvaf/typewriter/pkg/wrapper" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/pipeline/templates" ) // NewControllerGenerator returns a new ControllerGenerator. @@ -49,6 +50,7 @@ func (cg *ControllerGenerator) Generate(cfg *config.Resource, typesPkgPath strin "DisableNameInitializer": cfg.ExternalName.DisableNameInitializer, "TypePackageAlias": ctrlFile.Imports.UsePackage(typesPkgPath), "UseAsync": cfg.UseAsync, + "UseNoForkClient": cfg.ShouldUseNoForkClient(), "ResourceType": cfg.Name, "Initializers": cfg.InitializerFns, } diff --git a/pkg/pipeline/register.go b/pkg/pipeline/register.go index 5cd80fbb..f814547b 100644 --- a/pkg/pipeline/register.go +++ b/pkg/pipeline/register.go @@ -9,9 +9,10 @@ import ( "path/filepath" "sort" - "github.com/crossplane/upjet/pkg/pipeline/templates" "github.com/muvaf/typewriter/pkg/wrapper" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/pipeline/templates" ) // NewRegisterGenerator returns a new RegisterGenerator. diff --git a/pkg/pipeline/run.go b/pkg/pipeline/run.go index 4b1e3353..4d707984 100644 --- a/pkg/pipeline/run.go +++ b/pkg/pipeline/run.go @@ -11,10 +11,10 @@ import ( "sort" "strings" + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/examples" - - "github.com/crossplane/crossplane-runtime/pkg/errors" ) type terraformedInput struct { diff --git a/pkg/pipeline/setup.go b/pkg/pipeline/setup.go index 121f6cba..2b85b276 100644 --- a/pkg/pipeline/setup.go +++ b/pkg/pipeline/setup.go @@ -12,10 +12,11 @@ import ( "sort" "text/template" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/pipeline/templates" "github.com/muvaf/typewriter/pkg/wrapper" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/pipeline/templates" ) // NewProviderGenerator returns a new ProviderGenerator. diff --git a/pkg/pipeline/templates/controller.go.tmpl b/pkg/pipeline/templates/controller.go.tmpl index be17d390..e75d3714 100644 --- a/pkg/pipeline/templates/controller.go.tmpl +++ b/pkg/pipeline/templates/controller.go.tmpl @@ -42,17 +42,47 @@ func Setup(mgr ctrl.Manager, o tjcontroller.Options) error { } eventHandler := handler.NewEventHandler(handler.WithLogger(o.Logger.WithValues("gvk", {{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind))) {{- if .UseAsync }} - ac := tjcontroller.NewAPICallbacks(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind), tjcontroller.WithEventHandler(eventHandler)) + ac := tjcontroller.NewAPICallbacks(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind), tjcontroller.WithEventHandler(eventHandler){{ if .UseNoForkClient }}, tjcontroller.WithStatusUpdates(false){{ end }}) {{- end}} opts := []managed.ReconcilerOption{ - managed.WithExternalConnecter(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}} - )), + managed.WithExternalConnecter( + {{- if .UseNoForkClient -}} + {{- if .UseAsync }} + tjcontroller.NewNoForkAsyncConnector(mgr.GetClient(), o.OperationTrackerStore, o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], + tjcontroller.WithNoForkAsyncLogger(o.Logger), + tjcontroller.WithNoForkAsyncConnectorEventHandler(eventHandler), + tjcontroller.WithNoForkAsyncCallbackProvider(ac), + tjcontroller.WithNoForkAsyncMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), + {{if .FeaturesPackageAlias -}} + tjcontroller.WithNoForkAsyncManagementPolicies(o.Features.Enabled({{ .FeaturesPackageAlias }}EnableBetaManagementPolicies)) + {{- end -}} + ) + {{- else -}} + tjcontroller.NewNoForkConnector(mgr.GetClient(), o.SetupFn, o.Provider.Resources["{{ .ResourceType }}"], o.OperationTrackerStore, + tjcontroller.WithNoForkLogger(o.Logger), + tjcontroller.WithNoForkMetricRecorder(metrics.NewMetricRecorder({{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind, mgr, o.PollInterval)), + {{if .FeaturesPackageAlias -}} + tjcontroller.WithNoForkManagementPolicies(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 }} + tjcontroller.WithCallbackProvider(ac), + {{- end }} + ) + {{- end -}} + ), managed.WithLogger(o.Logger.WithValues("controller", name)), managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), - managed.WithFinalizer(terraform.NewWorkspaceFinalizer(o.WorkspaceStore, xpresource.NewAPIFinalizer(mgr.GetClient(), managed.FinalizerName))), + {{- if .UseNoForkClient }} + {{- if .UseAsync }} + managed.WithFinalizer(tjcontroller.NewNoForkFinalizer(o.OperationTrackerStore, xpresource.NewAPIFinalizer(mgr.GetClient(), managed.FinalizerName))), + {{- end }} + {{- else }} + managed.WithFinalizer(terraform.NewWorkspaceFinalizer(o.WorkspaceStore, xpresource.NewAPIFinalizer(mgr.GetClient(), managed.FinalizerName))), + {{- end }} managed.WithTimeout(3*time.Minute), managed.WithInitializers(initializers), managed.WithConnectionPublishers(cps...), diff --git a/pkg/pipeline/templates/terraformed.go.tmpl b/pkg/pipeline/templates/terraformed.go.tmpl index ee503fc4..4681e523 100644 --- a/pkg/pipeline/templates/terraformed.go.tmpl +++ b/pkg/pipeline/templates/terraformed.go.tmpl @@ -9,6 +9,7 @@ package {{ .APIVersion }} import ( + "dario.cat/mergo" "github.com/pkg/errors" "github.com/crossplane/upjet/pkg/resource" @@ -86,6 +87,36 @@ import ( return base, json.TFParser.Unmarshal(p, &base) } + // GetInitParameters of this {{ .CRD.Kind }} + func (tr *{{ .CRD.Kind }}) GetMergedParameters(shouldMergeInitProvider bool) (map[string]any, error) { + params, err := tr.GetParameters() + if err != nil { + return nil, errors.Wrapf(err, "cannot get parameters for resource '%q'", tr.GetName()) + } + if !shouldMergeInitProvider { + return params, nil + } + + initParams, err := tr.GetInitParameters() + if err != nil { + return nil, errors.Wrapf(err, "cannot get init parameters for resource '%q'", tr.GetName()) + } + + // Note(lsviben): mergo.WithSliceDeepCopy is needed to merge the + // slices from the initProvider to forProvider. As it also sets + // overwrite to true, we need to set it back to false, we don't + // want to overwrite the forProvider fields with the initProvider + // fields. + err = mergo.Merge(¶ms, initParams, mergo.WithSliceDeepCopy, func(c *mergo.Config) { + c.Overwrite = false + }) + if err != nil { + return nil, errors.Wrapf(err, "cannot merge spec.initProvider and spec.forProvider parameters for resource '%q'", tr.GetName()) + } + + return params, nil + } + // LateInitialize this {{ .CRD.Kind }} using its observed tfState. // returns True if there are any spec changes for the resource. func (tr *{{ .CRD.Kind }}) LateInitialize(attrs []byte) (bool, error) { diff --git a/pkg/pipeline/terraformed.go b/pkg/pipeline/terraformed.go index 10796b22..fea56e6a 100644 --- a/pkg/pipeline/terraformed.go +++ b/pkg/pipeline/terraformed.go @@ -10,9 +10,10 @@ import ( "path/filepath" "strings" - "github.com/crossplane/upjet/pkg/pipeline/templates" "github.com/muvaf/typewriter/pkg/wrapper" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/pipeline/templates" ) // NewTerraformedGenerator returns a new TerraformedGenerator. diff --git a/pkg/pipeline/version.go b/pkg/pipeline/version.go index c155a061..d0093b82 100644 --- a/pkg/pipeline/version.go +++ b/pkg/pipeline/version.go @@ -10,9 +10,10 @@ import ( "path/filepath" "strings" - "github.com/crossplane/upjet/pkg/pipeline/templates" "github.com/muvaf/typewriter/pkg/wrapper" "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/pipeline/templates" ) // NewVersionGenerator returns a new VersionGenerator. diff --git a/pkg/registry/meta_test.go b/pkg/registry/meta_test.go index 4fb46975..84a6717d 100644 --- a/pkg/registry/meta_test.go +++ b/pkg/registry/meta_test.go @@ -8,12 +8,11 @@ import ( "os" "testing" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + xptest "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "gopkg.in/yaml.v3" - - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" - xptest "github.com/crossplane/crossplane-runtime/pkg/test" ) func TestScrapeRepo(t *testing.T) { diff --git a/pkg/registry/reference/references.go b/pkg/registry/reference/references.go index f15b6dec..fc9fcc99 100644 --- a/pkg/registry/reference/references.go +++ b/pkg/registry/reference/references.go @@ -8,10 +8,11 @@ import ( "fmt" "strings" + "github.com/pkg/errors" + "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/registry" "github.com/crossplane/upjet/pkg/types" - "github.com/pkg/errors" ) const ( diff --git a/pkg/registry/reference/resolver.go b/pkg/registry/reference/resolver.go index 63941112..6fc6dccf 100644 --- a/pkg/registry/reference/resolver.go +++ b/pkg/registry/reference/resolver.go @@ -10,12 +10,12 @@ import ( "strconv" "strings" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/pkg/errors" + "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/registry" "github.com/crossplane/upjet/pkg/resource/json" - "github.com/pkg/errors" - - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" ) const ( diff --git a/pkg/registry/resource.go b/pkg/registry/resource.go index 53e9f781..e8f949b5 100644 --- a/pkg/registry/resource.go +++ b/pkg/registry/resource.go @@ -5,11 +5,11 @@ package registry import ( - "github.com/crossplane/upjet/pkg/resource/json" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/pkg/errors" "gopkg.in/yaml.v2" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/crossplane/upjet/pkg/resource/json" ) const ( diff --git a/pkg/resource/conditions.go b/pkg/resource/conditions.go index 62e8462b..24ae35ab 100644 --- a/pkg/resource/conditions.go +++ b/pkg/resource/conditions.go @@ -5,12 +5,12 @@ package resource import ( - tferrors "github.com/crossplane/upjet/pkg/terraform/errors" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + tferrors "github.com/crossplane/upjet/pkg/terraform/errors" ) // Condition constants. @@ -18,12 +18,15 @@ const ( TypeLastAsyncOperation = "LastAsyncOperation" TypeAsyncOperation = "AsyncOperation" - ReasonApplyFailure xpv1.ConditionReason = "ApplyFailure" - ReasonDestroyFailure xpv1.ConditionReason = "DestroyFailure" - ReasonSuccess xpv1.ConditionReason = "Success" - ReasonOngoing xpv1.ConditionReason = "Ongoing" - ReasonFinished xpv1.ConditionReason = "Finished" - ReasonResourceUpToDate xpv1.ConditionReason = "UpToDate" + ReasonApplyFailure xpv1.ConditionReason = "ApplyFailure" + ReasonDestroyFailure xpv1.ConditionReason = "DestroyFailure" + ReasonAsyncCreateFailure xpv1.ConditionReason = "AsyncCreateFailure" + ReasonAsyncUpdateFailure xpv1.ConditionReason = "AsyncUpdateFailure" + ReasonAsyncDeleteFailure xpv1.ConditionReason = "AsyncDeleteFailure" + ReasonSuccess xpv1.ConditionReason = "Success" + ReasonOngoing xpv1.ConditionReason = "Ongoing" + ReasonFinished xpv1.ConditionReason = "Finished" + ReasonResourceUpToDate xpv1.ConditionReason = "UpToDate" ) // LastAsyncOperationCondition returns the condition depending on the content @@ -53,6 +56,30 @@ func LastAsyncOperationCondition(err error) xpv1.Condition { Reason: ReasonDestroyFailure, Message: err.Error(), } + case tferrors.IsAsyncCreateFailed(err): + return xpv1.Condition{ + Type: TypeLastAsyncOperation, + Status: corev1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: ReasonAsyncCreateFailure, + Message: err.Error(), + } + case tferrors.IsAsyncUpdateFailed(err): + return xpv1.Condition{ + Type: TypeLastAsyncOperation, + Status: corev1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: ReasonAsyncUpdateFailure, + Message: err.Error(), + } + case tferrors.IsAsyncDeleteFailed(err): + return xpv1.Condition{ + Type: TypeLastAsyncOperation, + Status: corev1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: ReasonAsyncDeleteFailure, + Message: err.Error(), + } default: return xpv1.Condition{ Type: "Unknown", diff --git a/pkg/resource/fake/terraformed.go b/pkg/resource/fake/terraformed.go index 857788b8..ed71b356 100644 --- a/pkg/resource/fake/terraformed.go +++ b/pkg/resource/fake/terraformed.go @@ -5,11 +5,10 @@ package fake import ( + "github.com/crossplane/crossplane-runtime/pkg/resource/fake" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/json" - - "github.com/crossplane/crossplane-runtime/pkg/resource/fake" ) // Observable is mock Observable. @@ -46,6 +45,10 @@ type Parameterizable struct { InitParameters map[string]any } +func (t *Terraformed) GetMergedParameters(_ bool) (map[string]any, error) { + return t.Parameters, nil +} + // GetParameters is a mock. func (p *Parameterizable) GetParameters() (map[string]any, error) { return p.Parameters, nil diff --git a/pkg/resource/interfaces.go b/pkg/resource/interfaces.go index 2c68ea1d..6243181e 100644 --- a/pkg/resource/interfaces.go +++ b/pkg/resource/interfaces.go @@ -21,6 +21,7 @@ type Parameterizable interface { GetParameters() (map[string]any, error) SetParameters(map[string]any) error GetInitParameters() (map[string]any, error) + GetMergedParameters(shouldMergeInitProvider bool) (map[string]any, error) } // MetadataProvider provides Terraform metadata for the Terraform managed diff --git a/pkg/resource/lateinit.go b/pkg/resource/lateinit.go index 9d9ffa9a..6d592306 100644 --- a/pkg/resource/lateinit.go +++ b/pkg/resource/lateinit.go @@ -10,12 +10,12 @@ import ( "runtime/debug" "strings" - "github.com/crossplane/upjet/pkg/config" + xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/upjet/pkg/config" ) const ( diff --git a/pkg/resource/sensitive.go b/pkg/resource/sensitive.go index 70b21454..229ddd97 100644 --- a/pkg/resource/sensitive.go +++ b/pkg/resource/sensitive.go @@ -10,15 +10,15 @@ import ( "regexp" "strings" - "github.com/crossplane/upjet/pkg/config" - "github.com/pkg/errors" - kerrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/crossplane/upjet/pkg/config" ) const ( diff --git a/pkg/resource/sensitive_test.go b/pkg/resource/sensitive_test.go index 4aa71919..de2aabc8 100644 --- a/pkg/resource/sensitive_test.go +++ b/pkg/resource/sensitive_test.go @@ -8,10 +8,9 @@ import ( "context" "testing" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource/fake" - "github.com/crossplane/upjet/pkg/resource/fake/mocks" - "github.com/crossplane/upjet/pkg/resource/json" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" @@ -20,9 +19,10 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource/fake" + "github.com/crossplane/upjet/pkg/resource/fake/mocks" + "github.com/crossplane/upjet/pkg/resource/json" ) var ( diff --git a/pkg/terraform/errors/errors.go b/pkg/terraform/errors/errors.go index 58bb4d6c..66fa95e5 100644 --- a/pkg/terraform/errors/errors.go +++ b/pkg/terraform/errors/errors.go @@ -184,3 +184,66 @@ func IsRetryScheduleError(err error) bool { r := &retrySchedule{} return errors.As(err, &r) } + +type asyncCreateFailed struct { + error +} + +// NewAsyncCreateFailed returns a new async crate failure. +func NewAsyncCreateFailed(err error) error { + if err == nil { + return nil + } + return &asyncCreateFailed{ + error: errors.Wrap(err, "async create failed"), + } +} + +// IsAsyncCreateFailed returns whether error is due to failure of +// an async create operation. +func IsAsyncCreateFailed(err error) bool { + r := &asyncCreateFailed{} + return errors.As(err, &r) +} + +type asyncUpdateFailed struct { + error +} + +// NewAsyncUpdateFailed returns a new async update failure. +func NewAsyncUpdateFailed(err error) error { + if err == nil { + return nil + } + return &asyncUpdateFailed{ + error: errors.Wrap(err, "async update failed"), + } +} + +// IsAsyncUpdateFailed returns whether error is due to failure of +// an async update operation. +func IsAsyncUpdateFailed(err error) bool { + r := &asyncUpdateFailed{} + return errors.As(err, &r) +} + +type asyncDeleteFailed struct { + error +} + +// NewAsyncDeleteFailed returns a new async delete failure. +func NewAsyncDeleteFailed(err error) error { + if err == nil { + return nil + } + return &asyncDeleteFailed{ + error: errors.Wrap(err, "async delete failed"), + } +} + +// IsAsyncDeleteFailed returns whether error is due to failure of +// an async delete operation. +func IsAsyncDeleteFailed(err error) bool { + r := &asyncDeleteFailed{} + return errors.As(err, &r) +} diff --git a/pkg/terraform/errors/errors_test.go b/pkg/terraform/errors/errors_test.go index e1602254..a554c24b 100644 --- a/pkg/terraform/errors/errors_test.go +++ b/pkg/terraform/errors/errors_test.go @@ -321,3 +321,291 @@ func TestNewPlanFailed(t *testing.T) { }) } } + +func TestNewRetryScheduleError(t *testing.T) { + type args struct { + invocationCount, ttl int + } + tests := map[string]struct { + args + wantErrMessage string + }{ + "Successful": { + args: args{ + invocationCount: 101, + ttl: 100, + }, + wantErrMessage: "native provider reuse budget has been exceeded: invocationCount: 101, ttl: 100", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := NewRetryScheduleError(tc.args.invocationCount, tc.args.ttl) + got := err.Error() + if diff := cmp.Diff(tc.wantErrMessage, got); diff != "" { + t.Errorf("\nNewRetryScheduleError(...): -want message, +got message:\n%s", diff) + } + }) + } +} + +func TestIsRetryScheduleError(t *testing.T) { + var nilErr *retrySchedule + type args struct { + err error + } + tests := map[string]struct { + args + want bool + }{ + "NilError": { + args: args{}, + want: false, + }, + "NilRetryScheduleError": { + args: args{ + err: nilErr, + }, + want: true, + }, + "NonRetryScheduleError": { + args: args{ + err: errorBoom, + }, + want: false, + }, + "Successful": { + args: args{err: NewRetryScheduleError(101, 100)}, + want: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := IsRetryScheduleError(tc.args.err); got != tc.want { + t.Errorf("IsRetryScheduleError() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestNewAsyncCreateFailed(t *testing.T) { + type args struct { + err error + } + tests := map[string]struct { + args + wantErrMessage string + }{ + "Successful": { + args: args{ + err: errors.New("already exists"), + }, + wantErrMessage: "async create failed: already exists", + }, + "Nil": { + args: args{ + err: nil, + }, + wantErrMessage: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := NewAsyncCreateFailed(tc.args.err) + got := "" + if err != nil { + got = err.Error() + } + if diff := cmp.Diff(tc.wantErrMessage, got); diff != "" { + t.Errorf("\nNewAsyncCreateFailed(...): -want message, +got message:\n%s", diff) + } + }) + } +} + +func TestIsAsyncCreateFailed(t *testing.T) { + var nilErr *asyncCreateFailed + type args struct { + err error + } + tests := map[string]struct { + args + want bool + }{ + "NilError": { + args: args{}, + want: false, + }, + "NilAsyncCreateError": { + args: args{ + err: nilErr, + }, + want: true, + }, + "NonAsyncCreateError": { + args: args{ + err: errorBoom, + }, + want: false, + }, + "Successful": { + args: args{err: NewAsyncCreateFailed(errors.New("test"))}, + want: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := IsAsyncCreateFailed(tc.args.err); got != tc.want { + t.Errorf("IsAsyncCreateFailed() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestAsyncUpdateFailed(t *testing.T) { + type args struct { + err error + } + tests := map[string]struct { + args + wantErrMessage string + }{ + "Successful": { + args: args{ + err: errors.New("immutable field"), + }, + wantErrMessage: "async update failed: immutable field", + }, + "Nil": { + args: args{ + err: nil, + }, + wantErrMessage: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := NewAsyncUpdateFailed(tc.args.err) + got := "" + if err != nil { + got = err.Error() + } + if diff := cmp.Diff(tc.wantErrMessage, got); diff != "" { + t.Errorf("\nAsyncUpdateFailed(...): -want message, +got message:\n%s", diff) + } + }) + } +} + +func TestIsAsyncUpdateFailed(t *testing.T) { + var nilErr *asyncUpdateFailed + type args struct { + err error + } + tests := map[string]struct { + args + want bool + }{ + "NilError": { + args: args{}, + want: false, + }, + "NilAsyncUpdateError": { + args: args{ + err: nilErr, + }, + want: true, + }, + "NonAsyncUpdateError": { + args: args{ + err: errorBoom, + }, + want: false, + }, + "Successful": { + args: args{err: NewAsyncUpdateFailed(errors.New("test"))}, + want: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := IsAsyncUpdateFailed(tc.args.err); got != tc.want { + t.Errorf("IsAsyncUpdateFailed() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestAsyncDeleteFailed(t *testing.T) { + type args struct { + err error + } + tests := map[string]struct { + args + wantErrMessage string + }{ + "Successful": { + args: args{ + err: errors.New("dependency violation"), + }, + wantErrMessage: "async delete failed: dependency violation", + }, + "Nil": { + args: args{ + err: nil, + }, + wantErrMessage: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := NewAsyncDeleteFailed(tc.args.err) + got := "" + if err != nil { + got = err.Error() + } + if diff := cmp.Diff(tc.wantErrMessage, got); diff != "" { + t.Errorf("\nAsyncDeleteFailed(...): -want message, +got message:\n%s", diff) + } + }) + } +} + +func TestIsAsyncDeleteFailed(t *testing.T) { + var nilErr *asyncDeleteFailed + type args struct { + err error + } + tests := map[string]struct { + args + want bool + }{ + "NilError": { + args: args{}, + want: false, + }, + "NilAsyncUpdateError": { + args: args{ + err: nilErr, + }, + want: true, + }, + "NonAsyncUpdateError": { + args: args{ + err: errorBoom, + }, + want: false, + }, + "Successful": { + args: args{err: NewAsyncDeleteFailed(errors.New("test"))}, + want: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := IsAsyncDeleteFailed(tc.args.err); got != tc.want { + t.Errorf("IsAsyncDeleteFailed() = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index a8483012..78adac16 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -12,14 +12,14 @@ import ( "strings" "dario.cat/mergo" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/resource/json" + "github.com/crossplane/crossplane-runtime/pkg/feature" + "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/pkg/errors" "github.com/spf13/afero" - "github.com/crossplane/crossplane-runtime/pkg/feature" - "github.com/crossplane/crossplane-runtime/pkg/meta" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/resource/json" ) const ( @@ -184,7 +184,7 @@ func (fp *FileProducer) WriteMainTF() (ProviderHandle, error) { // EnsureTFState writes the Terraform state that should exist in the filesystem // to start any Terraform operation. -func (fp *FileProducer) EnsureTFState(ctx context.Context, tfID string) error { +func (fp *FileProducer) EnsureTFState(_ context.Context, tfID string) error { // TODO(muvaf): Reduce the cyclomatic complexity by separating the attributes // generation into its own function/interface. empty, err := fp.isStateEmpty() diff --git a/pkg/terraform/files_test.go b/pkg/terraform/files_test.go index 975b687d..926ac1f2 100644 --- a/pkg/terraform/files_test.go +++ b/pkg/terraform/files_test.go @@ -11,19 +11,19 @@ import ( "testing" "time" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/resource/fake" - "github.com/crossplane/upjet/pkg/resource/json" + "github.com/crossplane/crossplane-runtime/pkg/feature" + "github.com/crossplane/crossplane-runtime/pkg/meta" + xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "github.com/spf13/afero" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/crossplane/crossplane-runtime/pkg/feature" - "github.com/crossplane/crossplane-runtime/pkg/meta" - xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/resource/fake" + "github.com/crossplane/upjet/pkg/resource/json" ) const ( diff --git a/pkg/terraform/finalizer.go b/pkg/terraform/finalizer.go index bff2607e..cf0043f1 100644 --- a/pkg/terraform/finalizer.go +++ b/pkg/terraform/finalizer.go @@ -7,9 +7,8 @@ package terraform import ( "context" - "github.com/pkg/errors" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/pkg/errors" ) const ( diff --git a/pkg/terraform/finalizer_test.go b/pkg/terraform/finalizer_test.go index f2be6d11..1803950a 100644 --- a/pkg/terraform/finalizer_test.go +++ b/pkg/terraform/finalizer_test.go @@ -8,13 +8,13 @@ import ( "context" "testing" - "github.com/crossplane/upjet/pkg/resource" - "github.com/google/go-cmp/cmp" - "github.com/pkg/errors" - "github.com/crossplane/crossplane-runtime/pkg/logging" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" + + "github.com/crossplane/upjet/pkg/resource" ) var ( diff --git a/pkg/terraform/operation.go b/pkg/terraform/operation.go index 8f71ddb0..3229d2fa 100644 --- a/pkg/terraform/operation.go +++ b/pkg/terraform/operation.go @@ -15,6 +15,7 @@ type Operation struct { startTime *time.Time endTime *time.Time + err error mu sync.RWMutex } @@ -49,6 +50,7 @@ func (o *Operation) Flush() { o.Type = "" o.startTime = nil o.endTime = nil + o.err = nil } // IsEnded returns whether the operation has ended, regardless of its result. @@ -78,3 +80,17 @@ func (o *Operation) EndTime() time.Time { defer o.mu.RUnlock() return *o.endTime } + +// SetError records the given error on the current operation. +func (o *Operation) SetError(err error) { + o.mu.Lock() + defer o.mu.Unlock() + o.err = err +} + +// Error returns the recorded error of the current operation. +func (o *Operation) Error() error { + o.mu.RLock() + defer o.mu.RUnlock() + return o.err +} diff --git a/pkg/terraform/provider_runner.go b/pkg/terraform/provider_runner.go index 6653995e..e9753fca 100644 --- a/pkg/terraform/provider_runner.go +++ b/pkg/terraform/provider_runner.go @@ -12,11 +12,10 @@ import ( "sync" "time" + "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/pkg/errors" "k8s.io/utils/clock" "k8s.io/utils/exec" - - "github.com/crossplane/crossplane-runtime/pkg/logging" ) const ( diff --git a/pkg/terraform/provider_runner_test.go b/pkg/terraform/provider_runner_test.go index bd558110..fd584c4d 100644 --- a/pkg/terraform/provider_runner_test.go +++ b/pkg/terraform/provider_runner_test.go @@ -14,14 +14,13 @@ import ( "testing" "time" + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" clock "k8s.io/utils/clock/testing" "k8s.io/utils/exec" testingexec "k8s.io/utils/exec/testing" - - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/test" ) func TestStartSharedServer(t *testing.T) { diff --git a/pkg/terraform/provider_scheduler.go b/pkg/terraform/provider_scheduler.go index 60abc288..38240f9e 100644 --- a/pkg/terraform/provider_scheduler.go +++ b/pkg/terraform/provider_scheduler.go @@ -7,10 +7,10 @@ package terraform import ( "sync" - tferrors "github.com/crossplane/upjet/pkg/terraform/errors" + "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/pkg/errors" - "github.com/crossplane/crossplane-runtime/pkg/logging" + tferrors "github.com/crossplane/upjet/pkg/terraform/errors" ) // ProviderHandle represents native plugin (Terraform provider) process diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 2f9b7f57..862dab7c 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -15,9 +15,10 @@ import ( "sync" "time" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/metrics" - "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/crossplane-runtime/pkg/feature" + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/crossplane-runtime/pkg/meta" + xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/mitchellh/go-ps" "github.com/pkg/errors" "github.com/spf13/afero" @@ -25,10 +26,9 @@ import ( "k8s.io/utils/exec" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/crossplane/crossplane-runtime/pkg/feature" - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/meta" - xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/metrics" + "github.com/crossplane/upjet/pkg/resource" ) const ( @@ -120,6 +120,8 @@ type Setup struct { // the lifecycle of Terraform provider processes will be managed by // the Terraform CLI. Scheduler ProviderScheduler + + Meta any } // Map returns the Setup object in map form. The initial reason was so that diff --git a/pkg/terraform/timeouts.go b/pkg/terraform/timeouts.go index 93c3d240..c3defebd 100644 --- a/pkg/terraform/timeouts.go +++ b/pkg/terraform/timeouts.go @@ -5,10 +5,10 @@ package terraform import ( + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/resource/json" - - "github.com/crossplane/crossplane-runtime/pkg/errors" ) // "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" is a hardcoded string for Terraform diff --git a/pkg/terraform/timeouts_test.go b/pkg/terraform/timeouts_test.go index cc70cd72..3da97f9f 100644 --- a/pkg/terraform/timeouts_test.go +++ b/pkg/terraform/timeouts_test.go @@ -8,10 +8,9 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" - "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/google/go-cmp/cmp" ) func TestTimeoutsAsParameter(t *testing.T) { diff --git a/pkg/terraform/workspace.go b/pkg/terraform/workspace.go index beb6d2da..63b39d51 100644 --- a/pkg/terraform/workspace.go +++ b/pkg/terraform/workspace.go @@ -13,15 +13,15 @@ import ( "sync" "time" - "github.com/crossplane/upjet/pkg/metrics" - "github.com/crossplane/upjet/pkg/resource" - "github.com/crossplane/upjet/pkg/resource/json" - tferrors "github.com/crossplane/upjet/pkg/terraform/errors" + "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/pkg/errors" "github.com/spf13/afero" k8sExec "k8s.io/utils/exec" - "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/crossplane/upjet/pkg/metrics" + "github.com/crossplane/upjet/pkg/resource" + "github.com/crossplane/upjet/pkg/resource/json" + tferrors "github.com/crossplane/upjet/pkg/terraform/errors" ) const ( diff --git a/pkg/terraform/workspace_test.go b/pkg/terraform/workspace_test.go index b33c88ee..b0efaeed 100644 --- a/pkg/terraform/workspace_test.go +++ b/pkg/terraform/workspace_test.go @@ -9,15 +9,15 @@ import ( "testing" "time" - "github.com/crossplane/upjet/pkg/resource/json" - tferrors "github.com/crossplane/upjet/pkg/terraform/errors" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "github.com/spf13/afero" k8sExec "k8s.io/utils/exec" testingexec "k8s.io/utils/exec/testing" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/resource/json" + tferrors "github.com/crossplane/upjet/pkg/terraform/errors" ) var ( diff --git a/pkg/types/builder.go b/pkg/types/builder.go index 88b91039..b4dbc3ef 100644 --- a/pkg/types/builder.go +++ b/pkg/types/builder.go @@ -11,13 +11,13 @@ import ( "sort" "strings" - "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" twtypes "github.com/muvaf/typewriter/pkg/types" "github.com/pkg/errors" "k8s.io/utils/ptr" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/crossplane/upjet/pkg/config" ) const ( @@ -93,7 +93,8 @@ func (g *Builder) buildResource(res *schema.Resource, cfg *config.Resource, tfPa r := &resource{} for _, snakeFieldName := range keys { var reference *config.Reference - ref, ok := cfg.References[fieldPath(append(tfPath, snakeFieldName))] + cPath := fieldPath(append(tfPath, snakeFieldName)) + ref, ok := cfg.References[cPath] // if a reference is configured and the field does not belong to status if ok && !IsObservation(res.Schema[snakeFieldName]) { reference = &ref @@ -121,7 +122,7 @@ func (g *Builder) buildResource(res *schema.Resource, cfg *config.Resource, tfPa return nil, nil, nil, err } } - f.AddToResource(g, r, typeNames) + f.AddToResource(g, r, typeNames, cfg.SchemaElementOptions.AddToObservation(cPath)) } paramType, obsType, initType := g.AddToBuilder(typeNames, r) diff --git a/pkg/types/builder_test.go b/pkg/types/builder_test.go index 86fba91e..0322f2e9 100644 --- a/pkg/types/builder_test.go +++ b/pkg/types/builder_test.go @@ -10,12 +10,12 @@ import ( "go/types" "testing" - "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pkg/errors" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/config" ) func TestBuilder_generateTypeName(t *testing.T) { diff --git a/pkg/types/comments/comment.go b/pkg/types/comments/comment.go index baaec46e..53f583b2 100644 --- a/pkg/types/comments/comment.go +++ b/pkg/types/comments/comment.go @@ -90,3 +90,13 @@ func (c *Comment) Build() string { all := strings.ReplaceAll("// "+c.String(), "\n", "\n// ") return strings.TrimSuffix(all, "// ") } + +// CommentWithoutOptions returns a new Comment without the Options. +func (c *Comment) CommentWithoutOptions() *Comment { + if c == nil { + return nil + } + return &Comment{ + Text: c.Text, + } +} diff --git a/pkg/types/comments/comment_test.go b/pkg/types/comments/comment_test.go index 8050d203..8f561d1b 100644 --- a/pkg/types/comments/comment_test.go +++ b/pkg/types/comments/comment_test.go @@ -8,12 +8,12 @@ import ( "reflect" "testing" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/types/markers" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" - "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/types/markers" ) func TestComment_Build(t *testing.T) { diff --git a/pkg/types/field.go b/pkg/types/field.go index 838d9073..4a130eb7 100644 --- a/pkg/types/field.go +++ b/pkg/types/field.go @@ -12,13 +12,14 @@ import ( "sort" "strings" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pkg/errors" + "k8s.io/utils/ptr" + "github.com/crossplane/upjet/pkg" "github.com/crossplane/upjet/pkg/config" "github.com/crossplane/upjet/pkg/types/comments" "github.com/crossplane/upjet/pkg/types/name" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/pkg/errors" - "k8s.io/utils/ptr" ) var parentheses = regexp.MustCompile(`\(([^)]+)\)`) @@ -213,15 +214,21 @@ func NewReferenceField(g *Builder, cfg *config.Resource, r *resource, sch *schem } // AddToResource adds built field to the resource. -func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames) { - if f.Comment.UpjetOptions.FieldTFTag != nil { - f.TFTag = *f.Comment.UpjetOptions.FieldTFTag - } +func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames, addToObservation bool) { //nolint:gocyclo if f.Comment.UpjetOptions.FieldJSONTag != nil { f.JSONTag = *f.Comment.UpjetOptions.FieldJSONTag } field := types.NewField(token.NoPos, g.Package, f.FieldNameCamel, f.FieldType, false) + // if the field is explicitly configured to be added to + // the Observation type + if addToObservation { + r.addObservationField(f, field) + } + + if f.Comment.UpjetOptions.FieldTFTag != nil { + f.TFTag = *f.Comment.UpjetOptions.FieldTFTag + } // Note(turkenh): We want atProvider to be a superset of forProvider, so // we always add the field as an observation field and then add it as a @@ -230,9 +237,10 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames) { // We do this only if tf tag is not set to "-" because otherwise it won't // be populated from the tfstate. We typically set tf tag to "-" for // sensitive fields which were replaced with secretKeyRefs. - if f.TFTag != "-" { + if f.TFTag != "-" && !addToObservation { r.addObservationField(f, field) } + if !IsObservation(f.Schema) { if f.AsBlocksMode { f.TFTag = strings.TrimSuffix(f.TFTag, ",omitempty") @@ -259,12 +267,16 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames) { f.Comment.Required = nil g.comments.AddFieldComment(typeNames.InitTypeName, f.FieldNameCamel, f.Comment.Build()) - // Note(turkenh): We don't want reference resolver to be generated for - // fields under status.atProvider. So, we don't want reference comments to - // be added, hence we are unsetting reference on the field comment just - // before adding it as an observation field. - f.Comment.Reference = config.Reference{} - g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.Build()) + if addToObservation { + g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.CommentWithoutOptions().Build()) + } else { + // Note(turkenh): We don't want reference resolver to be generated for + // fields under status.atProvider. So, we don't want reference comments to + // be added, hence we are unsetting reference on the field comment just + // before adding it as an observation field. + f.Comment.Reference = config.Reference{} + g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.Build()) + } } // isInit returns true if the field should be added to initProvider. diff --git a/pkg/types/markers/crossplane_test.go b/pkg/types/markers/crossplane_test.go index 58269280..dbf4e1c6 100644 --- a/pkg/types/markers/crossplane_test.go +++ b/pkg/types/markers/crossplane_test.go @@ -7,8 +7,9 @@ package markers import ( "testing" - "github.com/crossplane/upjet/pkg/config" "github.com/google/go-cmp/cmp" + + "github.com/crossplane/upjet/pkg/config" ) func TestCrossplaneOptions_String(t *testing.T) { diff --git a/pkg/types/markers/terrajet_test.go b/pkg/types/markers/terrajet_test.go index 88d526e5..e196a4af 100644 --- a/pkg/types/markers/terrajet_test.go +++ b/pkg/types/markers/terrajet_test.go @@ -8,10 +8,9 @@ import ( "fmt" "testing" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" - - "github.com/crossplane/crossplane-runtime/pkg/test" ) func Test_parseAsUpjetOption(t *testing.T) { diff --git a/pkg/types/reference.go b/pkg/types/reference.go index 9fcf78f2..72c14c5c 100644 --- a/pkg/types/reference.go +++ b/pkg/types/reference.go @@ -11,10 +11,11 @@ import ( "reflect" "strings" + "k8s.io/utils/ptr" + "github.com/crossplane/upjet/pkg/types/comments" "github.com/crossplane/upjet/pkg/types/markers" "github.com/crossplane/upjet/pkg/types/name" - "k8s.io/utils/ptr" ) const ( diff --git a/pkg/types/reference_test.go b/pkg/types/reference_test.go index cf012348..ed239af8 100644 --- a/pkg/types/reference_test.go +++ b/pkg/types/reference_test.go @@ -9,10 +9,11 @@ import ( "go/types" "testing" - "github.com/crossplane/upjet/pkg/config" - "github.com/crossplane/upjet/pkg/types/name" "github.com/google/go-cmp/cmp" twtypes "github.com/muvaf/typewriter/pkg/types" + + "github.com/crossplane/upjet/pkg/config" + "github.com/crossplane/upjet/pkg/types/name" ) func TestBuilder_generateReferenceFields(t *testing.T) {