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

chore: remove redundant CreateRun magic string #555

Merged
merged 1 commit into from
Aug 2, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
<div class="actions-container">
<h3 class="font-semibold mb-2">Actions</h3>
<form id="workspace-start-run-form" action="{{ startRunWorkspacePath .Workspace.ID }}" method="POST">
<input type="hidden" name="connected" value="{{ ne .Workspace.Connection nil }}">
<select name="operation" id="start-run-operation" onchange="this.form.submit()">
<option value="" selected>-- start run --</option>
<option value="plan-only">plan only</option>
Expand Down
44 changes: 0 additions & 44 deletions internal/integration/run_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,50 +35,6 @@ func TestIntegration_RunAPI(t *testing.T) {
})
require.NoError(t, err)

// test the "magic string" behaviour specific to OTF: if
// run.PullVCSMagicString is specified for the config version ID then the
// config is pulled from the workspace's connected repo.
t.Run("magic string - create run using config from repo", func(t *testing.T) {
vcsProvider := daemon.createVCSProvider(t, ctx, org)
ws, err := daemon.CreateWorkspace(ctx, workspace.CreateOptions{
Name: internal.String("magic-connected-workspace"),
Organization: internal.String(org.Name),
ConnectOptions: &workspace.ConnectOptions{
RepoPath: &repo,
VCSProviderID: &vcsProvider.ID,
},
})
require.NoError(t, err)

created, err := tfeClient.Runs.Create(ctx, tfe.RunCreateOptions{
ConfigurationVersion: &tfe.ConfigurationVersion{
ID: run.PullVCSMagicString,
},
Workspace: &tfe.Workspace{
ID: ws.ID,
},
})
require.NoError(t, err)

// wait for run to reach planned status
for event := range daemon.sub {
if r, ok := event.Payload.(*run.Run); ok {
switch r.Status {
case internal.RunErrored:
t.Fatal("run unexpectedly errored")
case internal.RunPlanned:
// run should have planned two resources (defined in the config from the
// github repo)
planned, err := daemon.GetRun(ctx, created.ID)
require.NoError(t, err)

assert.Equal(t, 2, planned.Plan.ResourceReport.Additions)
return // success
}
}
}
})

