From 3ae6109815d2e885638df44180dfd95ac6262028 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 10 Jun 2022 17:22:35 -0700 Subject: [PATCH 1/7] Remove tfexec, allow TF_ environment vars and log them Signed-off-by: Spike Curtis --- go.mod | 5 +- go.sum | 1 + provisioner/terraform/executor.go | 338 ++++++++++++++++++++ provisioner/terraform/parse.go | 2 +- provisioner/terraform/provision.go | 407 +++++------------------- provisioner/terraform/provision_test.go | 147 +++++++-- provisioner/terraform/serve.go | 12 +- provisionersdk/logger.go | 71 +++++ provisionersdk/logger_test.go | 123 +++++++ provisionersdk/mocks/ParseStream.go | 42 +++ provisionersdk/mocks/ProvisionStream.go | 42 +++ 11 files changed, 831 insertions(+), 359 deletions(-) create mode 100644 provisioner/terraform/executor.go create mode 100644 provisionersdk/logger.go create mode 100644 provisionersdk/logger_test.go create mode 100644 provisionersdk/mocks/ParseStream.go create mode 100644 provisionersdk/mocks/ProvisionStream.go diff --git a/go.mod b/go.mod index 93f821a3ca5e2..0edeb575c512f 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,6 @@ go 1.18 // Required until https://github.com/manifoldco/promptui/pull/169 is merged. replace github.com/manifoldco/promptui => github.com/kylecarbs/promptui v0.8.1-0.20201231190244-d8f2159af2b2 -// Required until https://github.com/hashicorp/terraform-exec/pull/275 and https://github.com/hashicorp/terraform-exec/pull/276 are merged. -replace github.com/hashicorp/terraform-exec => github.com/kylecarbs/terraform-exec v0.15.1-0.20220202050609-a1ce7181b180 - // Required until https://github.com/hashicorp/terraform-config-inspect/pull/74 is merged. replace github.com/hashicorp/terraform-config-inspect => github.com/kylecarbs/terraform-config-inspect v0.0.0-20211215004401-bbc517866b88 @@ -74,7 +71,6 @@ require ( github.com/hashicorp/hc-install v0.3.2 github.com/hashicorp/hcl/v2 v2.12.0 github.com/hashicorp/terraform-config-inspect v0.0.0-20211115214459-90acf1ca460f - github.com/hashicorp/terraform-exec v0.15.0 github.com/hashicorp/terraform-json v0.14.0 github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87 github.com/jedib0t/go-pretty/v6 v6.3.1 @@ -129,6 +125,7 @@ require ( require ( github.com/agnivade/levenshtein v1.0.1 // indirect + github.com/stretchr/objx v0.2.0 // indirect github.com/vektah/gqlparser/v2 v2.4.4 // indirect github.com/yuin/goldmark v1.4.12 // indirect ) diff --git a/go.sum b/go.sum index 3ba4ccc9472f3..92f246ddb793c 100644 --- a/go.sum +++ b/go.sum @@ -1500,6 +1500,7 @@ github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag github.com/stretchr/objx v0.0.0-20180129172003-8a3f7159479f/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/testify v0.0.0-20180303142811-b89eecf5ca5d/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go new file mode 100644 index 0000000000000..f95484c7ea5d2 --- /dev/null +++ b/provisioner/terraform/executor.go @@ -0,0 +1,338 @@ +package terraform + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + + "golang.org/x/xerrors" + + "github.com/hashicorp/go-version" + tfjson "github.com/hashicorp/terraform-json" + + "github.com/coder/coder/provisionersdk" + "github.com/coder/coder/provisionersdk/proto" +) + +type executor struct { + binaryPath string + cachePath string + workdir string +} + +func (e executor) basicEnv() []string { + // Required for "terraform init" to find "git" to + // clone Terraform modules. + env := os.Environ() + // Only Linux reliably works with the Terraform plugin + // cache directory. It's unknown why this is. + if e.cachePath != "" && runtime.GOOS == "linux" { + env = append(env, "TF_PLUGIN_CACHE_DIR="+e.cachePath) + } + return env +} + +func (e executor) execWriteOutput(ctx context.Context, args, env []string, writer io.WriteCloser) (err error) { + defer func() { + closeErr := writer.Close() + if err == nil && closeErr != nil { + err = closeErr + } + }() + stdErr := &bytes.Buffer{} + // #nosec + cmd := exec.CommandContext(ctx, e.binaryPath, args...) + cmd.Dir = e.workdir + cmd.Stdout = writer + cmd.Stderr = stdErr + cmd.Env = env + if err = cmd.Run(); err != nil { + errString, _ := io.ReadAll(stdErr) + return xerrors.Errorf("%s: %w", errString, err) + } + return nil +} + +func (e executor) execParseJSON(ctx context.Context, args, env []string, v interface{}) error { + // #nosec + cmd := exec.CommandContext(ctx, e.binaryPath, args...) + cmd.Dir = e.workdir + cmd.Env = env + out := &bytes.Buffer{} + stdErr := &bytes.Buffer{} + cmd.Stdout = out + cmd.Stderr = stdErr + err := cmd.Run() + if err != nil { + // if noStateRegex.MatchString(err.Error()) { + // return nil, os.ErrNotExist + //} + errString, _ := io.ReadAll(stdErr) + return xerrors.Errorf("%s: %w", errString, err) + } + + dec := json.NewDecoder(out) + dec.UseNumber() + err = dec.Decode(v) + if err != nil { + return xerrors.Errorf("decode terraform json: %w", err) + } + return nil +} + +func (e executor) checkMinVersion(ctx context.Context) error { + v, err := e.getVersion(ctx) + if err != nil { + return err + } + if !v.GreaterThanOrEqual(minimumTerraformVersion) { + return xerrors.Errorf("terraform version %q is too old. required >= %q", v.String(), minimumTerraformVersion.String()) + } + return nil +} + +func (e executor) getVersion(ctx context.Context) (*version.Version, error) { + // #nosec + cmd := exec.CommandContext(ctx, e.binaryPath, "version", "-json") + out, err := cmd.Output() + if err != nil { + return nil, err + } + vj := tfjson.VersionOutput{} + err = json.Unmarshal(out, &vj) + if err != nil { + return nil, err + } + return version.NewVersion(vj.Version) +} + +func (e executor) init(ctx context.Context, logger provisionersdk.Logger) error { + writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_DEBUG) + defer func() { <-doneLogging }() + return e.execWriteOutput(ctx, []string{"init"}, e.basicEnv(), writer) +} + +// revive:disable-next-line:flag-parameter +func (e executor) plan(ctx context.Context, env, vars []string, logger provisionersdk.Logger, destroy bool) (*proto.Provision_Response, error) { + planfilePath := filepath.Join(e.workdir, "terraform.tfplan") + args := []string{ + "plan", + "-no-color", + "-input=false", + "-json", + "-refresh=true", + "-out=" + planfilePath, + } + if destroy { + args = append(args, "-destroy") + } + for _, variable := range vars { + args = append(args, "-var", variable) + } + + writer, doneLogging := provisionLogWriter(logger) + defer func() { <-doneLogging }() + + err := e.execWriteOutput(ctx, args, env, writer) + if err != nil { + return nil, xerrors.Errorf("terraform plan: %w", err) + } + resources, err := e.planResources(ctx, planfilePath) + if err != nil { + return nil, err + } + return &proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: resources, + }, + }, + }, nil +} + +func (e executor) planResources(ctx context.Context, planfilePath string) ([]*proto.Resource, error) { + plan, err := e.showPlan(ctx, planfilePath) + if err != nil { + return nil, xerrors.Errorf("show terraform plan file: %w", err) + } + + rawGraph, err := e.graph(ctx) + if err != nil { + return nil, xerrors.Errorf("graph: %w", err) + } + return ConvertResources(plan.PlannedValues.RootModule, rawGraph) +} + +func (e executor) showPlan(ctx context.Context, planfilePath string) (*tfjson.Plan, error) { + args := []string{"show", "-json", "-no-color", planfilePath} + p := new(tfjson.Plan) + err := e.execParseJSON(ctx, args, e.basicEnv(), p) + return p, err +} + +func (e executor) graph(ctx context.Context) (string, error) { + // #nosec + cmd := exec.CommandContext(ctx, e.binaryPath, "graph") + cmd.Dir = e.workdir + cmd.Env = e.basicEnv() + out, err := cmd.Output() + if err != nil { + return "", xerrors.Errorf("graph: %w", err) + } + return string(out), nil +} + +// revive:disable-next-line:flag-parameter +func (e executor) apply(ctx context.Context, env, vars []string, logger provisionersdk.Logger, destroy bool, +) (*proto.Provision_Response, error) { + args := []string{ + "apply", + "-no-color", + "-auto-approve", + "-input=false", + "-json", + "-refresh=true", + } + if destroy { + args = append(args, "-destroy") + } + for _, variable := range vars { + args = append(args, "-var", variable) + } + + writer, doneLogging := provisionLogWriter(logger) + defer func() { <-doneLogging }() + + err := e.execWriteOutput(ctx, args, env, writer) + if err != nil { + return nil, xerrors.Errorf("terraform apply: %w", err) + } + resources, err := e.stateResources(ctx) + if err != nil { + return nil, err + } + statefilePath := filepath.Join(e.workdir, "terraform.tfstate") + stateContent, err := os.ReadFile(statefilePath) + if err != nil { + return nil, xerrors.Errorf("read statefile %q: %w", statefilePath, err) + } + return &proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: resources, + State: stateContent, + }, + }, + }, nil +} + +func (e executor) stateResources(ctx context.Context) ([]*proto.Resource, error) { + state, err := e.getState(ctx) + if err != nil { + return nil, err + } + rawGraph, err := e.graph(ctx) + if err != nil { + return nil, xerrors.Errorf("get terraform graph: %w", err) + } + var resources []*proto.Resource + if state.Values != nil { + resources, err = ConvertResources(state.Values.RootModule, rawGraph) + if err != nil { + return nil, err + } + } + return resources, nil +} + +func (e executor) getState(ctx context.Context) (*tfjson.State, error) { + args := []string{"show", "-json"} + state := &tfjson.State{} + err := e.execParseJSON(ctx, args, e.basicEnv(), state) + if err != nil { + return nil, xerrors.Errorf("get terraform state: %w", err) + } + return state, nil +} + +func provisionLogWriter(logger provisionersdk.Logger) (io.WriteCloser, <-chan any) { + r, w := io.Pipe() + done := make(chan any) + go provisionReadAndLog(logger, r, done) + return w, done +} + +func provisionReadAndLog(logger provisionersdk.Logger, reader io.Reader, done chan<- any) { + defer close(done) + decoder := json.NewDecoder(reader) + for { + var log terraformProvisionLog + err := decoder.Decode(&log) + if err != nil { + return + } + logLevel := convertTerraformLogLevel(log.Level, logger) + + err = logger.Log(&proto.Log{Level: logLevel, Output: log.Message}) + if err != nil { + // Not much we can do. We can't log because logging is itself breaking! + return + } + + if log.Diagnostic == nil { + continue + } + + // If the diagnostic is provided, let's provide a bit more info! + logLevel = convertTerraformLogLevel(log.Diagnostic.Severity, logger) + if err != nil { + continue + } + err = logger.Log(&proto.Log{Level: logLevel, Output: log.Diagnostic.Detail}) + if err != nil { + // Not much we can do. We can't log because logging is itself breaking! + return + } + } +} + +func convertTerraformLogLevel(logLevel string, logger provisionersdk.Logger) proto.LogLevel { + switch strings.ToLower(logLevel) { + case "trace": + return proto.LogLevel_TRACE + case "debug": + return proto.LogLevel_DEBUG + case "info": + return proto.LogLevel_INFO + case "warn": + return proto.LogLevel_WARN + case "error": + return proto.LogLevel_ERROR + default: + _ = logger.Log(&proto.Log{ + Level: proto.LogLevel_WARN, + Output: fmt.Sprintf("unable to convert log level %s", logLevel), + }) + return proto.LogLevel_INFO + } +} + +type terraformProvisionLog struct { + Level string `json:"@level"` + Message string `json:"@message"` + + Diagnostic *terraformProvisionLogDiagnostic `json:"diagnostic"` +} + +type terraformProvisionLogDiagnostic struct { + Severity string `json:"severity"` + Summary string `json:"summary"` + Detail string `json:"detail"` +} diff --git a/provisioner/terraform/parse.go b/provisioner/terraform/parse.go index 6c8abe6f0aee1..8841b74d590f0 100644 --- a/provisioner/terraform/parse.go +++ b/provisioner/terraform/parse.go @@ -11,7 +11,7 @@ import ( ) // Parse extracts Terraform variables from source-code. -func (*terraform) Parse(request *proto.Parse_Request, stream proto.DRPCProvisioner_ParseStream) error { +func (*server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisioner_ParseStream) error { defer func() { _ = stream.CloseSend() }() diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index ec42f4b28cead..5090ea03c70f2 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -1,33 +1,21 @@ package terraform import ( - "bufio" "context" - "encoding/json" "fmt" - "io" "os" - "os/exec" "path/filepath" - "regexp" - "runtime" "strings" - "github.com/hashicorp/terraform-exec/tfexec" - tfjson "github.com/hashicorp/terraform-json" "golang.org/x/xerrors" "github.com/coder/coder/provisionersdk" "github.com/coder/coder/provisionersdk/proto" ) -var ( - // noStateRegex is matched against the output from `terraform state show` - noStateRegex = regexp.MustCompile(`no state`) -) - -// Provision executes `terraform apply`. -func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { +// Provision executes `terraform apply` or `terraform plan` for dry runs. +func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { + logger := provisionersdk.NewProvisionLogger(stream) shutdown, shutdownFunc := context.WithCancel(stream.Context()) defer shutdownFunc() @@ -58,16 +46,15 @@ func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) erro }() start := request.GetStart() - terraform, err := tfexec.NewTerraform(start.Directory, t.binaryPath) if err != nil { return xerrors.Errorf("create new terraform executor: %w", err) } - version, _, err := terraform.Version(shutdown, false) - if err != nil { - return xerrors.Errorf("get terraform version: %w", err) + e := t.executor(start.Directory) + if err := e.checkMinVersion(stream.Context()); err != nil { + return err } - if !version.GreaterThanOrEqual(minimumTerraformVersion) { - return xerrors.Errorf("terraform version %q is too old. required >= %q", version.String(), minimumTerraformVersion.String()) + if err := logTerraformEnvVars(logger); err != nil { + return err } statefilePath := filepath.Join(start.Directory, "terraform.tfstate") @@ -78,200 +65,51 @@ func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) erro } } - reader, writer := io.Pipe() - defer reader.Close() - defer writer.Close() - go func() { - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - _ = stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Log{ - Log: &proto.Log{ - Level: proto.LogLevel_DEBUG, - Output: scanner.Text(), - }, - }, - }) - } - }() - terraformEnv := map[string]string{} - // Required for "terraform init" to find "git" to - // clone Terraform modules. - for _, env := range os.Environ() { - parts := strings.SplitN(env, "=", 2) - if len(parts) < 2 { - continue - } - terraformEnv[parts[0]] = parts[1] - } - // Only Linux reliably works with the Terraform plugin - // cache directory. It's unknown why this is. - if t.cachePath != "" && runtime.GOOS == "linux" { - terraformEnv["TF_PLUGIN_CACHE_DIR"] = t.cachePath - } - err = terraform.SetEnv(terraformEnv) - if err != nil { - return xerrors.Errorf("set terraform env: %w", err) - } - terraform.SetStdout(writer) - t.logger.Debug(shutdown, "running initialization") - err = terraform.Init(shutdown) - if err != nil { - return xerrors.Errorf("initialize terraform: %w", err) - } - t.logger.Debug(shutdown, "ran initialization") - _ = reader.Close() - terraform.SetStdout(io.Discard) - - env := os.Environ() - env = append(env, - "CODER_AGENT_URL="+start.Metadata.CoderUrl, - "CODER_WORKSPACE_TRANSITION="+strings.ToLower(start.Metadata.WorkspaceTransition.String()), - "CODER_WORKSPACE_NAME="+start.Metadata.WorkspaceName, - "CODER_WORKSPACE_OWNER="+start.Metadata.WorkspaceOwner, - "CODER_WORKSPACE_ID="+start.Metadata.WorkspaceId, - "CODER_WORKSPACE_OWNER_ID="+start.Metadata.WorkspaceOwnerId, - ) - for key, value := range provisionersdk.AgentScriptEnv() { - env = append(env, key+"="+value) - } - vars := []string{} - for _, param := range start.ParameterValues { - switch param.DestinationScheme { - case proto.ParameterDestination_ENVIRONMENT_VARIABLE: - env = append(env, fmt.Sprintf("%s=%s", param.Name, param.Value)) - case proto.ParameterDestination_PROVISIONER_VARIABLE: - vars = append(vars, fmt.Sprintf("%s=%s", param.Name, param.Value)) - default: - return xerrors.Errorf("unsupported parameter type %q for %q", param.DestinationScheme, param.Name) - } - } - - closeChan := make(chan struct{}) - reader, writer = io.Pipe() - defer reader.Close() - defer writer.Close() - go func() { - defer close(closeChan) - decoder := json.NewDecoder(reader) - for { - var log terraformProvisionLog - err := decoder.Decode(&log) - if err != nil { - return - } - logLevel, err := convertTerraformLogLevel(log.Level) - if err != nil { - // Not a big deal, but we should handle this at some point! - continue - } - _ = stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Log{ - Log: &proto.Log{ - Level: logLevel, - Output: log.Message, - }, - }, - }) - - if log.Diagnostic == nil { - continue - } - - // If the diagnostic is provided, let's provide a bit more info! - logLevel, err = convertTerraformLogLevel(log.Diagnostic.Severity) - if err != nil { - continue - } - _ = stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Log{ - Log: &proto.Log{ - Level: logLevel, - Output: log.Diagnostic.Detail, - }, - }, - }) - } - }() - // If we're destroying, exit early if there's no state. This is necessary to // avoid any cases where a workspace is "locked out" of terraform due to // e.g. bad template param values and cannot be deleted. This is just for // contingency, in the future we will try harder to prevent workspaces being // broken this hard. - if start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY { - _, err := pullTerraformState(shutdown, terraform, statefilePath) - if xerrors.Is(err, os.ErrNotExist) { - _ = stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Log{ - Log: &proto.Log{ - Level: proto.LogLevel_INFO, - Output: "The terraform state does not exist, there is nothing to do", - }, + if start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY && len(start.State) == 0 { + _ = stream.Send(&proto.Provision_Response{ + Type: &proto.Provision_Response_Log{ + Log: &proto.Log{ + Level: proto.LogLevel_INFO, + Output: "The terraform state does not exist, there is nothing to do", }, - }) + }, + }) - return stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Complete{ - Complete: &proto.Provision_Complete{}, - }, - }) - } - if err != nil { - err = xerrors.Errorf("get terraform state: %w", err) - _ = stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Complete{ - Complete: &proto.Provision_Complete{ - Error: err.Error(), - }, - }, - }) + return stream.Send(&proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }) + } - return err - } + t.logger.Debug(shutdown, "running initialization") + err = e.init(stream.Context(), logger) + if err != nil { + return xerrors.Errorf("initialize terraform: %w", err) } + t.logger.Debug(shutdown, "ran initialization") - planfilePath := filepath.Join(start.Directory, "terraform.tfplan") - var args []string - if start.DryRun { - args = []string{ - "plan", - "-no-color", - "-input=false", - "-json", - "-refresh=true", - "-out=" + planfilePath, - } - } else { - args = []string{ - "apply", - "-no-color", - "-auto-approve", - "-input=false", - "-json", - "-refresh=true", - } + env, err := provisionEnv(start) + if err != nil { + return err } - if start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY { - args = append(args, "-destroy") + vars, err := provisionVars(start) + if err != nil { + return err } - for _, variable := range vars { - args = append(args, "-var", variable) + var resp *proto.Provision_Response + if start.DryRun { + resp, err = e.plan(shutdown, env, vars, logger, + start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) + } else { + resp, err = e.apply(shutdown, env, vars, logger, + start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) } - // #nosec - cmd := exec.CommandContext(stream.Context(), t.binaryPath, args...) - go func() { - select { - case <-stream.Context().Done(): - return - case <-shutdown.Done(): - _ = cmd.Process.Signal(os.Interrupt) - } - }() - cmd.Stdout = writer - cmd.Env = env - cmd.Dir = terraform.WorkingDir() - err = cmd.Run() if err != nil { if start.DryRun { if shutdown.Err() != nil { @@ -298,141 +136,60 @@ func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) erro }, }) } - _ = reader.Close() - <-closeChan - var resp *proto.Provision_Response - if start.DryRun { - resp, err = parseTerraformPlan(stream.Context(), terraform, planfilePath) - } else { - resp, err = parseTerraformApply(stream.Context(), terraform, statefilePath) - } - if err != nil { - return err - } return stream.Send(resp) } -func parseTerraformPlan(ctx context.Context, terraform *tfexec.Terraform, planfilePath string) (*proto.Provision_Response, error) { - plan, err := terraform.ShowPlanFile(ctx, planfilePath) - if err != nil { - return nil, xerrors.Errorf("show terraform plan file: %w", err) - } - - rawGraph, err := terraform.Graph(ctx) - if err != nil { - return nil, xerrors.Errorf("graph: %w", err) - } - resources, err := ConvertResources(plan.PlannedValues.RootModule, rawGraph) - if err != nil { - return nil, err - } - - return &proto.Provision_Response{ - Type: &proto.Provision_Response_Complete{ - Complete: &proto.Provision_Complete{ - Resources: resources, - }, - }, - }, nil -} - -func parseTerraformApply(ctx context.Context, terraform *tfexec.Terraform, statefilePath string) (*proto.Provision_Response, error) { - _, err := os.Stat(statefilePath) - statefileExisted := err == nil - - state, err := pullTerraformState(ctx, terraform, statefilePath) - if err != nil { - return nil, xerrors.Errorf("get terraform state: %w", err) - } - rawGraph, err := terraform.Graph(ctx) - if err != nil { - return nil, xerrors.Errorf("get terraform graph: %w", err) - } - var resources []*proto.Resource - if state.Values != nil { - resources, err = ConvertResources(state.Values.RootModule, rawGraph) - if err != nil { - return nil, err - } - } - - var stateContent []byte - // We only want to restore state if it's not hosted remotely. - if statefileExisted { - stateContent, err = os.ReadFile(statefilePath) - if err != nil { - return nil, xerrors.Errorf("read statefile %q: %w", statefilePath, err) +func provisionVars(start *proto.Provision_Start) ([]string, error) { + vars := []string{} + for _, param := range start.ParameterValues { + switch param.DestinationScheme { + case proto.ParameterDestination_ENVIRONMENT_VARIABLE: + continue + case proto.ParameterDestination_PROVISIONER_VARIABLE: + vars = append(vars, fmt.Sprintf("%s=%s", param.Name, param.Value)) + default: + return nil, xerrors.Errorf("unsupported parameter type %q for %q", param.DestinationScheme, param.Name) } } - - return &proto.Provision_Response{ - Type: &proto.Provision_Response_Complete{ - Complete: &proto.Provision_Complete{ - State: stateContent, - Resources: resources, - }, - }, - }, nil + return vars, nil } -// pullTerraformState pulls and merges any remote terraform state into the given -// path and reads the merged state. If there is no state, `os.ErrNotExist` will -// be returned. -func pullTerraformState(ctx context.Context, terraform *tfexec.Terraform, statefilePath string) (*tfjson.State, error) { - statefile, err := os.OpenFile(statefilePath, os.O_CREATE|os.O_RDWR, 0600) - if err != nil { - return nil, xerrors.Errorf("open statefile %q: %w", statefilePath, err) - } - defer statefile.Close() - - // #nosec - cmd := exec.CommandContext(ctx, terraform.ExecPath(), "state", "pull") - cmd.Dir = terraform.WorkingDir() - cmd.Stdout = statefile - err = cmd.Run() - if err != nil { - return nil, xerrors.Errorf("pull terraform state: %w", err) +func provisionEnv(start *proto.Provision_Start) ([]string, error) { + env := os.Environ() + env = append(env, + "CODER_AGENT_URL="+start.Metadata.CoderUrl, + "CODER_WORKSPACE_TRANSITION="+strings.ToLower(start.Metadata.WorkspaceTransition.String()), + "CODER_WORKSPACE_NAME="+start.Metadata.WorkspaceName, + "CODER_WORKSPACE_OWNER="+start.Metadata.WorkspaceOwner, + "CODER_WORKSPACE_ID="+start.Metadata.WorkspaceId, + "CODER_WORKSPACE_OWNER_ID="+start.Metadata.WorkspaceOwnerId, + ) + for key, value := range provisionersdk.AgentScriptEnv() { + env = append(env, key+"="+value) } - - state, err := terraform.ShowStateFile(ctx, statefilePath) - if err != nil { - if noStateRegex.MatchString(err.Error()) { - return nil, os.ErrNotExist + for _, param := range start.ParameterValues { + switch param.DestinationScheme { + case proto.ParameterDestination_ENVIRONMENT_VARIABLE: + env = append(env, fmt.Sprintf("%s=%s", param.Name, param.Value)) + case proto.ParameterDestination_PROVISIONER_VARIABLE: + continue + default: + return nil, xerrors.Errorf("unsupported parameter type %q for %q", param.DestinationScheme, param.Name) } - - return nil, xerrors.Errorf("show terraform state: %w", err) } - - return state, nil -} - -type terraformProvisionLog struct { - Level string `json:"@level"` - Message string `json:"@message"` - - Diagnostic *terraformProvisionLogDiagnostic `json:"diagnostic"` -} - -type terraformProvisionLogDiagnostic struct { - Severity string `json:"severity"` - Summary string `json:"summary"` - Detail string `json:"detail"` + return env, nil } -func convertTerraformLogLevel(logLevel string) (proto.LogLevel, error) { - switch strings.ToLower(logLevel) { - case "trace": - return proto.LogLevel_TRACE, nil - case "debug": - return proto.LogLevel_DEBUG, nil - case "info": - return proto.LogLevel_INFO, nil - case "warn": - return proto.LogLevel_WARN, nil - case "error": - return proto.LogLevel_ERROR, nil - default: - return proto.LogLevel(0), xerrors.Errorf("invalid log level %q", logLevel) +func logTerraformEnvVars(logger provisionersdk.Logger) error { + env := os.Environ() + for _, e := range env { + if strings.HasPrefix(e, "TF_") { + err := logger.Log(&proto.Log{Level: proto.LogLevel_WARN, Output: "terraform environment variable: " + e}) + if err != nil { + return err + } + } } + return nil } diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 9e381c63dab6e..e83ed31801ea3 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build linux || darwin package terraform_test @@ -22,24 +22,7 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -func TestProvision(t *testing.T) { - t.Parallel() - - provider := ` -terraform { - required_providers { - coder = { - source = "coder/coder" - version = "0.4.2" - } - } -} - -provider "coder" { -} - ` - t.Log(provider) - +func testProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) { client, server := provisionersdk.TransportPipe() ctx, cancelFunc := context.WithCancel(context.Background()) t.Cleanup(func() { @@ -57,6 +40,13 @@ provider "coder" { assert.NoError(t, err) }() api := proto.NewDRPCProvisionerClient(provisionersdk.Conn(client)) + return ctx, api +} + +func TestProvision(t *testing.T) { + t.Parallel() + + ctx, api := testProvisioner(t) for _, testCase := range []struct { Name string @@ -64,21 +54,30 @@ provider "coder" { Request *proto.Provision_Request Response *proto.Provision_Response Error bool + DryRun bool }{{ Name: "single-variable", Files: map[string]string{ "main.tf": `variable "A" { - description = "Testing!" - }`, + description = "Testing!" + }`, }, Request: &proto.Provision_Request{ Type: &proto.Provision_Request_Start{ Start: &proto.Provision_Start{ - ParameterValues: []*proto.ParameterValue{{ - DestinationScheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - Name: "A", - Value: "example", - }}, + ParameterValues: []*proto.ParameterValue{ + { + DestinationScheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + Name: "A", + Value: "example", + }, + // rando environment variable to exercise those code paths. + { + DestinationScheme: proto.ParameterDestination_ENVIRONMENT_VARIABLE, + Name: "GOALS", + Value: "AP Royal Oak", + }, + }, }, }, }, @@ -96,10 +95,44 @@ provider "coder" { Response: &proto.Provision_Response{ Type: &proto.Provision_Response_Complete{ Complete: &proto.Provision_Complete{ - Error: "exit status 1", + Error: "terraform apply: : exit status 1", }, }, }, + }, { + Name: "missing-variable-dry-run", + Files: map[string]string{ + "main.tf": `variable "A" { + }`, + }, + Response: &proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Error: "terraform plan: : exit status 1", + }, + }, + }, + DryRun: true, + Error: true, + }, { + Name: "unsupported-parameter-scheme", + Files: map[string]string{ + "main.tf": "", + }, + Request: &proto.Provision_Request{ + Type: &proto.Provision_Request_Start{ + Start: &proto.Provision_Start{ + ParameterValues: []*proto.ParameterValue{ + { + DestinationScheme: 88, + Name: "UNSUPPORTED", + Value: "sadface", + }, + }, + }, + }, + }, + Error: true, }, { Name: "single-resource", Files: map[string]string{ @@ -115,6 +148,22 @@ provider "coder" { }, }, }, + }, { + Name: "single-resource-dry-run", + Files: map[string]string{ + "main.tf": `resource "null_resource" "A" {}`, + }, + Response: &proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: []*proto.Resource{{ + Name: "A", + Type: "null_resource", + }}, + }, + }, + }, + DryRun: true, }, { Name: "invalid-sourcecode", Files: map[string]string{ @@ -136,6 +185,7 @@ provider "coder" { Type: &proto.Provision_Request_Start{ Start: &proto.Provision_Start{ Directory: directory, + DryRun: testCase.DryRun, }, }, } @@ -245,3 +295,46 @@ provider "coder" { } }) } + +// nolint:paralleltest +func TestProvision_ExtraEnv(t *testing.T) { + t.Setenv("TF_LOG", "INFO") + + ctx, api := testProvisioner(t) + + directory := t.TempDir() + path := filepath.Join(directory, "main.tf") + err := os.WriteFile(path, []byte(`resource "null_resource" "A" {}`), 0600) + require.NoError(t, err) + + request := &proto.Provision_Request{ + Type: &proto.Provision_Request_Start{ + Start: &proto.Provision_Start{ + Directory: directory, + Metadata: &proto.Provision_Metadata{ + WorkspaceTransition: proto.WorkspaceTransition_START, + }, + }, + }, + } + response, err := api.Provision(ctx) + require.NoError(t, err) + err = response.Send(request) + require.NoError(t, err) + found := false + for { + msg, err := response.Recv() + require.NoError(t, err) + + if log := msg.GetLog(); log != nil { + if strings.Contains(log.Output, "TF_LOG") { + found = true + } + } + if c := msg.GetComplete(); c != nil { + require.Empty(t, c.Error) + break + } + } + require.True(t, found) +} diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 5ef432e3728d9..60c03ed0dbe34 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -70,15 +70,23 @@ func Serve(ctx context.Context, options *ServeOptions) error { options.BinaryPath = absoluteBinary } } - return provisionersdk.Serve(ctx, &terraform{ + return provisionersdk.Serve(ctx, &server{ binaryPath: options.BinaryPath, cachePath: options.CachePath, logger: options.Logger, }, options.ServeOptions) } -type terraform struct { +type server struct { binaryPath string cachePath string logger slog.Logger } + +func (t server) executor(workdir string) executor { + return executor{ + binaryPath: t.binaryPath, + cachePath: t.cachePath, + workdir: workdir, + } +} diff --git a/provisionersdk/logger.go b/provisionersdk/logger.go new file mode 100644 index 0000000000000..af9980e017316 --- /dev/null +++ b/provisionersdk/logger.go @@ -0,0 +1,71 @@ +package provisionersdk + +import ( + "bufio" + "io" + + "github.com/coder/coder/provisionersdk/proto" +) + +type Logger interface { + Log(l *proto.Log) error +} + +type ProvisionStream interface { + Send(*proto.Provision_Response) error +} + +type ParseStream interface { + Send(response *proto.Parse_Response) error +} + +func NewProvisionLogger(s ProvisionStream) Logger { + return provisionLogger{s} +} + +func NewParseLogger(s ParseStream) Logger { + return parseLogger{s} +} + +type provisionLogger struct { + stream ProvisionStream +} + +func (p provisionLogger) Log(l *proto.Log) error { + return p.stream.Send(&proto.Provision_Response{ + Type: &proto.Provision_Response_Log{ + Log: l, + }, + }) +} + +type parseLogger struct { + stream ParseStream +} + +func (p parseLogger) Log(l *proto.Log) error { + return p.stream.Send(&proto.Parse_Response{ + Type: &proto.Parse_Response_Log{ + Log: l, + }, + }) +} + +func LogWriter(logger Logger, level proto.LogLevel) (io.WriteCloser, <-chan any) { + r, w := io.Pipe() + done := make(chan any) + go readAndLog(logger, r, done, level) + return w, done +} + +func readAndLog(logger Logger, r io.Reader, done chan<- any, level proto.LogLevel) { + defer close(done) + scanner := bufio.NewScanner(r) + for scanner.Scan() { + err := logger.Log(&proto.Log{Level: level, Output: scanner.Text()}) + if err != nil { + // Not much we can do. We can't log because logging is itself breaking! + return + } + } +} diff --git a/provisionersdk/logger_test.go b/provisionersdk/logger_test.go new file mode 100644 index 0000000000000..46c4e771a1581 --- /dev/null +++ b/provisionersdk/logger_test.go @@ -0,0 +1,123 @@ +package provisionersdk_test + +import ( + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/provisionersdk" + "github.com/coder/coder/provisionersdk/mocks" + "github.com/coder/coder/provisionersdk/proto" +) + +type logMatcher struct { + level proto.LogLevel + output string +} + +func (m logMatcher) Matches(x interface{}) bool { + switch r := x.(type) { + case *proto.Provision_Response: + return m.logMatches(r.GetLog()) + case *proto.Parse_Response: + return m.logMatches(r.GetLog()) + default: + return false + } +} + +func (m logMatcher) logMatches(l *proto.Log) bool { + if l.Level != m.level { + return false + } + if l.Output != m.output { + return false + } + return true +} + +func withLog(log *proto.Log) func(x interface{}) bool { + m := logMatcher{level: log.GetLevel(), output: log.GetOutput()} + return m.Matches +} + +func TestProvisionLogger_Log(t *testing.T) { + t.Parallel() + + mStream := new(mocks.ProvisionStream) + defer mStream.AssertExpectations(t) + + l := &proto.Log{Level: proto.LogLevel_ERROR, Output: "an error"} + mStream.On("Send", mock.MatchedBy(withLog(l))).Return(nil) + + uut := provisionersdk.NewProvisionLogger(mStream) + err := uut.Log(l) + require.NoError(t, err) +} + +func TestParseLogger_Log(t *testing.T) { + t.Parallel() + + mStream := new(mocks.ParseStream) + defer mStream.AssertExpectations(t) + + l := &proto.Log{Level: proto.LogLevel_ERROR, Output: "an error"} + mStream.On("Send", mock.MatchedBy(withLog(l))).Return(nil) + + uut := provisionersdk.NewParseLogger(mStream) + err := uut.Log(l) + require.NoError(t, err) +} + +func TestLogWriter_Mainline(t *testing.T) { + t.Parallel() + + mStream := new(mocks.ParseStream) + defer mStream.AssertExpectations(t) + + logger := provisionersdk.NewParseLogger(mStream) + writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_INFO) + + expected := []*proto.Log{ + {Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"}, + {Level: proto.LogLevel_INFO, Output: "Waiting for the sun"}, + {Level: proto.LogLevel_INFO, Output: "If the sun don't come you get a tan"}, + {Level: proto.LogLevel_INFO, Output: "From standing in the English rain"}, + } + for _, log := range expected { + mStream.On("Send", mock.MatchedBy(withLog(log))).Return(nil) + } + + _, err := writer.Write([]byte(`Sitting in an English garden +Waiting for the sun +If the sun don't come you get a tan +From standing in the English rain`)) + require.NoError(t, err) + err = writer.Close() + require.NoError(t, err) + <-doneLogging +} + +func TestLogWriter_SendError(t *testing.T) { + t.Parallel() + + mStream := new(mocks.ParseStream) + defer mStream.AssertExpectations(t) + + logger := provisionersdk.NewParseLogger(mStream) + writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_INFO) + + expected := &proto.Log{Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"} + mStream.On("Send", mock.MatchedBy(withLog(expected))).Return(xerrors.New("Goo goo g'joob")) + + _, err := writer.Write([]byte(`Sitting in an English garden +Waiting for the sun +If the sun don't come you get a tan +From standing in the English rain`)) + require.NoError(t, err) + err = writer.Close() + require.NoError(t, err) + <-doneLogging +} diff --git a/provisionersdk/mocks/ParseStream.go b/provisionersdk/mocks/ParseStream.go new file mode 100644 index 0000000000000..19c6530313d70 --- /dev/null +++ b/provisionersdk/mocks/ParseStream.go @@ -0,0 +1,42 @@ +// Code generated by mockery v2.12.3. DO NOT EDIT. + +package mocks + +import ( + proto "github.com/coder/coder/provisionersdk/proto" + mock "github.com/stretchr/testify/mock" +) + +// ParseStream is an autogenerated mock type for the ParseStream type +type ParseStream struct { + mock.Mock +} + +// Send provides a mock function with given fields: response +func (_m *ParseStream) Send(response *proto.Parse_Response) error { + ret := _m.Called(response) + + var r0 error + if rf, ok := ret.Get(0).(func(*proto.Parse_Response) error); ok { + r0 = rf(response) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type NewParseStreamT interface { + mock.TestingT + Cleanup(func()) +} + +// NewParseStream creates a new instance of ParseStream. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewParseStream(t NewParseStreamT) *ParseStream { + mock := &ParseStream{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/provisionersdk/mocks/ProvisionStream.go b/provisionersdk/mocks/ProvisionStream.go new file mode 100644 index 0000000000000..3d9fec048cb41 --- /dev/null +++ b/provisionersdk/mocks/ProvisionStream.go @@ -0,0 +1,42 @@ +// Code generated by mockery v2.12.3. DO NOT EDIT. + +package mocks + +import ( + proto "github.com/coder/coder/provisionersdk/proto" + mock "github.com/stretchr/testify/mock" +) + +// ProvisionStream is an autogenerated mock type for the ProvisionStream type +type ProvisionStream struct { + mock.Mock +} + +// Send provides a mock function with given fields: _a0 +func (_m *ProvisionStream) Send(_a0 *proto.Provision_Response) error { + ret := _m.Called(_a0) + + var r0 error + if rf, ok := ret.Get(0).(func(*proto.Provision_Response) error); ok { + r0 = rf(_a0) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type NewProvisionStreamT interface { + mock.TestingT + Cleanup(func()) +} + +// NewProvisionStream creates a new instance of ProvisionStream. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewProvisionStream(t NewProvisionStreamT) *ProvisionStream { + mock := &ProvisionStream{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 8052b505b8cb4b60d899668735a965f54486452a Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 10 Jun 2022 17:29:09 -0700 Subject: [PATCH 2/7] fixup: commented code, long lines Signed-off-by: Spike Curtis --- provisioner/terraform/executor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index f95484c7ea5d2..152296da0757f 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -71,9 +71,6 @@ func (e executor) execParseJSON(ctx context.Context, args, env []string, v inter cmd.Stderr = stdErr err := cmd.Run() if err != nil { - // if noStateRegex.MatchString(err.Error()) { - // return nil, os.ErrNotExist - //} errString, _ := io.ReadAll(stdErr) return xerrors.Errorf("%s: %w", errString, err) } @@ -93,7 +90,10 @@ func (e executor) checkMinVersion(ctx context.Context) error { return err } if !v.GreaterThanOrEqual(minimumTerraformVersion) { - return xerrors.Errorf("terraform version %q is too old. required >= %q", v.String(), minimumTerraformVersion.String()) + return xerrors.Errorf( + "terraform version %q is too old. required >= %q", + v.String(), + minimumTerraformVersion.String()) } return nil } From 9223a77522216345f54a50374cdc6fd6b419f698 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Mon, 13 Jun 2022 11:06:31 -0700 Subject: [PATCH 3/7] rename executor methods to remove get Signed-off-by: Spike Curtis --- provisioner/terraform/executor.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 152296da0757f..92b6df885d0e0 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -85,7 +85,7 @@ func (e executor) execParseJSON(ctx context.Context, args, env []string, v inter } func (e executor) checkMinVersion(ctx context.Context) error { - v, err := e.getVersion(ctx) + v, err := e.version(ctx) if err != nil { return err } @@ -98,7 +98,7 @@ func (e executor) checkMinVersion(ctx context.Context) error { return nil } -func (e executor) getVersion(ctx context.Context) (*version.Version, error) { +func (e executor) version(ctx context.Context) (*version.Version, error) { // #nosec cmd := exec.CommandContext(ctx, e.binaryPath, "version", "-json") out, err := cmd.Output() @@ -234,7 +234,7 @@ func (e executor) apply(ctx context.Context, env, vars []string, logger provisio } func (e executor) stateResources(ctx context.Context) ([]*proto.Resource, error) { - state, err := e.getState(ctx) + state, err := e.state(ctx) if err != nil { return nil, err } @@ -252,12 +252,12 @@ func (e executor) stateResources(ctx context.Context) ([]*proto.Resource, error) return resources, nil } -func (e executor) getState(ctx context.Context) (*tfjson.State, error) { +func (e executor) state(ctx context.Context) (*tfjson.State, error) { args := []string{"show", "-json"} state := &tfjson.State{} err := e.execParseJSON(ctx, args, e.basicEnv(), state) if err != nil { - return nil, xerrors.Errorf("get terraform state: %w", err) + return nil, xerrors.Errorf("terraform show state: %w", err) } return state, nil } From c6c42983a2f71b721b5f2fc3d55cc8cd7a6beefa Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Mon, 13 Jun 2022 16:51:22 -0700 Subject: [PATCH 4/7] don't log terraform environment variables we don't know are safe Signed-off-by: Spike Curtis --- provisioner/terraform/provision.go | 29 ++++++++++++++++++++++++- provisioner/terraform/provision_test.go | 9 +++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 5090ea03c70f2..c725c678b2696 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -181,11 +181,38 @@ func provisionEnv(start *proto.Provision_Start) ([]string, error) { return env, nil } +var ( + // tfEnvSafeToPrint is the set of terraform environment variables that we are quite sure won't contain secrets, + // and therefore it's ok to log their values + tfEnvSafeToPrint = map[string]bool{ + "TF_LOG": true, + "TF_LOG_PATH": true, + "TF_INPUT": true, + "TF_DATA_DIR": true, + "TF_WORKSPACE": true, + "TF_IN_AUTOMATION": true, + "TF_REGISTRY_DISCOVERY_RETRY": true, + "TF_REGISTRY_CLIENT_TIMEOUT": true, + "TF_CLI_CONFIG_FILE": true, + "TF_IGNORE": true, + } +) + func logTerraformEnvVars(logger provisionersdk.Logger) error { env := os.Environ() for _, e := range env { if strings.HasPrefix(e, "TF_") { - err := logger.Log(&proto.Log{Level: proto.LogLevel_WARN, Output: "terraform environment variable: " + e}) + parts := strings.SplitN(e, "=", 2) + if len(parts) != 2 { + panic("os.Environ() returned vars not in key=value form") + } + if !tfEnvSafeToPrint[parts[0]] { + parts[1] = "" + } + err := logger.Log(&proto.Log{ + Level: proto.LogLevel_WARN, + Output: fmt.Sprintf("terraform environment variable: %s=%s", parts[0], parts[1]), + }) if err != nil { return err } diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index e83ed31801ea3..cefb72ae9991c 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -22,7 +22,7 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -func testProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) { +func setupProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) { client, server := provisionersdk.TransportPipe() ctx, cancelFunc := context.WithCancel(context.Background()) t.Cleanup(func() { @@ -46,7 +46,7 @@ func testProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient func TestProvision(t *testing.T) { t.Parallel() - ctx, api := testProvisioner(t) + ctx, api := setupProvisioner(t) for _, testCase := range []struct { Name string @@ -298,9 +298,11 @@ func TestProvision(t *testing.T) { // nolint:paralleltest func TestProvision_ExtraEnv(t *testing.T) { + secretValue := "oinae3uinxase" t.Setenv("TF_LOG", "INFO") + t.Setenv("TF_SUPERSECRET", secretValue) - ctx, api := testProvisioner(t) + ctx, api := setupProvisioner(t) directory := t.TempDir() path := filepath.Join(directory, "main.tf") @@ -330,6 +332,7 @@ func TestProvision_ExtraEnv(t *testing.T) { if strings.Contains(log.Output, "TF_LOG") { found = true } + require.NotContains(t, log.Output, secretValue) } if c := msg.GetComplete(); c != nil { require.Empty(t, c.Error) From ceaa4023cb1d0525887c97381c63c3741a297972 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Mon, 13 Jun 2022 17:04:57 -0700 Subject: [PATCH 5/7] Disable linting of fake secret Signed-off-by: Spike Curtis --- provisioner/terraform/provision_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index cefb72ae9991c..27ab6af231b82 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -298,6 +298,7 @@ func TestProvision(t *testing.T) { // nolint:paralleltest func TestProvision_ExtraEnv(t *testing.T) { + // #nosec secretValue := "oinae3uinxase" t.Setenv("TF_LOG", "INFO") t.Setenv("TF_SUPERSECRET", secretValue) From 32fae1eeecb065e86a360aa3413b882d5a8fe62a Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 14 Jun 2022 09:59:20 -0700 Subject: [PATCH 6/7] drop parse support and move logger into terraform package Signed-off-by: Spike Curtis --- provisioner/terraform/executor.go | 73 +++++++++++--- provisioner/terraform/executor_test.go | 62 ++++++++++++ provisioner/terraform/provision.go | 14 +-- provisionersdk/logger.go | 71 -------------- provisionersdk/logger_test.go | 123 ------------------------ provisionersdk/mocks/ParseStream.go | 42 -------- provisionersdk/mocks/ProvisionStream.go | 42 -------- 7 files changed, 126 insertions(+), 301 deletions(-) create mode 100644 provisioner/terraform/executor_test.go delete mode 100644 provisionersdk/logger.go delete mode 100644 provisionersdk/logger_test.go delete mode 100644 provisionersdk/mocks/ParseStream.go delete mode 100644 provisionersdk/mocks/ProvisionStream.go diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 92b6df885d0e0..311a1461b412c 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -1,6 +1,7 @@ package terraform import ( + "bufio" "bytes" "context" "encoding/json" @@ -17,7 +18,6 @@ import ( "github.com/hashicorp/go-version" tfjson "github.com/hashicorp/terraform-json" - "github.com/coder/coder/provisionersdk" "github.com/coder/coder/provisionersdk/proto" ) @@ -113,14 +113,14 @@ func (e executor) version(ctx context.Context) (*version.Version, error) { return version.NewVersion(vj.Version) } -func (e executor) init(ctx context.Context, logger provisionersdk.Logger) error { - writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_DEBUG) +func (e executor) init(ctx context.Context, logr logger) error { + writer, doneLogging := logWriter(logr, proto.LogLevel_DEBUG) defer func() { <-doneLogging }() return e.execWriteOutput(ctx, []string{"init"}, e.basicEnv(), writer) } // revive:disable-next-line:flag-parameter -func (e executor) plan(ctx context.Context, env, vars []string, logger provisionersdk.Logger, destroy bool) (*proto.Provision_Response, error) { +func (e executor) plan(ctx context.Context, env, vars []string, logr logger, destroy bool) (*proto.Provision_Response, error) { planfilePath := filepath.Join(e.workdir, "terraform.tfplan") args := []string{ "plan", @@ -137,7 +137,7 @@ func (e executor) plan(ctx context.Context, env, vars []string, logger provision args = append(args, "-var", variable) } - writer, doneLogging := provisionLogWriter(logger) + writer, doneLogging := provisionLogWriter(logr) defer func() { <-doneLogging }() err := e.execWriteOutput(ctx, args, env, writer) @@ -190,7 +190,7 @@ func (e executor) graph(ctx context.Context) (string, error) { } // revive:disable-next-line:flag-parameter -func (e executor) apply(ctx context.Context, env, vars []string, logger provisionersdk.Logger, destroy bool, +func (e executor) apply(ctx context.Context, env, vars []string, logr logger, destroy bool, ) (*proto.Provision_Response, error) { args := []string{ "apply", @@ -207,7 +207,7 @@ func (e executor) apply(ctx context.Context, env, vars []string, logger provisio args = append(args, "-var", variable) } - writer, doneLogging := provisionLogWriter(logger) + writer, doneLogging := provisionLogWriter(logr) defer func() { <-doneLogging }() err := e.execWriteOutput(ctx, args, env, writer) @@ -262,14 +262,55 @@ func (e executor) state(ctx context.Context) (*tfjson.State, error) { return state, nil } -func provisionLogWriter(logger provisionersdk.Logger) (io.WriteCloser, <-chan any) { +type logger interface { + Log(*proto.Log) error +} + +type streamLogger struct { + stream proto.DRPCProvisioner_ProvisionStream +} + +func (s streamLogger) Log(l *proto.Log) error { + return s.stream.Send(&proto.Provision_Response{ + Type: &proto.Provision_Response_Log{ + Log: l, + }, + }) +} + +// logWriter creates a WriteCloser that will log each line of text at the given level. The WriteCloser must be closed +// by the caller to end logging, after which the returned channel will be closed to indicate that logging of the written +// data has finished. Failure to close the WriteCloser will leak a goroutine. +func logWriter(logr logger, level proto.LogLevel) (io.WriteCloser, <-chan any) { + r, w := io.Pipe() + done := make(chan any) + go readAndLog(logr, r, done, level) + return w, done +} + +func readAndLog(logr logger, r io.Reader, done chan<- any, level proto.LogLevel) { + defer close(done) + scanner := bufio.NewScanner(r) + for scanner.Scan() { + err := logr.Log(&proto.Log{Level: level, Output: scanner.Text()}) + if err != nil { + // Not much we can do. We can't log because logging is itself breaking! + return + } + } +} + +// provisionLogWriter creates a WriteCloser that will log each JSON formatted terraform log. The WriteCloser must be +// closed by the caller to end logging, after which the returned channel will be closed to indicate that logging of the +// written data has finished. Failure to close the WriteCloser will leak a goroutine. +func provisionLogWriter(logr logger) (io.WriteCloser, <-chan any) { r, w := io.Pipe() done := make(chan any) - go provisionReadAndLog(logger, r, done) + go provisionReadAndLog(logr, r, done) return w, done } -func provisionReadAndLog(logger provisionersdk.Logger, reader io.Reader, done chan<- any) { +func provisionReadAndLog(logr logger, reader io.Reader, done chan<- any) { defer close(done) decoder := json.NewDecoder(reader) for { @@ -278,9 +319,9 @@ func provisionReadAndLog(logger provisionersdk.Logger, reader io.Reader, done ch if err != nil { return } - logLevel := convertTerraformLogLevel(log.Level, logger) + logLevel := convertTerraformLogLevel(log.Level, logr) - err = logger.Log(&proto.Log{Level: logLevel, Output: log.Message}) + err = logr.Log(&proto.Log{Level: logLevel, Output: log.Message}) if err != nil { // Not much we can do. We can't log because logging is itself breaking! return @@ -291,11 +332,11 @@ func provisionReadAndLog(logger provisionersdk.Logger, reader io.Reader, done ch } // If the diagnostic is provided, let's provide a bit more info! - logLevel = convertTerraformLogLevel(log.Diagnostic.Severity, logger) + logLevel = convertTerraformLogLevel(log.Diagnostic.Severity, logr) if err != nil { continue } - err = logger.Log(&proto.Log{Level: logLevel, Output: log.Diagnostic.Detail}) + err = logr.Log(&proto.Log{Level: logLevel, Output: log.Diagnostic.Detail}) if err != nil { // Not much we can do. We can't log because logging is itself breaking! return @@ -303,7 +344,7 @@ func provisionReadAndLog(logger provisionersdk.Logger, reader io.Reader, done ch } } -func convertTerraformLogLevel(logLevel string, logger provisionersdk.Logger) proto.LogLevel { +func convertTerraformLogLevel(logLevel string, logr logger) proto.LogLevel { switch strings.ToLower(logLevel) { case "trace": return proto.LogLevel_TRACE @@ -316,7 +357,7 @@ func convertTerraformLogLevel(logLevel string, logger provisionersdk.Logger) pro case "error": return proto.LogLevel_ERROR default: - _ = logger.Log(&proto.Log{ + _ = logr.Log(&proto.Log{ Level: proto.LogLevel_WARN, Output: fmt.Sprintf("unable to convert log level %s", logLevel), }) diff --git a/provisioner/terraform/executor_test.go b/provisioner/terraform/executor_test.go new file mode 100644 index 0000000000000..e23a13e354234 --- /dev/null +++ b/provisioner/terraform/executor_test.go @@ -0,0 +1,62 @@ +package terraform + +import ( + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/provisionersdk/proto" +) + +type mockLogger struct { + logs []*proto.Log + retVal error +} + +func (m *mockLogger) Log(l *proto.Log) error { + m.logs = append(m.logs, l) + return m.retVal +} + +func TestLogWriter_Mainline(t *testing.T) { + t.Parallel() + + logr := &mockLogger{retVal: nil} + writer, doneLogging := logWriter(logr, proto.LogLevel_INFO) + + _, err := writer.Write([]byte(`Sitting in an English garden +Waiting for the sun +If the sun don't come you get a tan +From standing in the English rain`)) + require.NoError(t, err) + err = writer.Close() + require.NoError(t, err) + <-doneLogging + + expected := []*proto.Log{ + {Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"}, + {Level: proto.LogLevel_INFO, Output: "Waiting for the sun"}, + {Level: proto.LogLevel_INFO, Output: "If the sun don't come you get a tan"}, + {Level: proto.LogLevel_INFO, Output: "From standing in the English rain"}, + } + require.Equal(t, logr.logs, expected) +} + +func TestLogWriter_SendError(t *testing.T) { + t.Parallel() + + logr := &mockLogger{retVal: xerrors.New("Goo goo g'joob")} + writer, doneLogging := logWriter(logr, proto.LogLevel_INFO) + + _, err := writer.Write([]byte(`Sitting in an English garden +Waiting for the sun +If the sun don't come you get a tan +From standing in the English rain`)) + require.NoError(t, err) + err = writer.Close() + require.NoError(t, err) + <-doneLogging + expected := []*proto.Log{{Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"}} + require.Equal(t, logr.logs, expected) +} diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index c725c678b2696..574e8a1fc14ba 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -15,7 +15,7 @@ import ( // Provision executes `terraform apply` or `terraform plan` for dry runs. func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { - logger := provisionersdk.NewProvisionLogger(stream) + logr := streamLogger{stream: stream} shutdown, shutdownFunc := context.WithCancel(stream.Context()) defer shutdownFunc() @@ -53,7 +53,7 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { if err := e.checkMinVersion(stream.Context()); err != nil { return err } - if err := logTerraformEnvVars(logger); err != nil { + if err := logTerraformEnvVars(logr); err != nil { return err } @@ -88,7 +88,7 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { } t.logger.Debug(shutdown, "running initialization") - err = e.init(stream.Context(), logger) + err = e.init(stream.Context(), logr) if err != nil { return xerrors.Errorf("initialize terraform: %w", err) } @@ -104,10 +104,10 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { } var resp *proto.Provision_Response if start.DryRun { - resp, err = e.plan(shutdown, env, vars, logger, + resp, err = e.plan(shutdown, env, vars, logr, start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) } else { - resp, err = e.apply(shutdown, env, vars, logger, + resp, err = e.apply(shutdown, env, vars, logr, start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) } if err != nil { @@ -198,7 +198,7 @@ var ( } ) -func logTerraformEnvVars(logger provisionersdk.Logger) error { +func logTerraformEnvVars(logr logger) error { env := os.Environ() for _, e := range env { if strings.HasPrefix(e, "TF_") { @@ -209,7 +209,7 @@ func logTerraformEnvVars(logger provisionersdk.Logger) error { if !tfEnvSafeToPrint[parts[0]] { parts[1] = "" } - err := logger.Log(&proto.Log{ + err := logr.Log(&proto.Log{ Level: proto.LogLevel_WARN, Output: fmt.Sprintf("terraform environment variable: %s=%s", parts[0], parts[1]), }) diff --git a/provisionersdk/logger.go b/provisionersdk/logger.go deleted file mode 100644 index af9980e017316..0000000000000 --- a/provisionersdk/logger.go +++ /dev/null @@ -1,71 +0,0 @@ -package provisionersdk - -import ( - "bufio" - "io" - - "github.com/coder/coder/provisionersdk/proto" -) - -type Logger interface { - Log(l *proto.Log) error -} - -type ProvisionStream interface { - Send(*proto.Provision_Response) error -} - -type ParseStream interface { - Send(response *proto.Parse_Response) error -} - -func NewProvisionLogger(s ProvisionStream) Logger { - return provisionLogger{s} -} - -func NewParseLogger(s ParseStream) Logger { - return parseLogger{s} -} - -type provisionLogger struct { - stream ProvisionStream -} - -func (p provisionLogger) Log(l *proto.Log) error { - return p.stream.Send(&proto.Provision_Response{ - Type: &proto.Provision_Response_Log{ - Log: l, - }, - }) -} - -type parseLogger struct { - stream ParseStream -} - -func (p parseLogger) Log(l *proto.Log) error { - return p.stream.Send(&proto.Parse_Response{ - Type: &proto.Parse_Response_Log{ - Log: l, - }, - }) -} - -func LogWriter(logger Logger, level proto.LogLevel) (io.WriteCloser, <-chan any) { - r, w := io.Pipe() - done := make(chan any) - go readAndLog(logger, r, done, level) - return w, done -} - -func readAndLog(logger Logger, r io.Reader, done chan<- any, level proto.LogLevel) { - defer close(done) - scanner := bufio.NewScanner(r) - for scanner.Scan() { - err := logger.Log(&proto.Log{Level: level, Output: scanner.Text()}) - if err != nil { - // Not much we can do. We can't log because logging is itself breaking! - return - } - } -} diff --git a/provisionersdk/logger_test.go b/provisionersdk/logger_test.go deleted file mode 100644 index 46c4e771a1581..0000000000000 --- a/provisionersdk/logger_test.go +++ /dev/null @@ -1,123 +0,0 @@ -package provisionersdk_test - -import ( - "testing" - - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "golang.org/x/xerrors" - - "github.com/coder/coder/provisionersdk" - "github.com/coder/coder/provisionersdk/mocks" - "github.com/coder/coder/provisionersdk/proto" -) - -type logMatcher struct { - level proto.LogLevel - output string -} - -func (m logMatcher) Matches(x interface{}) bool { - switch r := x.(type) { - case *proto.Provision_Response: - return m.logMatches(r.GetLog()) - case *proto.Parse_Response: - return m.logMatches(r.GetLog()) - default: - return false - } -} - -func (m logMatcher) logMatches(l *proto.Log) bool { - if l.Level != m.level { - return false - } - if l.Output != m.output { - return false - } - return true -} - -func withLog(log *proto.Log) func(x interface{}) bool { - m := logMatcher{level: log.GetLevel(), output: log.GetOutput()} - return m.Matches -} - -func TestProvisionLogger_Log(t *testing.T) { - t.Parallel() - - mStream := new(mocks.ProvisionStream) - defer mStream.AssertExpectations(t) - - l := &proto.Log{Level: proto.LogLevel_ERROR, Output: "an error"} - mStream.On("Send", mock.MatchedBy(withLog(l))).Return(nil) - - uut := provisionersdk.NewProvisionLogger(mStream) - err := uut.Log(l) - require.NoError(t, err) -} - -func TestParseLogger_Log(t *testing.T) { - t.Parallel() - - mStream := new(mocks.ParseStream) - defer mStream.AssertExpectations(t) - - l := &proto.Log{Level: proto.LogLevel_ERROR, Output: "an error"} - mStream.On("Send", mock.MatchedBy(withLog(l))).Return(nil) - - uut := provisionersdk.NewParseLogger(mStream) - err := uut.Log(l) - require.NoError(t, err) -} - -func TestLogWriter_Mainline(t *testing.T) { - t.Parallel() - - mStream := new(mocks.ParseStream) - defer mStream.AssertExpectations(t) - - logger := provisionersdk.NewParseLogger(mStream) - writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_INFO) - - expected := []*proto.Log{ - {Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"}, - {Level: proto.LogLevel_INFO, Output: "Waiting for the sun"}, - {Level: proto.LogLevel_INFO, Output: "If the sun don't come you get a tan"}, - {Level: proto.LogLevel_INFO, Output: "From standing in the English rain"}, - } - for _, log := range expected { - mStream.On("Send", mock.MatchedBy(withLog(log))).Return(nil) - } - - _, err := writer.Write([]byte(`Sitting in an English garden -Waiting for the sun -If the sun don't come you get a tan -From standing in the English rain`)) - require.NoError(t, err) - err = writer.Close() - require.NoError(t, err) - <-doneLogging -} - -func TestLogWriter_SendError(t *testing.T) { - t.Parallel() - - mStream := new(mocks.ParseStream) - defer mStream.AssertExpectations(t) - - logger := provisionersdk.NewParseLogger(mStream) - writer, doneLogging := provisionersdk.LogWriter(logger, proto.LogLevel_INFO) - - expected := &proto.Log{Level: proto.LogLevel_INFO, Output: "Sitting in an English garden"} - mStream.On("Send", mock.MatchedBy(withLog(expected))).Return(xerrors.New("Goo goo g'joob")) - - _, err := writer.Write([]byte(`Sitting in an English garden -Waiting for the sun -If the sun don't come you get a tan -From standing in the English rain`)) - require.NoError(t, err) - err = writer.Close() - require.NoError(t, err) - <-doneLogging -} diff --git a/provisionersdk/mocks/ParseStream.go b/provisionersdk/mocks/ParseStream.go deleted file mode 100644 index 19c6530313d70..0000000000000 --- a/provisionersdk/mocks/ParseStream.go +++ /dev/null @@ -1,42 +0,0 @@ -// Code generated by mockery v2.12.3. DO NOT EDIT. - -package mocks - -import ( - proto "github.com/coder/coder/provisionersdk/proto" - mock "github.com/stretchr/testify/mock" -) - -// ParseStream is an autogenerated mock type for the ParseStream type -type ParseStream struct { - mock.Mock -} - -// Send provides a mock function with given fields: response -func (_m *ParseStream) Send(response *proto.Parse_Response) error { - ret := _m.Called(response) - - var r0 error - if rf, ok := ret.Get(0).(func(*proto.Parse_Response) error); ok { - r0 = rf(response) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -type NewParseStreamT interface { - mock.TestingT - Cleanup(func()) -} - -// NewParseStream creates a new instance of ParseStream. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewParseStream(t NewParseStreamT) *ParseStream { - mock := &ParseStream{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/provisionersdk/mocks/ProvisionStream.go b/provisionersdk/mocks/ProvisionStream.go deleted file mode 100644 index 3d9fec048cb41..0000000000000 --- a/provisionersdk/mocks/ProvisionStream.go +++ /dev/null @@ -1,42 +0,0 @@ -// Code generated by mockery v2.12.3. DO NOT EDIT. - -package mocks - -import ( - proto "github.com/coder/coder/provisionersdk/proto" - mock "github.com/stretchr/testify/mock" -) - -// ProvisionStream is an autogenerated mock type for the ProvisionStream type -type ProvisionStream struct { - mock.Mock -} - -// Send provides a mock function with given fields: _a0 -func (_m *ProvisionStream) Send(_a0 *proto.Provision_Response) error { - ret := _m.Called(_a0) - - var r0 error - if rf, ok := ret.Get(0).(func(*proto.Provision_Response) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -type NewProvisionStreamT interface { - mock.TestingT - Cleanup(func()) -} - -// NewProvisionStream creates a new instance of ProvisionStream. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewProvisionStream(t NewProvisionStreamT) *ProvisionStream { - mock := &ProvisionStream{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} From ca76052e801a4a22a507ed464c1416c527d89aab Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 14 Jun 2022 10:08:57 -0700 Subject: [PATCH 7/7] disable testpackage linter on internal package test Signed-off-by: Spike Curtis --- provisioner/terraform/executor_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/provisioner/terraform/executor_test.go b/provisioner/terraform/executor_test.go index e23a13e354234..6d091947ec493 100644 --- a/provisioner/terraform/executor_test.go +++ b/provisioner/terraform/executor_test.go @@ -1,3 +1,4 @@ +// nolint:testpackage package terraform import (