Skip to content

chore: track terraform modules in telemetry #15450

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.

Sign up for GitHub

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

Merged
merged 34 commits into from
Nov 16, 2024
Merged

chore: track terraform modules in telemetry #15450

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0f713ed
add the workspace_modules table, add the module column to workspace_r…
hugodutka Nov 6, 2024
82c8a3c
insert workspace modules on provisioner job completion
hugodutka Nov 6, 2024
307a57e
add foreign key to job on workspace_modules
hugodutka Nov 6, 2024
2ae16fc
don't return the root module
hugodutka Nov 7, 2024
7f2f155
add and populate the module_path column to workspace_resources
hugodutka Nov 7, 2024
863235d
TestCompleteJob - Modules WIP
hugodutka Nov 7, 2024
993f0ab
TemplateImport CompleteJob Modules test
hugodutka Nov 8, 2024
e8fbd40
TestCompleteJob Modules WorkspaceBuild
hugodutka Nov 8, 2024
5c542e5
add a test for returning modules in provision_test.go, fix test in re…
hugodutka Nov 8, 2024
9ec5ee1
make gen
hugodutka Nov 8, 2024
119a686
fix dbauthz tests
hugodutka Nov 8, 2024
3a5a025
add workspace modules and workspace resources's ModulePath to telemetry
hugodutka Nov 8, 2024
3377d22
add a workspace_modules fixture
hugodutka Nov 8, 2024
422b112
add a workspace resource's modulePath default to the e2e helper
hugodutka Nov 8, 2024
46b9b36
add modules default on PlanComplete for e2e tests
hugodutka Nov 8, 2024
f3f4d5c
lint
hugodutka Nov 8, 2024
93ed136
set module path to sentinel value when parsing failed and log an error
hugodutka Nov 13, 2024
301a153
change error string
hugodutka Nov 13, 2024
212e7d5
obfuscate workspace module source and version
hugodutka Nov 13, 2024
e9b7c46
fixes after rebase
hugodutka Nov 14, 2024
25cba6d
change migration number
hugodutka Nov 15, 2024
780730f
change fixture number
hugodutka Nov 15, 2024
b47a018
add index on created_at
hugodutka Nov 15, 2024
03be52e
add a comment about using modules from plan instead of apply
hugodutka Nov 15, 2024
00b8131
convert sentinel string to an error
hugodutka Nov 15, 2024
4187fbf
remove unnecessary comment
hugodutka Nov 15, 2024
7e82c3d
don't fail if we can't get modules from disk
hugodutka Nov 15, 2024
42e57ca
add a test for a malformed module
hugodutka Nov 16, 2024
368fc25
add a comment
hugodutka Nov 16, 2024
73b22cf
combine assignment and conditional
hugodutka Nov 16, 2024
7b9d70a
make isCoderModule into a top level function
hugodutka Nov 16, 2024
ea82313
use dbtestutil.NewDB instead of dbmem
hugodutka Nov 16, 2024
8564e9a
make gen
hugodutka Nov 16, 2024
d11fc82
combine 2 more assignments and conditionals
hugodutka Nov 16, 2024
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
Prev Previous commit
Next Next commit
set module path to sentinel value when parsing failed and log an error
  • Loading branch information
@hugodutka
hugodutka committed Nov 15, 2024
commit 93ed1360c19fb4dcc4453edfc1c2fbea84d48272
6 changes: 3 additions & 3 deletions provisioner/terraform/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func (e *executor) planResources(ctx, killCtx context.Context, planfilePath stri
}
modules = append(modules, plan.PlannedValues.RootModule)

state, err := ConvertState(modules, rawGraph)
state, err := ConvertState(ctx, modules, rawGraph, e.server.logger)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -484,9 +484,9 @@ func (e *executor) stateResources(ctx, killCtx context.Context) (*State, error)
return converted, nil
}

