From baa6108443e016a2a152493def744ccb78ec447a Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Thu, 21 Apr 2022 03:12:27 +0300 Subject: [PATCH 1/3] Respect timeouts during refresh Signed-off-by: Hasan Turken --- pkg/terraform/files.go | 75 +++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index b2e60969..dde2a4c2 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -114,6 +114,10 @@ func (fp *FileProducer) WriteTFState(ctx context.Context) error { if pr, ok := fp.Resource.GetAnnotations()[resource.AnnotationKeyPrivateRawAttribute]; ok { privateRaw = []byte(pr) } + privateRawWithTimeout, err := insertTimeoutsMeta(privateRaw, fp.Config.OperationTimeouts) + if err != nil { + return errors.Wrap(err, "cannot insert timeouts meta") + } s := json.NewStateV4() s.TerraformVersion = fp.Setup.Version s.Lineage = string(fp.Resource.GetUID()) @@ -128,7 +132,7 @@ func (fp *FileProducer) WriteTFState(ctx context.Context) error { Instances: []json.InstanceObjectStateV4{ { SchemaVersion: uint64(fp.Resource.GetTerraformSchemaVersion()), - PrivateRaw: privateRaw, + PrivateRaw: privateRawWithTimeout, AttributesRaw: attr, }, }, @@ -152,21 +156,9 @@ func (fp *FileProducer) WriteMainTF() error { } // Add operation timeouts if any timeout configured for the resource - timeouts := map[string]string{} - if t := fp.Config.OperationTimeouts.Read.String(); t != "0s" { - timeouts["read"] = t - } - if t := fp.Config.OperationTimeouts.Create.String(); t != "0s" { - timeouts["create"] = t - } - if t := fp.Config.OperationTimeouts.Update.String(); t != "0s" { - timeouts["update"] = t - } - if t := fp.Config.OperationTimeouts.Delete.String(); t != "0s" { - timeouts["delete"] = t - } - if len(timeouts) != 0 { - fp.parameters["timeouts"] = timeouts + tp := timeoutsAsParameter(fp.Config.OperationTimeouts) + if len(tp) != 0 { + fp.parameters["timeouts"] = tp } // Note(turkenh): To use third party providers, we need to configure @@ -196,3 +188,54 @@ func (fp *FileProducer) WriteMainTF() error { } return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "main.tf.json"), rawMainTF, os.ModePerm), "cannot write tfstate file") } + +func timeoutsAsParameter(ot config.OperationTimeouts) map[string]string { + timeouts := make(map[string]string) + if t := ot.Read.String(); t != "0s" { + timeouts["read"] = t + } + if t := ot.Create.String(); t != "0s" { + timeouts["create"] = t + } + if t := ot.Update.String(); t != "0s" { + timeouts["update"] = t + } + if t := ot.Delete.String(); t != "0s" { + timeouts["delete"] = t + } + return timeouts +} + +// "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" is the TimeoutKey: +// https://github.com/hashicorp/terraform-plugin-sdk/blob/112e2164c381d80e8ada3170dac9a8a5db01079a/helper/schema/resource_timeout.go#L14 +const tfMetaTimeoutKey = "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" + +func insertTimeoutsMeta(rawMeta []byte, ot config.OperationTimeouts) ([]byte, error) { + m := make(map[string]interface{}) + if t := ot.Read.String(); t != "0s" { + m["read"] = ot.Read.Nanoseconds() + } + if t := ot.Create.String(); t != "0s" { + m["create"] = ot.Create.Nanoseconds() + } + if t := ot.Update.String(); t != "0s" { + m["update"] = ot.Update.Nanoseconds() + } + if t := ot.Delete.String(); t != "0s" { + m["delete"] = ot.Delete.Nanoseconds() + } + if len(m) == 0 { + // no timeout configured + return rawMeta, nil + } + meta := make(map[string]interface{}) + if len(rawMeta) == 0 { + meta[tfMetaTimeoutKey] = m + return json.JSParser.Marshal(meta) + } + if err := json.JSParser.Unmarshal(rawMeta, &meta); err != nil { + return rawMeta, errors.Wrap(err, "cannot unmarshall private raw meta") + } + meta[tfMetaTimeoutKey] = m + return json.JSParser.Marshal(meta) +} From 6c4fbe9a14fac293fdcab779abad1d87e3fd53a4 Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Thu, 21 Apr 2022 16:19:52 +0300 Subject: [PATCH 2/3] Add unit tests for refresh timeouts Signed-off-by: Hasan Turken --- pkg/terraform/files.go | 60 +-------- pkg/terraform/files_test.go | 65 +++++++--- pkg/terraform/timeouts.go | 97 ++++++++++++++ pkg/terraform/timeouts_test.go | 229 +++++++++++++++++++++++++++++++++ 4 files changed, 378 insertions(+), 73 deletions(-) create mode 100644 pkg/terraform/timeouts.go create mode 100644 pkg/terraform/timeouts_test.go diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index dde2a4c2..197a6ad4 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -114,9 +114,8 @@ func (fp *FileProducer) WriteTFState(ctx context.Context) error { if pr, ok := fp.Resource.GetAnnotations()[resource.AnnotationKeyPrivateRawAttribute]; ok { privateRaw = []byte(pr) } - privateRawWithTimeout, err := insertTimeoutsMeta(privateRaw, fp.Config.OperationTimeouts) - if err != nil { - return errors.Wrap(err, "cannot insert timeouts meta") + if privateRaw, err = insertTimeoutsMeta(privateRaw, timeouts(fp.Config.OperationTimeouts)); err != nil { + return errors.Wrap(err, "cannot insert timeouts metadata to private raw") } s := json.NewStateV4() s.TerraformVersion = fp.Setup.Version @@ -132,7 +131,7 @@ func (fp *FileProducer) WriteTFState(ctx context.Context) error { Instances: []json.InstanceObjectStateV4{ { SchemaVersion: uint64(fp.Resource.GetTerraformSchemaVersion()), - PrivateRaw: privateRawWithTimeout, + PrivateRaw: privateRaw, AttributesRaw: attr, }, }, @@ -156,7 +155,7 @@ func (fp *FileProducer) WriteMainTF() error { } // Add operation timeouts if any timeout configured for the resource - tp := timeoutsAsParameter(fp.Config.OperationTimeouts) + tp := timeouts(fp.Config.OperationTimeouts).asParameter() if len(tp) != 0 { fp.parameters["timeouts"] = tp } @@ -188,54 +187,3 @@ func (fp *FileProducer) WriteMainTF() error { } return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "main.tf.json"), rawMainTF, os.ModePerm), "cannot write tfstate file") } - -func timeoutsAsParameter(ot config.OperationTimeouts) map[string]string { - timeouts := make(map[string]string) - if t := ot.Read.String(); t != "0s" { - timeouts["read"] = t - } - if t := ot.Create.String(); t != "0s" { - timeouts["create"] = t - } - if t := ot.Update.String(); t != "0s" { - timeouts["update"] = t - } - if t := ot.Delete.String(); t != "0s" { - timeouts["delete"] = t - } - return timeouts -} - -// "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" is the TimeoutKey: -// https://github.com/hashicorp/terraform-plugin-sdk/blob/112e2164c381d80e8ada3170dac9a8a5db01079a/helper/schema/resource_timeout.go#L14 -const tfMetaTimeoutKey = "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" - -func insertTimeoutsMeta(rawMeta []byte, ot config.OperationTimeouts) ([]byte, error) { - m := make(map[string]interface{}) - if t := ot.Read.String(); t != "0s" { - m["read"] = ot.Read.Nanoseconds() - } - if t := ot.Create.String(); t != "0s" { - m["create"] = ot.Create.Nanoseconds() - } - if t := ot.Update.String(); t != "0s" { - m["update"] = ot.Update.Nanoseconds() - } - if t := ot.Delete.String(); t != "0s" { - m["delete"] = ot.Delete.Nanoseconds() - } - if len(m) == 0 { - // no timeout configured - return rawMeta, nil - } - meta := make(map[string]interface{}) - if len(rawMeta) == 0 { - meta[tfMetaTimeoutKey] = m - return json.JSParser.Marshal(meta) - } - if err := json.JSParser.Unmarshal(rawMeta, &meta); err != nil { - return rawMeta, errors.Wrap(err, "cannot unmarshall private raw meta") - } - meta[tfMetaTimeoutKey] = m - return json.JSParser.Marshal(meta) -} diff --git a/pkg/terraform/files_test.go b/pkg/terraform/files_test.go index 9cf823f6..a953a435 100644 --- a/pkg/terraform/files_test.go +++ b/pkg/terraform/files_test.go @@ -40,8 +40,9 @@ const ( func TestWriteTFState(t *testing.T) { type args struct { - tr resource.Terraformed - s Setup + tr resource.Terraformed + cfg *config.Resource + s Setup } type want struct { tfstate string @@ -53,33 +54,63 @@ func TestWriteTFState(t *testing.T) { want }{ "Success": { - reason: "Standard 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", + reason: "Standard resources should be able to write everything it has into tfstate 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", + }}, }, - Parameterizable: fake.Parameterizable{Parameters: map[string]interface{}{ - "param": "paramval", - }}, - Observable: fake.Observable{Observation: map[string]interface{}{ - "obs": "obsval", - }}, - }}, + cfg: config.DefaultResource("terrajet_resource", nil), + }, want: want{ tfstate: `{"version":4,"terraform_version":"","serial":1,"lineage":"","outputs":null,"resources":[{"mode":"managed","type":"","name":"","provider":"provider[\"registry.terraform.io/\"]","instances":[{"schema_version":0,"attributes":{"id":"some-id","name":"some-id","obs":"obsval","param":"paramval"},"private":"cHJpdmF0ZXJhdw=="}]}]}`, }, }, + "SuccessWithTimeout": { + reason: "Configured timeouts should be reflected tfstate as private meta", + args: args{ + tr: &fake.Terraformed{ + Managed: xpfake.Managed{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + resource.AnnotationKeyPrivateRawAttribute: "{}", + 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.Read = 2 * time.Minute + }), + }, + want: want{ + tfstate: `{"version":4,"terraform_version":"","serial":1,"lineage":"","outputs":null,"resources":[{"mode":"managed","type":"","name":"","provider":"provider[\"registry.terraform.io/\"]","instances":[{"schema_version":0,"attributes":{"id":"some-id","name":"some-id","obs":"obsval","param":"paramval"},"private":"eyJlMmJmYjczMC1lY2FhLTExZTYtOGY4OC0zNDM2M2JjN2M0YzAiOnsicmVhZCI6MTIwMDAwMDAwMDAwfX0="}]}]}`, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { fs := afero.NewMemMapFs() ctx := context.TODO() - fp, err := NewFileProducer(ctx, nil, dir, tc.args.tr, tc.args.s, config.DefaultResource("terrajet_resource", nil), WithFileSystem(fs)) + fp, err := NewFileProducer(ctx, 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()) } diff --git a/pkg/terraform/timeouts.go b/pkg/terraform/timeouts.go new file mode 100644 index 00000000..b35796b5 --- /dev/null +++ b/pkg/terraform/timeouts.go @@ -0,0 +1,97 @@ +/* + Copyright 2022 The Crossplane Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package terraform + +import ( + "github.com/crossplane/crossplane-runtime/pkg/errors" + + "github.com/crossplane/terrajet/pkg/config" + "github.com/crossplane/terrajet/pkg/resource/json" +) + +// "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" is a hardcoded string for Terraform +// timeout key in private raw, i.e. provider specific metadata: +// https://github.com/hashicorp/terraform-plugin-sdk/blob/112e2164c381d80e8ada3170dac9a8a5db01079a/helper/schema/resource_timeout.go#L14 +const tfMetaTimeoutKey = "e2bfb730-ecaa-11e6-8f88-34363bc7c4c0" + +type timeouts config.OperationTimeouts + +func (ts timeouts) asParameter() map[string]string { + param := make(map[string]string) + if t := ts.Read.String(); t != "0s" { + param["read"] = t + } + if t := ts.Create.String(); t != "0s" { + param["create"] = t + } + if t := ts.Update.String(); t != "0s" { + param["update"] = t + } + if t := ts.Delete.String(); t != "0s" { + param["delete"] = t + } + return param +} + +func (ts timeouts) asMetadata() map[string]interface{} { + // See how timeouts encoded as metadata on Terraform side: + // https://github.com/hashicorp/terraform-plugin-sdk/blob/112e2164c381d80e8ada3170dac9a8a5db01079a/helper/schema/resource_timeout.go#L170 + meta := make(map[string]interface{}) + if t := ts.Read.String(); t != "0s" { + meta["read"] = ts.Read.Nanoseconds() + } + if t := ts.Create.String(); t != "0s" { + meta["create"] = ts.Create.Nanoseconds() + } + if t := ts.Update.String(); t != "0s" { + meta["update"] = ts.Update.Nanoseconds() + } + if t := ts.Delete.String(); t != "0s" { + meta["delete"] = ts.Delete.Nanoseconds() + } + return meta +} + +func insertTimeoutsMeta(existingMeta []byte, to timeouts) ([]byte, error) { + customTimeouts := to.asMetadata() + if len(customTimeouts) == 0 { + // No custom timeout configured, nothing to do. + return existingMeta, nil + } + meta := make(map[string]interface{}) + if len(existingMeta) == 0 { + // No existing data, just initialize a new meta with custom timeouts. + meta[tfMetaTimeoutKey] = customTimeouts + return json.JSParser.Marshal(meta) + } + // There are some existing metadata, let's parse it to insert custom + // timeouts properly. + if err := json.JSParser.Unmarshal(existingMeta, &meta); err != nil { + return nil, errors.Wrap(err, "cannot parse existing metadata") + } + if existingTimeouts, ok := meta[tfMetaTimeoutKey].(map[string]interface{}); ok { + // There are some timeout configuration exists in existing metadata. + // Only override custom timeouts. + for k, v := range customTimeouts { + existingTimeouts[k] = v + } + return json.JSParser.Marshal(meta) + } + // No existing timeout configuration, initialize it with custom timeouts. + meta[tfMetaTimeoutKey] = customTimeouts + return json.JSParser.Marshal(meta) +} diff --git a/pkg/terraform/timeouts_test.go b/pkg/terraform/timeouts_test.go new file mode 100644 index 00000000..db9a1848 --- /dev/null +++ b/pkg/terraform/timeouts_test.go @@ -0,0 +1,229 @@ +/* + Copyright 2022 The Crossplane Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package terraform + +import ( + "testing" + "time" + + "github.com/google/go-cmp/cmp" + + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/test" +) + +func TestTimeoutsAsParameter(t *testing.T) { + type args struct { + to timeouts + } + type want struct { + out map[string]string + } + cases := map[string]struct { + args + want + }{ + "NoTimeouts": { + want: want{ + out: map[string]string{}, + }, + }, + "SomeTimeout": { + args: args{ + to: timeouts{ + Read: 3 * time.Minute, + }, + }, + want: want{ + out: map[string]string{ + "read": "3m0s", + }, + }, + }, + "AllTimeouts": { + args: args{ + to: timeouts{ + Create: time.Minute, + Update: 2 * time.Minute, + Read: 3 * time.Minute, + Delete: 4 * time.Minute, + }, + }, + want: want{ + out: map[string]string{ + "create": "1m0s", + "update": "2m0s", + "read": "3m0s", + "delete": "4m0s", + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := tc.args.to.asParameter() + if diff := cmp.Diff(tc.want.out, got); diff != "" { + t.Errorf("\n%s\nasParameter(...): -want out, +got out:\n%s", name, diff) + } + }) + } +} +func TestTimeoutsAsMetadata(t *testing.T) { + type args struct { + to timeouts + } + type want struct { + out map[string]interface{} + } + cases := map[string]struct { + args + want + }{ + "NoTimeouts": { + want: want{ + out: map[string]interface{}{}, + }, + }, + "SomeTimeout": { + args: args{ + to: timeouts{ + Read: 3 * time.Minute, + }, + }, + want: want{ + out: map[string]interface{}{ + "read": int64(180000000000), + }, + }, + }, + "AllTimeouts": { + args: args{ + to: timeouts{ + Create: time.Minute, + Update: 2 * time.Minute, + Read: 3 * time.Minute, + Delete: 4 * time.Minute, + }, + }, + want: want{ + out: map[string]interface{}{ + "create": int64(60000000000), + "update": int64(120000000000), + "read": int64(180000000000), + "delete": int64(240000000000), + }, + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := tc.args.to.asMetadata() + if diff := cmp.Diff(tc.want.out, got); diff != "" { + t.Errorf("\n%s\nasParameter(...): -want out, +got out:\n%s", name, diff) + } + }) + } +} +func TestInsertTimeoutsMeta(t *testing.T) { + type args struct { + rawMeta []byte + to timeouts + } + type want struct { + out []byte + err error + } + cases := map[string]struct { + args + want + }{ + "NoTimeoutNoMeta": {}, + "NoMetaButTimeout": { + args: args{ + to: timeouts{ + Read: 2 * time.Minute, + }, + }, + want: want{ + out: []byte(`{"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0":{"read":120000000000}}`), + }, + }, + "NonNilMetaButTimeout": { + args: args{ + rawMeta: []byte(`{}`), + to: timeouts{ + Read: 2 * time.Minute, + }, + }, + want: want{ + out: []byte(`{"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0":{"read":120000000000}}`), + }, + }, + "CannotParseExistingMeta": { + args: args{ + rawMeta: []byte(`{malformed}`), + to: timeouts{ + Read: 2 * time.Minute, + }, + }, + want: want{ + err: errors.Wrap(errors.New(`ReadString: expects " or n, but found m, error found in #2 byte of ...|{malformed}|..., bigger context ...|{malformed}|...`), `cannot parse existing metadata`), // nolint: golint + }, + }, + "ExistingMetaAndTimeout": { + args: args{ + rawMeta: []byte(`{"some-key":"some-value"}`), + to: timeouts{ + Read: 2 * time.Minute, + }, + }, + want: want{ + out: []byte(`{"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0":{"read":120000000000},"some-key":"some-value"}`), + }, + }, + "ExistingMetaNoTimeout": { + args: args{ + rawMeta: []byte(`{"some-key":"some-value"}`), + }, + want: want{ + out: []byte(`{"some-key":"some-value"}`), + }, + }, + "ExistingMetaOverridesSomeTimeout": { + args: args{ + rawMeta: []byte(`{"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0":{"create":240000000000,"read":120000000000},"some-key":"some-value"}`), + to: timeouts{ + Read: 1 * time.Minute, + }, + }, + want: want{ + out: []byte(`{"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0":{"create":240000000000,"read":60000000000},"some-key":"some-value"}`), + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got, err := insertTimeoutsMeta(tc.args.rawMeta, tc.args.to) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ninsertTimeoutsMeta(...): -want error, +got error:\n%s", name, diff) + } + if diff := cmp.Diff(tc.want.out, got); diff != "" { + t.Errorf("\n%s\ninsertTimeoutsMeta(...): -want out, +got out:\n%s", name, diff) + } + }) + } +} From 4cddcfec3acbb86af546796aae1c0c7f4954ccba Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Fri, 6 May 2022 10:04:34 +0300 Subject: [PATCH 3/3] Resolve comments in #276 Signed-off-by: Hasan Turken --- pkg/terraform/files.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index 197a6ad4..c035c7ba 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -155,8 +155,7 @@ func (fp *FileProducer) WriteMainTF() error { } // Add operation timeouts if any timeout configured for the resource - tp := timeouts(fp.Config.OperationTimeouts).asParameter() - if len(tp) != 0 { + if tp := timeouts(fp.Config.OperationTimeouts).asParameter(); len(tp) != 0 { fp.parameters["timeouts"] = tp }