fix: apply on output changes (#501)

OTF does not apply plans with only changed outputs. This PR fixes that.

internal/api/run_marshaler.go

@@ -186,7 +186,7 @@ func (m *jsonapiMarshaler) toApply(apply run.Phase, r *http.Request) (*types.App
 	}, nil
 }
 
-func (m *jsonapiMarshaler) toResourceReport(from *run.ResourceReport) types.ResourceReport {
+func (m *jsonapiMarshaler) toResourceReport(from *run.Report) types.ResourceReport {
 	var to types.ResourceReport
 	if from != nil {
 		to.Additions = &from.Additions

internal/integration/daemon_helpers_test.go

@@ -283,7 +283,8 @@ func (s *testDaemon) createAndUploadConfigurationVersion(t *testing.T, ctx conte
 	cv := s.createConfigurationVersion(t, ctx, ws, opts)
 	tarball, err := os.ReadFile("./testdata/root.tar.gz")
 	require.NoError(t, err)
-	s.UploadConfig(ctx, cv.ID, tarball)
+	err = s.UploadConfig(ctx, cv.ID, tarball)
+	require.NoError(t, err)
 	return cv
 }
 

internal/integration/run_api_test.go

@@ -72,7 +72,7 @@ func TestIntegration_RunAPI(t *testing.T) {
 					planned, err := daemon.GetRun(ctx, created.ID)
 					require.NoError(t, err)
 
-					assert.Equal(t, 2, planned.Plan.Additions)
+					assert.Equal(t, 2, planned.Plan.ResourceReport.Additions)
 					return // success
 				}
 			}
@@ -113,7 +113,7 @@ func TestIntegration_RunAPI(t *testing.T) {
 					planned, err := daemon.GetRun(ctx, created.ID)
 					require.NoError(t, err)
 
-					assert.Equal(t, 2, planned.Plan.Additions)
+					assert.Equal(t, 2, planned.Plan.ResourceReport.Additions)
 					return // success
 				}
 			}

internal/integration/run_status_test.go

@@ -0,0 +1,107 @@
+package integration
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/leg100/otf/internal"
+	"github.com/leg100/otf/internal/run"
+	"github.com/leg100/otf/internal/workspace"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestIntegration_RunStatus creates successive runs on a workspace, each time
+// making changes to the configuration, and checking that the change is
+// reflected in the run status and resource/output change reports.
+func TestIntegration_RunStatus(t *testing.T) {
+	integrationTest(t)
+
+	// Create a workspace with auto-apply enabled
+	daemon, org, ctx := setup(t, nil)
+	ws, err := daemon.CreateWorkspace(ctx, workspace.CreateOptions{
+		Name:         internal.String(t.Name()),
+		Organization: internal.String(org.Name),
+		AutoApply:    internal.Bool(true),
+	})
+	require.NoError(t, err)
+
+	// directory for root module
+	root := t.TempDir()
+
+	// create a sequence of steps that the test will execute sequentially.
+	steps := []struct {
+		name               string
+		config             string
+		wantStatus         internal.RunStatus
+		wantResourceReport run.Report
+		wantOutputReport   run.Report
+	}{
+		{
+			name:       "add resource",
+			config:     `resource "random_pet" "cat" { prefix = "mr-" }`,
+			wantStatus: internal.RunApplied,
+			wantResourceReport: run.Report{
+				Additions: 1,
+			},
+		},
+		{
+			name: "replace resource",
+			config: `resource "random_pet" "cat" { prefix = "sir-" }
+`,
+			wantStatus: internal.RunApplied,
+			wantResourceReport: run.Report{
+				Additions:    1,
+				Destructions: 1,
+			},
+		},
+		{
+			name: "new output",
+			config: `resource "random_pet" "cat" { prefix = "sir-" }
+output "cat_name" { value = random_pet.cat.id }
+`,
+			wantStatus: internal.RunApplied,
+			wantOutputReport: run.Report{
+				Additions: 1,
+			},
+		},
+		{
+			name:       "destroy all",
+			wantStatus: internal.RunApplied,
+			wantResourceReport: run.Report{
+				Destructions: 1,
+			},
+			wantOutputReport: run.Report{
+				Destructions: 1,
+			},
+		},
+	}
+	for _, step := range steps {
+		t.Run(step.name, func(t *testing.T) {
+			cv := daemon.createConfigurationVersion(t, ctx, ws, nil)
+
+			// create tarball of root module and upload
+			err := os.WriteFile(filepath.Join(root, "main.tf"), []byte(step.config), 0o777)
+			require.NoError(t, err)
+			tarball, err := internal.Pack(root)
+			require.NoError(t, err)
+			err = daemon.UploadConfig(ctx, cv.ID, tarball)
+			require.NoError(t, err)
+
+			// create run and wait for it to reach wanted status
+			_ = daemon.createRun(t, ctx, ws, cv)
+			for event := range daemon.sub {
+				if r, ok := event.Payload.(*run.Run); ok {
+					if r.Status == step.wantStatus {
+						// status matches, now check whether reports match as well
+						assert.Equal(t, &step.wantResourceReport, r.Plan.ResourceReport)
+						assert.Equal(t, &step.wantOutputReport, r.Plan.OutputReport)
+						break
+					}
+					require.False(t, r.Done(), "run unexpectedly finished with status %s", r.Status)
+				}
+			}
+		})
+	}
+}

internal/run/db.go

@@ -35,8 +35,9 @@ type (
 		ReplaceAddrs           []string                      `json:"replace_addrs"`
 		TargetAddrs            []string                      `json:"target_addrs"`
 		AutoApply              bool                          `json:"auto_apply"`
-		PlannedChanges         *pggen.Report                 `json:"planned_changes"`
-		AppliedChanges         *pggen.Report                 `json:"applied_changes"`
+		PlanResourceReport     *pggen.Report                 `json:"plan_resource_report"`
+		PlanOutputReport       *pggen.Report                 `json:"plan_output_report"`
+		ApplyResourceReport    *pggen.Report                 `json:"apply_resource_report"`
 		ConfigurationVersionID pgtype.Text                   `json:"configuration_version_id"`
 		WorkspaceID            pgtype.Text                   `json:"workspace_id"`
 		PlanOnly               bool                          `json:"plan_only"`
@@ -158,20 +159,23 @@ func (db *pgdb) UpdateStatus(ctx context.Context, runID string, fn func(*Run) er
 	return run, err
 }
 
-func (db *pgdb) CreatePlanReport(ctx context.Context, runID string, report ResourceReport) error {
+func (db *pgdb) CreatePlanReport(ctx context.Context, runID string, resource, output Report) error {
 	_, err := db.UpdatePlannedChangesByID(ctx, pggen.UpdatePlannedChangesByIDParams{
-		RunID:        sql.String(runID),
-		Additions:    sql.Int4(report.Additions),
-		Changes:      sql.Int4(report.Changes),
-		Destructions: sql.Int4(report.Destructions),
+		RunID:                sql.String(runID),
+		ResourceAdditions:    sql.Int4(resource.Additions),
+		ResourceChanges:      sql.Int4(resource.Changes),
+		ResourceDestructions: sql.Int4(resource.Destructions),
+		OutputAdditions:      sql.Int4(output.Additions),
+		OutputChanges:        sql.Int4(output.Changes),
+		OutputDestructions:   sql.Int4(output.Destructions),
 	})
 	if err != nil {
 		return sql.Error(err)
 	}
 	return err
 }
 
-func (db *pgdb) CreateApplyReport(ctx context.Context, runID string, report ResourceReport) error {
+func (db *pgdb) CreateApplyReport(ctx context.Context, runID string, report Report) error {
 	_, err := db.UpdateAppliedChangesByID(ctx, pggen.UpdateAppliedChangesByIDParams{
 		RunID:        sql.String(runID),
 		Additions:    sql.Int4(report.Additions),
@@ -360,14 +364,15 @@ func (result pgresult) toRun() *Run {
 			PhaseType:        internal.PlanPhase,
 			Status:           PhaseStatus(result.PlanStatus.String),
 			StatusTimestamps: unmarshalPlanStatusTimestampRows(result.PlanStatusTimestamps),
-			ResourceReport:   reportFromDB(result.PlannedChanges),
+			ResourceReport:   reportFromDB(result.PlanResourceReport),
+			OutputReport:     reportFromDB(result.PlanOutputReport),
 		},
 		Apply: Phase{
 			RunID:            result.RunID.String,
 			PhaseType:        internal.ApplyPhase,
 			Status:           PhaseStatus(result.ApplyStatus.String),
 			StatusTimestamps: unmarshalApplyStatusTimestampRows(result.ApplyStatusTimestamps),
-			ResourceReport:   reportFromDB(result.AppliedChanges),
+			ResourceReport:   reportFromDB(result.ApplyResourceReport),
 		},
 	}
 	if result.ForceCancelAvailableAt.Status == pgtype.Present {

internal/run/parse_apply_output.go

@@ -8,26 +8,26 @@ import (
 
 var applyChangesRegex = regexp.MustCompile(`(?m)^Apply complete! Resources: (\d+) added, (\d+) changed, (\d+) destroyed.`)
 
-func ParseApplyOutput(output string) (ResourceReport, error) {
+func ParseApplyOutput(output string) (Report, error) {
 	matches := applyChangesRegex.FindStringSubmatch(output)
 	if matches == nil {
-		return ResourceReport{}, fmt.Errorf("regexes unexpectedly did not match apply output")
+		return Report{}, fmt.Errorf("regexes unexpectedly did not match apply output")
 	}
 
 	adds, err := strconv.ParseInt(matches[1], 10, 0)
 	if err != nil {
-		return ResourceReport{}, err
+		return Report{}, err
 	}
 	changes, err := strconv.ParseInt(matches[2], 10, 0)
 	if err != nil {
-		return ResourceReport{}, err
+		return Report{}, err
 	}
 	deletions, err := strconv.ParseInt(matches[3], 10, 0)
 	if err != nil {
-		return ResourceReport{}, err
+		return Report{}, err
 	}
 
-	return ResourceReport{
+	return Report{
 		Additions:    int(adds),
 		Changes:      int(changes),
 		Destructions: int(deletions),

internal/run/parse_apply_output_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestParseApplyOutputChanges(t *testing.T) {
-	want := ResourceReport{
+	want := Report{
 		Additions:    1,
 		Changes:      0,
 		Destructions: 0,
@@ -24,7 +24,7 @@ func TestParseApplyOutputChanges(t *testing.T) {
 }
 
 func TestParseApplyOutputNoChanges(t *testing.T) {
-	want := ResourceReport{
+	want := Report{
 		Additions:    0,
 		Changes:      0,
 		Destructions: 0,

internal/run/phase.go

@@ -22,13 +22,15 @@ var ErrPhaseAlreadyStarted = errors.New("phase already started")
 type (
 	// Phase is a section of work performed by a run.
 	Phase struct {
-		RunID string `json:"run_id"`
-
+		RunID              string `json:"run_id"`
 		internal.PhaseType `json:"phase"`
-		*ResourceReport    `json:"report"` // report of planned or applied resource changes
+		Status             PhaseStatus            `json:"status"`
+		StatusTimestamps   []PhaseStatusTimestamp `json:"status_timestamps"`
 
-		Status           PhaseStatus            `json:"status"` // current phase status
-		StatusTimestamps []PhaseStatusTimestamp `json:"status_timestamps"`
+		// report of planned or applied resource changes
+		ResourceReport *Report `json:"resource_report"`
+		// report of planned or applied output changes
+		OutputReport *Report `json:"output_report"`
 	}
 
 	PhaseStatus string
@@ -59,12 +61,11 @@ func NewPhase(runID string, t internal.PhaseType) Phase {
 }
 
 func (p *Phase) HasChanges() bool {
-	if p.ResourceReport != nil {
-		return p.ResourceReport.HasChanges()
-	}
-	// no report has been published yet, which means there are no proposed
-	// changes yet.
-	return false
+	var (
+		hasResourceChanges = p.ResourceReport != nil && p.ResourceReport.HasChanges()
+		hasOutputChanges   = p.OutputReport != nil && p.OutputReport.HasChanges()
+	)
+	return hasResourceChanges || hasOutputChanges
 }
 
 // StatusTimestamp looks up the timestamp for a status