From ad840132c805ac1966248c35c72df42a66821dee Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 3 Feb 2026 17:05:25 +0200 Subject: [PATCH] Adopt env package from toolhive-core Migrate from internal pkg/env to toolhive-core/env as the third adoption from the shared library per THV-0032. The env package provides a testable abstraction for environment variable access via the Reader interface and OSReader implementation, along with generated mocks for testing. - Replace pkg/env imports with toolhive-core/env - Replace pkg/env/mocks imports with toolhive-core/env/mocks - Remove internal pkg/env package Co-Authored-By: Claude Opus 4.5 --- cmd/vmcp/app/commands.go | 2 +- pkg/config/config.go | 2 +- pkg/config/config_test.go | 2 +- pkg/container/runtime/types.go | 2 +- pkg/env/env.go | 23 ------- pkg/env/env_test.go | 67 ------------------- pkg/env/mocks/mock_reader.go | 54 --------------- pkg/logger/logger.go | 2 +- pkg/logger/logger_test.go | 2 +- pkg/usagemetrics/client.go | 2 +- pkg/usagemetrics/client_test.go | 2 +- pkg/vmcp/auth/factory/integration_test.go | 2 +- pkg/vmcp/auth/factory/outgoing.go | 2 +- pkg/vmcp/auth/factory/outgoing_test.go | 2 +- pkg/vmcp/auth/strategies/tokenexchange.go | 2 +- .../auth/strategies/tokenexchange_test.go | 2 +- pkg/vmcp/config/crd_cli_roundtrip_test.go | 2 +- pkg/vmcp/config/yaml_loader.go | 2 +- pkg/vmcp/config/yaml_loader_test.go | 4 +- pkg/vmcp/config/yaml_loader_transform_test.go | 2 +- pkg/workloads/statuses/status.go | 2 +- pkg/workloads/statuses/status_test.go | 2 +- test/integration/vmcp/helpers/vmcp_server.go | 2 +- 23 files changed, 21 insertions(+), 165 deletions(-) delete mode 100644 pkg/env/env.go delete mode 100644 pkg/env/env_test.go delete mode 100644 pkg/env/mocks/mock_reader.go diff --git a/cmd/vmcp/app/commands.go b/cmd/vmcp/app/commands.go index 00077bc02e..e6c30d7132 100644 --- a/cmd/vmcp/app/commands.go +++ b/cmd/vmcp/app/commands.go @@ -15,8 +15,8 @@ import ( "go.opentelemetry.io/otel/trace" "k8s.io/client-go/rest" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive/pkg/audit" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/telemetry" diff --git a/pkg/config/config.go b/pkg/config/config.go index 75f5abcba2..508886009b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -16,7 +16,7 @@ import ( "github.com/adrg/xdg" "gopkg.in/yaml.v3" - "github.com/stacklok/toolhive/pkg/env" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive/pkg/lockfile" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/secrets" diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index cd8d2fc00c..b0873c3f9a 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -13,7 +13,7 @@ import ( "go.uber.org/mock/gomock" "gopkg.in/yaml.v3" - "github.com/stacklok/toolhive/pkg/env/mocks" + "github.com/stacklok/toolhive-core/env/mocks" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/secrets" ) diff --git a/pkg/container/runtime/types.go b/pkg/container/runtime/types.go index 1ee464d8bc..d4550b4ceb 100644 --- a/pkg/container/runtime/types.go +++ b/pkg/container/runtime/types.go @@ -13,8 +13,8 @@ import ( "strings" "time" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive-core/httperr" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/ignore" "github.com/stacklok/toolhive/pkg/permissions" ) diff --git a/pkg/env/env.go b/pkg/env/env.go deleted file mode 100644 index 62640ce221..0000000000 --- a/pkg/env/env.go +++ /dev/null @@ -1,23 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -// Package env provides abstractions for environment variable access -// to enable dependency injection and testing isolation. -package env - -//go:generate mockgen -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader - -import "os" - -// Reader defines an interface for environment variable access -type Reader interface { - Getenv(key string) string -} - -// OSReader implements Reader using the standard os package -type OSReader struct{} - -// Getenv returns the value of the environment variable named by the key -func (*OSReader) Getenv(key string) string { - return os.Getenv(key) -} diff --git a/pkg/env/env_test.go b/pkg/env/env_test.go deleted file mode 100644 index ce219161a7..0000000000 --- a/pkg/env/env_test.go +++ /dev/null @@ -1,67 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package env - -import ( - "os" - "testing" -) - -func TestOSReader_Getenv(t *testing.T) { //nolint:paralleltest // Modifies environment variables - // Cannot run in parallel because it modifies environment variables - testKey := "TEST_ENV_VARIABLE_FOR_TESTING" - testValue := "test_value_123" - - // Set an environment variable for testing - originalValue, wasSet := os.LookupEnv(testKey) - os.Setenv(testKey, testValue) - t.Cleanup(func() { - if wasSet { - os.Setenv(testKey, originalValue) - } else { - os.Unsetenv(testKey) - } - }) - - reader := &OSReader{} - - tests := []struct { - name string - key string - want string - }{ - { - name: "existing environment variable", - key: testKey, - want: testValue, - }, - { - name: "non-existing environment variable", - key: "NONEXISTENT_ENV_VAR_TESTING_12345", - want: "", - }, - { - name: "empty key", - key: "", - want: "", - }, - } - - for _, tt := range tests { //nolint:paralleltest // Test modifies environment variables - t.Run(tt.name, func(t *testing.T) { - // Cannot run in parallel because parent test modifies environment variables - got := reader.Getenv(tt.key) - if got != tt.want { - t.Errorf("OSReader.Getenv() = %v, want %v", got, tt.want) - } - }) - } -} - -// TestReader_InterfaceCompliance ensures OSReader implements the Reader interface -func TestReader_InterfaceCompliance(t *testing.T) { - t.Parallel() - var _ Reader = &OSReader{} - // If this compiles, the test passes -} diff --git a/pkg/env/mocks/mock_reader.go b/pkg/env/mocks/mock_reader.go deleted file mode 100644 index cd8227e5fb..0000000000 --- a/pkg/env/mocks/mock_reader.go +++ /dev/null @@ -1,54 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: env.go -// -// Generated by this command: -// -// mockgen -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader -// - -// Package mocks is a generated GoMock package. -package mocks - -import ( - reflect "reflect" - - gomock "go.uber.org/mock/gomock" -) - -// MockReader is a mock of Reader interface. -type MockReader struct { - ctrl *gomock.Controller - recorder *MockReaderMockRecorder - isgomock struct{} -} - -// MockReaderMockRecorder is the mock recorder for MockReader. -type MockReaderMockRecorder struct { - mock *MockReader -} - -// NewMockReader creates a new mock instance. -func NewMockReader(ctrl *gomock.Controller) *MockReader { - mock := &MockReader{ctrl: ctrl} - mock.recorder = &MockReaderMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockReader) EXPECT() *MockReaderMockRecorder { - return m.recorder -} - -// Getenv mocks base method. -func (m *MockReader) Getenv(key string) string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Getenv", key) - ret0, _ := ret[0].(string) - return ret0 -} - -// Getenv indicates an expected call of Getenv. -func (mr *MockReaderMockRecorder) Getenv(key any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Getenv", reflect.TypeOf((*MockReader)(nil).Getenv), key) -} diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 29b7807fcd..704399664b 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -14,7 +14,7 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" - "github.com/stacklok/toolhive/pkg/env" + "github.com/stacklok/toolhive-core/env" ) // Debug logs a message at debug level using the singleton logger. diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index ac01b1f8c4..53412ecf94 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -15,7 +15,7 @@ import ( "go.uber.org/zap/zapcore" "go.uber.org/zap/zaptest/observer" - "github.com/stacklok/toolhive/pkg/env/mocks" + "github.com/stacklok/toolhive-core/env/mocks" ) // TestUnstructuredLogsCheck tests the unstructuredLogs function diff --git a/pkg/usagemetrics/client.go b/pkg/usagemetrics/client.go index 70ec103e60..40cde305d6 100644 --- a/pkg/usagemetrics/client.go +++ b/pkg/usagemetrics/client.go @@ -11,8 +11,8 @@ import ( "runtime" "time" + "github.com/stacklok/toolhive-core/env" rt "github.com/stacklok/toolhive/pkg/container/runtime" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/updates" "github.com/stacklok/toolhive/pkg/versions" diff --git a/pkg/usagemetrics/client_test.go b/pkg/usagemetrics/client_test.go index 0ab7b3f51c..4cc9800e3b 100644 --- a/pkg/usagemetrics/client_test.go +++ b/pkg/usagemetrics/client_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - envmocks "github.com/stacklok/toolhive/pkg/env/mocks" + envmocks "github.com/stacklok/toolhive-core/env/mocks" ) // newTestClient creates a client for testing with a pre-set anonymous ID diff --git a/pkg/vmcp/auth/factory/integration_test.go b/pkg/vmcp/auth/factory/integration_test.go index fa0e826dd3..9eef6d272f 100644 --- a/pkg/vmcp/auth/factory/integration_test.go +++ b/pkg/vmcp/auth/factory/integration_test.go @@ -16,8 +16,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/stacklok/toolhive-core/env" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/vmcp/auth/converters" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) diff --git a/pkg/vmcp/auth/factory/outgoing.go b/pkg/vmcp/auth/factory/outgoing.go index 81e45ee718..27f05c89c7 100644 --- a/pkg/vmcp/auth/factory/outgoing.go +++ b/pkg/vmcp/auth/factory/outgoing.go @@ -18,7 +18,7 @@ package factory import ( "context" - "github.com/stacklok/toolhive/pkg/env" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive/pkg/vmcp/auth" "github.com/stacklok/toolhive/pkg/vmcp/auth/strategies" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" diff --git a/pkg/vmcp/auth/factory/outgoing_test.go b/pkg/vmcp/auth/factory/outgoing_test.go index e3b3d3ecfb..9feecefc00 100644 --- a/pkg/vmcp/auth/factory/outgoing_test.go +++ b/pkg/vmcp/auth/factory/outgoing_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/stacklok/toolhive/pkg/env" + "github.com/stacklok/toolhive-core/env" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) diff --git a/pkg/vmcp/auth/strategies/tokenexchange.go b/pkg/vmcp/auth/strategies/tokenexchange.go index 1878becc82..be7c7e94b1 100644 --- a/pkg/vmcp/auth/strategies/tokenexchange.go +++ b/pkg/vmcp/auth/strategies/tokenexchange.go @@ -11,9 +11,9 @@ import ( "strings" "sync" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive/pkg/auth" "github.com/stacklok/toolhive/pkg/auth/tokenexchange" - "github.com/stacklok/toolhive/pkg/env" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" "github.com/stacklok/toolhive/pkg/vmcp/health" ) diff --git a/pkg/vmcp/auth/strategies/tokenexchange_test.go b/pkg/vmcp/auth/strategies/tokenexchange_test.go index 175e1480f5..292000d78b 100644 --- a/pkg/vmcp/auth/strategies/tokenexchange_test.go +++ b/pkg/vmcp/auth/strategies/tokenexchange_test.go @@ -14,8 +14,8 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "github.com/stacklok/toolhive-core/env/mocks" "github.com/stacklok/toolhive/pkg/auth" - "github.com/stacklok/toolhive/pkg/env/mocks" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" "github.com/stacklok/toolhive/pkg/vmcp/health" ) diff --git a/pkg/vmcp/config/crd_cli_roundtrip_test.go b/pkg/vmcp/config/crd_cli_roundtrip_test.go index 7409a63fc7..515b63aa6e 100644 --- a/pkg/vmcp/config/crd_cli_roundtrip_test.go +++ b/pkg/vmcp/config/crd_cli_roundtrip_test.go @@ -11,7 +11,7 @@ import ( "go.uber.org/mock/gomock" "gopkg.in/yaml.v3" - "github.com/stacklok/toolhive/pkg/env/mocks" + "github.com/stacklok/toolhive-core/env/mocks" thvjson "github.com/stacklok/toolhive/pkg/json" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) diff --git a/pkg/vmcp/config/yaml_loader.go b/pkg/vmcp/config/yaml_loader.go index a6880520b7..5fb9a724bc 100644 --- a/pkg/vmcp/config/yaml_loader.go +++ b/pkg/vmcp/config/yaml_loader.go @@ -11,7 +11,7 @@ import ( "gopkg.in/yaml.v3" - "github.com/stacklok/toolhive/pkg/env" + "github.com/stacklok/toolhive-core/env" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) diff --git a/pkg/vmcp/config/yaml_loader_test.go b/pkg/vmcp/config/yaml_loader_test.go index 2fbe8bf7f6..4fe04ed3b0 100644 --- a/pkg/vmcp/config/yaml_loader_test.go +++ b/pkg/vmcp/config/yaml_loader_test.go @@ -12,8 +12,8 @@ import ( "go.uber.org/mock/gomock" - "github.com/stacklok/toolhive/pkg/env" - "github.com/stacklok/toolhive/pkg/env/mocks" + "github.com/stacklok/toolhive-core/env" + "github.com/stacklok/toolhive-core/env/mocks" "github.com/stacklok/toolhive/pkg/vmcp" ) diff --git a/pkg/vmcp/config/yaml_loader_transform_test.go b/pkg/vmcp/config/yaml_loader_transform_test.go index 051113572c..1f1373a14f 100644 --- a/pkg/vmcp/config/yaml_loader_transform_test.go +++ b/pkg/vmcp/config/yaml_loader_transform_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - "github.com/stacklok/toolhive/pkg/env/mocks" + "github.com/stacklok/toolhive-core/env/mocks" thvjson "github.com/stacklok/toolhive/pkg/json" "github.com/stacklok/toolhive/pkg/telemetry" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" diff --git a/pkg/workloads/statuses/status.go b/pkg/workloads/statuses/status.go index f8f13ad2a0..38f31e89ed 100644 --- a/pkg/workloads/statuses/status.go +++ b/pkg/workloads/statuses/status.go @@ -8,9 +8,9 @@ import ( "context" "fmt" + "github.com/stacklok/toolhive-core/env" rt "github.com/stacklok/toolhive/pkg/container/runtime" "github.com/stacklok/toolhive/pkg/core" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/workloads/types" ) diff --git a/pkg/workloads/statuses/status_test.go b/pkg/workloads/statuses/status_test.go index 4373d6cafd..065c3567b3 100644 --- a/pkg/workloads/statuses/status_test.go +++ b/pkg/workloads/statuses/status_test.go @@ -12,10 +12,10 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" + envmocks "github.com/stacklok/toolhive-core/env/mocks" rt "github.com/stacklok/toolhive/pkg/container/runtime" rtmocks "github.com/stacklok/toolhive/pkg/container/runtime/mocks" "github.com/stacklok/toolhive/pkg/core" - envmocks "github.com/stacklok/toolhive/pkg/env/mocks" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/workloads/types" ) diff --git a/test/integration/vmcp/helpers/vmcp_server.go b/test/integration/vmcp/helpers/vmcp_server.go index 99caa6c086..9b3240bff2 100644 --- a/test/integration/vmcp/helpers/vmcp_server.go +++ b/test/integration/vmcp/helpers/vmcp_server.go @@ -11,8 +11,8 @@ import ( "github.com/stretchr/testify/require" + "github.com/stacklok/toolhive-core/env" "github.com/stacklok/toolhive/pkg/auth" - "github.com/stacklok/toolhive/pkg/env" "github.com/stacklok/toolhive/pkg/telemetry" vmcptypes "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/aggregator"