// pull config from workspace's vcs repo
t.Run("create run using config from repo", func(t *testing.T) {
vcsProvider := daemon.createVCSProvider(t, ctx, org)
Expand Down
28 changes: 0 additions & 28 deletions internal/integration/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,6 @@ func TestRun(t *testing.T) {
require.NoError(t, err)
})

// test the "magic string" behaviour specific to OTF: if
// run.PullVCSMagicString is specified for the config version ID then the
// config is pulled from the workspace's connected repo.
t.Run("magic string - create run using config from repo", func(t *testing.T) {
// setup daemon along with fake github repo
repo := cloud.NewTestRepo()
daemon, _, ctx := setup(t, nil,
github.WithRepo(repo),
github.WithArchive(testutils.ReadFile(t, "../testdata/github.tar.gz")),
)
org := daemon.createOrganization(t, ctx)
vcsProvider := daemon.createVCSProvider(t, ctx, org)
ws, err := daemon.CreateWorkspace(ctx, workspace.CreateOptions{
Name: internal.String("connected-workspace"),
Organization: internal.String(org.Name),
ConnectOptions: &workspace.ConnectOptions{
RepoPath: &repo,
VCSProviderID: &vcsProvider.ID,
},
})
require.NoError(t, err)

_, err = daemon.CreateRun(ctx, ws.ID, run.RunCreateOptions{
ConfigurationVersionID: internal.String(run.PullVCSMagicString),
})
require.NoError(t, err)
})

t.Run("create run using config from repo", func(t *testing.T) {
// setup daemon along with fake github repo
repo := cloud.NewTestRepo()
Expand Down
20 changes: 4 additions & 16 deletions internal/run/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,16 @@ func (f *factory) NewRun(ctx context.Context, workspaceID string, opts RunCreate
return nil, err
}

// TODO(@leg100): remove PullVCSMagicString because (c)(ii) triggers the
// same behaviour.
//
// There are three possibilities for the ConfigurationVersionID value:
// (a) equals PullVCSMagicString in which case a config version is first
// created from the contents of the vcs repo connected to the workspace
// (b) non-nil, in which case it is deemed to be a configuration version id
// There are two possibilities for the ConfigurationVersionID value:
// (a) non-nil, in which case it is deemed to be a configuration version id
// and an existing config version with that ID is retrieved.
// (c) nil, in which it depends whether the workspace is connected to a vcs
// (b) nil, in which it depends whether the workspace is connected to a vcs
// repo:
// (i) not connected: the latest config version is retrieved.
// (ii) connected: same behaviour as (a): vcs repo contents are retrieved.
var cv *configversion.ConfigurationVersion
if opts.ConfigurationVersionID != nil {
if *opts.ConfigurationVersionID == PullVCSMagicString {
cv, err = f.createConfigVersionFromVCS(ctx, ws)
} else {
cv, err = f.GetConfigurationVersion(ctx, *opts.ConfigurationVersionID)
}
cv, err = f.GetConfigurationVersion(ctx, *opts.ConfigurationVersionID)
} else if ws.Connection == nil {
cv, err = f.GetLatestConfigurationVersion(ctx, workspaceID)
} else {
Expand All @@ -57,9 +48,6 @@ func (f *factory) NewRun(ctx context.Context, workspaceID string, opts RunCreate
// createConfigVersionFromVCS creates a config version from the vcs repo
// connected to the workspace using the contents of the vcs repo.
func (f *factory) createConfigVersionFromVCS(ctx context.Context, ws *workspace.Workspace) (*configversion.ConfigurationVersion, error) {
if ws.Connection == nil {
return nil, workspace.ErrNoVCSConnection
}
client, err := f.GetVCSClient(ctx, ws.Connection.VCSProviderID)
if err != nil {
return nil, err
Expand Down
30 changes: 0 additions & 30 deletions internal/run/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,24 +82,6 @@ func TestFactory(t *testing.T) {
assert.True(t, got.AutoApply)
})

t.Run("magic string - pull from vcs", func(t *testing.T) {
f := newTestFactory(
&workspace.Workspace{
Connection: &workspace.Connection{},
},
&configversion.ConfigurationVersion{},
)

got, err := f.NewRun(ctx, "", RunCreateOptions{
ConfigurationVersionID: internal.String(PullVCSMagicString),
})
require.NoError(t, err)

// fake config version service sets the config version ID to "created"
// if it was newly created
assert.Equal(t, "created", got.ConfigurationVersionID)
})

t.Run("pull from vcs", func(t *testing.T) {
f := newTestFactory(
&workspace.Workspace{
Expand All @@ -115,18 +97,6 @@ func TestFactory(t *testing.T) {
// if it was newly created
assert.Equal(t, "created", got.ConfigurationVersionID)
})

t.Run("pull from vcs workspace not connected error", func(t *testing.T) {
f := newTestFactory(
&workspace.Workspace{}, // workspace with no connection
&configversion.ConfigurationVersion{},
)

_, err := f.NewRun(ctx, "", RunCreateOptions{
ConfigurationVersionID: internal.String(PullVCSMagicString),
})
require.Equal(t, err, workspace.ErrNoVCSConnection)
})
}

type (
Expand Down
14 changes: 1 addition & 13 deletions internal/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ const (
PlanFormatBinary = "bin" // plan file in binary format
PlanFormatJSON = "json" // plan file in json format

// When specified in place of a configuration version ID passed to
// RunCreateOptions this magic string instructs the run factory to
// automatically create a configuration version from the workspace connected
// vcs repo.
PullVCSMagicString = "__pull_vcs__"

PlanOnlyOperation Operation = "plan-only"
PlanAndApplyOperation Operation = "plan-and-apply"
DestroyAllOperation Operation = "destroy-all"
Expand Down Expand Up @@ -89,14 +83,8 @@ type (
RefreshOnly *bool
Message *string
// Specifies the configuration version to use for this run. If the
// configuration version object is omitted, the run will be created using the
// configuration version ID is nil, the run will be created using the
// workspace's latest configuration version.
//
// Alternatively, if PullVCSMagicString is specified, and the workspace
// is connected to a vcs repo, then a configuration version is
// automatically created from the vcs repo and the run uses that
// configuration version. If the workspace is not connected to a
// workspace then an error is returned.
ConfigurationVersionID *string
TargetAddrs []string
ReplaceAddrs []string
Expand Down
12 changes: 2 additions & 10 deletions internal/run/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,16 @@ func (h *webHandlers) createRun(w http.ResponseWriter, r *http.Request) {
var params struct {
WorkspaceID string `schema:"workspace_id,required"`
Operation Operation `schema:"operation,required"`
Connected bool `schema:"connected,required"`
}
if err := decode.All(&params, r); err != nil {
h.Error(w, err.Error(), http.StatusUnprocessableEntity)
return
}

opts := RunCreateOptions{
run, err := h.svc.CreateRun(r.Context(), params.WorkspaceID, RunCreateOptions{
IsDestroy: internal.Bool(params.Operation == DestroyAllOperation),
PlanOnly: internal.Bool(params.Operation == PlanOnlyOperation),
}
// if run's workspace is connected to a VCS repo then pull config from
// the repo
if params.Connected {
opts.ConfigurationVersionID = internal.String(PullVCSMagicString)
}

run, err := h.svc.CreateRun(r.Context(), params.WorkspaceID, opts)
})
if err != nil {
html.FlashError(w, err.Error())
http.Redirect(w, r, paths.Workspace(params.WorkspaceID), http.StatusFound)
Expand Down
1 change: 0 additions & 1 deletion internal/workspace/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const (
)

var (
ErrNoVCSConnection = errors.New("workspace is not connected to a vcs repo")
ErrTagsRegexAndTriggerPatterns = errors.New("cannot specify both tags-regex and trigger-patterns")
ErrTagsRegexAndAlwaysTrigger = errors.New("cannot specify both tags-regex and always-trigger")
ErrTriggerPatternsAndAlwaysTrigger = errors.New("cannot specify both trigger-patterns and always-trigger")
Expand Down