Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write Terraform CLI logs to container stdout #258

Merged
merged 16 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apis/v1beta1/workspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ type WorkspaceParameters struct {

// Arguments to be included in the terraform destroy CLI command
DestroyArgs []string `json:"destroyArgs,omitempty"`

// Boolean value to indicate CLI logging of terraform execution is enabled or not
// +optional
EnableTerraformCLILogging bool `json:"enableTerraformCLILogging,omitempty"`
}

// WorkspaceObservation are the observable fields of a Workspace.
Expand Down
12 changes: 8 additions & 4 deletions cmd/provider/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"path/filepath"
"time"

zapuber "go.uber.org/zap"

"github.com/crossplane/crossplane-runtime/pkg/certificates"
"github.com/crossplane/crossplane-runtime/pkg/controller"
"github.com/crossplane/crossplane-runtime/pkg/feature"
Expand Down Expand Up @@ -64,7 +66,7 @@ func main() {
)
kingpin.MustParse(app.Parse(os.Args[1:]))

zl := zap.New(zap.UseDevMode(*debug), UseISO8601())
zl := zap.New(zap.UseDevMode(*debug), UseJSONencoder())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree that the logging format change should be in a separate PR and should be able to be specified as a runtime argument. It should default to the current format to prevent breaking existing deployments and allow for a json option to be specified. If we want to change the default log format in the future we can plan for it and announce it ahead of time, but for now I think it's better not to change the default format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bobh66, we reverted the changes for the logging format. Please review them when you get the chance and let us know if you require anything else to get this PR merged!

log := logging.NewLogrLogger(zl.WithName("provider-terraform"))
if *debug {
// The controller-runtime runs with a no-op logger by default. It is
Expand Down Expand Up @@ -148,9 +150,11 @@ func main() {
kingpin.FatalIfError(mgr.Start(ctrl.SetupSignalHandler()), "Cannot start controller manager")
}

// UseISO8601 sets the logger to use ISO8601 timestamp format
func UseISO8601() zap.Opts {
// UseJSONencoder sets the logger to use json format for log records
func UseJSONencoder() zap.Opts {
return func(o *zap.Options) {
o.TimeEncoder = zapcore.ISO8601TimeEncoder
encoderConfig := zapuber.NewProductionEncoderConfig()
encoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
o.Encoder = zapcore.NewJSONEncoder(encoderConfig)
}
}
22 changes: 22 additions & 0 deletions docs/monolith/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -355,3 +355,25 @@ spec:
At Vault side configuration is also needed to allow the write operation, see
[example](https://docs.crossplane.io/knowledge-base/integrations/vault-as-secret-store/)
here for inspiration.


## Enable Terraform CLI logs

Terraform CLI output can be written to the container logs to assist with debugging and to view detailed information about Terraform operations.
To enable it, the `Workspace` spec has an **optional** `EnableTerraformCLILogging` field.
```yaml
apiVersion: tf.upbound.io/v1beta1
kind: Workspace
metadata:
name: example-random-generator
annotations:
meta.upbound.io/example-id: tf/v1beta1/workspace
crossplane.io/external-name: random
spec:
forProvider:
source: Inline
enableTerraformCLILogging: true
...
```

- `enableTerraformCLILogging`: Specifies whether logging is enabled (`true`) or disabled (`false`). When enabled, Terraform CLI command output will be written to the container logs. Default is `false`
34 changes: 34 additions & 0 deletions examples/workspace-enable-logging.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
apiVersion: tf.upbound.io/v1beta1
kind: Workspace
metadata:
name: example-random-generator
annotations:
meta.upbound.io/example-id: tf/v1beta1/workspace
# The terraform workspace will be named 'random'. If you omit this
# annotation it would be derived from metadata.name - e.g. 'example-random-generator.
crossplane.io/external-name: random
spec:
forProvider:
enableTerraformCLILogging: true
source: Inline
module: |
resource "random_id" "example_id" {
byte_length = 8
}
resource "random_password" "password" {
length = 16
special = true
}
// Non-sensitive Outputs are written to status.atProvider.outputs and to the connection secret.
output "random_id_hex" {
value = random_id.example_id.hex
}
// Sensitive Outputs are only written to the connection secret
output "random_password" {
value = random_password.password
sensitive = true
}
// Terraform has several other random resources, see the random provider for details
writeConnectionSecretToRef:
namespace: default
name: terraform-workspace-example-random-generator
8 changes: 4 additions & 4 deletions internal/controller/workspace/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, envs ...string) tfclient {
return terraform.Harness{Path: tfPath, Dir: dir, UsePluginCache: usePluginCache, 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}
},
}

Expand Down Expand Up @@ -168,7 +168,7 @@ type connector struct {
usage resource.Tracker
logger logging.Logger
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
}

func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { //nolint:gocyclo
Expand Down Expand Up @@ -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, envs...)
tf := c.terraform(dir, *pc.Spec.PluginCache, cr.Spec.ForProvider.EnableTerraformCLILogging, l, envs...)
if cr.Status.AtProvider.Checksum != "" {
checksum, err := tf.GenerateChecksum(ctx)
if err != nil {
Expand Down
30 changes: 15 additions & 15 deletions internal/controller/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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 }}
},
},
Expand All @@ -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 },
Expand All @@ -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 },
}
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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)
Expand Down
61 changes: 55 additions & 6 deletions internal/terraform/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ import (
"sort"
"strconv"
"strings"
"syscall"

"sync"
"syscall"

"github.com/crossplane/crossplane-runtime/pkg/logging"
"github.com/pkg/errors"
)

