From 268f04aae4d928dac244a68bf833ed53963464b4 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Mon, 21 Mar 2022 18:40:29 +0300 Subject: [PATCH 1/6] Add config.Provider.SharedGRPC config option to control shared gRPC mode for a provider Signed-off-by: Alper Rifat Ulucinar --- pkg/config/provider.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/config/provider.go b/pkg/config/provider.go index 0843b8f..a36ba4a 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -96,6 +96,12 @@ type Provider struct { // resource name. Resources map[string]*Resource + // SharedGRPC set to `true` to run the native Terraform plugin in + // shared gRPC mode. In shared gRPC mode, the Terraform CLI does not fork + // the binary plugin at each request but rather send requests to a shared + // instance forked once. + SharedGRPC bool + // resourceConfigurators is a map holding resource configurators where key // is Terraform resource name. resourceConfigurators map[string]ResourceConfiguratorChain @@ -146,6 +152,13 @@ func WithDefaultResourceFn(f DefaultResourceFn) ProviderOption { } } +// WithSharedGRPC configures SharedGRPC for this Provider +func WithSharedGRPC(sharedGRPC bool) ProviderOption { + return func(p *Provider) { + p.SharedGRPC = sharedGRPC + } +} + // NewProviderWithSchema builds and returns a new Provider from provider // tfjson schema, that is generated using Terraform CLI with: // `terraform providers schema --json` From 172f610912f40e3d70c8beee0cffd3dcea1e50db Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Tue, 22 Mar 2022 09:39:43 +0300 Subject: [PATCH 2/6] Implement shared gRPC server in terraform.WorkspaceStore Signed-off-by: Alper Rifat Ulucinar --- pkg/config/provider.go | 13 ---- pkg/terraform/files.go | 5 +- pkg/terraform/shared_grpc_server.go | 95 +++++++++++++++++++++++++++++ pkg/terraform/store.go | 28 ++++++++- 4 files changed, 122 insertions(+), 19 deletions(-) create mode 100644 pkg/terraform/shared_grpc_server.go diff --git a/pkg/config/provider.go b/pkg/config/provider.go index a36ba4a..0843b8f 100644 --- a/pkg/config/provider.go +++ b/pkg/config/provider.go @@ -96,12 +96,6 @@ type Provider struct { // resource name. Resources map[string]*Resource - // SharedGRPC set to `true` to run the native Terraform plugin in - // shared gRPC mode. In shared gRPC mode, the Terraform CLI does not fork - // the binary plugin at each request but rather send requests to a shared - // instance forked once. - SharedGRPC bool - // resourceConfigurators is a map holding resource configurators where key // is Terraform resource name. resourceConfigurators map[string]ResourceConfiguratorChain @@ -152,13 +146,6 @@ func WithDefaultResourceFn(f DefaultResourceFn) ProviderOption { } } -// WithSharedGRPC configures SharedGRPC for this Provider -func WithSharedGRPC(sharedGRPC bool) ProviderOption { - return func(p *Provider) { - p.SharedGRPC = sharedGRPC - } -} - // NewProviderWithSchema builds and returns a new Provider from provider // tfjson schema, that is generated using Terraform CLI with: // `terraform providers schema --json` diff --git a/pkg/terraform/files.go b/pkg/terraform/files.go index b2e6096..c1b91f9 100644 --- a/pkg/terraform/files.go +++ b/pkg/terraform/files.go @@ -19,7 +19,6 @@ package terraform import ( "context" "fmt" - "os" "path/filepath" "strings" @@ -139,7 +138,7 @@ func (fp *FileProducer) WriteTFState(ctx context.Context) error { if err != nil { return errors.Wrap(err, "cannot marshal state object") } - return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "terraform.tfstate"), rawState, os.ModePerm), "cannot write tfstate file") + return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "terraform.tfstate"), rawState, 0600), "cannot write tfstate file") } // WriteMainTF writes the content main configuration file that has the desired @@ -194,5 +193,5 @@ func (fp *FileProducer) WriteMainTF() error { if err != nil { return errors.Wrap(err, "cannot marshal main hcl object") } - return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "main.tf.json"), rawMainTF, os.ModePerm), "cannot write tfstate file") + return errors.Wrap(fp.fs.WriteFile(filepath.Join(fp.Dir, "main.tf.json"), rawMainTF, 0600), "cannot write tfstate file") } diff --git a/pkg/terraform/shared_grpc_server.go b/pkg/terraform/shared_grpc_server.go new file mode 100644 index 0000000..ae8a2ab --- /dev/null +++ b/pkg/terraform/shared_grpc_server.go @@ -0,0 +1,95 @@ +/* +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 ( + "bufio" + "os" + "regexp" + "time" + + "github.com/pkg/errors" +) + +const ( + envReattachConfig = "TF_REATTACH_PROVIDERS" + regexReattachLine = envReattachConfig + `='(.*)'` + reattachTimeout = 1 * time.Minute +) + +func (ws *WorkspaceStore) startSharedServer() error { //nolint:gocyclo + if len(ws.nativeProviderPath) == 0 { + return nil + } + log := ws.logger.WithValues("nativeProviderPath", ws.nativeProviderPath, "nativeProviderArgs", ws.nativeProviderArgs) + if ws.reattachConfig != "" { + log.Debug("Shared gRPC server is running...", "reattachConfig", ws.reattachConfig) + return nil + } + errCh := make(chan error, 1) + reattachCh := make(chan string, 1) + re, err := regexp.Compile(regexReattachLine) + if err != nil { + return errors.Wrap(err, "failed to compile regexp") + } + + go func() { + defer close(errCh) + defer close(reattachCh) + defer func() { + ws.mu.Lock() + ws.reattachConfig = "" + ws.mu.Unlock() + }() + //#nosec G204 no user input + cmd := ws.executor.Command(ws.nativeProviderPath, ws.nativeProviderArgs...) + stdout, err := cmd.StdoutPipe() + if err != nil { + errCh <- err + return + } + if err := cmd.Start(); err != nil { + errCh <- err + return + } + scanner := bufio.NewScanner(stdout) + for scanner.Scan() { + t := scanner.Text() + matches := re.FindStringSubmatch(t) + if matches == nil { + continue + } + reattachCh <- matches[1] + break + } + if err := cmd.Wait(); err != nil { + log.Info("Native Terraform provider process error", "error", err) + errCh <- err + } + }() + + select { + case reattachConfig := <-reattachCh: + ws.reattachConfig = reattachConfig + return errors.Wrapf(os.Setenv(envReattachConfig, ws.reattachConfig), + "could not set reattach config env variable %q to value: %s", envReattachConfig, ws.reattachConfig) + case err := <-errCh: + return err + case <-time.After(reattachTimeout): + return errors.Errorf("timed out after %v while waiting for the reattach configuration string", reattachTimeout) + } +} diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index d2cb3bf..68bce96 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -66,6 +66,22 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption { } } +// WithNativeProviderPath enables shared gRPC mode and configures the path +// of the Terraform native provider. When set, Terraform CLI does not fork +// the native plugin for each request but a shared server is used instead. +func WithNativeProviderPath(path string) WorkspaceStoreOption { + return func(ws *WorkspaceStore) { + ws.nativeProviderPath = path + } +} + +// WithNativeProviderArgs are the arguments to be passed to the native provider +func WithNativeProviderArgs(args []string) WorkspaceStoreOption { + return func(ws *WorkspaceStore) { + ws.nativeProviderArgs = args + } +} + // NewWorkspaceStore returns a new WorkspaceStore. func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore { ws := &WorkspaceStore{ @@ -87,8 +103,11 @@ type WorkspaceStore struct { // 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 // cause rehashing in some cases. - store map[types.UID]*Workspace - logger logging.Logger + store map[types.UID]*Workspace + logger logging.Logger + nativeProviderPath string + nativeProviderArgs []string + reattachConfig string mu sync.Mutex @@ -99,7 +118,7 @@ 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, cfg *config.Resource) (*Workspace, error) { +func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient, tr resource.Terraformed, ts Setup, cfg *config.Resource) (*Workspace, error) { //nolint:gocyclo 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") @@ -122,6 +141,9 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient } l := ws.logger.WithValues("workspace", dir) ws.mu.Lock() + if err := ws.startSharedServer(); err != nil { + return nil, err + } w, ok := ws.store[tr.GetUID()] if !ok { ws.store[tr.GetUID()] = NewWorkspace(dir, WithLogger(l), WithExecutor(ws.executor)) From 5b31f82a587e8d0fd213b2e46dc018b08419eee9 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Wed, 30 Mar 2022 01:54:19 +0300 Subject: [PATCH 3/6] Add NativeProviderRunner interface - Add tests Signed-off-by: Alper Rifat Ulucinar --- pkg/terraform/provider_runner.go | 93 +++++++++++ pkg/terraform/provider_runner_test.go | 222 ++++++++++++++++++++++++++ pkg/terraform/shared_grpc_server.go | 47 +++--- pkg/terraform/store.go | 39 ++--- 4 files changed, 357 insertions(+), 44 deletions(-) create mode 100644 pkg/terraform/provider_runner.go create mode 100644 pkg/terraform/provider_runner_test.go diff --git a/pkg/terraform/provider_runner.go b/pkg/terraform/provider_runner.go new file mode 100644 index 0000000..03a6217 --- /dev/null +++ b/pkg/terraform/provider_runner.go @@ -0,0 +1,93 @@ +/* +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 ( + "sync" + + "github.com/crossplane/crossplane-runtime/pkg/logging" + "k8s.io/apimachinery/pkg/util/clock" + "k8s.io/utils/exec" +) + +// NativeProviderRunner is the interface for running +// Terraform native provider processes in the shared +// gRPC server mode +type NativeProviderRunner interface { + StartSharedServer() (string, error) +} + +// NoOpProviderRunner is a no-op NativeProviderRunner +type NoOpProviderRunner struct{} + +// StartSharedServer takes no action +func (NoOpProviderRunner) StartSharedServer() (string, error) { + return "", nil +} + +// SharedGRPCRunner runs the configured native provider plugin +// using the supplied command-line args +type SharedGRPCRunner struct { + nativeProviderPath string + nativeProviderArgs []string + reattachConfig string + logger logging.Logger + executor exec.Interface + clock clock.Clock + mu *sync.Mutex +} + +// SharedGRPCRunnerOption lets you configure the shared gRPC runner. +type SharedGRPCRunnerOption func(runner *SharedGRPCRunner) + +// WithNativeProviderPath enables shared gRPC mode and configures the path +// of the Terraform native provider. When set, Terraform CLI does not fork +// the native plugin for each request but a shared server is used instead. +func WithNativeProviderPath(path string) SharedGRPCRunnerOption { + return func(sr *SharedGRPCRunner) { + sr.nativeProviderPath = path + } +} + +// WithNativeProviderArgs are the arguments to be passed to the native provider +func WithNativeProviderArgs(args ...string) SharedGRPCRunnerOption { + return func(sr *SharedGRPCRunner) { + sr.nativeProviderArgs = args + } +} + +// WithNativeProviderExecutor sets the process executor to be used +func WithNativeProviderExecutor(e exec.Interface) SharedGRPCRunnerOption { + return func(sr *SharedGRPCRunner) { + sr.executor = e + } +} + +// NewSharedGRPCRunner instantiates a SharedGRPCRunner with an +// OS executor using the supplied logger +func NewSharedGRPCRunner(l logging.Logger, opts ...SharedGRPCRunnerOption) *SharedGRPCRunner { + sr := &SharedGRPCRunner{ + logger: l, + executor: exec.New(), + clock: clock.RealClock{}, + mu: &sync.Mutex{}, + } + for _, o := range opts { + o(sr) + } + return sr +} diff --git a/pkg/terraform/provider_runner_test.go b/pkg/terraform/provider_runner_test.go new file mode 100644 index 0000000..69cf214 --- /dev/null +++ b/pkg/terraform/provider_runner_test.go @@ -0,0 +1,222 @@ +/* +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 ( + "io" + "reflect" + "strings" + "sync" + "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" + "k8s.io/apimachinery/pkg/util/clock" + "k8s.io/utils/exec" + testingexec "k8s.io/utils/exec/testing" +) + +func TestStartSharedServer(t *testing.T) { + testPath := "path" + testArgs := []string{"arg1", "arg2"} + testReattachConfig1 := `TF_REATTACH_PROVIDERS='test1'` + testReattachConfig2 := `TF_REATTACH_PROVIDERS='test2'` + testErr := errors.New("boom") + type args struct { + runner NativeProviderRunner + } + type want struct { + reattachConfig string + err error + } + tests := map[string]struct { + args args + want want + }{ + "NotConfiguredNoOp": { + args: args{ + runner: NoOpProviderRunner{}, + }, + }, + "NotConfiguredSharedGRPC": { + args: args{ + runner: NewSharedGRPCRunner(logging.NewNopLogger()), + }, + want: want{ + err: errors.New(errNativeProviderPath), + }, + }, + "SuccessfullyStarted": { + args: args{ + runner: NewSharedGRPCRunner(logging.NewNopLogger(), WithNativeProviderPath(testPath), WithNativeProviderArgs(testArgs...), + WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, nil))), + }, + want: want{ + reattachConfig: "test1", + }, + }, + "AlreadyRunning": { + args: args{ + runner: &SharedGRPCRunner{ + nativeProviderPath: testPath, + reattachConfig: "test1", + logger: logging.NewNopLogger(), + executor: newExecutorWithStoutPipe(testReattachConfig2, nil), + mu: &sync.Mutex{}, + }, + }, + want: want{ + reattachConfig: "test1", + }, + }, + "NativeProviderError": { + args: args{ + runner: NewSharedGRPCRunner(logging.NewNopLogger(), WithNativeProviderPath(testPath), + WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, testErr))), + }, + want: want{ + err: testErr, + }, + }, + "NativeProviderTimeout": { + args: args{ + runner: &SharedGRPCRunner{ + nativeProviderPath: testPath, + logger: logging.NewNopLogger(), + executor: newExecutorWithStoutPipe("invalid", nil), + mu: &sync.Mutex{}, + clock: &fakeClock{}, + }, + }, + want: want{ + err: errors.Errorf(errFmtTimeout, reattachTimeout), + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + reattachConfig, err := tt.args.runner.StartSharedServer() + if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nStartSharedServer(): -want error, +got error:\n%s", name, diff) + } + if err != nil { + return + } + if diff := cmp.Diff(reattachConfig, tt.want.reattachConfig); diff != "" { + t.Errorf("\n%s\nStartSharedServer(): -want reattachConfig, +got reattachConfig:\n%s", name, diff) + } + }) + } +} + +type fakeClock struct { + clock.FakeClock +} + +func (f *fakeClock) After(d time.Duration) <-chan time.Time { + defer func() { + f.Step(reattachTimeout) + }() + return f.FakeClock.After(d) +} + +func newExecutorWithStoutPipe(reattachConfig string, err error) exec.Interface { + return &testingexec.FakeExec{ + CommandScript: []testingexec.FakeCommandAction{ + func(cmd string, args ...string) exec.Cmd { + return &testingexec.FakeCmd{ + StdoutPipeResponse: testingexec.FakeStdIOPipeResponse{ + ReadCloser: io.NopCloser(strings.NewReader(reattachConfig)), + Error: err, + }, + } + }, + }, + } +} + +func TestWithNativeProviderPath(t *testing.T) { + tests := map[string]struct { + path string + want string + }{ + "NotConfigured": { + path: "", + want: "", + }, + "Configured": { + path: "a/b/c", + want: "a/b/c", + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + sr := &SharedGRPCRunner{} + WithNativeProviderPath(tt.path)(sr) + if !reflect.DeepEqual(sr.nativeProviderPath, tt.want) { + t.Errorf("WithNativeProviderPath(tt.path) = %v, want %v", sr.nativeProviderArgs, tt.want) + } + }) + } +} + +func TestWithNativeProviderArgs(t *testing.T) { + tests := map[string]struct { + args []string + want []string + }{ + "NotConfigured": {}, + "Configured": { + args: []string{"a", "b", "c"}, + want: []string{"a", "b", "c"}, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + sr := &SharedGRPCRunner{} + WithNativeProviderArgs(tt.args...)(sr) + if !reflect.DeepEqual(sr.nativeProviderArgs, tt.want) { + t.Errorf("WithNativeProviderArgs(tt.args) = %v, want %v", sr.nativeProviderArgs, tt.want) + } + }) + } +} + +func TestWithNativeProviderExecutor(t *testing.T) { + tests := map[string]struct { + executor exec.Interface + want exec.Interface + }{ + "NotConfigured": {}, + "Configured": { + executor: &testingexec.FakeExec{}, + want: &testingexec.FakeExec{}, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + sr := &SharedGRPCRunner{} + WithNativeProviderExecutor(tt.executor)(sr) + if !reflect.DeepEqual(sr.executor, tt.want) { + t.Errorf("WithNativeProviderExecutor(tt.executor) = %v, want %v", sr.executor, tt.want) + } + }) + } +} diff --git a/pkg/terraform/shared_grpc_server.go b/pkg/terraform/shared_grpc_server.go index ae8a2ab..e9be4f6 100644 --- a/pkg/terraform/shared_grpc_server.go +++ b/pkg/terraform/shared_grpc_server.go @@ -18,7 +18,6 @@ package terraform import ( "bufio" - "os" "regexp" "time" @@ -26,37 +25,48 @@ import ( ) const ( + // error messages + errNativeProviderPath = "native provider path is not configured" + errFmtTimeout = "timed out after %v while waiting for the reattach configuration string" + envReattachConfig = "TF_REATTACH_PROVIDERS" regexReattachLine = envReattachConfig + `='(.*)'` reattachTimeout = 1 * time.Minute ) -func (ws *WorkspaceStore) startSharedServer() error { //nolint:gocyclo - if len(ws.nativeProviderPath) == 0 { - return nil +// StartSharedServer starts a shared gRPC server if not already running +// A logger, native provider's path and command-line arguments to be +// passed to it must have been properly configured. +// Returns any errors encountered and the reattachment configuration for +// the native provider. +func (sr *SharedGRPCRunner) StartSharedServer() (string, error) { //nolint:gocyclo + sr.mu.Lock() + defer sr.mu.Unlock() + if len(sr.nativeProviderPath) == 0 { + return "", errors.New(errNativeProviderPath) } - log := ws.logger.WithValues("nativeProviderPath", ws.nativeProviderPath, "nativeProviderArgs", ws.nativeProviderArgs) - if ws.reattachConfig != "" { - log.Debug("Shared gRPC server is running...", "reattachConfig", ws.reattachConfig) - return nil + log := sr.logger.WithValues("nativeProviderPath", sr.nativeProviderPath, "nativeProviderArgs", sr.nativeProviderArgs) + if sr.reattachConfig != "" { + log.Debug("Shared gRPC server is running...", "reattachConfig", sr.reattachConfig) + return sr.reattachConfig, nil } errCh := make(chan error, 1) reattachCh := make(chan string, 1) re, err := regexp.Compile(regexReattachLine) if err != nil { - return errors.Wrap(err, "failed to compile regexp") + return "", errors.Wrap(err, "failed to compile regexp") } go func() { defer close(errCh) defer close(reattachCh) defer func() { - ws.mu.Lock() - ws.reattachConfig = "" - ws.mu.Unlock() + sr.mu.Lock() + sr.reattachConfig = "" + sr.mu.Unlock() }() //#nosec G204 no user input - cmd := ws.executor.Command(ws.nativeProviderPath, ws.nativeProviderArgs...) + cmd := sr.executor.Command(sr.nativeProviderPath, sr.nativeProviderArgs...) stdout, err := cmd.StdoutPipe() if err != nil { errCh <- err @@ -84,12 +94,11 @@ func (ws *WorkspaceStore) startSharedServer() error { //nolint:gocyclo select { case reattachConfig := <-reattachCh: - ws.reattachConfig = reattachConfig - return errors.Wrapf(os.Setenv(envReattachConfig, ws.reattachConfig), - "could not set reattach config env variable %q to value: %s", envReattachConfig, ws.reattachConfig) + sr.reattachConfig = reattachConfig + return sr.reattachConfig, nil case err := <-errCh: - return err - case <-time.After(reattachTimeout): - return errors.Errorf("timed out after %v while waiting for the reattach configuration string", reattachTimeout) + return "", err + case <-sr.clock.After(reattachTimeout): + return "", errors.Errorf(errFmtTimeout, reattachTimeout) } } diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 68bce96..7e23ad1 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -34,6 +34,10 @@ import ( "github.com/crossplane/terrajet/pkg/resource" ) +const ( + fmtErrSharedServerEnv = "could not set reattach config env variable %q to value: %s" +) + // SetupFn is a function that returns Terraform setup which contains // provider requirement, configuration and Terraform version. type SetupFn func(ctx context.Context, client client.Client, mg xpresource.Managed) (Setup, error) @@ -66,22 +70,6 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption { } } -// WithNativeProviderPath enables shared gRPC mode and configures the path -// of the Terraform native provider. When set, Terraform CLI does not fork -// the native plugin for each request but a shared server is used instead. -func WithNativeProviderPath(path string) WorkspaceStoreOption { - return func(ws *WorkspaceStore) { - ws.nativeProviderPath = path - } -} - -// WithNativeProviderArgs are the arguments to be passed to the native provider -func WithNativeProviderArgs(args []string) WorkspaceStoreOption { - return func(ws *WorkspaceStore) { - ws.nativeProviderArgs = args - } -} - // NewWorkspaceStore returns a new WorkspaceStore. func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore { ws := &WorkspaceStore{ @@ -103,13 +91,10 @@ type WorkspaceStore struct { // 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 // cause rehashing in some cases. - store map[types.UID]*Workspace - logger logging.Logger - nativeProviderPath string - nativeProviderArgs []string - reattachConfig string - - mu sync.Mutex + store map[types.UID]*Workspace + logger logging.Logger + providerRunner NativeProviderRunner + mu sync.Mutex fs afero.Afero executor exec.Interface @@ -140,10 +125,14 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient return nil, errors.Wrap(err, "cannot write main tf file") } l := ws.logger.WithValues("workspace", dir) - ws.mu.Lock() - if err := ws.startSharedServer(); err != nil { + reattachConfig, err := ws.providerRunner.StartSharedServer() + if err != nil { return nil, err } + if err := os.Setenv(envReattachConfig, reattachConfig); err != nil { + return nil, errors.Wrapf(err, fmtErrSharedServerEnv, envReattachConfig, reattachConfig) + } + ws.mu.Lock() w, ok := ws.store[tr.GetUID()] if !ok { ws.store[tr.GetUID()] = NewWorkspace(dir, WithLogger(l), WithExecutor(ws.executor)) From a9d407d6bdf77f0d4feb9f4cb56c50bf9b073610 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Wed, 30 Mar 2022 09:24:19 +0300 Subject: [PATCH 4/6] Use terraform.NoOpProviderRunner as the default WorkspaceStore.providerRunner Signed-off-by: Alper Rifat Ulucinar --- pkg/terraform/store.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 7e23ad1..a174795 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -70,14 +70,22 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption { } } +// WithNativeProviderRunner sets the NativeProviderRunner to be used. +func WithNativeProviderRunner(pr NativeProviderRunner) WorkspaceStoreOption { + return func(ws *WorkspaceStore) { + ws.providerRunner = pr + } +} + // NewWorkspaceStore returns a new WorkspaceStore. func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *WorkspaceStore { ws := &WorkspaceStore{ - store: map[types.UID]*Workspace{}, - logger: l, - mu: sync.Mutex{}, - fs: afero.Afero{Fs: afero.NewOsFs()}, - executor: exec.New(), + store: map[types.UID]*Workspace{}, + logger: l, + mu: sync.Mutex{}, + fs: afero.Afero{Fs: afero.NewOsFs()}, + executor: exec.New(), + providerRunner: NoOpProviderRunner{}, } for _, f := range opts { f(ws) From 0da01ccc9f01c1c2224e65f0bdf95dc51016893a Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Wed, 30 Mar 2022 17:52:22 +0300 Subject: [PATCH 5/6] Rename terraform.NativeProviderRunner as terraform.ProviderRunner Signed-off-by: Alper Rifat Ulucinar --- pkg/terraform/provider_runner.go | 118 ++++++++++++++++++++++---- pkg/terraform/provider_runner_test.go | 22 ++--- pkg/terraform/shared_grpc_server.go | 104 ----------------------- pkg/terraform/store.go | 14 +-- 4 files changed, 121 insertions(+), 137 deletions(-) delete mode 100644 pkg/terraform/shared_grpc_server.go diff --git a/pkg/terraform/provider_runner.go b/pkg/terraform/provider_runner.go index 03a6217..fce7143 100644 --- a/pkg/terraform/provider_runner.go +++ b/pkg/terraform/provider_runner.go @@ -17,31 +17,50 @@ limitations under the License. package terraform import ( + "bufio" + "regexp" "sync" + "time" "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/utils/exec" ) -// NativeProviderRunner is the interface for running +const ( + // error messages + errNativeProviderPath = "native provider path is not configured" + errFmtTimeout = "timed out after %v while waiting for the reattach configuration string" + + // an example value would be: '{"registry.terraform.io/hashicorp/aws": {"Protocol": "grpc", "ProtocolVersion":5, "Pid":... "Addr":{"Network": "unix","String": "..."}}}' + envReattachConfig = "TF_REATTACH_PROVIDERS" + regexReattachLine = envReattachConfig + `='(.*)'` + reattachTimeout = 1 * time.Minute +) + +// ProviderRunner is the interface for running // Terraform native provider processes in the shared // gRPC server mode -type NativeProviderRunner interface { - StartSharedServer() (string, error) +type ProviderRunner interface { + Start() (string, error) } -// NoOpProviderRunner is a no-op NativeProviderRunner +// NoOpProviderRunner is a no-op ProviderRunner type NoOpProviderRunner struct{} -// StartSharedServer takes no action -func (NoOpProviderRunner) StartSharedServer() (string, error) { +func NewNoOpProviderRunner() NoOpProviderRunner { + return NoOpProviderRunner{} +} + +// Start takes no action +func (NoOpProviderRunner) Start() (string, error) { return "", nil } -// SharedGRPCRunner runs the configured native provider plugin +// SharedProvider runs the configured native provider plugin // using the supplied command-line args -type SharedGRPCRunner struct { +type SharedProvider struct { nativeProviderPath string nativeProviderArgs []string reattachConfig string @@ -52,35 +71,35 @@ type SharedGRPCRunner struct { } // SharedGRPCRunnerOption lets you configure the shared gRPC runner. -type SharedGRPCRunnerOption func(runner *SharedGRPCRunner) +type SharedGRPCRunnerOption func(runner *SharedProvider) // WithNativeProviderPath enables shared gRPC mode and configures the path // of the Terraform native provider. When set, Terraform CLI does not fork // the native plugin for each request but a shared server is used instead. func WithNativeProviderPath(path string) SharedGRPCRunnerOption { - return func(sr *SharedGRPCRunner) { + return func(sr *SharedProvider) { sr.nativeProviderPath = path } } // WithNativeProviderArgs are the arguments to be passed to the native provider func WithNativeProviderArgs(args ...string) SharedGRPCRunnerOption { - return func(sr *SharedGRPCRunner) { + return func(sr *SharedProvider) { sr.nativeProviderArgs = args } } // WithNativeProviderExecutor sets the process executor to be used func WithNativeProviderExecutor(e exec.Interface) SharedGRPCRunnerOption { - return func(sr *SharedGRPCRunner) { + return func(sr *SharedProvider) { sr.executor = e } } -// NewSharedGRPCRunner instantiates a SharedGRPCRunner with an +// NewSharedProvider instantiates a SharedProvider with an // OS executor using the supplied logger -func NewSharedGRPCRunner(l logging.Logger, opts ...SharedGRPCRunnerOption) *SharedGRPCRunner { - sr := &SharedGRPCRunner{ +func NewSharedProvider(l logging.Logger, opts ...SharedGRPCRunnerOption) *SharedProvider { + sr := &SharedProvider{ logger: l, executor: exec.New(), clock: clock.RealClock{}, @@ -91,3 +110,72 @@ func NewSharedGRPCRunner(l logging.Logger, opts ...SharedGRPCRunnerOption) *Shar } return sr } + +// Start starts a shared gRPC server if not already running +// A logger, native provider's path and command-line arguments to be +// passed to it must have been properly configured. +// Returns any errors encountered and the reattachment configuration for +// the native provider. +func (sr *SharedProvider) Start() (string, error) { //nolint:gocyclo + sr.mu.Lock() + defer sr.mu.Unlock() + if len(sr.nativeProviderPath) == 0 { + return "", errors.New(errNativeProviderPath) + } + log := sr.logger.WithValues("nativeProviderPath", sr.nativeProviderPath, "nativeProviderArgs", sr.nativeProviderArgs) + if sr.reattachConfig != "" { + log.Debug("Shared gRPC server is running...", "reattachConfig", sr.reattachConfig) + return sr.reattachConfig, nil + } + errCh := make(chan error, 1) + reattachCh := make(chan string, 1) + re, err := regexp.Compile(regexReattachLine) + if err != nil { + return "", errors.Wrap(err, "failed to compile regexp") + } + + go func() { + defer close(errCh) + defer close(reattachCh) + defer func() { + sr.mu.Lock() + sr.reattachConfig = "" + sr.mu.Unlock() + }() + //#nosec G204 no user input + cmd := sr.executor.Command(sr.nativeProviderPath, sr.nativeProviderArgs...) + stdout, err := cmd.StdoutPipe() + if err != nil { + errCh <- err + return + } + if err := cmd.Start(); err != nil { + errCh <- err + return + } + scanner := bufio.NewScanner(stdout) + for scanner.Scan() { + t := scanner.Text() + matches := re.FindStringSubmatch(t) + if matches == nil { + continue + } + reattachCh <- matches[1] + break + } + if err := cmd.Wait(); err != nil { + log.Info("Native Terraform provider process error", "error", err) + errCh <- err + } + }() + + select { + case reattachConfig := <-reattachCh: + sr.reattachConfig = reattachConfig + return sr.reattachConfig, nil + case err := <-errCh: + return "", err + case <-sr.clock.After(reattachTimeout): + return "", errors.Errorf(errFmtTimeout, reattachTimeout) + } +} diff --git a/pkg/terraform/provider_runner_test.go b/pkg/terraform/provider_runner_test.go index 69cf214..f706e87 100644 --- a/pkg/terraform/provider_runner_test.go +++ b/pkg/terraform/provider_runner_test.go @@ -40,7 +40,7 @@ func TestStartSharedServer(t *testing.T) { testReattachConfig2 := `TF_REATTACH_PROVIDERS='test2'` testErr := errors.New("boom") type args struct { - runner NativeProviderRunner + runner ProviderRunner } type want struct { reattachConfig string @@ -52,12 +52,12 @@ func TestStartSharedServer(t *testing.T) { }{ "NotConfiguredNoOp": { args: args{ - runner: NoOpProviderRunner{}, + runner: NewNoOpProviderRunner(), }, }, "NotConfiguredSharedGRPC": { args: args{ - runner: NewSharedGRPCRunner(logging.NewNopLogger()), + runner: NewSharedProvider(logging.NewNopLogger()), }, want: want{ err: errors.New(errNativeProviderPath), @@ -65,7 +65,7 @@ func TestStartSharedServer(t *testing.T) { }, "SuccessfullyStarted": { args: args{ - runner: NewSharedGRPCRunner(logging.NewNopLogger(), WithNativeProviderPath(testPath), WithNativeProviderArgs(testArgs...), + runner: NewSharedProvider(logging.NewNopLogger(), WithNativeProviderPath(testPath), WithNativeProviderArgs(testArgs...), WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, nil))), }, want: want{ @@ -74,7 +74,7 @@ func TestStartSharedServer(t *testing.T) { }, "AlreadyRunning": { args: args{ - runner: &SharedGRPCRunner{ + runner: &SharedProvider{ nativeProviderPath: testPath, reattachConfig: "test1", logger: logging.NewNopLogger(), @@ -88,7 +88,7 @@ func TestStartSharedServer(t *testing.T) { }, "NativeProviderError": { args: args{ - runner: NewSharedGRPCRunner(logging.NewNopLogger(), WithNativeProviderPath(testPath), + runner: NewSharedProvider(logging.NewNopLogger(), WithNativeProviderPath(testPath), WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, testErr))), }, want: want{ @@ -97,7 +97,7 @@ func TestStartSharedServer(t *testing.T) { }, "NativeProviderTimeout": { args: args{ - runner: &SharedGRPCRunner{ + runner: &SharedProvider{ nativeProviderPath: testPath, logger: logging.NewNopLogger(), executor: newExecutorWithStoutPipe("invalid", nil), @@ -112,7 +112,7 @@ func TestStartSharedServer(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - reattachConfig, err := tt.args.runner.StartSharedServer() + reattachConfig, err := tt.args.runner.Start() if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("\n%s\nStartSharedServer(): -want error, +got error:\n%s", name, diff) } @@ -168,7 +168,7 @@ func TestWithNativeProviderPath(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - sr := &SharedGRPCRunner{} + sr := &SharedProvider{} WithNativeProviderPath(tt.path)(sr) if !reflect.DeepEqual(sr.nativeProviderPath, tt.want) { t.Errorf("WithNativeProviderPath(tt.path) = %v, want %v", sr.nativeProviderArgs, tt.want) @@ -190,7 +190,7 @@ func TestWithNativeProviderArgs(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - sr := &SharedGRPCRunner{} + sr := &SharedProvider{} WithNativeProviderArgs(tt.args...)(sr) if !reflect.DeepEqual(sr.nativeProviderArgs, tt.want) { t.Errorf("WithNativeProviderArgs(tt.args) = %v, want %v", sr.nativeProviderArgs, tt.want) @@ -212,7 +212,7 @@ func TestWithNativeProviderExecutor(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - sr := &SharedGRPCRunner{} + sr := &SharedProvider{} WithNativeProviderExecutor(tt.executor)(sr) if !reflect.DeepEqual(sr.executor, tt.want) { t.Errorf("WithNativeProviderExecutor(tt.executor) = %v, want %v", sr.executor, tt.want) diff --git a/pkg/terraform/shared_grpc_server.go b/pkg/terraform/shared_grpc_server.go deleted file mode 100644 index e9be4f6..0000000 --- a/pkg/terraform/shared_grpc_server.go +++ /dev/null @@ -1,104 +0,0 @@ -/* -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 ( - "bufio" - "regexp" - "time" - - "github.com/pkg/errors" -) - -const ( - // error messages - errNativeProviderPath = "native provider path is not configured" - errFmtTimeout = "timed out after %v while waiting for the reattach configuration string" - - envReattachConfig = "TF_REATTACH_PROVIDERS" - regexReattachLine = envReattachConfig + `='(.*)'` - reattachTimeout = 1 * time.Minute -) - -// StartSharedServer starts a shared gRPC server if not already running -// A logger, native provider's path and command-line arguments to be -// passed to it must have been properly configured. -// Returns any errors encountered and the reattachment configuration for -// the native provider. -func (sr *SharedGRPCRunner) StartSharedServer() (string, error) { //nolint:gocyclo - sr.mu.Lock() - defer sr.mu.Unlock() - if len(sr.nativeProviderPath) == 0 { - return "", errors.New(errNativeProviderPath) - } - log := sr.logger.WithValues("nativeProviderPath", sr.nativeProviderPath, "nativeProviderArgs", sr.nativeProviderArgs) - if sr.reattachConfig != "" { - log.Debug("Shared gRPC server is running...", "reattachConfig", sr.reattachConfig) - return sr.reattachConfig, nil - } - errCh := make(chan error, 1) - reattachCh := make(chan string, 1) - re, err := regexp.Compile(regexReattachLine) - if err != nil { - return "", errors.Wrap(err, "failed to compile regexp") - } - - go func() { - defer close(errCh) - defer close(reattachCh) - defer func() { - sr.mu.Lock() - sr.reattachConfig = "" - sr.mu.Unlock() - }() - //#nosec G204 no user input - cmd := sr.executor.Command(sr.nativeProviderPath, sr.nativeProviderArgs...) - stdout, err := cmd.StdoutPipe() - if err != nil { - errCh <- err - return - } - if err := cmd.Start(); err != nil { - errCh <- err - return - } - scanner := bufio.NewScanner(stdout) - for scanner.Scan() { - t := scanner.Text() - matches := re.FindStringSubmatch(t) - if matches == nil { - continue - } - reattachCh <- matches[1] - break - } - if err := cmd.Wait(); err != nil { - log.Info("Native Terraform provider process error", "error", err) - errCh <- err - } - }() - - select { - case reattachConfig := <-reattachCh: - sr.reattachConfig = reattachConfig - return sr.reattachConfig, nil - case err := <-errCh: - return "", err - case <-sr.clock.After(reattachTimeout): - return "", errors.Errorf(errFmtTimeout, reattachTimeout) - } -} diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index a174795..59dcb33 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -70,8 +70,8 @@ func WithFs(fs afero.Fs) WorkspaceStoreOption { } } -// WithNativeProviderRunner sets the NativeProviderRunner to be used. -func WithNativeProviderRunner(pr NativeProviderRunner) WorkspaceStoreOption { +// WithProviderRunner sets the ProviderRunner to be used. +func WithProviderRunner(pr ProviderRunner) WorkspaceStoreOption { return func(ws *WorkspaceStore) { ws.providerRunner = pr } @@ -85,7 +85,7 @@ func NewWorkspaceStore(l logging.Logger, opts ...WorkspaceStoreOption) *Workspac mu: sync.Mutex{}, fs: afero.Afero{Fs: afero.NewOsFs()}, executor: exec.New(), - providerRunner: NoOpProviderRunner{}, + providerRunner: NewNoOpProviderRunner(), } for _, f := range opts { f(ws) @@ -101,7 +101,7 @@ type WorkspaceStore struct { // cause rehashing in some cases. store map[types.UID]*Workspace logger logging.Logger - providerRunner NativeProviderRunner + providerRunner ProviderRunner mu sync.Mutex fs afero.Afero @@ -133,12 +133,12 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient return nil, errors.Wrap(err, "cannot write main tf file") } l := ws.logger.WithValues("workspace", dir) - reattachConfig, err := ws.providerRunner.StartSharedServer() + attachmentConfig, err := ws.providerRunner.Start() if err != nil { return nil, err } - if err := os.Setenv(envReattachConfig, reattachConfig); err != nil { - return nil, errors.Wrapf(err, fmtErrSharedServerEnv, envReattachConfig, reattachConfig) + if err := os.Setenv(envReattachConfig, attachmentConfig); err != nil { + return nil, errors.Wrapf(err, fmtErrSharedServerEnv, envReattachConfig, attachmentConfig) } ws.mu.Lock() w, ok := ws.store[tr.GetUID()] From 43541014e7838a39993be93007e96237cbc68bee Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Wed, 30 Mar 2022 17:57:03 +0300 Subject: [PATCH 6/6] Set TF_REATTACH_PROVIDERS env. variable per Workspace Signed-off-by: Alper Rifat Ulucinar --- pkg/terraform/provider_runner.go | 27 ++++++------------- pkg/terraform/provider_runner_test.go | 37 ++------------------------- pkg/terraform/store.go | 7 +++-- 3 files changed, 13 insertions(+), 58 deletions(-) diff --git a/pkg/terraform/provider_runner.go b/pkg/terraform/provider_runner.go index fce7143..db8d33b 100644 --- a/pkg/terraform/provider_runner.go +++ b/pkg/terraform/provider_runner.go @@ -30,8 +30,7 @@ import ( const ( // error messages - errNativeProviderPath = "native provider path is not configured" - errFmtTimeout = "timed out after %v while waiting for the reattach configuration string" + errFmtTimeout = "timed out after %v while waiting for the reattach configuration string" // an example value would be: '{"registry.terraform.io/hashicorp/aws": {"Protocol": "grpc", "ProtocolVersion":5, "Pid":... "Addr":{"Network": "unix","String": "..."}}}' envReattachConfig = "TF_REATTACH_PROVIDERS" @@ -49,6 +48,7 @@ type ProviderRunner interface { // NoOpProviderRunner is a no-op ProviderRunner type NoOpProviderRunner struct{} +// NewNoOpProviderRunner constructs a new NoOpProviderRunner func NewNoOpProviderRunner() NoOpProviderRunner { return NoOpProviderRunner{} } @@ -73,15 +73,6 @@ type SharedProvider struct { // SharedGRPCRunnerOption lets you configure the shared gRPC runner. type SharedGRPCRunnerOption func(runner *SharedProvider) -// WithNativeProviderPath enables shared gRPC mode and configures the path -// of the Terraform native provider. When set, Terraform CLI does not fork -// the native plugin for each request but a shared server is used instead. -func WithNativeProviderPath(path string) SharedGRPCRunnerOption { - return func(sr *SharedProvider) { - sr.nativeProviderPath = path - } -} - // WithNativeProviderArgs are the arguments to be passed to the native provider func WithNativeProviderArgs(args ...string) SharedGRPCRunnerOption { return func(sr *SharedProvider) { @@ -98,12 +89,13 @@ func WithNativeProviderExecutor(e exec.Interface) SharedGRPCRunnerOption { // NewSharedProvider instantiates a SharedProvider with an // OS executor using the supplied logger -func NewSharedProvider(l logging.Logger, opts ...SharedGRPCRunnerOption) *SharedProvider { +func NewSharedProvider(l logging.Logger, nativeProviderPath string, opts ...SharedGRPCRunnerOption) *SharedProvider { sr := &SharedProvider{ - logger: l, - executor: exec.New(), - clock: clock.RealClock{}, - mu: &sync.Mutex{}, + logger: l, + nativeProviderPath: nativeProviderPath, + executor: exec.New(), + clock: clock.RealClock{}, + mu: &sync.Mutex{}, } for _, o := range opts { o(sr) @@ -119,9 +111,6 @@ func NewSharedProvider(l logging.Logger, opts ...SharedGRPCRunnerOption) *Shared func (sr *SharedProvider) Start() (string, error) { //nolint:gocyclo sr.mu.Lock() defer sr.mu.Unlock() - if len(sr.nativeProviderPath) == 0 { - return "", errors.New(errNativeProviderPath) - } log := sr.logger.WithValues("nativeProviderPath", sr.nativeProviderPath, "nativeProviderArgs", sr.nativeProviderArgs) if sr.reattachConfig != "" { log.Debug("Shared gRPC server is running...", "reattachConfig", sr.reattachConfig) diff --git a/pkg/terraform/provider_runner_test.go b/pkg/terraform/provider_runner_test.go index f706e87..c382695 100644 --- a/pkg/terraform/provider_runner_test.go +++ b/pkg/terraform/provider_runner_test.go @@ -55,17 +55,9 @@ func TestStartSharedServer(t *testing.T) { runner: NewNoOpProviderRunner(), }, }, - "NotConfiguredSharedGRPC": { - args: args{ - runner: NewSharedProvider(logging.NewNopLogger()), - }, - want: want{ - err: errors.New(errNativeProviderPath), - }, - }, "SuccessfullyStarted": { args: args{ - runner: NewSharedProvider(logging.NewNopLogger(), WithNativeProviderPath(testPath), WithNativeProviderArgs(testArgs...), + runner: NewSharedProvider(logging.NewNopLogger(), testPath, WithNativeProviderArgs(testArgs...), WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, nil))), }, want: want{ @@ -88,7 +80,7 @@ func TestStartSharedServer(t *testing.T) { }, "NativeProviderError": { args: args{ - runner: NewSharedProvider(logging.NewNopLogger(), WithNativeProviderPath(testPath), + runner: NewSharedProvider(logging.NewNopLogger(), testPath, WithNativeProviderExecutor(newExecutorWithStoutPipe(testReattachConfig1, testErr))), }, want: want{ @@ -152,31 +144,6 @@ func newExecutorWithStoutPipe(reattachConfig string, err error) exec.Interface { } } -func TestWithNativeProviderPath(t *testing.T) { - tests := map[string]struct { - path string - want string - }{ - "NotConfigured": { - path: "", - want: "", - }, - "Configured": { - path: "a/b/c", - want: "a/b/c", - }, - } - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - sr := &SharedProvider{} - WithNativeProviderPath(tt.path)(sr) - if !reflect.DeepEqual(sr.nativeProviderPath, tt.want) { - t.Errorf("WithNativeProviderPath(tt.path) = %v, want %v", sr.nativeProviderArgs, tt.want) - } - }) - } -} - func TestWithNativeProviderArgs(t *testing.T) { tests := map[string]struct { args []string diff --git a/pkg/terraform/store.go b/pkg/terraform/store.go index 59dcb33..a668494 100644 --- a/pkg/terraform/store.go +++ b/pkg/terraform/store.go @@ -18,6 +18,7 @@ package terraform import ( "context" + "fmt" "os" "path/filepath" "sync" @@ -35,7 +36,7 @@ import ( ) const ( - fmtErrSharedServerEnv = "could not set reattach config env variable %q to value: %s" + fmtEnv = "%s=%s" ) // SetupFn is a function that returns Terraform setup which contains @@ -137,9 +138,6 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient if err != nil { return nil, err } - if err := os.Setenv(envReattachConfig, attachmentConfig); err != nil { - return nil, errors.Wrapf(err, fmtErrSharedServerEnv, envReattachConfig, attachmentConfig) - } ws.mu.Lock() w, ok := ws.store[tr.GetUID()] if !ok { @@ -152,6 +150,7 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient return nil, errors.Wrap(err, "cannot stat init lock file") } w.env = ts.Env + w.env = append(w.env, fmt.Sprintf(fmtEnv, envReattachConfig, attachmentConfig)) // We need to initialize only if the workspace hasn't been initialized yet. if !os.IsNotExist(err) { return w, nil