Skip to content

Commit

Permalink
fix: improve error messaging when mandatory flag is missing in file c…
Browse files Browse the repository at this point in the history
…onvert (#1429) (#1487)

* fix: improve error messaging when mandatory flag is missing in file convert (#1429)

When mandatory flags for file convert are missing, the error message is not helpful. In this commit, we have added additional validation to communicate more details in the message.

* add a reusable validation function for strings

* assert converted file contents and address review feedback

* limit file convert tests to yaml format for now
  • Loading branch information
harshadixit12 authored Jan 15, 2025
1 parent 2c9a599 commit 6472d17
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 0 deletions.
16 changes: 16 additions & 0 deletions cmd/file_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,22 @@ into another compatible format. For example, a configuration for 'kong-gateway-2
can be converted into a 'kong-gateway-3.x' configuration file.`,
Args: validateNoArgs,
RunE: execute,
PreRunE: func(_ *cobra.Command, _ []string) error {
validSourceFormats := []string{string(convert.FormatKongGateway), string(convert.FormatKongGateway2x)}
validDestinationFormats := []string{string(convert.FormatKonnect), string(convert.FormatKongGateway3x)}

err := validateInputFlag("from", convertCmdSourceFormat, validSourceFormats, "")
if err != nil {
return err
}

err = validateInputFlag("to", convertCmdDestinationFormat, validDestinationFormats, "")
if err != nil {
return err
}

return nil
},
}

sourceFormats := []convert.Format{convert.FormatKongGateway, convert.FormatKongGateway2x}
Expand Down
17 changes: 17 additions & 0 deletions cmd/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package cmd

import (
"errors"
"fmt"
"slices"

"github.com/fatih/color"
"github.com/kong/go-database-reconciler/pkg/cprint"
"github.com/kong/go-database-reconciler/pkg/diff"
Expand Down Expand Up @@ -30,3 +34,16 @@ func addSilenceEventsFlag(set *pflag.FlagSet) {
set.BoolVar(&silenceEvents, "silence-events", false,
"disable printing events to stdout")
}

func validateInputFlag(flagName string, flagValue string, allowedValues []string, errorMessage string) error {
if slices.Contains(allowedValues, flagValue) {
return nil
}

if errorMessage != "" {
return errors.New(errorMessage)
}

return fmt.Errorf("invalid value '%s' found for the '%s' flag. Allowed values: %v",
flagValue, flagName, allowedValues)
}
73 changes: 73 additions & 0 deletions tests/integration/file_convert_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package integration

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"sigs.k8s.io/yaml"
)

func Test_FileConvert(t *testing.T) {
tests := []struct {
name string
convertCmdSourceFormat string
convertCmdDestinationFormat string
errorExpected bool
errorString string
expectedOutputFile string
}{
{
name: "Valid source and destination format",
convertCmdSourceFormat: "kong-gateway-2.x",
convertCmdDestinationFormat: "kong-gateway-3.x",
errorExpected: false,
expectedOutputFile: "testdata/file-convert/001-kong-gateway-config/kong-gateway-3-x.yaml",
},
{
name: "Invalid source format",
convertCmdSourceFormat: "random-value",
convertCmdDestinationFormat: "kong-gateway-3.x",
errorExpected: true,
errorString: "invalid value 'random-value' found for the 'from' flag." +
" Allowed values: [kong-gateway kong-gateway-2.x]",
},
{
name: "Invalid destination format",
convertCmdSourceFormat: "kong-gateway-2.x",
convertCmdDestinationFormat: "random-value",
errorExpected: true,
errorString: "invalid value 'random-value' found for the 'to' flag." +
" Allowed values: [konnect kong-gateway-3.x]",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
output, err := fileConvert(
"--from", tc.convertCmdSourceFormat,
"--to", tc.convertCmdDestinationFormat,
"--input-file", "testdata/file-convert/001-kong-gateway-config/kong-gateway-2-x.yaml",
)

if tc.errorExpected {
assert.Error(t, err)
assert.Contains(t, err.Error(), tc.errorString)

return
}

assert.NoError(t, err)

var expectedOutput interface{}
var currentOutput interface{}
content, err := os.ReadFile(tc.expectedOutputFile)
assert.NoError(t, err)

err = yaml.Unmarshal(content, &expectedOutput)
assert.NoError(t, err)
err = yaml.Unmarshal([]byte(output), &currentOutput)
assert.NoError(t, err)
assert.Equal(t, expectedOutput, currentOutput)
})
}
}
22 changes: 22 additions & 0 deletions tests/integration/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,28 @@ func lint(opts ...string) (string, error) {
return stripansi.Strip(string(out)), cmdErr
}

func fileConvert(opts ...string) (string, error) {
deckCmd := cmd.NewRootCmd()
args := []string{"file", "convert"}
if len(opts) > 0 {
args = append(args, opts...)
}
deckCmd.SetArgs(args)

// capture command output to be used during tests
rescueStdout := os.Stdout
r, w, _ := os.Pipe()
os.Stdout = w

cmdErr := deckCmd.ExecuteContext(context.Background())

w.Close()
out, _ := io.ReadAll(r)
os.Stdout = rescueStdout

return stripansi.Strip(string(out)), cmdErr
}

func ping(opts ...string) error {
deckCmd := cmd.NewRootCmd()
args := []string{"gateway", "ping"}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
_format_version: "1.1"
services:
- connect_timeout: 60000
id: 58076db2-28b6-423b-ba39-a797193017f7
host: mockbin.org
name: svc1
port: 80
protocol: http
read_timeout: 60000
retries: 5
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
_format_version: "3.0"
services:
- connect_timeout: 60000
id: 58076db2-28b6-423b-ba39-a797193017f7
host: mockbin.org
name: svc1
port: 80
protocol: http
read_timeout: 60000
retries: 5

0 comments on commit 6472d17

Please sign in to comment.