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.

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

Jump to bottom

flightplan: fix various potential nil panics #119

Merged
merged 1 commit into from
Dec 1, 2023
Merged

flightplan: fix various potential nil panics #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 22 additions & 8 deletions acceptance/acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package acceptance

import (
"context"
"fmt"
"io"
"os"
"os/exec"
Expand Down Expand Up @@ -102,6 +103,10 @@ func hasAWSCredentials() bool {
return false
}

if cfg.Credentials == nil {
return false
}

creds, err := cfg.Credentials.Retrieve(ctx)
if err != nil {
return false
Expand Down Expand Up @@ -245,9 +250,13 @@ func requireEqualOperationResponses(t *testing.T, expected *pb.OperationResponse
sortResponses(expectedResponses)
sortResponses(gotResponses)

for i := range expected.GetResponses() {
got := gotResponses[i]
require.Lenf(t, gotResponses, len(expectedResponses),
fmt.Sprintf("expected %d responses, got %d", len(expectedResponses), len(gotResponses)),
)
for i := range expectedResponses {
require.NotNil(t, gotResponses)
expected := expectedResponses[i]
got := gotResponses[i]

// Scenario reference
require.Equal(t, expected.GetOp().GetScenario().String(), got.GetOp().GetScenario().String())
Expand Down Expand Up @@ -355,13 +364,18 @@ func requireEqualOutput(t *testing.T, expected, got *pb.Terraform_Command_Output
t.Helper()

require.Len(t, expected.GetMeta(), len(got.GetMeta()))
for i := range expected.GetMeta() {
require.Equal(t, expected.GetMeta()[i].GetName(), got.GetMeta()[i].GetName())
for i, eMeta := range expected.GetMeta() {
gotMetas := got.GetMeta()
require.NotNil(t, gotMetas)
gotMeta := gotMetas[i]
require.NotNil(t, gotMeta)

require.Equal(t, eMeta.GetName(), gotMeta.GetName())
// Skip the type and the value by default since they're encoded
// require.Equal(t, expected.GetMeta()[i].GetType(), got.GetMeta()[i].GetType())
// require.Equal(t, expected.GetMeta()[i].GetValue(), got.GetMeta()[i].GetValue())
require.Equal(t, expected.GetMeta()[i].GetSensitive(), got.GetMeta()[i].GetSensitive())
require.Equal(t, expected.GetMeta()[i].GetStderr(), got.GetMeta()[i].GetStderr())
// require.Equal(t, eMeta.GetType(), gotMeta.GetType())
// require.Equal(t, eMeta.GetValue(), gotMeta.GetValue())
require.Equal(t, eMeta.GetSensitive(), gotMeta.GetSensitive())
require.Equal(t, eMeta.GetStderr(), gotMeta.GetStderr())
}
require.Equal(t, expected.GetDiagnostics(), got.GetDiagnostics())
}
Expand Down
12 changes: 7 additions & 5 deletions acceptance/fmt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ func TestAcc_Cmd_Fmt(t *testing.T) {
got := &pb.FormatResponse{}
require.NoErrorf(t, protojson.Unmarshal(out, got), string(out))
require.Len(t, got.GetResponses(), len(expected.GetResponses()))
for i := range expected.GetResponses() {
got := got.GetResponses()[i]
expected := expected.GetResponses()[i]
require.Equal(t, expected.GetPath(), got.GetPath())
require.Equal(t, expected.GetChanged(), got.GetChanged())
gotResps := got.GetResponses()
require.NotNil(t, gotResps)
for i, eRes := range expected.GetResponses() {
gotRes := gotResps[i]
require.NotNil(t, gotRes)
require.Equal(t, eRes.GetPath(), gotRes.GetPath())
require.Equal(t, eRes.GetChanged(), gotRes.GetChanged())
}
}
17 changes: 11 additions & 6 deletions acceptance/scenario_generate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,19 @@ func TestAcc_Cmd_Scenario_Generate(t *testing.T) {
})
}

var variants *pb.Matrix_Vector
if len(elements) > 0 {
variants = &pb.Matrix_Vector{
Elements: elements,
}
}

scenarioRef := &pb.Ref_Scenario{
Id: &pb.Scenario_ID{
Name: test.name,
Filter: filter,
Uid: test.uid,
Variants: &pb.Matrix_Vector{
Elements: elements,
},
Name: test.name,
Filter: filter,
Uid: test.uid,
Variants: variants,
},
}

Expand Down
21 changes: 9 additions & 12 deletions acceptance/scenario_list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ func TestAcc_Cmd_Scenario_List(t *testing.T) {
out: &pb.ListScenariosResponse{
Scenarios: []*pb.Ref_Scenario{{
Id: &pb.Scenario_ID{
Name: "test",
Filter: "test",
Uid: "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
Variants: &pb.Matrix_Vector{},
Name: "test",
Filter: "test",
Uid: "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
},
}},
},
Expand All @@ -42,18 +41,16 @@ func TestAcc_Cmd_Scenario_List(t *testing.T) {
Scenarios: []*pb.Ref_Scenario{
{
Id: &pb.Scenario_ID{
Name: "consul",
Uid: "b713f0bd8f48dfad2263cabc455ade78f7e4e99a548101f31f935686dff67124",
Filter: "consul",
Variants: &pb.Matrix_Vector{},
Name: "consul",
Uid: "b713f0bd8f48dfad2263cabc455ade78f7e4e99a548101f31f935686dff67124",
Filter: "consul",
},
},
{
Id: &pb.Scenario_ID{
Name: "vault",
Filter: "vault",
Uid: "e6f0a1fbb43c89196dcfcbef85908f19ab4c5f7cc4f4c452284697757683d7ef",
Variants: &pb.Matrix_Vector{},
Name: "vault",
Filter: "vault",
Uid: "e6f0a1fbb43c89196dcfcbef85908f19ab4c5f7cc4f4c452284697757683d7ef",
},
},
},
Expand Down
3 changes: 1 addition & 2 deletions internal/command/enos/cmd/fmt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ func runFmtCmd(cmd *cobra.Command, args []string) error {
}
}

/// Scan STDIN for content if we've been told to use STDIN either implicitly
// of explicitly.
// Scan STDIN for content if we've been told to use STDIN either implicitly of explicitly.
if (argP == "-" || argP == "") && len(req.GetFiles()) == 0 {
bytes, err := io.ReadAll(cmd.InOrStdin())
if err != nil {
Expand Down
15 changes: 10 additions & 5 deletions internal/flightplan/decoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,14 @@ type Decoder struct {

// Parse locates enos configuration files and parses them.
func (d *Decoder) Parse() hcl.Diagnostics {
var diags hcl.Diagnostics
diags := hcl.Diagnostics{}

// Parse and raw configuration bytes we've been configured with
return diags.Extend(d.parseRawFiles())
}

func (d *Decoder) parseRawFiles() hcl.Diagnostics {
var diags hcl.Diagnostics
diags := hcl.Diagnostics{}

for path, bytes := range d.fpFiles {
_, moreDiags := d.FPParser.ParseHCL(bytes, path)
Expand Down Expand Up @@ -262,7 +262,7 @@ func (d *Decoder) baseEvalContext() *hcl.EvalContext {

// Decode decodes the HCL into a flight plan.
func (d *Decoder) Decode(ctx context.Context) (*FlightPlan, hcl.Diagnostics) {
var diags hcl.Diagnostics
diags := hcl.Diagnostics{}

fp, err := NewFlightPlan(WithFlightPlanBaseDirectory(d.dir))
if err != nil {
Expand Down Expand Up @@ -299,14 +299,19 @@ func (d *Decoder) Decode(ctx context.Context) (*FlightPlan, hcl.Diagnostics) {
body := hcl.MergeFiles(files)

// Decode our top-level schema
fp.BodyContent, diags = body.Content(flightPlanSchema)
var moreDiags hcl.Diagnostics
fp.BodyContent, moreDiags = body.Content(flightPlanSchema)
diags = diags.Extend(moreDiags)
if diags.HasErrors() {
return fp, diags
}

// Decode to our desired target level. Start with the lowest level and continue until we've
// reached our desired target. Each target level includes more blocks. Where appropriate, each
// decoder is responsible for extending the eval context and/or falling through to the next
// level.
decodeToLevel := func() hcl.Diagnostics {
var diags hcl.Diagnostics
diags := hcl.Diagnostics{}

if d.target < DecodeTargetUnset {
return diags.Append(&hcl.Diagnostic{
Expand Down
68 changes: 44 additions & 24 deletions internal/flightplan/decoder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func testDiagsToError(files map[string]*hcl.File, diags hcl.Diagnostics) error {
if !diags.HasErrors() {
if diags == nil || !diags.HasErrors() {
return nil
}
msg := &strings.Builder{}
Expand All @@ -44,7 +44,9 @@ func testDecodeHCL(t *testing.T, hcl []byte, dt DecodeTarget, env ...string) (*F
)
require.NoError(t, err)
_, diags := decoder.FPParser.ParseHCL(hcl, "decoder-test.hcl")
require.False(t, diags.HasErrors(), testDiagsToError(decoder.ParserFiles(), diags))
if diags != nil {
require.False(t, diags.HasErrors(), testDiagsToError(decoder.ParserFiles(), diags))
}
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
fp, diags := decoder.Decode(ctx)
Expand Down Expand Up @@ -139,26 +141,37 @@ func testRequireEqualFP(t *testing.T, fp, expected *FlightPlan) {
}
}

if expected.ScenarioBlocks != nil {
require.NotNil(t, fp.ScenarioBlocks)
}
for i := range expected.ScenarioBlocks {
require.NotNil(t, fp.ScenarioBlocks)
gotBlock := fp.ScenarioBlocks[i]
require.NotNil(t, gotBlock)

for j := range expected.ScenarioBlocks[i].Scenarios {
require.EqualValues(t, expected.ScenarioBlocks[i].Name, fp.ScenarioBlocks[i].Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Name, fp.ScenarioBlocks[i].Scenarios[j].Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].TerraformSetting, fp.ScenarioBlocks[i].Scenarios[j].TerraformSetting)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].TerraformCLI, fp.ScenarioBlocks[i].Scenarios[j].TerraformCLI)
require.EqualValues(t, expected.ScenarioBlocks[i].Name, gotBlock.Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Name, gotBlock.Scenarios[j].Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].TerraformSetting, gotBlock.Scenarios[j].TerraformSetting)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].TerraformCLI, gotBlock.Scenarios[j].TerraformCLI)
if expected.ScenarioBlocks[i].Scenarios[j].Variants == nil {
require.Nil(t, fp.ScenarioBlocks[i].Scenarios[j].Variants)
require.Nil(t, gotBlock.Scenarios[j].Variants)
} else {
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Variants.elements, fp.ScenarioBlocks[i].Scenarios[j].Variants.elements)
expectedVariants := expected.ScenarioBlocks[i].Scenarios[j].Variants
gotVariants := gotBlock.Scenarios[j].Variants
require.NotNil(t, expectedVariants)
require.NotNil(t, gotVariants)
require.EqualValues(t, expectedVariants.Elements(), gotVariants.Elements())
}
require.Len(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs, len(fp.ScenarioBlocks[i].Scenarios[j].Outputs))
require.Len(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs, len(gotBlock.Scenarios[j].Outputs))

if len(fp.ScenarioBlocks[i].Scenarios[j].Outputs) > 0 {
if len(gotBlock.Scenarios[j].Outputs) > 0 {
for oi := range expected.ScenarioBlocks[i].Scenarios[j].Outputs {
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Name, fp.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Name)
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Description, fp.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Description)
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Sensitive, fp.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Sensitive)
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Name, gotBlock.Scenarios[j].Outputs[oi].Name)
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Description, gotBlock.Scenarios[j].Outputs[oi].Description)
require.Equal(t, expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Sensitive, gotBlock.Scenarios[j].Outputs[oi].Sensitive)
eVal := expected.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Value
aVal := fp.ScenarioBlocks[i].Scenarios[j].Outputs[oi].Value
aVal := gotBlock.Scenarios[j].Outputs[oi].Value

require.True(t, eVal.Type().Equals(aVal.Type()),
fmt.Sprintf("expected type %s, got %s", eVal.Type().FriendlyName(), aVal.Type().FriendlyName()),
Expand All @@ -169,15 +182,15 @@ func testRequireEqualFP(t *testing.T, fp, expected *FlightPlan) {
}
}

require.Len(t, expected.ScenarioBlocks[i].Scenarios[j].Steps, len(fp.ScenarioBlocks[i].Scenarios[j].Steps))
require.Len(t, expected.ScenarioBlocks[i].Scenarios[j].Steps, len(gotBlock.Scenarios[j].Steps))
for is := range expected.ScenarioBlocks[i].Scenarios[j].Steps {
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Name, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Providers, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Providers)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].DependsOn, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].DependsOn)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Skip, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Skip)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Name, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Source, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Source)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Version, fp.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Version)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Name, gotBlock.Scenarios[j].Steps[is].Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Providers, gotBlock.Scenarios[j].Steps[is].Providers)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].DependsOn, gotBlock.Scenarios[j].Steps[is].DependsOn)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Skip, gotBlock.Scenarios[j].Steps[is].Skip)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Name, gotBlock.Scenarios[j].Steps[is].Module.Name)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Source, gotBlock.Scenarios[j].Steps[is].Module.Source)
require.EqualValues(t, expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Version, gotBlock.Scenarios[j].Steps[is].Module.Version)

for isa := range expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Attrs {
eAttr := expected.ScenarioBlocks[i].Scenarios[j].Steps[is].Module.Attrs[isa]
Expand Down Expand Up @@ -210,8 +223,10 @@ func testMostlyEqualStepVar(t *testing.T, expected cty.Value, got cty.Value) {
t.Helper()

eVal, diags := StepVariableFromVal(expected)
require.NotNil(t, eVal)
require.False(t, diags.HasErrors(), diags.Error())
aVal, diags := StepVariableFromVal(got)
require.NotNil(t, aVal)
require.False(t, diags.HasErrors(), diags.Error())
require.EqualValues(t, eVal.Value, aVal.Value)
require.Lenf(t, eVal.Traversal, len(aVal.Traversal),
Expand All @@ -221,9 +236,14 @@ func testMostlyEqualStepVar(t *testing.T, expected cty.Value, got cty.Value) {

for i := range eVal.Traversal {
if i == 0 {
eAttr, ok := eVal.Traversal[i].(hcl.TraverseRoot)
require.NotNil(t, aVal.Traversal)
eTrav := eVal.Traversal[i]
require.NotNil(t, eTrav)
eAttr, ok := eTrav.(hcl.TraverseRoot)
require.True(t, ok)
aAttr, ok := aVal.Traversal[i].(hcl.TraverseRoot)
aTrav := aVal.Traversal[i]
require.NotNil(t, aTrav)
aAttr, ok := aTrav.(hcl.TraverseRoot)
require.True(t, ok)
require.EqualValues(t, eAttr.Name, aAttr.Name)

Expand Down
Loading