converted, err = ConvertState([]*tfjson.StateModule{
converted, err = ConvertState(ctx, []*tfjson.StateModule{
state.Values.RootModule,
}, rawGraph)
}, rawGraph, e.server.logger)
if err != nil {
return nil, err
}
Expand Down
18 changes: 14 additions & 4 deletions provisioner/terraform/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package terraform

import (
"context"
"fmt"
"strings"

Expand All @@ -9,6 +10,8 @@ import (
"github.com/mitchellh/mapstructure"
"golang.org/x/xerrors"

"cdr.dev/slog"

"github.com/coder/terraform-provider-coder/provider"

tfaddr "github.com/hashicorp/go-terraform-address"
Expand Down Expand Up @@ -134,7 +137,7 @@ type State struct {
// ConvertState consumes Terraform state and a GraphViz representation
// produced by `terraform graph` to produce resources consumable by Coder.
// nolint:gocognit // This function makes more sense being large for now, until refactored.
func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error) {
func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph string, logger slog.Logger) (*State, error) {
parsedGraph, err := gographviz.ParseString(rawGraph)
if err != nil {
return nil, xerrors.Errorf("parse graph: %w", err)
Expand Down Expand Up @@ -598,9 +601,16 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error
modulePath, err := convertAddressToModulePath(resource.Address)
if err != nil {
// Module path recording was added primarily to keep track of
// modules in telemetry. We're adding this fallback so we can
// detect if there are any issues with the address parsing.
modulePath = fmt.Sprintf("error parsing address: %s", resource.Address)
// modules in telemetry. We're adding this sentinel value so
// we can detect if there are any issues with the address
// parsing.
//
// We don't want to set modulePath to null here because, in
// the database, a null value in WorkspaceResource's ModulePath
// indicates "this resource was created before module paths
// were tracked."
modulePath = "FAILED_TO_PARSE_TERRAFORM_ADDRESS"
Copy link
Member

@johnstcn johnstcn Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could we extract this to a constant we can refer to elsewhere?

EDIT: it might actually be even better as a sentinel error ErrInvalidTerraformAddr

logger.Error(ctx, "failed to parse Terraform address", slog.F("address", resource.Address))
}

agents, exists := resourceAgents[label]
Expand Down
39 changes: 27 additions & 12 deletions provisioner/terraform/resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package terraform_test

import (
"context"
"encoding/json"
"fmt"
"os"
Expand All @@ -14,11 +15,18 @@ import (
"github.com/stretchr/testify/require"
protobuf "google.golang.org/protobuf/proto"

"cdr.dev/slog"
"cdr.dev/slog/sloggers/slogtest"

"github.com/coder/coder/v2/cryptorand"
"github.com/coder/coder/v2/provisioner/terraform"
"github.com/coder/coder/v2/provisionersdk/proto"
)

func ctxAndLogger(t *testing.T) (context.Context, slog.Logger) {
return context.Background(), slogtest.Make(t, nil).Leveled(slog.LevelDebug)
}

func TestConvertResources(t *testing.T) {
t.Parallel()
// nolint:dogsled
Expand Down Expand Up @@ -688,6 +696,7 @@ func TestConvertResources(t *testing.T) {
dir := filepath.Join(filepath.Dir(filename), "testdata", folderName)
t.Run("Plan", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

tfPlanRaw, err := os.ReadFile(filepath.Join(dir, folderName+".tfplan.json"))
require.NoError(t, err)
Expand All @@ -705,7 +714,7 @@ func TestConvertResources(t *testing.T) {
// and that no errors occur!
modules = append(modules, tfPlan.PlannedValues.RootModule)
}
state, err := terraform.ConvertState(modules, string(tfPlanGraph))
state, err := terraform.ConvertState(ctx, modules, string(tfPlanGraph), logger)
require.NoError(t, err)
sortResources(state.Resources)
sortExternalAuthProviders(state.ExternalAuthProviders)
Expand Down Expand Up @@ -763,6 +772,7 @@ func TestConvertResources(t *testing.T) {

t.Run("Provision", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)
tfStateRaw, err := os.ReadFile(filepath.Join(dir, folderName+".tfstate.json"))
require.NoError(t, err)
var tfState tfjson.State
Expand All @@ -771,7 +781,7 @@ func TestConvertResources(t *testing.T) {
tfStateGraph, err := os.ReadFile(filepath.Join(dir, folderName+".tfstate.dot"))
require.NoError(t, err)

state, err := terraform.ConvertState([]*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph))
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph), logger)
require.NoError(t, err)
sortResources(state.Resources)
sortExternalAuthProviders(state.ExternalAuthProviders)
Expand Down Expand Up @@ -808,6 +818,7 @@ func TestConvertResources(t *testing.T) {

func TestAppSlugValidation(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// nolint:dogsled
_, filename, _, _ := runtime.Caller(0)
Expand All @@ -829,7 +840,7 @@ func TestAppSlugValidation(t *testing.T) {
}
}

state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph))
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "invalid app slug")
Expand All @@ -841,14 +852,15 @@ func TestAppSlugValidation(t *testing.T) {
}
}

state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph))
state, err = terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "duplicate app slug")
}

