Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit28dc854

Browse files
committed
set module path to sentinel value when parsing failed and log an error
1 parentea2b6b7 commit28dc854

File tree

3 files changed

+44
-19
lines changed

3 files changed

+44
-19
lines changed

‎provisioner/terraform/executor.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ func (e *executor) planResources(ctx, killCtx context.Context, planfilePath stri
356356
}
357357
modules=append(modules,plan.PlannedValues.RootModule)
358358

359-
state,err:=ConvertState(modules,rawGraph)
359+
state,err:=ConvertState(ctx,modules,rawGraph,e.server.logger)
360360
iferr!=nil {
361361
returnnil,err
362362
}
@@ -484,9 +484,9 @@ func (e *executor) stateResources(ctx, killCtx context.Context) (*State, error)
484484
returnconverted,nil
485485
}
486486

487-
converted,err=ConvertState([]*tfjson.StateModule{
487+
converted,err=ConvertState(ctx,[]*tfjson.StateModule{
488488
state.Values.RootModule,
489-
},rawGraph)
489+
},rawGraph,e.server.logger)
490490
iferr!=nil {
491491
returnnil,err
492492
}

‎provisioner/terraform/resources.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package terraform
22

33
import (
4+
"context"
45
"fmt"
56
"strings"
67

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

13+
"cdr.dev/slog"
14+
1215
"github.com/coder/terraform-provider-coder/provider"
1316

1417
tfaddr"github.com/hashicorp/go-terraform-address"
@@ -134,7 +137,7 @@ type State struct {
134137
// ConvertState consumes Terraform state and a GraphViz representation
135138
// produced by `terraform graph` to produce resources consumable by Coder.
136139
// nolint:gocognit // This function makes more sense being large for now, until refactored.
137-
funcConvertState(modules []*tfjson.StateModule,rawGraphstring) (*State,error) {
140+
funcConvertState(ctx context.Context,modules []*tfjson.StateModule,rawGraphstring,logger slog.Logger) (*State,error) {
138141
parsedGraph,err:=gographviz.ParseString(rawGraph)
139142
iferr!=nil {
140143
returnnil,xerrors.Errorf("parse graph: %w",err)
@@ -598,9 +601,16 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error
598601
modulePath,err:=convertAddressToModulePath(resource.Address)
599602
iferr!=nil {
600603
// Module path recording was added primarily to keep track of
601-
// modules in telemetry. We're adding this fallback so we can
602-
// detect if there are any issues with the address parsing.
603-
modulePath=fmt.Sprintf("error parsing address: %s",resource.Address)
604+
// modules in telemetry. We're adding this sentinel value so
605+
// we can detect if there are any issues with the address
606+
// parsing.
607+
//
608+
// We don't want to set modulePath to null here because, in
609+
// the database, a null value in WorkspaceResource's ModulePath
610+
// indicates "this resource was created before module paths
611+
// were tracked."
612+
modulePath="FAILED_TO_PARSE_TERRAFORM_ADDRESS"
613+
logger.Error(ctx,"failed to parse Terraform address",slog.F("address",resource.Address))
604614
}
605615

606616
agents,exists:=resourceAgents[label]

‎provisioner/terraform/resources_test.go

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package terraform_test
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"os"
@@ -14,11 +15,18 @@ import (
1415
"github.com/stretchr/testify/require"
1516
protobuf"google.golang.org/protobuf/proto"
1617

18+
"cdr.dev/slog"
19+
"cdr.dev/slog/sloggers/slogtest"
20+
1721
"github.com/coder/coder/v2/cryptorand"
1822
"github.com/coder/coder/v2/provisioner/terraform"
1923
"github.com/coder/coder/v2/provisionersdk/proto"
2024
)
2125

26+
funcctxAndLogger(t*testing.T) (context.Context, slog.Logger) {
27+
returncontext.Background(),slogtest.Make(t,nil).Leveled(slog.LevelDebug)
28+
}
29+
2230
funcTestConvertResources(t*testing.T) {
2331
t.Parallel()
2432
// nolint:dogsled
@@ -688,6 +696,7 @@ func TestConvertResources(t *testing.T) {
688696
dir:=filepath.Join(filepath.Dir(filename),"testdata",folderName)
689697
t.Run("Plan",func(t*testing.T) {
690698
t.Parallel()
699+
ctx,logger:=ctxAndLogger(t)
691700

692701
tfPlanRaw,err:=os.ReadFile(filepath.Join(dir,folderName+".tfplan.json"))
693702
require.NoError(t,err)
@@ -705,7 +714,7 @@ func TestConvertResources(t *testing.T) {
705714
// and that no errors occur!
706715
modules=append(modules,tfPlan.PlannedValues.RootModule)
707716
}
708-
state,err:=terraform.ConvertState(modules,string(tfPlanGraph))
717+
state,err:=terraform.ConvertState(ctx,modules,string(tfPlanGraph),logger)
709718
require.NoError(t,err)
710719
sortResources(state.Resources)
711720
sortExternalAuthProviders(state.ExternalAuthProviders)
@@ -763,6 +772,7 @@ func TestConvertResources(t *testing.T) {
763772

764773
t.Run("Provision",func(t*testing.T) {
765774
t.Parallel()
775+
ctx,logger:=ctxAndLogger(t)
766776
tfStateRaw,err:=os.ReadFile(filepath.Join(dir,folderName+".tfstate.json"))
767777
require.NoError(t,err)
768778
vartfState tfjson.State
@@ -771,7 +781,7 @@ func TestConvertResources(t *testing.T) {
771781
tfStateGraph,err:=os.ReadFile(filepath.Join(dir,folderName+".tfstate.dot"))
772782
require.NoError(t,err)
773783

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

809819
funcTestAppSlugValidation(t*testing.T) {
810820
t.Parallel()
821+
ctx,logger:=ctxAndLogger(t)
811822

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

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

844-
state,err=terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule},string(tfPlanGraph))
855+
state,err=terraform.ConvertState(ctx,[]*tfjson.StateModule{tfPlan.PlannedValues.RootModule},string(tfPlanGraph),logger)
845856
require.Nil(t,state)
846857
require.Error(t,err)
847858
require.ErrorContains(t,err,"duplicate app slug")
848859
}
849860

850861
funcTestMetadataResourceDuplicate(t*testing.T) {
851862
t.Parallel()
863+
ctx,logger:=ctxAndLogger(t)
852864

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

863-
state,err:=terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule},string(tfPlanGraph))
875+
state,err:=terraform.ConvertState(ctx,[]*tfjson.StateModule{tfPlan.PlannedValues.RootModule},string(tfPlanGraph),logger)
864876
require.Nil(t,state)
865877
require.Error(t,err)
866878
require.ErrorContains(t,err,"duplicate metadata resource: null_resource.about")
867879
}
868880

869881
funcTestParameterValidation(t*testing.T) {
870882
t.Parallel()
883+
ctx,logger:=ctxAndLogger(t)
871884

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

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

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

926-
state,err=terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule},string(tfPlanGraph))
939+
state,err=terraform.ConvertState(ctx,[]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule},string(tfPlanGraph),logger)
927940
require.Nil(t,state)
928941
require.Error(t,err)
929942
require.ErrorContains(t,err,"coder_parameter names must be unique but\"identical-0\",\"identical-1\" and\"identical-2\" appear multiple times")
@@ -954,9 +967,10 @@ func TestInstanceTypeAssociation(t *testing.T) {
954967
tc:=tc
955968
t.Run(tc.ResourceType,func(t*testing.T) {
956969
t.Parallel()
970+
ctx,logger:=ctxAndLogger(t)
957971
instanceType,err:=cryptorand.String(12)
958972
require.NoError(t,err)
959-
state,err:=terraform.ConvertState([]*tfjson.StateModule{{
973+
state,err:=terraform.ConvertState(ctx,[]*tfjson.StateModule{{
960974
Resources: []*tfjson.StateResource{{
961975
Address:tc.ResourceType+".dev",
962976
Type:tc.ResourceType,
@@ -973,7 +987,7 @@ func TestInstanceTypeAssociation(t *testing.T) {
973987
subgraph "root" {
974988
"[root] `+tc.ResourceType+`.dev" [label = "`+tc.ResourceType+`.dev", shape = "box"]
975989
}
976-
}`)
990+
}`,logger)
977991
require.NoError(t,err)
978992
require.Len(t,state.Resources,1)
979993
require.Equal(t,state.Resources[0].GetInstanceType(),instanceType)
@@ -1012,9 +1026,10 @@ func TestInstanceIDAssociation(t *testing.T) {
10121026
tc:=tc
10131027
t.Run(tc.ResourceType,func(t*testing.T) {
10141028
t.Parallel()
1029+
ctx,logger:=ctxAndLogger(t)
10151030
instanceID,err:=cryptorand.String(12)
10161031
require.NoError(t,err)
1017-
state,err:=terraform.ConvertState([]*tfjson.StateModule{{
1032+
state,err:=terraform.ConvertState(ctx,[]*tfjson.StateModule{{
10181033
Resources: []*tfjson.StateResource{{
10191034
Address:"coder_agent.dev",
10201035
Type:"coder_agent",
@@ -1044,7 +1059,7 @@ func TestInstanceIDAssociation(t *testing.T) {
10441059
"[root] `+tc.ResourceType+`.dev" -> "[root] coder_agent.dev"
10451060
}
10461061
}
1047-
`)
1062+
`,logger)
10481063
require.NoError(t,err)
10491064
require.Len(t,state.Resources,1)
10501065
require.Len(t,state.Resources[0].Agents,1)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp