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

VAULT-30189: enos_remote_exec: Use non-static ID as prefix for scripts #21

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
uses: ./.github/actions/build-provider
with:
target: ${{matrix.os}}/${{ matrix.arch }}
- uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
- uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: ${{ steps.build.outputs.name }}
path: dist/${{ steps.build.outputs.name }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
uses: ./.github/actions/build-provider
with:
target: ${{matrix.os}}/${{ matrix.arch }}
- uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
- uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: ${{ steps.build.outputs.name }}
path: dist/${{ steps.build.outputs.name }}
Expand Down
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ linters:
- nestif
- nonamedreturns
- nosprintfhostport
- predeclared
- promlinter
- rowserrcheck
- sqlclosecheck
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.5.4
0.5.5
2 changes: 1 addition & 1 deletion docs/resources/remote_exec.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ If a double quote must be included in the command it should be escaped as follow

### Read-Only

- `id` (String) The resource identifier is always static
- `id` (String) A random ID number associated with the resource. This is created a single time during the initial 'apply' phase. It is utilized as a prefix when copying file contents to the remote target
- `stderr` (String) The aggregate STDERR of all inline commnads, scripts, or content. If nothing is output this value will be set to a blank string
- `stdout` (String) The aggregate STDOUT of all inline commnads, scripts, or content. If nothing is output this value will be set to a blank string
- `sum` (String) A digest of the inline commands, source files, and environment variables. If the sum changes between runs all commands will execute again
2 changes: 1 addition & 1 deletion internal/flightcontrol/download/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (c *Command) Download() error {
}
}

dst, err := os.OpenFile(c.args.destination, os.O_RDWR|os.O_CREATE, fs.FileMode(c.args.mode))
dst, err := os.OpenFile(c.args.destination, os.O_RDWR|os.O_CREATE, fs.FileMode(c.args.mode)) //#nosec: G115
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/flightcontrol/zip/upzip_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (c *UnzipCommand) Unzip() error {
}
defer archive.Close()

fileMode := fs.FileMode(c.args.mode)
fileMode := fs.FileMode(c.args.mode) //#nosec:G115

// Make sure we've got a destination directory
dstDir, err := os.Open(c.args.destination)
Expand All @@ -128,7 +128,7 @@ func (c *UnzipCommand) Unzip() error {
return err
}

err = os.MkdirAll(c.args.destination, fs.FileMode(c.args.destinationMode))
err = os.MkdirAll(c.args.destination, fs.FileMode(c.args.destinationMode)) //#nosec:G115
if err != nil {
return err
}
Expand Down
20 changes: 12 additions & 8 deletions internal/plugin/resource_remote_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/hashicorp/terraform-plugin-go/tftypes"

"github.com/hashicorp-forge/terraform-provider-enos/internal/diags"
"github.com/hashicorp-forge/terraform-provider-enos/internal/random"
"github.com/hashicorp-forge/terraform-provider-enos/internal/remoteflight"
resource "github.com/hashicorp-forge/terraform-provider-enos/internal/server/resourcerouter"
"github.com/hashicorp-forge/terraform-provider-enos/internal/server/state"
Expand Down Expand Up @@ -196,7 +197,6 @@ func (r *remoteExec) ApplyResourceChange(ctx context.Context, req resource.Apply
// nothing to do on delete
return
}
plannedState.ID.Set("static")

