Skip to content

Commit

Permalink
updates from review.
Browse files Browse the repository at this point in the history
Also appied comments to nonkube connector commands.
  • Loading branch information
lynnemorrison committed Dec 10, 2024
1 parent 43f536b commit 0d87003
Show file tree
Hide file tree
Showing 17 changed files with 253 additions and 113 deletions.
5 changes: 4 additions & 1 deletion internal/cmd/skupper/connector/nonkube/connector_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error {
validationErrors = append(validationErrors, fmt.Errorf("host name must be configured: an IP address or hostname is expected"))
}
if cmd.Flags.TlsCredentials != "" {
// TBD what is valid TlsCredentials
ok, err := resourceStringValidator.Evaluate(cmd.Flags.TlsCredentials)
if !ok {
validationErrors = append(validationErrors, fmt.Errorf("tlsCredentials is not valid: %s", err))
}
}

return validationErrors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ func TestNonKubeCmdConnectorCreate_ValidateInput(t *testing.T) {
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: "TlsCredentials is not valid",
args: []string{"my-connector-tls", "8080"},
flags: &common.CommandConnectorCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"},
expectedErrors: []string{"tlsCredentials 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"},
Expand Down
9 changes: 9 additions & 0 deletions internal/cmd/skupper/connector/nonkube/connector_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func (cmd *CmdConnectorDelete) NewClient(cobraCommand *cobra.Command, args []str

func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error {
var validationErrors []error
opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false}
resourceStringValidator := validator.NewResourceStringValidator()

if cmd.CobraCmd != nil && cmd.CobraCmd.Flag(common.FlagNameContext) != nil && cmd.CobraCmd.Flag(common.FlagNameContext).Value.String() != "" {
Expand All @@ -57,6 +58,14 @@ func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error {
}
}

if cmd.connectorName != "" {
// Validate that there is already a connector with this name
connector, err := cmd.connectorHandler.Get(cmd.connectorName, opts)
if connector == nil || err != nil {
validationErrors = append(validationErrors, fmt.Errorf("connector %s does not exist", cmd.connectorName))
}
}

return validationErrors
}

Expand Down
43 changes: 39 additions & 4 deletions internal/cmd/skupper/connector/nonkube/connector_delete_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package nonkube

import (
"os"
"path/filepath"
"testing"

"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/v2alpha1"
"github.com/skupperproject/skupper/pkg/nonkube/api"
"github.com/spf13/cobra"

"gotest.tools/v3/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
)

Expand All @@ -23,6 +27,10 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) {
expectedErrors []string
}

homeDir, err := os.UserHomeDir()
assert.Check(t, err == nil)
path := filepath.Join(homeDir, "/.local/share/skupper/namespaces/test/", string(api.InputSiteStatePath))

testTable := []test{
{
name: "connector name is not specified",
Expand All @@ -48,6 +56,12 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) {
flags: &common.CommandConnectorDeleteFlags{},
expectedErrors: []string{"only one argument is allowed for this command"},
},
{
name: "connector doesn't exist ",
args: []string{"no-connector"},
flags: &common.CommandConnectorDeleteFlags{},
expectedErrors: []string{"connector no-connector does not exist"},
},
{
name: "kubernetes flags are not valid on this platform",
args: []string{"my-connector"},
Expand All @@ -60,11 +74,32 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) {
},
}

//Add a temp file so connector exists for update tests will pass
connectorResource := v2alpha1.Connector{
TypeMeta: metav1.TypeMeta{
APIVersion: "skupper.io/v2alpha1",
Kind: "Connector",
},
ObjectMeta: metav1.ObjectMeta{
Name: "my-connector",
Namespace: "test",
},
}

command := &CmdConnectorDelete{Flags: &common.CommandConnectorDeleteFlags{}}
command.CobraCmd = &cobra.Command{Use: "test"}
command.namespace = "test"
command.connectorHandler = fs.NewConnectorHandler(command.namespace)

defer command.connectorHandler.Delete("my-connector")
content, err := command.connectorHandler.EncodeToYaml(connectorResource)
assert.Check(t, err == nil)
err = command.connectorHandler.WriteFile(path, "my-connector.yaml", content, common.Connectors)
assert.Check(t, err == nil)