Expand All @@ -47,6 +47,7 @@ const (
errRunCommand = "shutdown while running terraform command"
errSigTerm = "error sending SIGTERM to child process"
errWaitTerm = "error waiting for child process to terminate"
errWriteLogs = "error writing terraform logs to stdout"

tfDefault = "default"
)
Expand Down Expand Up @@ -125,6 +126,12 @@ type Harness struct {
// Whether to use the terraform plugin cache
UsePluginCache bool

// Whether to enable writing Terraform CLI logs to container stdout
EnableTerraformCLILogging bool

// Logger
Logger logging.Logger

// Environment Variables
Envs []string

Expand Down Expand Up @@ -559,8 +566,18 @@ func (h Harness) Diff(ctx context.Context, o ...Option) (bool, error) {
// 0 - Succeeded, diff is empty (no changes)
// 1 - Errored
// 2 - Succeeded, there is a diff
_, err := runCommand(ctx, cmd)
if cmd.ProcessState.ExitCode() == 2 {
log, err := runCommand(ctx, cmd)
switch cmd.ProcessState.ExitCode() {
case 1:
ee := &exec.ExitError{}
errors.As(err, &ee)
if h.EnableTerraformCLILogging {
h.Logger.Info(string(ee.Stderr), "operation", "plan")
}
case 2:
if h.EnableTerraformCLILogging {
h.Logger.Info(string(log), "operation", "plan")
}
return true, nil
}
return false, Classify(err)
Expand Down Expand Up @@ -591,7 +608,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), "operation", "apply")
}
default:
ee := &exec.ExitError{}
errors.As(err, &ee)
if h.EnableTerraformCLILogging {
h.Logger.Info(string(ee.Stderr), "operation", "apply")
}
}
return Classify(err)
}

Expand Down Expand Up @@ -620,7 +653,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), "operation", "delete")
}
default:
ee := &exec.ExitError{}
errors.As(err, &ee)
if h.EnableTerraformCLILogging {
h.Logger.Info(string(ee.Stderr), "operation", "delete")
}
}
return Classify(err)
}

Expand Down
4 changes: 4 additions & 0 deletions package/crds/tf.upbound.io_workspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ spec:
items:
type: string
type: array
enableTerraformCLILogging:
description: Boolean value to indicate CLI logging of terraform
execution is enabled or not
type: boolean
entrypoint:
default: ""
description: Entrypoint for `terraform init` within the module
Expand Down