From 8bd021ff412510c1446623086f39e669bfe6a3a9 Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Wed, 20 Oct 2021 15:18:07 +0300 Subject: [PATCH] Configure custom keys with function instead of string Signed-off-by: Hasan Turken --- pkg/config/resource.go | 2 +- pkg/controller/external.go | 44 +++++++-------- pkg/controller/external_test.go | 66 +++++++++++----------- pkg/controller/interfaces.go | 3 +- pkg/pipeline/controller.go | 2 +- pkg/pipeline/templates/controller.go.tmpl | 6 +- pkg/pipeline/templates/terraformed.go.tmpl | 9 --- pkg/pipeline/terraformed.go | 7 +-- pkg/resource/interfaces.go | 1 - pkg/terraform/store.go | 17 +++--- 10 files changed, 67 insertions(+), 90 deletions(-) diff --git a/pkg/config/resource.go b/pkg/config/resource.go index 56a195af..2fee395c 100644 --- a/pkg/config/resource.go +++ b/pkg/config/resource.go @@ -89,7 +89,7 @@ type Sensitive struct { // CustomKeysFunctionPath is the path for function adding custom connection // details keys - CustomKeysFunctionPath string + CustomKeysFn CustomConnectionKeysFn fieldPaths map[string]string } diff --git a/pkg/controller/external.go b/pkg/controller/external.go index 13a8413f..01f864f3 100644 --- a/pkg/controller/external.go +++ b/pkg/controller/external.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/crossplane-contrib/terrajet/pkg/config" "github.com/crossplane-contrib/terrajet/pkg/resource" "github.com/crossplane-contrib/terrajet/pkg/resource/json" "github.com/crossplane-contrib/terrajet/pkg/terraform" @@ -47,20 +48,13 @@ const ( // Option allows you to configure Connector. type Option func(*Connector) -// UseAsync configures the controller to use async variant of the functions -// of the Terraform client. -func UseAsync() Option { - return func(c *Connector) { - c.async = true - } -} - // NewConnector returns a new Connector object. -func NewConnector(kube client.Client, ws Store, sf terraform.SetupFn, opts ...Option) *Connector { +func NewConnector(kube client.Client, ws Store, sf terraform.SetupFn, cfg config.Resource, opts ...Option) *Connector { c := &Connector{ kube: kube, getTerraformSetup: sf, store: ws, + config: cfg, } for _, f := range opts { f(c) @@ -74,7 +68,7 @@ type Connector struct { kube client.Client store Store getTerraformSetup terraform.SetupFn - async bool + config config.Resource } // Connect makes sure the underlying client is ready to issue requests to the @@ -90,7 +84,7 @@ func (c *Connector) Connect(ctx context.Context, mg xpresource.Managed) (managed return nil, errors.Wrap(err, errGetTerraformSetup) } - tf, err := c.store.Workspace(ctx, &APISecretClient{kube: c.kube}, tr, ts) + tf, err := c.store.Workspace(ctx, &APISecretClient{kube: c.kube}, tr, ts, c.config) if err != nil { return nil, errors.Wrap(err, errGetWorkspace) } @@ -98,15 +92,14 @@ func (c *Connector) Connect(ctx context.Context, mg xpresource.Managed) (managed return &external{ kube: c.kube, workspace: tf, - async: c.async, + config: c.config, }, nil } type external struct { kube client.Client workspace Workspace - - async bool + config config.Resource } func (e *external) Observe(ctx context.Context, mg xpresource.Managed) (managed.ExternalObservation, error) { // nolint:gocyclo @@ -123,7 +116,7 @@ func (e *external) Observe(ctx context.Context, mg xpresource.Managed) (managed. if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, errRefresh) } - if e.async { + if e.config.UseAsync { tr.SetConditions(resource.LastOperationCondition(res.LastOperationError)) if err := e.kube.Status().Update(ctx, tr); err != nil { return managed.ExternalObservation{}, errors.Wrap(err, errStatusUpdate) @@ -160,7 +153,7 @@ func (e *external) Observe(ctx context.Context, mg xpresource.Managed) (managed. return managed.ExternalObservation{}, errors.Wrap(err, "cannot late initialize parameters") } - conn, err := getConnectionDetails(attr, tr) + conn, err := getConnectionDetails(attr, tr, e.config) if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot get connection details") } @@ -188,7 +181,7 @@ func (e *external) Create(ctx context.Context, mg xpresource.Managed) (managed.E if !ok { return managed.ExternalCreation{}, errors.New(errUnexpectedObject) } - if e.async { + if e.config.UseAsync { return managed.ExternalCreation{}, errors.Wrap(e.workspace.ApplyAsync(CriticalAnnotationsCallback(e.kube, tr)), errStartAsyncApply) } res, err := e.workspace.Apply(ctx) @@ -200,7 +193,7 @@ func (e *external) Create(ctx context.Context, mg xpresource.Managed) (managed.E return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal state attributes") } - conn, err := getConnectionDetails(attr, tr) + conn, err := getConnectionDetails(attr, tr, e.config) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") } @@ -211,7 +204,7 @@ func (e *external) Create(ctx context.Context, mg xpresource.Managed) (managed.E } func (e *external) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { - if e.async { + if e.config.UseAsync { return managed.ExternalUpdate{}, errors.Wrap(e.workspace.ApplyAsync(terraform.NopCallbackFn), errStartAsyncApply) } tr, ok := mg.(resource.Terraformed) @@ -230,7 +223,7 @@ func (e *external) Update(ctx context.Context, mg xpresource.Managed) (managed.E } func (e *external) Delete(ctx context.Context, _ xpresource.Managed) error { - if e.async { + if e.config.UseAsync { return errors.Wrap(e.workspace.DestroyAsync(), errStartAsyncDestroy) } return errors.Wrap(e.workspace.Destroy(ctx), errDestroy) @@ -283,15 +276,18 @@ func CriticalAnnotationsCallback(kube client.Client, tr resource.Terraformed) te } } -func getConnectionDetails(attr map[string]interface{}, tr resource.Terraformed) (managed.ConnectionDetails, error) { +func getConnectionDetails(attr map[string]interface{}, tr resource.Terraformed, cfg config.Resource) (managed.ConnectionDetails, error) { conn, err := resource.GetSensitiveAttributes(attr, tr.GetConnectionDetailsMapping(), tr.GetTerraformResourceIDField()) if err != nil { return nil, errors.Wrap(err, "cannot get connection details") } - custom, err := tr.GetAdditionalConnectionDetails(attr) - if err != nil { - return nil, errors.Wrap(err, "cannot get custom connection keys") + custom := map[string][]byte{} + // TODO(turkenh): Once we have automatic defaulting, remove this if check. + if cfg.Sensitive.CustomKeysFn != nil { + if custom, err = cfg.Sensitive.CustomKeysFn(attr); err != nil { + return nil, errors.Wrap(err, "cannot get custom connection keys") + } } if conn == nil { diff --git a/pkg/controller/external_test.go b/pkg/controller/external_test.go index c1a04ad6..8f50fcaf 100644 --- a/pkg/controller/external_test.go +++ b/pkg/controller/external_test.go @@ -28,6 +28,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/crossplane-contrib/terrajet/pkg/config" "github.com/crossplane-contrib/terrajet/pkg/resource" "github.com/crossplane-contrib/terrajet/pkg/resource/fake" "github.com/crossplane-contrib/terrajet/pkg/resource/json" @@ -83,11 +84,11 @@ func (c WorkspaceFns) Plan(ctx context.Context) (terraform.PlanResult, error) { } type StoreFns struct { - WorkspaceFn func(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup) (*terraform.Workspace, error) + WorkspaceFn func(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup, cfg config.Resource) (*terraform.Workspace, error) } -func (s StoreFns) Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup) (*terraform.Workspace, error) { - return s.WorkspaceFn(ctx, c, tr, ts) +func (s StoreFns) Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup, cfg config.Resource) (*terraform.Workspace, error) { + return s.WorkspaceFn(ctx, c, tr, ts, cfg) } func TestConnect(t *testing.T) { @@ -95,6 +96,7 @@ func TestConnect(t *testing.T) { setupFn terraform.SetupFn store Store obj xpresource.Managed + cfg config.Resource } type want struct { err error @@ -132,7 +134,7 @@ func TestConnect(t *testing.T) { return terraform.Setup{}, nil }, store: StoreFns{ - WorkspaceFn: func(_ context.Context, _ resource.SecretClient, _ resource.Terraformed, _ terraform.Setup) (*terraform.Workspace, error) { + WorkspaceFn: func(_ context.Context, _ resource.SecretClient, _ resource.Terraformed, _ terraform.Setup, _ config.Resource) (*terraform.Workspace, error) { return nil, errBoom }, }, @@ -148,7 +150,7 @@ func TestConnect(t *testing.T) { return terraform.Setup{}, nil }, store: StoreFns{ - WorkspaceFn: func(_ context.Context, _ resource.SecretClient, _ resource.Terraformed, _ terraform.Setup) (*terraform.Workspace, error) { + WorkspaceFn: func(_ context.Context, _ resource.SecretClient, _ resource.Terraformed, _ terraform.Setup, _ config.Resource) (*terraform.Workspace, error) { return nil, nil }, }, @@ -157,7 +159,7 @@ func TestConnect(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - c := NewConnector(nil, tc.args.store, tc.args.setupFn) + c := NewConnector(nil, tc.args.store, tc.args.setupFn, tc.args.cfg) _, 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%s", tc.reason, diff) @@ -168,10 +170,10 @@ func TestConnect(t *testing.T) { func TestObserve(t *testing.T) { type args struct { - w Workspace - kube client.Client - async bool - obj xpresource.Managed + w Workspace + kube client.Client + cfg config.Resource + obj xpresource.Managed } type want struct { obs managed.ExternalObservation @@ -237,7 +239,7 @@ func TestObserve(t *testing.T) { "LastOperationFailed": { reason: "It should report the last operation error without failing", args: args{ - async: true, + cfg: config.Resource{UseAsync: true}, obj: &fake.Terraformed{ MetadataProvider: fake.MetadataProvider{ IDField: "id", @@ -269,8 +271,8 @@ func TestObserve(t *testing.T) { "StatusUpdateFailed": { reason: "It should fail if status cannot be updated", args: args{ - async: true, - obj: &fake.Terraformed{}, + cfg: config.Resource{UseAsync: true}, + obj: &fake.Terraformed{}, kube: &test.MockClient{ MockStatusUpdate: test.NewMockStatusUpdateFn(errBoom), }, @@ -340,7 +342,7 @@ func TestObserve(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := &external{workspace: tc.w, kube: tc.kube, async: tc.async} + e := &external{workspace: tc.w, kube: tc.kube, config: tc.cfg} _, err := e.Observe(context.TODO(), tc.args.obj) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nObserve(...): -want error, +got error:\n%s", tc.reason, diff) @@ -351,9 +353,9 @@ func TestObserve(t *testing.T) { func TestCreate(t *testing.T) { type args struct { - w Workspace - async bool - obj xpresource.Managed + w Workspace + cfg config.Resource + obj xpresource.Managed } type want struct { err error @@ -374,8 +376,8 @@ func TestCreate(t *testing.T) { "AsyncFailed": { reason: "It should return error if it cannot trigger the async apply", args: args{ - async: true, - obj: &fake.Terraformed{}, + cfg: config.Resource{UseAsync: true}, + obj: &fake.Terraformed{}, w: WorkspaceFns{ ApplyAsyncFn: func(_ terraform.CallbackFn) error { return errBoom @@ -403,7 +405,7 @@ func TestCreate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := &external{workspace: tc.w, async: tc.async} + e := &external{workspace: tc.w, config: tc.cfg} _, err := e.Create(context.TODO(), tc.args.obj) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nCreate(...): -want error, +got error:\n%s", tc.reason, diff) @@ -414,9 +416,9 @@ func TestCreate(t *testing.T) { func TestUpdate(t *testing.T) { type args struct { - w Workspace - async bool - obj xpresource.Managed + w Workspace + cfg config.Resource + obj xpresource.Managed } type want struct { err error @@ -437,8 +439,8 @@ func TestUpdate(t *testing.T) { "AsyncFailed": { reason: "It should return error if it cannot trigger the async apply", args: args{ - async: true, - obj: &fake.Terraformed{}, + cfg: config.Resource{UseAsync: true}, + obj: &fake.Terraformed{}, w: WorkspaceFns{ ApplyAsyncFn: func(_ terraform.CallbackFn) error { return errBoom @@ -466,7 +468,7 @@ func TestUpdate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := &external{workspace: tc.w, async: tc.async} + e := &external{workspace: tc.w, config: tc.cfg} _, err := e.Update(context.TODO(), tc.args.obj) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nCreate(...): -want error, +got error:\n%s", tc.reason, diff) @@ -477,9 +479,9 @@ func TestUpdate(t *testing.T) { func TestDelete(t *testing.T) { type args struct { - w Workspace - async bool - obj xpresource.Managed + w Workspace + cfg config.Resource + obj xpresource.Managed } type want struct { err error @@ -492,8 +494,8 @@ func TestDelete(t *testing.T) { "AsyncFailed": { reason: "It should return error if it cannot trigger the async destroy", args: args{ - async: true, - obj: &fake.Terraformed{}, + cfg: config.Resource{UseAsync: true}, + obj: &fake.Terraformed{}, w: WorkspaceFns{ DestroyAsyncFn: func() error { return errBoom @@ -521,7 +523,7 @@ func TestDelete(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := &external{workspace: tc.w, async: tc.async} + e := &external{workspace: tc.w, config: tc.cfg} err := e.Delete(context.TODO(), tc.args.obj) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nCreate(...): -want error, +got error:\n%s", tc.reason, diff) diff --git a/pkg/controller/interfaces.go b/pkg/controller/interfaces.go index cbf1dc90..173b236a 100644 --- a/pkg/controller/interfaces.go +++ b/pkg/controller/interfaces.go @@ -19,6 +19,7 @@ package controller import ( "context" + "github.com/crossplane-contrib/terrajet/pkg/config" "github.com/crossplane-contrib/terrajet/pkg/resource" "github.com/crossplane-contrib/terrajet/pkg/terraform" ) @@ -39,5 +40,5 @@ type Workspace interface { // Store is where we can get access to the Terraform workspace of given resource. type Store interface { - Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup) (*terraform.Workspace, error) + Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts terraform.Setup, cfg config.Resource) (*terraform.Workspace, error) } diff --git a/pkg/pipeline/controller.go b/pkg/pipeline/controller.go index dd9ff159..b87f3a3e 100644 --- a/pkg/pipeline/controller.go +++ b/pkg/pipeline/controller.go @@ -59,7 +59,7 @@ func (cg *ControllerGenerator) Generate(c *config.Resource, typesPkgPath string) }, "DisableNameInitializer": c.ExternalName.DisableNameInitializer, "TypePackageAlias": ctrlFile.Imports.UsePackage(typesPkgPath), - "UseAsync": c.UseAsync, + "ResourceType": c.TerraformResourceType, } filePath := filepath.Join(cg.ControllerGroupDir, strings.ToLower(c.Kind), "zz_controller.go") diff --git a/pkg/pipeline/templates/controller.go.tmpl b/pkg/pipeline/templates/controller.go.tmpl index 3c335e87..b54425e8 100644 --- a/pkg/pipeline/templates/controller.go.tmpl +++ b/pkg/pipeline/templates/controller.go.tmpl @@ -27,11 +27,7 @@ func Setup(mgr ctrl.Manager, l logging.Logger, rl workqueue.RateLimiter, s terra name := managed.ControllerName({{ .TypePackageAlias }}{{ .CRD.Kind }}GroupVersionKind.String()) r := managed.NewReconciler(mgr, xpresource.ManagedKind({{ .TypePackageAlias }}{{ .CRD.Kind }}GroupVersionKind), - managed.WithExternalConnecter(tjcontroller.NewConnector(mgr.GetClient(), ws, s, - {{- if .UseAsync }} - tjcontroller.UseAsync(), - {{- end}} - )), + managed.WithExternalConnecter(tjcontroller.NewConnector(mgr.GetClient(), ws, s, config.Store.GetForResource("{{ .ResourceType }}"))), managed.WithLogger(l.WithValues("controller", name)), managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), managed.WithFinalizer(terraform.NewWorkspaceFinalizer(ws, xpresource.NewAPIFinalizer(mgr.GetClient(), managed.FinalizerName))), diff --git a/pkg/pipeline/templates/terraformed.go.tmpl b/pkg/pipeline/templates/terraformed.go.tmpl index eb8bcf5f..355fcbd5 100644 --- a/pkg/pipeline/templates/terraformed.go.tmpl +++ b/pkg/pipeline/templates/terraformed.go.tmpl @@ -31,15 +31,6 @@ func (tr *{{ .CRD.Kind }}) GetConnectionDetailsMapping() map[string]string { {{- end }} } -// GetCustomConnectionKeys for this {{ .CRD.Kind }} -func (tr *{{ .CRD.Kind }}) GetAdditionalConnectionDetails(attributes map[string]interface{}) (map[string][]byte, error) { - {{- if .Sensitive.CustomKeysFunctionName }} - return {{ .Sensitive.CustomKeysFunctionName }}(attributes) - {{- else }} - return nil, nil - {{- end }} -} - // GetObservation of this {{ .CRD.Kind }} func (tr *{{ .CRD.Kind }}) GetObservation() (map[string]interface{}, error) { o, err := json.TFParser.Marshal(tr.Status.AtProvider) diff --git a/pkg/pipeline/terraformed.go b/pkg/pipeline/terraformed.go index 3b292dea..94217f15 100644 --- a/pkg/pipeline/terraformed.go +++ b/pkg/pipeline/terraformed.go @@ -53,10 +53,6 @@ func (tg *TerraformedGenerator) Generate(c *config.Resource, sch *schema.Resourc wrapper.WithGenStatement(GenStatement), wrapper.WithHeaderPath("hack/boilerplate.go.txt"), // todo ) - customKeysPath := "" - if c.Sensitive.CustomKeysFunctionPath != "" { - customKeysPath = trFile.Imports.UseType(c.Sensitive.CustomKeysFunctionPath) - } vars := map[string]interface{}{ "CRD": map[string]string{ "APIVersion": c.Version, @@ -74,8 +70,7 @@ func (tg *TerraformedGenerator) Generate(c *config.Resource, sch *schema.Resourc "SchemaVersion": sch.SchemaVersion, }, "Sensitive": map[string]interface{}{ - "Fields": c.Sensitive.GetFieldPaths(), - "CustomKeysFunctionName": customKeysPath, + "Fields": c.Sensitive.GetFieldPaths(), }, "LateInitializer": map[string]interface{}{ "IgnoredFields": c.LateInitializer.IgnoredFields, diff --git a/pkg/resource/interfaces.go b/pkg/resource/interfaces.go index b8277bf1..e495bdb2 100644 --- a/pkg/resource/interfaces.go +++ b/pkg/resource/interfaces.go @@ -24,7 +24,6 @@ import ( type Observable interface { GetObservation() (map[string]interface{}, error) SetObservation(map[string]interface{}) error - GetAdditionalConnectionDetails(map[string]interface{}) (map[string][]byte, error) } // Parameterizable structs can get and set parameters of the managed resource diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 7802f5d0..35f532ec 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -67,13 +67,12 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption { } // NewWorkspaceStore returns a new WorkspaceStore. -func NewWorkspaceStore(configs *config.Provider, l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore { +func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore { ws := &WorkspaceStore{ - Configurations: configs, - store: map[types.UID]*Workspace{}, - logger: l, - mu: sync.Mutex{}, - fs: afero.Afero{Fs: afero.NewOsFs()}, + store: map[types.UID]*Workspace{}, + logger: l, + mu: sync.Mutex{}, + fs: afero.Afero{Fs: afero.NewOsFs()}, } for _, f := range opts { f(ws) @@ -83,8 +82,6 @@ func NewWorkspaceStore(configs *config.Provider, l logging.Logger, opts ...Works // WorkspaceStore allows you to manage multiple Terraform workspaces. type WorkspaceStore struct { - Configurations *config.Provider - // store holds information about ongoing operations of given resource. // Since there can be multiple calls that add/remove values from the map at // the same time, it has to be safe for concurrency since those operations @@ -99,12 +96,12 @@ type WorkspaceStore struct { // Workspace makes sure the Terraform workspace for the given resource is ready // to be used and returns the Workspace object configured to work in that // workspace folder in the filesystem. -func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts Setup) (*Workspace, error) { +func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts Setup, cfg config.Resource) (*Workspace, error) { dir := filepath.Join(ws.fs.GetTempDir(""), string(tr.GetUID())) if err := ws.fs.MkdirAll(dir, os.ModePerm); err != nil { return nil, errors.Wrap(err, "cannot create directory for workspace") } - fp, err := NewFileProducer(ctx, c, dir, tr, ts, WithConfig(ws.Configurations.GetForResource(tr.GetTerraformResourceType()))) + fp, err := NewFileProducer(ctx, c, dir, tr, ts, WithConfig(cfg)) if err != nil { return nil, errors.Wrap(err, "cannot create a new file producer") }