for _, test := range testTable {
t.Run(test.name, func(t *testing.T) {
command := &CmdConnectorDelete{Flags: &common.CommandConnectorDeleteFlags{}}
command.CobraCmd = &cobra.Command{Use: "test"}

command.connectorName = ""
if test.flags != nil {
command.Flags = test.flags
}
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/skupper/connector/nonkube/connector_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (cmd *CmdConnectorStatus) NewClient(cobraCommand *cobra.Command, args []str

func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error {
var validationErrors []error
opts := fs.GetOptions{RuntimeFirst: true}
opts := fs.GetOptions{RuntimeFirst: true, LogWarning: false}
resourceStringValidator := validator.NewResourceStringValidator()
outputTypeValidator := validator.NewOptionValidator(common.OutputTypes)

Expand Down Expand Up @@ -75,7 +75,7 @@ func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error {
}

func (cmd *CmdConnectorStatus) Run() error {
opts := fs.GetOptions{RuntimeFirst: true}
opts := fs.GetOptions{RuntimeFirst: true, LogWarning: true}
if cmd.connectorName == "" {
connectors, err := cmd.connectorHandler.List()
if connectors == nil || err != nil {
Expand Down
11 changes: 9 additions & 2 deletions internal/cmd/skupper/connector/nonkube/connector_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (cmd *CmdConnectorUpdate) NewClient(cobraCommand *cobra.Command, args []str

func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error {
var validationErrors []error
opts := fs.GetOptions{RuntimeFirst: false}
opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false}
resourceStringValidator := validator.NewResourceStringValidator()
numberValidator := validator.NewNumberValidator()
connectorTypeValidator := validator.NewOptionValidator(common.ConnectorTypes)
Expand Down Expand Up @@ -132,7 +132,14 @@ func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error {
cmd.output = cmd.Flags.Output
}
}

if cmd.Flags.TlsCredentials != "" {
ok, err := resourceStringValidator.Evaluate(cmd.Flags.TlsCredentials)
if !ok {
validationErrors = append(validationErrors, fmt.Errorf("tlsCredentials is not valid: %s", err))
} else {
cmd.newSettings.tlsCredentials = cmd.Flags.TlsCredentials
}
}
return validationErrors
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) {
flags: &common.CommandConnectorUpdateFlags{RoutingKey: "not-valid$"},
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: "tlsCredentials is not valid",
args: []string{"my-connector"},
flags: &common.CommandConnectorUpdateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"},
expectedErrors: []string{"tlsCredentials 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"},
Expand Down
23 changes: 15 additions & 8 deletions internal/cmd/skupper/listener/nonkube/listener_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type CmdListenerCreate struct {
listenerName string
port int
host string
tlsSecret string
tlsCredentials string
listenerType string
routingKey string
output string
Expand Down Expand Up @@ -105,11 +105,13 @@ func (cmd *CmdListenerCreate) ValidateInput(args []string) []error {
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

if cmd.Flags.TlsCredentials != "" {
ok, err := resourceStringValidator.Evaluate(cmd.Flags.TlsCredentials)
if !ok {
validationErrors = append(validationErrors, fmt.Errorf("tlsCredentials is not valid: %s", err))
}
}

if cmd.Flags.Output != "" {
Expand All @@ -133,8 +135,13 @@ func (cmd *CmdListenerCreate) InputToOptions() {
cmd.namespace = "default"
}

cmd.host = cmd.Flags.Host
cmd.tlsSecret = cmd.Flags.TlsSecret
if cmd.Flags.Host == "" {
cmd.host = "0.0.0.0"
} else {
cmd.host = cmd.Flags.Host
}

cmd.tlsCredentials = cmd.Flags.TlsCredentials
cmd.listenerType = cmd.Flags.ListenerType
cmd.output = cmd.Flags.Output
}
Expand All @@ -153,7 +160,7 @@ func (cmd *CmdListenerCreate) Run() error {
Host: cmd.host,
Port: cmd.port,
RoutingKey: cmd.routingKey,
TlsCredentials: cmd.tlsSecret,
TlsCredentials: cmd.tlsCredentials,
Type: cmd.listenerType,
},
}
Expand Down
102 changes: 51 additions & 51 deletions internal/cmd/skupper/listener/nonkube/listener_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) {
flags: &common.CommandListenerCreateFlags{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: "TlsCredentials key is not valid",
args: []string{"my-listener-tls", "8080"},
flags: &common.CommandListenerCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"},
expectedErrors: []string{"tlsCredentials 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-listener-host", "8080"},
flags: &common.CommandListenerCreateFlags{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-listener-host", "8080"},
flags: &common.CommandListenerCreateFlags{},
expectedErrors: []string{"host name must be configured: an IP address or hostname is expected"},
},
{
name: "output format is not valid",
args: []string{"my-listener", "8080"},
Expand All @@ -110,11 +110,11 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) {
name: "flags all valid",
args: []string{"my-listener-flags", "8080"},
flags: &common.CommandListenerCreateFlags{
RoutingKey: "routingkeyname",
TlsSecret: "secretname",
ListenerType: "tcp",
Output: "json",
Host: "1.2.3.4",
RoutingKey: "routingkeyname",
TlsCredentials: "secretname",
ListenerType: "tcp",
Output: "json",
Host: "1.2.3.4",
},
expectedErrors: []string{},
},
Expand Down Expand Up @@ -148,51 +148,51 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) {
func TestNonKubeCmdListenerCreate_InputToOptions(t *testing.T) {

type test struct {
name string
args []string
namespace string
flags common.CommandListenerCreateFlags
expectedNamespace string
listenerName string
expectedTlsSecret string
expectedHost string
expectedRoutingKey string
expectedListenerType string
expectedOutput string
name string
args []string
namespace string
flags common.CommandListenerCreateFlags
expectedNamespace string
listenerName string
expectedTlsCredentials string
expectedHost string
expectedRoutingKey string
expectedListenerType string
expectedOutput string
}

testTable := []test{
{
name: "test1",
flags: common.CommandListenerCreateFlags{"backend", "", "secret", "tcp", 0, "json"},
expectedTlsSecret: "secret",
expectedHost: "",
expectedRoutingKey: "backend",
expectedListenerType: "tcp",
expectedOutput: "json",
expectedNamespace: "default",
name: "test1",
flags: common.CommandListenerCreateFlags{"backend", "", "secret", "tcp", 0, "json"},
expectedTlsCredentials: "secret",
expectedHost: "0.0.0.0",
expectedRoutingKey: "backend",
expectedListenerType: "tcp",
expectedOutput: "json",
expectedNamespace: "default",
},
{
name: "test2",
namespace: "test",
flags: common.CommandListenerCreateFlags{"backend", "1.2.3.4", "secret", "tcp", 0, "json"},
expectedTlsSecret: "secret",
expectedHost: "1.2.3.4",
expectedRoutingKey: "backend",
expectedListenerType: "tcp",
expectedOutput: "json",
expectedNamespace: "test",
name: "test2",
namespace: "test",
flags: common.CommandListenerCreateFlags{"backend", "1.2.3.4", "secret", "tcp", 0, "json"},
expectedTlsCredentials: "secret",
expectedHost: "1.2.3.4",
expectedRoutingKey: "backend",
expectedListenerType: "tcp",
expectedOutput: "json",
expectedNamespace: "test",
},
{
name: "test3",
namespace: "default",
flags: common.CommandListenerCreateFlags{"", "", "secret", "tcp", 0, "yaml"},
expectedTlsSecret: "secret",
expectedHost: "",
expectedRoutingKey: "my-listener",
expectedListenerType: "tcp",
expectedOutput: "yaml",
expectedNamespace: "default",
name: "test3",
namespace: "default",
flags: common.CommandListenerCreateFlags{"", "", "secret", "tcp", 0, "yaml"},
expectedTlsCredentials: "secret",
expectedHost: "0.0.0.0",
expectedRoutingKey: "my-listener",
expectedListenerType: "tcp",
expectedOutput: "yaml",
expectedNamespace: "default",
},
}

Expand All @@ -207,7 +207,7 @@ func TestNonKubeCmdListenerCreate_InputToOptions(t *testing.T) {
cmd.InputToOptions()

assert.Check(t, cmd.routingKey == test.expectedRoutingKey)
assert.Check(t, cmd.tlsSecret == test.expectedTlsSecret)
assert.Check(t, cmd.tlsCredentials == test.expectedTlsCredentials)
assert.Check(t, cmd.host == test.expectedHost)
assert.Check(t, cmd.listenerType == test.expectedListenerType)
assert.Check(t, cmd.output == test.expectedOutput)
Expand All @@ -227,7 +227,7 @@ func TestNonKubeCmdListenerCreate_Run(t *testing.T) {
output string
errorMessage string
routingKey string
tlsSecret string
tlsCredentials string
listenerType string
listenerPort int
}
Expand All @@ -241,7 +241,7 @@ func TestNonKubeCmdListenerCreate_Run(t *testing.T) {
listenerPort: 8080,
listenerType: "tcp",
routingKey: "keyname",
tlsSecret: "secretname",
tlsCredentials: "secretname",
host: "1.2.3.4",
},
{
Expand Down
Loading

0 comments on commit 0d87003

Please sign in to comment.