transport := transportUtil.ApplyValidatePlannedAndBuildTransport(ctx, plannedState, r, res)
if diags.HasErrors(res.Diagnostics) {
Expand All @@ -206,6 +206,10 @@ func (r *remoteExec) ApplyResourceChange(ctx context.Context, req resource.Apply
// If our priorState Sum is blank then we're creating the resource. If
// it's not blank and doesn't match the planned state we're updating.
_, pok := priorState.ID.Get()
if !pok {
plannedState.ID.Set(random.ID())
}

priorSum, prsumok := priorState.Sum.Get()
plannedSum, plsumok := plannedState.Sum.Get()

Expand Down Expand Up @@ -271,7 +275,7 @@ If a double quote must be included in the command it should be escaped as follow
Name: "id",
Type: tftypes.String,
Computed: true,
Description: resourceStaticIDDescription,
Description: "A random ID number associated with the resource. This is created a single time during the initial 'apply' phase. It is utilized as a prefix when copying file contents to the remote target",
},
{
Name: "sum",
Expand Down Expand Up @@ -334,7 +338,6 @@ If a double quote must be included in the command it should be escaped as follow
func (r *remoteExec) ExecuteCommands(ctx context.Context, state *remoteExecStateV1, client it.Transport) (ui.UI, error) {
var err error
ui := ui.NewBuffered()
env, _ := state.Env.GetStrings()

if inline, ok := state.Inline.GetStrings(); ok {
for _, cmd := range inline {
Expand All @@ -353,7 +356,7 @@ func (r *remoteExec) ExecuteCommands(ctx context.Context, state *remoteExecState
source := tfile.NewReader(cmd)
defer source.Close()

return r.copyAndRun(ctx, ui, client, source, "inline", env)
return r.copyAndRun(ctx, ui, client, source, "inline", state)
}
if err := exec(cmd); err != nil {
return ui, fmt.Errorf("running inline command failed, due to: %w", err)
Expand All @@ -370,7 +373,7 @@ func (r *remoteExec) ExecuteCommands(ctx context.Context, state *remoteExecState
}
defer script.Close()

return r.copyAndRun(ctx, ui, client, script, "script", env)
return r.copyAndRun(ctx, ui, client, script, "script", state)
}

if err := exec(path); err != nil {
Expand All @@ -383,7 +386,7 @@ func (r *remoteExec) ExecuteCommands(ctx context.Context, state *remoteExecState
content := tfile.NewReader(cont)
defer content.Close()

err = r.copyAndRun(ctx, ui, client, content, "content", env)
err = r.copyAndRun(ctx, ui, client, content, "content", state)
if err != nil {
return ui, fmt.Errorf("running command content failed, due to: %w", err)
}
Expand All @@ -395,7 +398,7 @@ func (r *remoteExec) ExecuteCommands(ctx context.Context, state *remoteExecState
// copyAndRun copies the copyable source to the target using the configured transport,
// sets the environment variables and executes the content of the source.
// It returns STDOUT, STDERR, and any errors encountered.
func (r *remoteExec) copyAndRun(ctx context.Context, ui ui.UI, client it.Transport, src it.Copyable, srcType string, env map[string]string) error {
func (r *remoteExec) copyAndRun(ctx context.Context, ui ui.UI, client it.Transport, src it.Copyable, srcType string, state *remoteExecStateV1) error {
select {
case <-ctx.Done():
return ctx.Err()
Expand All @@ -417,9 +420,10 @@ func (r *remoteExec) copyAndRun(ctx context.Context, ui ui.UI, client it.Transpo
// TODO: Eventually we'll probably have to support /tmp being mounted
// with no exec. In those cases we'll have to make this configurable
// or find another strategy for executing scripts.
env, _ := state.Env.GetStrings()
res, err := remoteflight.RunScript(ctx, client, remoteflight.NewRunScriptRequest(
remoteflight.WithRunScriptContent(src),
remoteflight.WithRunScriptDestination(fmt.Sprintf("/tmp/%s.sh", sha)),
remoteflight.WithRunScriptDestination(fmt.Sprintf("/tmp/%s-%s.sh", state.ID.Value(), sha)),
remoteflight.WithRunScriptEnv(env),
remoteflight.WithRunScriptChmod("0777"),
))
Expand Down
2 changes: 1 addition & 1 deletion internal/plugin/tftypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func encodeTfObjectDynamicPseudoType(

// MarshalMsgPack is deprecated but it's by far the easiest way to inspect the serialized value
// of the raw attribute.
//nolint:staticcheck
//nolint:staticcheck,nolintlint
//lint:ignore SA1019 we have to use this internal only API to determine DynamicPseudoType types.
msgpackBytes, err := planVal.MarshalMsgPack(tftypes.DynamicPseudoType)
if err != nil {
Expand Down
3 changes: 0 additions & 3 deletions internal/transport/test/test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func NewTransportTestSuite(transportFn func(t *testing.T) it.Transport) *Transpo
func (s *TransportTestSuite) TestRun() {
t := s.T()
transport := s.transportFn(t)
t.Parallel()

type args struct {
command it.Command
Expand Down Expand Up @@ -117,7 +116,6 @@ func (s *TransportTestSuite) TestRun() {
func (s *TransportTestSuite) TestCopy() {
t := s.T()
transport := s.transportFn(t)
t.Parallel()

type args struct {
ctx context.Context
Expand Down Expand Up @@ -194,7 +192,6 @@ func (s *TransportTestSuite) TestCopy() {
func (s *TransportTestSuite) TestStream() {
t := s.T()
transport := s.transportFn(t)
t.Parallel()

type args struct {
ctx context.Context
Expand Down