diff --git a/apis/v1beta1/workspace_types.go b/apis/v1beta1/workspace_types.go index b2d0e505..d0e4ddfe 100644 --- a/apis/v1beta1/workspace_types.go +++ b/apis/v1beta1/workspace_types.go @@ -159,7 +159,7 @@ type WorkspaceObservation struct { type WorkspaceSpec struct { xpv1.ResourceSpec `json:",inline"` ForProvider WorkspaceParameters `json:"forProvider"` - EnableLogging bool `json:"enableLogging,omitempty"` + EnableTerraformCLILogging bool `json:"enableTerraformCLILogging,omitempty"` } // A WorkspaceStatus represents the observed state of a Workspace. diff --git a/cmd/provider/main.go b/cmd/provider/main.go index 160e65ac..1b86c77b 100644 --- a/cmd/provider/main.go +++ b/cmd/provider/main.go @@ -18,6 +18,7 @@ package main import ( "context" + zapuber "go.uber.org/zap" "os" "path/filepath" "time" @@ -149,19 +150,10 @@ func main() { } func UseJSONencoder() zap.Opts { - // Create a zap logger with JSON encoding - encoderCfg := zapcore.EncoderConfig{ - TimeKey: "time", - LevelKey: "level", - NameKey: "logger", - MessageKey: "message", - StacktraceKey: "stacktrace", - LineEnding: zapcore.DefaultLineEnding, - EncodeLevel: zapcore.LowercaseLevelEncoder, - EncodeTime: zapcore.ISO8601TimeEncoder, - } return func(o *zap.Options) { - o.Encoder = zapcore.NewJSONEncoder(encoderCfg) + encoderConfig := zapuber.NewProductionEncoderConfig() + encoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder + o.Encoder = zapcore.NewJSONEncoder(encoderConfig) } } diff --git a/examples/workspace-enable-logging.yaml b/examples/workspace-enable-logging.yaml index 330244b4..4e029ff4 100644 --- a/examples/workspace-enable-logging.yaml +++ b/examples/workspace-enable-logging.yaml @@ -8,7 +8,7 @@ metadata: # annotation it would be derived from metadata.name - e.g. 'example-random-generator. crossplane.io/external-name: random spec: - enableLogging: true + enableTerraformCLILogging: true forProvider: source: Inline module: | diff --git a/internal/controller/workspace/workspace.go b/internal/controller/workspace/workspace.go index 74adfc16..82def011 100644 --- a/internal/controller/workspace/workspace.go +++ b/internal/controller/workspace/workspace.go @@ -132,8 +132,8 @@ func Setup(mgr ctrl.Manager, o controller.Options, timeout, pollJitter time.Dura usage: resource.NewProviderConfigUsageTracker(mgr.GetClient(), &v1beta1.ProviderConfigUsage{}), logger: o.Logger, fs: fs, - terraform: func(dir string, usePluginCache bool, enableLogging bool, logger logging.Logger, envs ...string) tfclient { - return terraform.Harness{Path: tfPath, Dir: dir, UsePluginCache: usePluginCache, EnableLogging: enableLogging, Logger: logger, Envs: envs} + terraform: func(dir string, usePluginCache bool, enableTerraformCLILogging bool, logger logging.Logger, envs ...string) tfclient { + return terraform.Harness{Path: tfPath, Dir: dir, UsePluginCache: usePluginCache, EnableTerraformCLILogging: enableTerraformCLILogging, Logger: logger, Envs: envs} }, } @@ -168,7 +168,7 @@ type connector struct { usage resource.Tracker logger logging.Logger fs afero.Afero - terraform func(dir string, usePluginCache bool, enableLogging bool, logger logging.Logger, envs ...string) tfclient + terraform func(dir string, usePluginCache bool, enableTerraformCLILogging bool, logger logging.Logger, envs ...string) tfclient } func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { //nolint:gocyclo @@ -320,7 +320,7 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E envs[idx] = strings.Join([]string{env.Name, runtimeVal}, "=") } - tf := c.terraform(dir, *pc.Spec.PluginCache, cr.Spec.EnableLogging, l, envs...) + tf := c.terraform(dir, *pc.Spec.PluginCache, cr.Spec.EnableTerraformCLILogging, l, envs...) if cr.Status.AtProvider.Checksum != "" { checksum, err := tf.GenerateChecksum(ctx) if err != nil { diff --git a/internal/controller/workspace/workspace_test.go b/internal/controller/workspace/workspace_test.go index 8856098c..75776efe 100644 --- a/internal/controller/workspace/workspace_test.go +++ b/internal/controller/workspace/workspace_test.go @@ -124,7 +124,7 @@ func TestConnect(t *testing.T) { kube client.Client usage resource.Tracker fs afero.Afero - terraform func(dir string, usePluginCache bool, envs ...string) tfclient + terraform func(dir string, usePluginCache bool, enableTerraformCLILogging bool, logger logging.Logger, envs ...string) tfclient } type args struct { @@ -216,7 +216,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -255,7 +255,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join(tfDir, string(uid), tfCreds): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -294,7 +294,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join(tfDir, string(uid), "subdir", tfCreds): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -338,7 +338,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join("/tmp", tfDir, string(uid), ".git-credentials"): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -381,7 +381,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join("/tmp", tfDir, string(uid)): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -422,7 +422,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join(tfDir, string(uid), tfConfig): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -463,7 +463,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join(tfDir, string(uid), "subdir", tfConfig): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -499,7 +499,7 @@ func TestConnect(t *testing.T) { errs: map[string]error{filepath.Join(tfDir, string(uid), tfMain): errBoom}, }, }, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, } @@ -529,7 +529,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{MockInit: func(_ context.Context, _ ...terraform.InitOption) error { return errBoom }} }, }, @@ -553,7 +553,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, MockWorkspace: func(_ context.Context, _ string) error { return errBoom }, @@ -579,7 +579,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockGenerateChecksum: func(ctx context.Context) (string, error) { return "", errBoom }, } @@ -613,7 +613,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockGenerateChecksum: func(ctx context.Context) (string, error) { return tfChecksum, nil }, MockWorkspace: func(_ context.Context, _ string) error { return nil }, @@ -649,7 +649,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { return nil }, MockGenerateChecksum: func(ctx context.Context) (string, error) { return tfChecksum, nil }, @@ -688,7 +688,7 @@ func TestConnect(t *testing.T) { }, usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }), fs: afero.Afero{Fs: afero.NewMemMapFs()}, - terraform: func(_ string, _ bool, _ ...string) tfclient { + terraform: func(_ string, _ bool, _ bool, _ logging.Logger, _ ...string) tfclient { return &MockTf{ MockInit: func(ctx context.Context, o ...terraform.InitOption) error { args := terraform.InitArgsToString(o) diff --git a/internal/terraform/terraform.go b/internal/terraform/terraform.go index 0e6c31d5..aab58fd0 100644 --- a/internal/terraform/terraform.go +++ b/internal/terraform/terraform.go @@ -125,8 +125,8 @@ type Harness struct { // Whether to use the terraform plugin cache UsePluginCache bool - // Whether to enable logging to container stdout - EnableLogging bool + // Whether to enable writing Terraform CLI logs to container stdout + EnableTerraformCLILogging bool // Logger Logger logging.Logger @@ -570,26 +570,18 @@ func (h Harness) Diff(ctx context.Context, o ...Option) (bool, error) { case 1: ee := &exec.ExitError{} errors.As(err, &ee) - if err = h.writeLogs(ee.Stderr, h.EnableLogging ); err != nil { - return false, errors.Wrap(err, errWriteLogs) + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(ee.Stderr)) } case 2: - if err = h.writeLogs(log, h.EnableLogging); err != nil { - return false, errors.Wrap(err, errWriteLogs) + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(log)) } return true, nil } return false, Classify(err) } -func (h Harness) writeLogs(out []byte, enableLogging bool) error { - if !enableLogging { - return nil - } - h.Logger.Debug(string(out)) - return nil -} - // Apply a Terraform configuration. func (h Harness) Apply(ctx context.Context, o ...Option) error { ao := &options{} @@ -615,7 +607,23 @@ func (h Harness) Apply(ctx context.Context, o ...Option) error { defer rwmutex.RUnlock() } - _, err := runCommand(ctx, cmd) + // In case of terraform apply + // 0 - Succeeded + // Non Zero output - Errored + + log, err := runCommand(ctx, cmd) + switch cmd.ProcessState.ExitCode() { + case 0: + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(log)) + } + default: + ee := &exec.ExitError{} + errors.As(err, &ee) + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(ee.Stderr)) + } + } return Classify(err) } @@ -644,7 +652,23 @@ func (h Harness) Destroy(ctx context.Context, o ...Option) error { defer rwmutex.RUnlock() } - _, err := runCommand(ctx, cmd) + log, err := runCommand(ctx, cmd) + + // In case of terraform destroy + // 0 - Succeeded + // Non Zero output - Errored + switch cmd.ProcessState.ExitCode() { + case 0: + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(log)) + } + default: + ee := &exec.ExitError{} + errors.As(err, &ee) + if h.EnableTerraformCLILogging{ + h.Logger.Info(string(ee.Stderr)) + } + } return Classify(err) } diff --git a/package/crds/tf.upbound.io_workspaces.yaml b/package/crds/tf.upbound.io_workspaces.yaml index 13f76cd1..1b0e0484 100644 --- a/package/crds/tf.upbound.io_workspaces.yaml +++ b/package/crds/tf.upbound.io_workspaces.yaml @@ -61,7 +61,7 @@ spec: - Orphan - Delete type: string - enableLogging: + enableTerraformCLILogging: type: boolean forProvider: description: WorkspaceParameters are the configurable fields of a