From bd0b3a8063cd0fd1767892423a448ce492ba25d0 Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Sat, 16 Jan 2021 11:18:49 -0700 Subject: [PATCH] feat(security): Add more tests to increase coverage Added some more missing testable tests to increase the test coverage. Signed-off-by: Jim Wang --- .../bootstrapper/command/gate/command.go | 2 +- .../bootstrapper/command/gate/command_test.go | 160 ++++++++++++++++++ .../command/genpassword/command.go | 2 +- .../command/genpassword/command_test.go | 75 ++++++++ .../command/gethttpstatus/command.go | 2 +- .../command/gethttpstatus/command_test.go | 101 +++++++++++ .../bootstrapper/command/listen/command.go | 2 +- .../command/listen/command_test.go | 136 +++++++++++++++ .../bootstrapper/command/ping/command.go | 3 +- .../bootstrapper/command/ping/command_test.go | 57 +++++++ .../bootstrapper/interfaces/command.go | 6 +- internal/security/bootstrapper/tcp/client.go | 4 +- .../security/bootstrapper/tcp/listener.go | 4 + .../bootstrapper/tcp/listener_test.go | 4 - 14 files changed, 545 insertions(+), 13 deletions(-) create mode 100644 internal/security/bootstrapper/command/gate/command_test.go create mode 100644 internal/security/bootstrapper/command/genpassword/command_test.go create mode 100644 internal/security/bootstrapper/command/gethttpstatus/command_test.go create mode 100644 internal/security/bootstrapper/command/listen/command_test.go create mode 100644 internal/security/bootstrapper/command/ping/command_test.go diff --git a/internal/security/bootstrapper/command/gate/command.go b/internal/security/bootstrapper/command/gate/command.go index 7349018630..a314e346c5 100644 --- a/internal/security/bootstrapper/command/gate/command.go +++ b/internal/security/bootstrapper/command/gate/command.go @@ -66,7 +66,7 @@ func NewCommand( return nil, fmt.Errorf("Unable to parse command: %s: %w", strings.Join(args, " "), err) } - return &cmd, err + return &cmd, nil } // Execute implements Command and runs this command diff --git a/internal/security/bootstrapper/command/gate/command_test.go b/internal/security/bootstrapper/command/gate/command_test.go new file mode 100644 index 0000000000..36031fba5e --- /dev/null +++ b/internal/security/bootstrapper/command/gate/command_test.go @@ -0,0 +1,160 @@ +/******************************************************************************* + * Copyright 2021 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + *******************************************************************************/ + +package gate + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/config" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/interfaces" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/tcp" + + "github.com/stretchr/testify/require" + + "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" +) + +func TestNewCommand(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: gate cmd empty option", []string{}, false}, + {"Bad: gate invalid option", []string{"--invalid=xxx"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + command, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, command) + } + }) + } +} + +type testConfig struct { + testHost string + bootstrapperStartPort int + consulReadyPort int + redisReadyPort int + postgresReadyPort int + readyToRunPort int +} + +func TestExecuteWithAllDependentsRun(t *testing.T) { + // Arrange + lc := logger.MockLogger{} + testHost := "localhost" + testConfig := &testConfig{ + testHost: "localhost", + bootstrapperStartPort: 28001, + consulReadyPort: 28002, + redisReadyPort: 28003, + postgresReadyPort: 28004, + readyToRunPort: 28009, + } + config := setupMockServiceConfigs(testConfig) + + type executeReturn struct { + code int + err error + } + + ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) + wg := &sync.WaitGroup{} + gate, err := NewCommand(ctx, wg, lc, config, []string{}) + defer func() { + cancelFunc() + wg.Wait() + }() + require.NoError(t, err) + require.NotNil(t, gate) + require.Equal(t, "gate", gate.GetCommandName()) + execRet := make(chan executeReturn, 1) + // in a separate go-routine since the listenTcp is a blocking call + go func() { + statusCode, err := gate.Execute() + execRet <- executeReturn{code: statusCode, err: err} + }() + + tcpSrvErr := make(chan error) + // start up all other dependent mock services: + go func() { + tcpSrvErr <- tcp.NewTcpServer().StartListener(testConfig.consulReadyPort, + lc, testHost) + }() + go func() { + tcpSrvErr <- tcp.NewTcpServer().StartListener(testConfig.postgresReadyPort, + lc, testHost) + }() + go func() { + tcpSrvErr <- tcp.NewTcpServer().StartListener(testConfig.redisReadyPort, + lc, testHost) + }() + + select { + case ret := <-execRet: + require.NoError(t, ret.err) + require.Equal(t, interfaces.StatusCodeExitNormal, ret.code) + case err := <-tcpSrvErr: + require.NoError(t, err) + case <-time.After(5 * time.Second): + require.Fail(t, "security bootstrapper gate never returned") + } +} + +func setupMockServiceConfigs(testConf *testConfig) *config.ConfigurationStruct { + conf := &config.ConfigurationStruct{} + conf.StageGate = config.StageGateInfo{ + BootStrapper: config.BootStrapperInfo{ + Host: testConf.testHost, + StartPort: testConf.bootstrapperStartPort, + }, + Consul: config.ConsulInfo{ + Host: testConf.testHost, + Port: 12001, + ReadyPort: testConf.consulReadyPort, + }, + Redis: config.RedisInfo{ + Host: testConf.testHost, + Port: 12002, + ReadyPort: testConf.redisReadyPort, + }, + PG: config.PostgresInfo{ + Host: testConf.testHost, + Port: 12003, + ReadyPort: testConf.postgresReadyPort, + }, + Ready: config.ReadyInfo{ + ToRunPort: testConf.readyToRunPort, + }, + } + return conf +} diff --git a/internal/security/bootstrapper/command/genpassword/command.go b/internal/security/bootstrapper/command/genpassword/command.go index bc40bbe827..b0e241ecf6 100644 --- a/internal/security/bootstrapper/command/genpassword/command.go +++ b/internal/security/bootstrapper/command/genpassword/command.go @@ -64,7 +64,7 @@ func NewCommand( return nil, fmt.Errorf("Unable to parse command: %s: %w", strings.Join(args, " "), err) } - return &cmd, err + return &cmd, nil } // Execute implements Command and runs this command diff --git a/internal/security/bootstrapper/command/genpassword/command_test.go b/internal/security/bootstrapper/command/genpassword/command_test.go new file mode 100644 index 0000000000..96b4db6067 --- /dev/null +++ b/internal/security/bootstrapper/command/genpassword/command_test.go @@ -0,0 +1,75 @@ +/******************************************************************************* + * Copyright 2021 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + *******************************************************************************/ + +package genpassword + +import ( + "context" + "sync" + "testing" + + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/config" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/interfaces" + + "github.com/stretchr/testify/require" + + "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" +) + +func TestNewCommand(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: genPasswd cmd empty option", []string{}, false}, + {"Bad: genPasswd invalid option", []string{"--invalid=xxx"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + command, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, command) + } + }) + } +} + +func TestExecute(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + genPwd, err := NewCommand(ctx, wg, lc, config, []string{}) + require.NoError(t, err) + require.NotNil(t, genPwd) + require.Equal(t, "genPassword", genPwd.GetCommandName()) + + statusCode, err := genPwd.Execute() + require.NoError(t, err) + require.Equal(t, interfaces.StatusCodeExitNormal, statusCode) +} diff --git a/internal/security/bootstrapper/command/gethttpstatus/command.go b/internal/security/bootstrapper/command/gethttpstatus/command.go index 622d26af63..bb2c1f21cc 100644 --- a/internal/security/bootstrapper/command/gethttpstatus/command.go +++ b/internal/security/bootstrapper/command/gethttpstatus/command.go @@ -74,7 +74,7 @@ func NewCommand( return nil, fmt.Errorf("%s %s: argument --url is required", os.Args[0], CommandName) } - return &cmd, err + return &cmd, nil } // GetCommandName returns the name of this command diff --git a/internal/security/bootstrapper/command/gethttpstatus/command_test.go b/internal/security/bootstrapper/command/gethttpstatus/command_test.go new file mode 100644 index 0000000000..83173e0d0a --- /dev/null +++ b/internal/security/bootstrapper/command/gethttpstatus/command_test.go @@ -0,0 +1,101 @@ +/******************************************************************************* + * Copyright 2021 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + *******************************************************************************/ + +package gethttpstatus + +import ( + "context" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/config" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/interfaces" + + "github.com/stretchr/testify/require" + + "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" +) + +func TestNewCommand(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: getHttpStatus required --url option", []string{"--url=http://localhost:32323"}, false}, + {"Bad: getHttpStatus invalid option", []string{"--invalid=http://localhost:123"}, true}, + {"Bad: getHttpStatus empty option", []string{""}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + command, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, command) + } + }) + } +} + +func TestExecute(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + testSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer testSrv.Close() + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: getHttpStatus with existing server", []string{"--url=" + testSrv.URL}, false}, + {"Bad: getHttpStatus with non-existing server", []string{"--url=http://non-existing:1111"}, true}, + {"Bad: getHttpStatus with malformed URL", []string{"--url=_http!@xxxxxx:1111"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + getHttpStatus, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + require.NoError(t, err) + require.NotNil(t, getHttpStatus) + require.Equal(t, "getHttpStatus", getHttpStatus.GetCommandName()) + + statusCode, err := getHttpStatus.Execute() + + if tt.expectedErr { + require.Error(t, err) + require.Equal(t, interfaces.StatusCodeExitWithError, statusCode) + } else { + require.NoError(t, err) + require.Equal(t, interfaces.StatusCodeExitNormal, statusCode) + } + }) + } +} diff --git a/internal/security/bootstrapper/command/listen/command.go b/internal/security/bootstrapper/command/listen/command.go index fb0de21a3f..d1758021b9 100644 --- a/internal/security/bootstrapper/command/listen/command.go +++ b/internal/security/bootstrapper/command/listen/command.go @@ -72,7 +72,7 @@ func NewCommand( return nil, fmt.Errorf("%s %s: argument --port is required", os.Args[0], CommandName) } - return &cmd, err + return &cmd, nil } // Execute implements Command and runs this command diff --git a/internal/security/bootstrapper/command/listen/command_test.go b/internal/security/bootstrapper/command/listen/command_test.go new file mode 100644 index 0000000000..281de5b7eb --- /dev/null +++ b/internal/security/bootstrapper/command/listen/command_test.go @@ -0,0 +1,136 @@ +/******************************************************************************* + * Copyright 2021 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + *******************************************************************************/ + +package listen + +import ( + "context" + "strconv" + "sync" + "testing" + "time" + + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/config" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/interfaces" + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/tcp" + + "github.com/stretchr/testify/require" + + "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" +) + +func TestNewCommand(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: listenTcp required --port option", []string{"--port=32323"}, false}, + {"Good: listenTcp both options", []string{"--host=test", "--port=32323"}, false}, + {"Bad: listenTcp invalid option", []string{"--invalid=xxxxx"}, true}, + {"Bad: listenTcp empty option", []string{""}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + command, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, command) + } + }) + } +} + +func TestExecute(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + testListenPort1 := 65111 + testListenPort2 := 65112 + + tests := []struct { + name string + cmdArgs []string + testPort int + expectedErr bool + }{ + {"Good: listenTcp with unbound port", + []string{"--port=" + strconv.Itoa(testListenPort1)}, testListenPort1, false}, + {"Good: listenTcp with unbound port specific host", + []string{"--host=localhost", "--port=" + strconv.Itoa(testListenPort2)}, testListenPort2, false}, + {"Bad: listenTcp with already bound port", + []string{"--host=localhost", "--port=" + strconv.Itoa(testListenPort2)}, testListenPort2, true}, + } + + type executeReturn struct { + code int + err error + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + listenTcp, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + require.NoError(t, err) + require.NotNil(t, listenTcp) + require.Equal(t, "listenTcp", listenTcp.GetCommandName()) + + execRet := make(chan executeReturn, 1) + // in a separate go-routine since the listenTcp is a blocking call + go func() { + statusCode, err := listenTcp.Execute() + execRet <- executeReturn{code: statusCode, err: err} + }() + + dialErr := make(chan error) + // dial to tcp server to check the server being listening + go func() { + dialErr <- tcp.DialTcp("", tt.testPort, lc) + }() + + select { + case testErr := <-dialErr: + require.NoError(t, testErr) + case <-time.After(3 * time.Second): + require.Fail(t, "DialTcp never returned") + } + + // test to wait for some time to check the running tcp server is not errorred out + // since receiving execRet channel will block forever if no error occurs + select { + case ret := <-execRet: + if tt.expectedErr { + require.Error(t, ret.err) + require.Equal(t, interfaces.StatusCodeExitWithError, ret.code) + } else { + require.NoError(t, ret.err) + require.Equal(t, interfaces.StatusCodeExitNormal, ret.code) + } + case <-time.After(5 * time.Second): + t.Logf("tcp server %s listening ok", tt.cmdArgs) + } + }) + } +} diff --git a/internal/security/bootstrapper/command/ping/command.go b/internal/security/bootstrapper/command/ping/command.go index 3f43ee58cc..08bcd90523 100644 --- a/internal/security/bootstrapper/command/ping/command.go +++ b/internal/security/bootstrapper/command/ping/command.go @@ -90,7 +90,8 @@ func NewCommand( if err != nil { return nil, fmt.Errorf("Unable to parse command: %s: %w", strings.Join(args, " "), err) } - return &cmd, err + + return &cmd, nil } // Execute implements Command and runs this command diff --git a/internal/security/bootstrapper/command/ping/command_test.go b/internal/security/bootstrapper/command/ping/command_test.go new file mode 100644 index 0000000000..870df609ca --- /dev/null +++ b/internal/security/bootstrapper/command/ping/command_test.go @@ -0,0 +1,57 @@ +/******************************************************************************* + * Copyright 2021 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + * + *******************************************************************************/ + +package ping + +import ( + "context" + "sync" + "testing" + + "github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/config" + + "github.com/stretchr/testify/require" + + "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" +) + +func TestNewCommand(t *testing.T) { + // Arrange + ctx := context.Background() + wg := &sync.WaitGroup{} + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + + tests := []struct { + name string + cmdArgs []string + expectedErr bool + }{ + {"Good: ping cmd empty option", []string{}, false}, + {"Bad: ping invalid option", []string{"--invalid=xxx"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + command, err := NewCommand(ctx, wg, lc, config, tt.cmdArgs) + if tt.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, command) + } + }) + } +} diff --git a/internal/security/bootstrapper/interfaces/command.go b/internal/security/bootstrapper/interfaces/command.go index 7a4fd7fce5..ee81dddf49 100644 --- a/internal/security/bootstrapper/interfaces/command.go +++ b/internal/security/bootstrapper/interfaces/command.go @@ -16,11 +16,11 @@ package interfaces const ( - // StatusCodeExitNormal exit code + // StatusCodeExitNormal exit code for normal case StatusCodeExitNormal = 0 - // StatusCodeNoOptionSelected exit code + // StatusCodeNoOptionSelected exit code for missing options case StatusCodeNoOptionSelected = 1 - // StatusCodeExitWithError is exit code for error + // StatusCodeExitWithError is exit code for error case StatusCodeExitWithError = 2 ) diff --git a/internal/security/bootstrapper/tcp/client.go b/internal/security/bootstrapper/tcp/client.go index 445a60dbbe..140747c83e 100644 --- a/internal/security/bootstrapper/tcp/client.go +++ b/internal/security/bootstrapper/tcp/client.go @@ -16,6 +16,7 @@ package tcp import ( + "errors" "fmt" "net" "strconv" @@ -44,7 +45,8 @@ func DialTcp(host string, port int, lc logger.LoggingClient) error { lc.Debugf("Trying to connecting to TCP server at address: %s", tcpServerAddr) c, err := net.DialTimeout("tcp", tcpServerAddr, dialTimeoutDuration) if err != nil { - if opErr, ok := err.(*net.OpError); ok && opErr.Op == "dial" { + var opErr *net.OpError + if errors.As(err, &opErr) && opErr.Op == "dial" { lc.Infof("TCP server %s may be not ready yet, retry in 1 second", tcpServerAddr) time.Sleep(time.Second) continue diff --git a/internal/security/bootstrapper/tcp/listener.go b/internal/security/bootstrapper/tcp/listener.go index 06c7503625..2c34c3a691 100644 --- a/internal/security/bootstrapper/tcp/listener.go +++ b/internal/security/bootstrapper/tcp/listener.go @@ -51,6 +51,10 @@ func (tcpSrv *TcpServer) StartListener(port int, lc logger.LoggingClient, host s return fmt.Errorf("Failed to create TCP listener: %v", err) } + defer func() { + _ = listener.Close() + }() + lc.Infof("Security bootstrapper starts listening on tcp://%s", doneSrv) for { conn, err := listener.Accept() diff --git a/internal/security/bootstrapper/tcp/listener_test.go b/internal/security/bootstrapper/tcp/listener_test.go index 39a107bdee..997a0a9fca 100644 --- a/internal/security/bootstrapper/tcp/listener_test.go +++ b/internal/security/bootstrapper/tcp/listener_test.go @@ -65,10 +65,6 @@ func TestStartListenerAlreadyInUse(t *testing.T) { errs <- srv2.StartListener(testPort, lc, "") }() - // in this test case we want listener starts so that the second listener srv2 attempting to start - // on the same port fails; hence wait for some time duration for that to be happening - time.Sleep(2 * time.Second) - select { case err := <-errs: require.Error(t, err)