diff --git a/cmd/cleanup.go b/cmd/cleanup.go index acab0c2..aa6a265 100644 --- a/cmd/cleanup.go +++ b/cmd/cleanup.go @@ -20,7 +20,7 @@ import ( "log/slog" "os" - "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" "github.com/ncode/tagit/pkg/tagit" "github.com/spf13/cobra" ) @@ -34,21 +34,20 @@ var cleanupCmd = &cobra.Command{ Level: slog.LevelInfo, })) - config := api.DefaultConfig() - config.Address = cmd.InheritedFlags().Lookup("consul-addr").Value.String() - config.Token = cmd.InheritedFlags().Lookup("token").Value.String() + consulAddr := cmd.InheritedFlags().Lookup("consul-addr").Value.String() + token := cmd.InheritedFlags().Lookup("token").Value.String() - consulClient, err := api.NewClient(config) + consulClient, err := consul.CreateClient(consulAddr, token) if err != nil { logger.Error("Failed to create Consul client", "error", err) - return fmt.Errorf("failed to create Consul client: %w", err) + return err } serviceID := cmd.InheritedFlags().Lookup("service-id").Value.String() tagPrefix := cmd.InheritedFlags().Lookup("tag-prefix").Value.String() t := tagit.New( - tagit.NewConsulAPIWrapper(consulClient), + consulClient, &tagit.CmdExecutor{}, serviceID, "", // script is not needed for cleanup diff --git a/cmd/cleanup_test.go b/cmd/cleanup_test.go index d4cddeb..4a5e0d1 100644 --- a/cmd/cleanup_test.go +++ b/cmd/cleanup_test.go @@ -2,8 +2,11 @@ package cmd import ( "bytes" + "fmt" "testing" + "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" ) @@ -244,6 +247,8 @@ func TestCleanupCmdFlagRetrieval(t *testing.T) { func TestCleanupCmdSuccessFlow(t *testing.T) { // Test the successful flow of cleanup command + // Since the actual cleanupCmd creates a real Consul client internally, + // we test with a mock command that simulates the successful path cmd := &cobra.Command{Use: "tagit"} cmd.PersistentFlags().StringP("consul-addr", "c", "127.0.0.1:8500", "consul address") cmd.PersistentFlags().StringP("service-id", "s", "", "consul service id") @@ -251,12 +256,27 @@ func TestCleanupCmdSuccessFlow(t *testing.T) { cmd.PersistentFlags().StringP("tag-prefix", "p", "tagged", "prefix to be added to tags") cmd.PersistentFlags().StringP("token", "t", "", "consul token") + var logOutput bytes.Buffer testCleanupCmd := &cobra.Command{ Use: "cleanup", Short: "cleanup removes all services with the tag prefix from a given consul service", RunE: func(cmd *cobra.Command, args []string) error { - // Simulate successful cleanup without actual consul connection - // This tests the success path that returns nil + // Verify all required flags are accessible + serviceID := cmd.InheritedFlags().Lookup("service-id").Value.String() + tagPrefix := cmd.InheritedFlags().Lookup("tag-prefix").Value.String() + consulAddr := cmd.InheritedFlags().Lookup("consul-addr").Value.String() + token := cmd.InheritedFlags().Lookup("token").Value.String() + + // Simulate the logging that would happen + fmt.Fprintf(&logOutput, "Starting tag cleanup, serviceID=%s, tagPrefix=%s, consulAddr=%s\n", + serviceID, tagPrefix, consulAddr) + + if token != "" { + fmt.Fprintf(&logOutput, "Using token authentication\n") + } + + // Simulate successful cleanup + fmt.Fprintf(&logOutput, "Tag cleanup completed successfully\n") return nil }, } @@ -268,8 +288,215 @@ func TestCleanupCmdSuccessFlow(t *testing.T) { "--script=/tmp/test.sh", "--consul-addr=localhost:8500", "--tag-prefix=test", + "--token=secret-token", }) err := cmd.Execute() assert.NoError(t, err) + + // Verify the command would have executed with the right parameters + output := logOutput.String() + assert.Contains(t, output, "serviceID=test-service") + assert.Contains(t, output, "tagPrefix=test") + assert.Contains(t, output, "consulAddr=localhost:8500") + assert.Contains(t, output, "Using token authentication") + assert.Contains(t, output, "Tag cleanup completed successfully") +} + +// MockConsulClient for testing +type MockConsulClient struct { + MockAgent consul.Agent +} + +func (m *MockConsulClient) Agent() consul.Agent { + return m.MockAgent +} + +// MockAgent implements the Agent interface +type MockAgent struct { + ServiceFunc func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) + ServiceRegisterFunc func(reg *api.AgentServiceRegistration) error +} + +func (m *MockAgent) Service(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + if m.ServiceFunc != nil { + return m.ServiceFunc(serviceID, q) + } + return &api.AgentService{ + ID: "test-service", + Service: "test", + Tags: []string{"tagged:old", "other-tag"}, + }, nil, nil +} + +func (m *MockAgent) ServiceRegister(reg *api.AgentServiceRegistration) error { + if m.ServiceRegisterFunc != nil { + return m.ServiceRegisterFunc(reg) + } + return nil +} + +func TestCleanupCmdWithMockFactory(t *testing.T) { + // Save and restore the original factory + originalFactory := consul.Factory + defer func() { + consul.Factory = originalFactory + }() + + t.Run("Successful cleanup with mock", func(t *testing.T) { + // Create a mock agent that simulates a service with tags + mockAgent := &MockAgent{ + ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + return &api.AgentService{ + ID: serviceID, + Service: "test", + Tags: []string{"tagged-value1", "tagged-value2", "other-tag"}, + }, nil, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + // Verify that the tags were cleaned up + assert.Equal(t, "test-service", reg.ID) + assert.NotContains(t, reg.Tags, "tagged-value1") + assert.NotContains(t, reg.Tags, "tagged-value2") + assert.Contains(t, reg.Tags, "other-tag") + return nil + }, + } + + // Create mock client with the mock agent + mockClient := &MockConsulClient{ + MockAgent: mockAgent, + } + + // Set up the mock factory + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "cleanup", + RunE: cleanupCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "tagged", "") + parent.AddCommand(cmd) + + // Run the actual cleanup command + err := cmd.RunE(cmd, []string{}) + assert.NoError(t, err) + }) + + t.Run("Cleanup with connection error", func(t *testing.T) { + // Set up a factory that returns an error + mockFactory := &consul.MockFactory{ + MockError: fmt.Errorf("connection failed"), + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "cleanup", + RunE: cleanupCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "tagged", "") + parent.AddCommand(cmd) + + // Run the cleanup command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + }) + + t.Run("Cleanup with service not found", func(t *testing.T) { + // Create a mock agent that returns nil service + mockAgent := &MockAgent{ + ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + return nil, nil, nil + }, + } + + // Create mock client with the mock agent + mockClient := &MockConsulClient{ + MockAgent: mockAgent, + } + + // Set up the mock factory + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "cleanup", + RunE: cleanupCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "tagged", "") + parent.AddCommand(cmd) + + // Run the cleanup command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "service test-service not found") + }) + + t.Run("Cleanup with service register error", func(t *testing.T) { + // Create a mock agent that simulates a service with tags but fails on register + mockAgent := &MockAgent{ + ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + return &api.AgentService{ + ID: serviceID, + Service: "test", + Tags: []string{"tagged-value1", "other-tag"}, + }, nil, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + return fmt.Errorf("failed to register service") + }, + } + + // Create mock client with the mock agent + mockClient := &MockConsulClient{ + MockAgent: mockAgent, + } + + // Set up the mock factory + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "cleanup", + RunE: cleanupCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "tagged", "") + parent.AddCommand(cmd) + + // Run the cleanup command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to clean up tags") + }) } diff --git a/cmd/run.go b/cmd/run.go index 224f666..3572873 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -24,7 +24,7 @@ import ( "syscall" "time" - "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" "github.com/ncode/tagit/pkg/tagit" "github.com/spf13/cobra" ) @@ -59,22 +59,21 @@ example: tagit run -s my-super-service -x '/tmp/tag-role.sh' return fmt.Errorf("invalid interval %q: %w", interval, err) } - config := api.DefaultConfig() - config.Address, err = cmd.InheritedFlags().GetString("consul-addr") + consulAddr, err := cmd.InheritedFlags().GetString("consul-addr") if err != nil { logger.Error("Failed to get consul-addr flag", "error", err) return err } - config.Token, err = cmd.InheritedFlags().GetString("token") + token, err := cmd.InheritedFlags().GetString("token") if err != nil { logger.Error("Failed to get token flag", "error", err) return err } - consulClient, err := api.NewClient(config) + consulClient, err := consul.CreateClient(consulAddr, token) if err != nil { logger.Error("Failed to create Consul client", "error", err) - return fmt.Errorf("failed to create Consul client: %w", err) + return err } serviceID, err := cmd.InheritedFlags().GetString("service-id") @@ -94,7 +93,7 @@ example: tagit run -s my-super-service -x '/tmp/tag-role.sh' } t := tagit.New( - tagit.NewConsulAPIWrapper(consulClient), + consulClient, &tagit.CmdExecutor{}, serviceID, script, diff --git a/cmd/run_test.go b/cmd/run_test.go index 9ed0c31..8426c5c 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -5,9 +5,12 @@ import ( "context" "fmt" "os" + "sync/atomic" "testing" "time" + "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" ) @@ -484,3 +487,196 @@ func TestRunCmdCompleteFlow(t *testing.T) { }) } } + +func TestRunCmdWithMockFactory(t *testing.T) { + // Save and restore the original factory + originalFactory := consul.Factory + defer func() { + consul.Factory = originalFactory + }() + + t.Run("Successful run with mock", func(t *testing.T) { + // Track if service was registered at least once + var registered atomic.Bool + + // Create a mock agent that simulates a service + mockAgent := &MockAgent{ + ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + return &api.AgentService{ + ID: serviceID, + Service: "test", + Tags: []string{"existing-tag"}, + }, nil, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + // Verify that new tags were added + registered.Store(true) + assert.Contains(t, reg.Tags, "existing-tag") + assert.Contains(t, reg.Tags, "test-tag1") + assert.Contains(t, reg.Tags, "test-tag2") + return nil + }, + } + + // Create mock client with the mock agent + mockClient := &MockConsulClient{ + MockAgent: mockAgent, + } + + // Set up the mock factory + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "run", + RunE: runCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "test", "") + parent.PersistentFlags().String("script", "echo 'tag1 tag2'", "") + parent.PersistentFlags().String("interval", "100ms", "") // Short interval for testing + parent.AddCommand(cmd) + + // Run the command in a goroutine with timeout + done := make(chan error) + go func() { + done <- cmd.RunE(cmd, []string{}) + }() + + // Let it run for a short time + time.Sleep(250 * time.Millisecond) + + // The command should have registered the service at least once + assert.True(t, registered.Load(), "Service should have been registered at least once") + + // Note: The run command runs forever, so we can't test it finishing cleanly + // This test verifies it starts correctly and processes at least one update + }) + + t.Run("Run with invalid interval", func(t *testing.T) { + // Set up a valid mock factory + mockClient := &MockConsulClient{ + MockAgent: &MockAgent{}, + } + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "run", + RunE: runCmd.RunE, + } + // Set up parent command with invalid interval + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "test", "") + parent.PersistentFlags().String("script", "echo 'tag1'", "") + parent.PersistentFlags().String("interval", "invalid", "") + parent.AddCommand(cmd) + + // Run the command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid interval") + }) + + t.Run("Run with empty interval", func(t *testing.T) { + // Set up a valid mock factory + mockClient := &MockConsulClient{ + MockAgent: &MockAgent{}, + } + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "run", + RunE: runCmd.RunE, + } + // Set up parent command with empty interval + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "test", "") + parent.PersistentFlags().String("script", "echo 'tag1'", "") + parent.PersistentFlags().String("interval", "", "") + parent.AddCommand(cmd) + + // Run the command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "interval is required") + }) + + t.Run("Run with zero interval", func(t *testing.T) { + // Set up a valid mock factory + mockClient := &MockConsulClient{ + MockAgent: &MockAgent{}, + } + mockFactory := &consul.MockFactory{ + MockClient: mockClient, + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "run", + RunE: runCmd.RunE, + } + // Set up parent command with zero interval + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "test", "") + parent.PersistentFlags().String("script", "echo 'tag1'", "") + parent.PersistentFlags().String("interval", "0", "") + parent.AddCommand(cmd) + + // Run the command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "interval is required") + }) + + t.Run("Run with connection error", func(t *testing.T) { + // Set up a factory that returns an error + mockFactory := &consul.MockFactory{ + MockError: fmt.Errorf("connection failed"), + } + consul.SetFactory(mockFactory) + + // Create a new command instance for this test + cmd := &cobra.Command{ + Use: "run", + RunE: runCmd.RunE, + } + // Set up parent command for flags inheritance + parent := &cobra.Command{} + parent.PersistentFlags().String("consul-addr", "127.0.0.1:8500", "") + parent.PersistentFlags().String("token", "", "") + parent.PersistentFlags().String("service-id", "test-service", "") + parent.PersistentFlags().String("tag-prefix", "test", "") + parent.PersistentFlags().String("script", "echo 'tag1'", "") + parent.PersistentFlags().String("interval", "1s", "") + parent.AddCommand(cmd) + + // Run the command - should fail + err := cmd.RunE(cmd, []string{}) + assert.Error(t, err) + }) +} diff --git a/pkg/consul/factory.go b/pkg/consul/factory.go new file mode 100644 index 0000000..6787c10 --- /dev/null +++ b/pkg/consul/factory.go @@ -0,0 +1,102 @@ +/* +Copyright © 2025 Juliano Martinez + +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 consul + +import ( + "fmt" + + "github.com/hashicorp/consul/api" +) + +// Client is an interface for the Consul client. +type Client interface { + Agent() Agent +} + +// Agent is an interface for the Consul agent. +type Agent interface { + Service(string, *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) + ServiceRegister(*api.AgentServiceRegistration) error +} + +// ApiWrapper wraps the Consul API client to conform to the Client interface. +type ApiWrapper struct { + client *api.Client +} + +// NewConsulAPIWrapper creates a new instance of ApiWrapper. +func NewConsulAPIWrapper(client *api.Client) *ApiWrapper { + return &ApiWrapper{client: client} +} + +// Agent returns an object that conforms to the Agent interface. +func (w *ApiWrapper) Agent() Agent { + return w.client.Agent() +} + +// ClientFactory is an interface for creating Consul clients +type ClientFactory interface { + NewClient(address, token string) (Client, error) +} + +// DefaultFactory creates real Consul clients +type DefaultFactory struct{} + +// NewClient creates a new Consul client with the given configuration +func (f *DefaultFactory) NewClient(address, token string) (Client, error) { + config := api.DefaultConfig() + config.Address = address + config.Token = token + + client, err := api.NewClient(config) + if err != nil { + return nil, fmt.Errorf("failed to create Consul client: %w", err) + } + + return NewConsulAPIWrapper(client), nil +} + +// MockFactory creates mock Consul clients for testing +type MockFactory struct { + MockClient Client + MockError error +} + +// NewClient returns the mock client or error +func (f *MockFactory) NewClient(address, token string) (Client, error) { + if f.MockError != nil { + return nil, f.MockError + } + return f.MockClient, nil +} + +// Factory is the global factory instance (can be overridden for testing) +var Factory ClientFactory = &DefaultFactory{} + +// SetFactory allows tests to inject a mock factory +func SetFactory(f ClientFactory) { + Factory = f +} + +// ResetFactory resets to the default factory +func ResetFactory() { + Factory = &DefaultFactory{} +} + +// CreateClient is a convenience function that uses the global factory +func CreateClient(address, token string) (Client, error) { + return Factory.NewClient(address, token) +} diff --git a/pkg/consul/factory_test.go b/pkg/consul/factory_test.go new file mode 100644 index 0000000..9f4e449 --- /dev/null +++ b/pkg/consul/factory_test.go @@ -0,0 +1,184 @@ +package consul + +import ( + "fmt" + "testing" + + "github.com/hashicorp/consul/api" + "github.com/stretchr/testify/assert" +) + +// MockConsulClient for testing +type MockConsulClient struct { + MockAgent *MockAgent +} + +func (m *MockConsulClient) Agent() Agent { + return m.MockAgent +} + +type MockAgent struct { + ServiceFunc func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) + ServiceRegisterFunc func(reg *api.AgentServiceRegistration) error +} + +func (m *MockAgent) Service(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + if m.ServiceFunc != nil { + return m.ServiceFunc(serviceID, q) + } + return nil, nil, nil +} + +func (m *MockAgent) ServiceRegister(reg *api.AgentServiceRegistration) error { + if m.ServiceRegisterFunc != nil { + return m.ServiceRegisterFunc(reg) + } + return nil +} + +func TestDefaultFactory(t *testing.T) { + factory := &DefaultFactory{} + + // Test with valid configuration + // Note: This will actually try to connect to Consul, so it might fail + // in environments without Consul running + t.Run("Create client with valid config", func(t *testing.T) { + client, err := factory.NewClient("127.0.0.1:8500", "test-token") + // We expect this to succeed in creating a client object, + // even if Consul isn't actually running + assert.NoError(t, err) + assert.NotNil(t, client) + }) + + // Test with environment that causes api.NewClient to fail + t.Run("Create client with invalid TLS config", func(t *testing.T) { + // Set environment variables that should cause TLS config to fail + t.Setenv("CONSUL_CACERT", "/nonexistent/ca.pem") + t.Setenv("CONSUL_CLIENT_CERT", "/nonexistent/client.pem") + t.Setenv("CONSUL_CLIENT_KEY", "/nonexistent/client.key") + + // These environment variables should cause api.NewClient to fail + // when it tries to load the TLS certificates + client, err := factory.NewClient("127.0.0.1:8500", "test-token") + + // Check if we got an error (which we expect due to invalid cert paths) + if err != nil { + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create Consul client") + assert.Nil(t, client) + } else { + // If no error, the API might have changed or env vars weren't processed + t.Log("Expected error but got none - Consul API may have changed behavior") + assert.NotNil(t, client) + } + }) + + // Test with invalid HTTP proxy that should cause client creation to fail + t.Run("Create client with invalid HTTP proxy", func(t *testing.T) { + // Set an invalid HTTP proxy that should cause the client to fail + t.Setenv("HTTP_PROXY", "://invalid-proxy-url") + + client, err := factory.NewClient("127.0.0.1:8500", "test-token") + + // The invalid proxy URL should cause an error + if err != nil { + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create Consul client") + assert.Nil(t, client) + } else { + // If no error, log it for debugging + t.Log("Expected error with invalid proxy but got none") + assert.NotNil(t, client) + } + }) + + // Test with malformed TLS configuration + t.Run("Create client with malformed TLS env", func(t *testing.T) { + // Set CONSUL_HTTP_SSL=true to force TLS but without proper certs + t.Setenv("CONSUL_HTTP_SSL", "true") + t.Setenv("CONSUL_CACERT", "not-a-file.pem") + + client, err := factory.NewClient("127.0.0.1:8500", "test-token") + + // This should fail when trying to set up TLS + if err != nil { + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create Consul client") + assert.Nil(t, client) + } else { + t.Log("Expected error with invalid TLS setup but got none") + assert.NotNil(t, client) + } + }) +} + +func TestMockFactory(t *testing.T) { + t.Run("Returns mock client", func(t *testing.T) { + mockClient := &MockConsulClient{ + MockAgent: &MockAgent{}, + } + factory := &MockFactory{ + MockClient: mockClient, + } + + client, err := factory.NewClient("any-address", "any-token") + assert.NoError(t, err) + assert.Equal(t, mockClient, client) + }) + + t.Run("Returns error when configured", func(t *testing.T) { + expectedErr := fmt.Errorf("connection failed") + factory := &MockFactory{ + MockError: expectedErr, + } + + client, err := factory.NewClient("any-address", "any-token") + assert.Error(t, err) + assert.Equal(t, expectedErr, err) + assert.Nil(t, client) + }) +} + +func TestGlobalFactory(t *testing.T) { + // Save original factory + originalFactory := Factory + defer func() { + Factory = originalFactory + }() + + t.Run("Default factory is set", func(t *testing.T) { + ResetFactory() + _, ok := Factory.(*DefaultFactory) + assert.True(t, ok, "Default factory should be DefaultFactory type") + }) + + t.Run("Can set mock factory", func(t *testing.T) { + mockFactory := &MockFactory{ + MockClient: &MockConsulClient{}, + } + SetFactory(mockFactory) + assert.Equal(t, mockFactory, Factory) + }) + + t.Run("CreateClient uses global factory", func(t *testing.T) { + mockClient := &MockConsulClient{ + MockAgent: &MockAgent{}, + } + mockFactory := &MockFactory{ + MockClient: mockClient, + } + SetFactory(mockFactory) + + client, err := CreateClient("test-addr", "test-token") + assert.NoError(t, err) + assert.Equal(t, mockClient, client) + }) + + t.Run("ResetFactory restores default", func(t *testing.T) { + mockFactory := &MockFactory{} + SetFactory(mockFactory) + ResetFactory() + _, ok := Factory.(*DefaultFactory) + assert.True(t, ok, "Factory should be reset to DefaultFactory") + }) +} diff --git a/pkg/tagit/tagit.go b/pkg/tagit/tagit.go index 68069be..0572af4 100644 --- a/pkg/tagit/tagit.go +++ b/pkg/tagit/tagit.go @@ -11,6 +11,7 @@ import ( "github.com/google/shlex" "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" ) // TagIt is the main struct for the tagit flow. @@ -19,37 +20,11 @@ type TagIt struct { Script string Interval time.Duration TagPrefix string - client ConsulClient + client consul.Client commandExecutor CommandExecutor logger *slog.Logger } -// ConsulClient is an interface for the Consul client. -type ConsulClient interface { - Agent() ConsulAgent -} - -// ConsulAgent is an interface for the Consul agent. -type ConsulAgent interface { - Service(string, *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) - ServiceRegister(*api.AgentServiceRegistration) error -} - -// ConsulAPIWrapper wraps the Consul API client to conform to the ConsulClient interface. -type ConsulAPIWrapper struct { - client *api.Client -} - -// NewConsulAPIWrapper creates a new instance of ConsulAPIWrapper. -func NewConsulAPIWrapper(client *api.Client) *ConsulAPIWrapper { - return &ConsulAPIWrapper{client: client} -} - -// Agent returns an object that conforms to the ConsulAgent interface. -func (w *ConsulAPIWrapper) Agent() ConsulAgent { - return w.client.Agent() -} - // CommandExecutor is an interface for running commands. type CommandExecutor interface { Execute(command string) ([]byte, error) @@ -72,7 +47,7 @@ func (e *CmdExecutor) Execute(command string) ([]byte, error) { } // New creates a new TagIt struct. -func New(consulClient ConsulClient, commandExecutor CommandExecutor, serviceID string, script string, interval time.Duration, tagPrefix string, logger *slog.Logger) *TagIt { +func New(consulClient consul.Client, commandExecutor CommandExecutor, serviceID string, script string, interval time.Duration, tagPrefix string, logger *slog.Logger) *TagIt { return &TagIt{ ServiceID: serviceID, Script: script, diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index 680fbf9..148aecb 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -11,15 +11,16 @@ import ( "time" "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" "github.com/stretchr/testify/assert" ) -// MockConsulClient implements the ConsulClient interface for testing. +// MockConsulClient implements the Client interface for testing. type MockConsulClient struct { MockAgent *MockAgent } -func (m *MockConsulClient) Agent() ConsulAgent { +func (m *MockConsulClient) Agent() consul.Agent { return m.MockAgent } @@ -596,19 +597,34 @@ func TestRun(t *testing.T) { assert.LessOrEqual(t, updateServiceTagsCalled.Load(), int32(4), "Expected updateServiceTags to be called at most 4 times") } -func TestNewConsulAPIWrapper(t *testing.T) { - consulClient, err := api.NewClient(api.DefaultConfig()) - assert.NoError(t, err, "Failed to create Consul client") +func TestConsulInterfaceCompatibility(t *testing.T) { + // Test that our mocks implement the consul package interfaces correctly + mockAgent := &MockAgent{ + ServiceFunc: func(serviceID string, q *api.QueryOptions) (*api.AgentService, *api.QueryMeta, error) { + return &api.AgentService{ID: serviceID}, nil, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + return nil + }, + } + + mockClient := &MockConsulClient{ + MockAgent: mockAgent, + } - wrapper := NewConsulAPIWrapper(consulClient) + // Verify that MockConsulClient implements consul.Client + var _ consul.Client = mockClient - assert.NotNil(t, wrapper, "NewConsulAPIWrapper returned nil") + // Verify that MockAgent implements consul.Agent + var _ consul.Agent = mockAgent - _, isConsulClient := interface{}(wrapper).(ConsulClient) - assert.True(t, isConsulClient, "NewConsulAPIWrapper does not implement ConsulClient interface") + // Test that the mock client works correctly + agent := mockClient.Agent() + assert.NotNil(t, agent, "Agent() should return non-nil") - _, isConsulAgent := wrapper.Agent().(ConsulAgent) - assert.True(t, isConsulAgent, "Wrapper's Agent method does not return a ConsulAgent") + service, _, err := agent.Service("test-service", nil) + assert.NoError(t, err) + assert.Equal(t, "test-service", service.ID) } func TestCmdExecutor_Execute(t *testing.T) {