func TestMetadataResourceDuplicate(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// Load the multiple-apps state file and edit it.
dir := filepath.Join("testdata", "resource-metadata-duplicate")
Expand All @@ -860,14 +872,15 @@ func TestMetadataResourceDuplicate(t *testing.T) {
tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "resource-metadata-duplicate.tfplan.dot"))
require.NoError(t, err)

state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph))
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "duplicate metadata resource: null_resource.about")
}

func TestParameterValidation(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// nolint:dogsled
_, filename, _, _ := runtime.Caller(0)
Expand All @@ -891,7 +904,7 @@ func TestParameterValidation(t *testing.T) {
}
}

state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph))
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical\" appears multiple times")
Expand All @@ -907,7 +920,7 @@ func TestParameterValidation(t *testing.T) {
}
}

state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph))
state, err = terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\" and \"identical-1\" appear multiple times")
Expand All @@ -923,7 +936,7 @@ func TestParameterValidation(t *testing.T) {
}
}

state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph))
state, err = terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\", \"identical-1\" and \"identical-2\" appear multiple times")
Expand Down Expand Up @@ -954,9 +967,10 @@ func TestInstanceTypeAssociation(t *testing.T) {
tc := tc
t.Run(tc.ResourceType, func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)
instanceType, err := cryptorand.String(12)
require.NoError(t, err)
state, err := terraform.ConvertState([]*tfjson.StateModule{{
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
Resources: []*tfjson.StateResource{{
Address: tc.ResourceType + ".dev",
Type: tc.ResourceType,
Expand All @@ -973,7 +987,7 @@ func TestInstanceTypeAssociation(t *testing.T) {
subgraph "root" {
"[root] `+tc.ResourceType+`.dev" [label = "`+tc.ResourceType+`.dev", shape = "box"]
}
}`)
}`, logger)
require.NoError(t, err)
require.Len(t, state.Resources, 1)
require.Equal(t, state.Resources[0].GetInstanceType(), instanceType)
Expand Down Expand Up @@ -1012,9 +1026,10 @@ func TestInstanceIDAssociation(t *testing.T) {
tc := tc
t.Run(tc.ResourceType, func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)
instanceID, err := cryptorand.String(12)
require.NoError(t, err)
state, err := terraform.ConvertState([]*tfjson.StateModule{{
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
Resources: []*tfjson.StateResource{{
Address: "coder_agent.dev",
Type: "coder_agent",
Expand Down Expand Up @@ -1044,7 +1059,7 @@ func TestInstanceIDAssociation(t *testing.T) {
"[root] `+tc.ResourceType+`.dev" -> "[root] coder_agent.dev"
}
}
`)
`, logger)
require.NoError(t, err)
require.Len(t, state.Resources, 1)
require.Len(t, state.Resources[0].Agents, 1)
Expand Down