Skip to content

Commit

Permalink
code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@lynnemorrison
lynnemorrison committed Oct 28, 2024
1 parent f81df8a commit 5f216e2
Show file tree
Hide file tree
Showing 13 changed files with 204 additions and 167 deletions.
6 changes: 6 additions & 0 deletions internal/cmd/skupper/common/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,9 @@ var (
ConnectorTypes = []string{"tcp"}
WorkloadTypes = []string{"deployment", "service", "daemonset", "statefulset"}
)

const (
Connectors string = "connectors"
Listeners string = "listeners"
Sites string = "sites"
)
18 changes: 13 additions & 5 deletions internal/cmd/skupper/connector/nonkube/connector_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ package nonkube

import (
"fmt"
"net"
"strconv"

"github.com/skupperproject/skupper/internal/cmd/skupper/common"
"github.com/skupperproject/skupper/internal/cmd/skupper/common/utils"
"github.com/skupperproject/skupper/internal/nonkube/client/fs"
"github.com/skupperproject/skupper/pkg/apis/skupper/v1alpha1"
"github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1"
"github.com/skupperproject/skupper/pkg/utils/validator"
"github.com/spf13/cobra"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -54,6 +55,7 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error {
numberValidator := validator.NewNumberValidator()
connectorTypeValidator := validator.NewOptionValidator(common.ConnectorTypes)
outputTypeValidator := validator.NewOptionValidator(common.OutputTypes)
hostStringValidator := validator.NewHostStringValidator()

// Validate arguments name and port
if len(args) < 2 {
Expand Down Expand Up @@ -101,7 +103,13 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error {
}
}
if cmd.Flags.Host != "" {
// TBD what is valid host
ip := net.ParseIP(cmd.Flags.Host)
ok, _ := hostStringValidator.Evaluate(cmd.Flags.Host)
if !ok || ip == nil {
validationErrors = append(validationErrors, fmt.Errorf("host is not valid: a valid IP address or hostname is expected"))
}
} else {
validationErrors = append(validationErrors, fmt.Errorf("host name must be configured: an IP address or hostname is expected"))
}
if cmd.Flags.TlsSecret != "" {
// TBD what is valid TlsSecret
Expand Down Expand Up @@ -129,16 +137,16 @@ func (cmd *CmdConnectorCreate) InputToOptions() {
}

func (cmd *CmdConnectorCreate) Run() error {
connectorResource := v1alpha1.Connector{
connectorResource := v2alpha1.Connector{
TypeMeta: metav1.TypeMeta{
APIVersion: "skupper.io/v1alpha1",
APIVersion: "skupper.io/v2alpha1",
Kind: "Connector",
},
ObjectMeta: metav1.ObjectMeta{
Name: cmd.connectorName,
Namespace: cmd.namespace,
},
Spec: v1alpha1.ConnectorSpec{
Spec: v2alpha1.ConnectorSpec{
Host: cmd.host,
Port: cmd.port,
RoutingKey: cmd.routingKey,
Expand Down
40 changes: 26 additions & 14 deletions internal/cmd/skupper/connector/nonkube/connector_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/skupperproject/skupper/internal/cmd/skupper/common"
"github.com/skupperproject/skupper/internal/cmd/skupper/common/utils"
fs2 "github.com/skupperproject/skupper/internal/nonkube/client/fs"
"github.com/skupperproject/skupper/internal/nonkube/client/fs"
"github.com/spf13/cobra"

"gotest.tools/assert"
Expand All @@ -27,67 +27,79 @@ func TestNonKubeCmdConnectorCreate_ValidateInput(t *testing.T) {
{
name: "Connector name and port are not specified",
args: []string{},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector name and port must be configured"},
},
{
name: "Connector name is not valid",
args: []string{"my new Connector", "8080"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"},
},
{
name: "Connector name is empty",
args: []string{"", "1234"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector name must not be empty"},
},
{
name: "connector port empty",
args: []string{"my-name-port-empty", ""},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector port must not be empty"},
},
{
name: "port is not valid",
args: []string{"my-connector-port", "abcd"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax"},
},
{
name: "port not positive",
args: []string{"my-port-positive", "-45"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"connector port is not valid: value is not positive"},
},
{
name: "more than two arguments was specified",
args: []string{"my", "Connector", "test"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{"only two arguments are allowed for this command"},
},
{
name: "type is not valid",
args: []string{"my-connector", "8080"},
flags: &common.CommandConnectorCreateFlags{ConnectorType: "not-valid"},
flags: &common.CommandConnectorCreateFlags{ConnectorType: "not-valid", Host: "1.2.3.4"},
expectedErrors: []string{"connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"},
},
{
name: "routing key is not valid",
args: []string{"my-connector-rk", "8080"},
flags: &common.CommandConnectorCreateFlags{RoutingKey: "not-valid$"},
flags: &common.CommandConnectorCreateFlags{RoutingKey: "not-valid$", Host: "1.2.3.4"},
expectedErrors: []string{"routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"},
},
{
name: "host is not valid",
args: []string{"my-connector-host", "8080"},
flags: &common.CommandConnectorCreateFlags{Host: "not-valid$"},
expectedErrors: []string{"host is not valid: a valid IP address or hostname is expected"},
},
{
name: "host is not configued",
args: []string{"my-connector-host", "8080"},
flags: &common.CommandConnectorCreateFlags{},
expectedErrors: []string{"host name must be configured: an IP address or hostname is expected"},
},
{
name: "output format is not valid",
args: []string{"my-connector", "8080"},
flags: &common.CommandConnectorCreateFlags{Output: "not-valid"},
flags: &common.CommandConnectorCreateFlags{Output: "not-valid", Host: "1.2.3.4"},
expectedErrors: []string{"output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]"},
},
{
name: "kubernetes flags are not valid on this platform",
args: []string{"my-connector", "8080"},
flags: &common.CommandConnectorCreateFlags{},
flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"},
expectedErrors: []string{},
cobraGenericFlags: map[string]string{
common.FlagNameContext: "test",
Expand Down Expand Up @@ -190,7 +202,7 @@ func TestNonKubeCmdConnectorCreate_InputToOptions(t *testing.T) {
cmd.Flags = &test.flags
cmd.connectorName = "my-Connector"
cmd.namespace = test.namespace
cmd.connectorHandler = fs2.NewConnectorHandler(cmd.namespace)
cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace)

cmd.InputToOptions()

Expand Down Expand Up @@ -266,7 +278,7 @@ func TestNonKubeCmdConnectorCreate_Run(t *testing.T) {
command.host = test.host
command.connectorType = test.connectorType
command.namespace = test.namespace
command.connectorHandler = fs2.NewConnectorHandler(command.namespace)
command.connectorHandler = fs.NewConnectorHandler(command.namespace)
defer command.connectorHandler.Delete("test1")
t.Run(test.name, func(t *testing.T) {

Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/skupper/connector/nonkube/connector_delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/skupperproject/skupper/internal/cmd/skupper/common"
"github.com/skupperproject/skupper/internal/cmd/skupper/common/utils"
fs2 "github.com/skupperproject/skupper/internal/nonkube/client/fs"
"github.com/skupperproject/skupper/internal/nonkube/client/fs"
"github.com/spf13/cobra"

"gotest.tools/assert"
Expand Down Expand Up @@ -119,7 +119,7 @@ func TestCmdConnectorDelete_Run(t *testing.T) {

cmd.connectorName = test.deleteName
cmd.namespace = test.namespace
cmd.connectorHandler = fs2.NewConnectorHandler(cmd.namespace)
cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace)
cmd.InputToOptions()

err := cmd.Run()
Expand Down
26 changes: 7 additions & 19 deletions internal/cmd/skupper/connector/nonkube/connector_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,15 @@ func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error {

func (cmd *CmdConnectorStatus) Run() error {
if cmd.connectorName == "" {
_, connectors, err := cmd.connectorHandler.GetRuntime("")
if err != nil {
fmt.Println("failed getting directory")
connectors, err := cmd.connectorHandler.List()
if connectors == nil || err != nil {
fmt.Println("No connectors found:")
return err
}
if cmd.output != "" {
for _, file := range connectors {
fmt.Println("file name:", file.Name())
connector, _, err := cmd.connectorHandler.GetRuntime(file.Name())
if err != nil {
fmt.Println("failed getting connector")
return err
}
for _, connector := range connectors {
encodedOutput, err := utils.Encode(cmd.output, connector)
if err != nil {
fmt.Println("failed encoding connector output")
return err
}
fmt.Println(encodedOutput)
Expand All @@ -99,12 +92,7 @@ func (cmd *CmdConnectorStatus) Run() error {
tw := tabwriter.NewWriter(os.Stdout, 8, 8, 1, '\t', tabwriter.TabIndent)
_, _ = fmt.Fprintln(tw, fmt.Sprintf("%s\t%s\t%s\t%s\t%s",
"NAME", "STATUS", "ROUTING-KEY", "HOST", "PORT"))
for _, file := range connectors {
connector, _, err := cmd.connectorHandler.GetRuntime(file.Name())
if err != nil {
fmt.Println("failed getting connector")
return err
}
for _, connector := range connectors {
status := "Not Ready"
if connector.IsConfigured() {
status = "Ok"
Expand All @@ -115,9 +103,9 @@ func (cmd *CmdConnectorStatus) Run() error {
_ = tw.Flush()
}
} else {
connector, _, err := cmd.connectorHandler.GetRuntime(cmd.connectorName + ".yaml")
connector, err := cmd.connectorHandler.Get(cmd.connectorName)
if connector == nil || err != nil {
fmt.Println("No connectors found:", err)
fmt.Println("No connectors found:")
return err
}
if cmd.output != "" {
Expand Down
Loading

0 comments on commit 5f216e2

Please sign in to comment.