From 8332b9d2d8031f41bfdf616af6ed95d227f41e67 Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Wed, 5 Jan 2022 11:41:15 +0300 Subject: [PATCH 1/2] Configurable operation timeouts Signed-off-by: Hasan Turken --- pkg/config/resource.go | 19 +++++++++++++++ pkg/terraform/files.go | 24 +++++++++++++++++++ pkg/terraform/files_test.go | 47 ++++++++++++++++++++++++++++++++++--- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 5671ebda..fca4fd11 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -18,6 +18,7 @@ package config import ( "context" + "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pkg/errors" @@ -184,6 +185,21 @@ func (s *Sensitive) AddFieldPath(tf, xp string) { s.fieldPaths[tf] = xp } +// OperationTimeouts allows configuring resource operation timeouts: +// https://www.terraform.io/language/resources/syntax#operation-timeouts +// Please note that, not all resources support configuring timeouts. +type OperationTimeouts struct { + Read *time.Duration + Create *time.Duration + Update *time.Duration + Delete *time.Duration +} + +// TimeDuration returns a pointer to a given time.Duration. +func TimeDuration(d time.Duration) *time.Duration { + return &d +} + // Resource is the set of information that you can override at different steps // of the code generation pipeline. type Resource struct { @@ -212,6 +228,9 @@ type Resource struct { // databases. UseAsync bool + // OperationTimeouts allows configuring resource operation timeouts. + OperationTimeouts OperationTimeouts + // ExternalName allows you to specify a custom ExternalName. ExternalName ExternalName diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index eddb6fec..d67d333e 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -150,6 +150,30 @@ func (fp *FileProducer) WriteMainTF() error { fp.parameters["lifecycle"] = map[string]bool{ "prevent_destroy": !meta.WasDeleted(fp.Resource), } + + // Add operation timeouts if any timeout configured for the resource + timeouts := map[string]string{} + timeoutsConfigured := false + if fp.Config.OperationTimeouts.Read != nil { + timeouts["read"] = fp.Config.OperationTimeouts.Read.String() + timeoutsConfigured = true + } + if fp.Config.OperationTimeouts.Create != nil { + timeouts["create"] = fp.Config.OperationTimeouts.Create.String() + timeoutsConfigured = true + } + if fp.Config.OperationTimeouts.Update != nil { + timeouts["update"] = fp.Config.OperationTimeouts.Update.String() + timeoutsConfigured = true + } + if fp.Config.OperationTimeouts.Delete != nil { + timeouts["delete"] = fp.Config.OperationTimeouts.Delete.String() + timeoutsConfigured = true + } + if timeoutsConfigured { + fp.parameters["timeouts"] = timeouts + } + // Note(turkenh): To use third party providers, we need to configure // provider name in required_providers. providerSource := strings.Split(fp.Setup.Requirement.Source, "/") diff --git a/pkg/terraform/files_test.go b/pkg/terraform/files_test.go index b8ddae78..42843dbc 100644 --- a/pkg/terraform/files_test.go +++ b/pkg/terraform/files_test.go @@ -20,6 +20,7 @@ import ( "context" "path/filepath" "testing" + "time" "github.com/crossplane/crossplane-runtime/pkg/meta" xpfake "github.com/crossplane/crossplane-runtime/pkg/resource/fake" @@ -96,8 +97,9 @@ func TestWriteTFState(t *testing.T) { func TestWriteMainTF(t *testing.T) { type args struct { - tr resource.Terraformed - s Setup + tr resource.Terraformed + cfg *config.Resource + s Setup } type want struct { maintf string @@ -108,6 +110,44 @@ func TestWriteMainTF(t *testing.T) { args want }{ + "TimeoutsConfigured": { + reason: "Configured resources should be able to write everything it has into maintf file", + args: args{ + tr: &fake.Terraformed{ + Managed: xpfake.Managed{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + resource.AnnotationKeyPrivateRawAttribute: "privateraw", + meta.AnnotationKeyExternalName: "some-id", + }, + }, + }, + Parameterizable: fake.Parameterizable{Parameters: map[string]interface{}{ + "param": "paramval", + }}, + Observable: fake.Observable{Observation: map[string]interface{}{ + "obs": "obsval", + }}, + }, + cfg: config.DefaultResource("terrajet_resource", nil, func(r *config.Resource) { + r.OperationTimeouts = config.OperationTimeouts{ + Read: config.TimeDuration(30 * time.Second), + Update: config.TimeDuration(2 * time.Minute), + } + }), + s: Setup{ + Requirement: ProviderRequirement{ + Source: "hashicorp/provider-test", + Version: "1.2.3", + }, + Configuration: nil, + Env: nil, + }, + }, + want: want{ + maintf: `{"provider":{"provider-test":null},"resource":{"":{"":{"lifecycle":{"prevent_destroy":true},"name":"some-id","param":"paramval","timeouts":{"read":"30s","update":"2m0s"}}}},"terraform":{"required_providers":{"provider-test":{"source":"hashicorp/provider-test","version":"1.2.3"}}}}`, + }, + }, "Success": { reason: "Standard resources should be able to write everything it has into maintf file", args: args{ @@ -127,6 +167,7 @@ func TestWriteMainTF(t *testing.T) { "obs": "obsval", }}, }, + cfg: config.DefaultResource("terrajet_resource", nil), s: Setup{ Requirement: ProviderRequirement{ Source: "hashicorp/provider-test", @@ -144,7 +185,7 @@ func TestWriteMainTF(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { fs := afero.NewMemMapFs() - fp, err := NewFileProducer(context.TODO(), nil, dir, tc.args.tr, tc.args.s, config.DefaultResource("terrajet_resource", nil), WithFileSystem(fs)) + fp, err := NewFileProducer(context.TODO(), nil, dir, tc.args.tr, tc.args.s, tc.args.cfg, WithFileSystem(fs)) if err != nil { t.Errorf("cannot initialize a file producer: %s", err.Error()) } From ad715a163ead05ffc639fb4f4c3fafaec6e8d2f8 Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Thu, 6 Jan 2022 10:22:50 +0300 Subject: [PATCH 2/2] Configurable timeouts: address comments Signed-off-by: Hasan Turken --- pkg/config/resource.go | 13 ++++--------- pkg/terraform/files.go | 23 +++++++++-------------- pkg/terraform/files_test.go | 4 ++-- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/pkg/config/resource.go b/pkg/config/resource.go index fca4fd11..33ce7328 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -189,15 +189,10 @@ func (s *Sensitive) AddFieldPath(tf, xp string) { // https://www.terraform.io/language/resources/syntax#operation-timeouts // Please note that, not all resources support configuring timeouts. type OperationTimeouts struct { - Read *time.Duration - Create *time.Duration - Update *time.Duration - Delete *time.Duration -} - -// TimeDuration returns a pointer to a given time.Duration. -func TimeDuration(d time.Duration) *time.Duration { - return &d + Read time.Duration + Create time.Duration + Update time.Duration + Delete time.Duration } // Resource is the set of information that you can override at different steps diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index d67d333e..b2e60969 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -153,24 +153,19 @@ func (fp *FileProducer) WriteMainTF() error { // Add operation timeouts if any timeout configured for the resource timeouts := map[string]string{} - timeoutsConfigured := false - if fp.Config.OperationTimeouts.Read != nil { - timeouts["read"] = fp.Config.OperationTimeouts.Read.String() - timeoutsConfigured = true + if t := fp.Config.OperationTimeouts.Read.String(); t != "0s" { + timeouts["read"] = t } - if fp.Config.OperationTimeouts.Create != nil { - timeouts["create"] = fp.Config.OperationTimeouts.Create.String() - timeoutsConfigured = true + if t := fp.Config.OperationTimeouts.Create.String(); t != "0s" { + timeouts["create"] = t } - if fp.Config.OperationTimeouts.Update != nil { - timeouts["update"] = fp.Config.OperationTimeouts.Update.String() - timeoutsConfigured = true + if t := fp.Config.OperationTimeouts.Update.String(); t != "0s" { + timeouts["update"] = t } - if fp.Config.OperationTimeouts.Delete != nil { - timeouts["delete"] = fp.Config.OperationTimeouts.Delete.String() - timeoutsConfigured = true + if t := fp.Config.OperationTimeouts.Delete.String(); t != "0s" { + timeouts["delete"] = t } - if timeoutsConfigured { + if len(timeouts) != 0 { fp.parameters["timeouts"] = timeouts } diff --git a/pkg/terraform/files_test.go b/pkg/terraform/files_test.go index 42843dbc..9cf823f6 100644 --- a/pkg/terraform/files_test.go +++ b/pkg/terraform/files_test.go @@ -131,8 +131,8 @@ func TestWriteMainTF(t *testing.T) { }, cfg: config.DefaultResource("terrajet_resource", nil, func(r *config.Resource) { r.OperationTimeouts = config.OperationTimeouts{ - Read: config.TimeDuration(30 * time.Second), - Update: config.TimeDuration(2 * time.Minute), + Read: 30 * time.Second, + Update: 2 * time.Minute, } }), s: Setup{