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.

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

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

Merged
merged 4 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)}
Copy link
Contributor

Choose a reason for hiding this comment

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

We have sourceFormats and destinationFormats defined. We can use direct type-conversion on them.
Could you try this:

sourceFormatStrings := []string(sourceFormats)
destinationFormatStrings := []string(destinationFormats)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried, direct type conversion does not seem to be supported for composite types (here array)

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can return from here to avoid the else block.


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
Loading