From da11a4ae910b955f1d7a82d225338830d9c0133e Mon Sep 17 00:00:00 2001 From: Joshua Duffney Date: Fri, 6 Sep 2024 10:01:41 -0500 Subject: [PATCH] refactor: switch refresherConfig from map to struct --- .../keymanagementprovider_controller.go | 14 +- .../keymanagementprovider_controller_test.go | 8 +- .../keymanagementprovider_controller.go | 14 +- .../keymanagementprovider_controller_test.go | 9 +- pkg/keymanagementprovider/refresh/factory.go | 20 +- .../refresh/factory_test.go | 10 +- .../refresh/kubeRefresh.go | 49 +---- .../refresh/kubeRefresh_test.go | 180 ++++++------------ pkg/keymanagementprovider/refresh/refresh.go | 10 + 9 files changed, 108 insertions(+), 206 deletions(-) diff --git a/pkg/controllers/clusterresource/keymanagementprovider_controller.go b/pkg/controllers/clusterresource/keymanagementprovider_controller.go index efb4606203..0e2dd428ec 100644 --- a/pkg/controllers/clusterresource/keymanagementprovider_controller.go +++ b/pkg/controllers/clusterresource/keymanagementprovider_controller.go @@ -87,15 +87,15 @@ func (r *KeyManagementProviderReconciler) ReconcileWithType(ctx context.Context, return ctrl.Result{}, err } - config := map[string]interface{}{ - "refresherType": refresherType, - "provider": provider, - "providerType": keyManagementProvider.Spec.Type, - "providerRefreshInterval": keyManagementProvider.Spec.RefreshInterval, - "resource": resource, + refresherConfig := refresh.RefresherConfig{ + RefresherType: refresherType, + Provider: provider, + ProviderType: keyManagementProvider.Spec.Type, + ProviderRefreshInterval: keyManagementProvider.Spec.RefreshInterval, + Resource: resource, } - refresher, err := refresh.CreateRefresherFromConfig(config) + refresher, err := refresh.CreateRefresherFromConfig(refresherConfig) if err != nil { writeKMProviderStatus(ctx, r, &keyManagementProvider, logger, false, err.Error(), lastFetchedTime, nil) return ctrl.Result{}, err diff --git a/pkg/controllers/clusterresource/keymanagementprovider_controller_test.go b/pkg/controllers/clusterresource/keymanagementprovider_controller_test.go index b83d71e064..9beb497034 100644 --- a/pkg/controllers/clusterresource/keymanagementprovider_controller_test.go +++ b/pkg/controllers/clusterresource/keymanagementprovider_controller_test.go @@ -74,18 +74,18 @@ func (mr *MockRefresher) GetStatus() interface{} { return mr.Status } -func (mr *MockRefresher) Create(config map[string]interface{}) (refresh.Refresher, error) { - if resource, ok := config["resource"].(string); ok && resource == "refreshError" { +func (mr *MockRefresher) Create(config refresh.RefresherConfig) (refresh.Refresher, error) { + if config.Resource == "refreshError" { return &MockRefresher{ RefreshError: true, }, nil } - if resource, ok := config["resource"].(string); ok && resource == "resultError" { + if config.Resource == "resultError" { return &MockRefresher{ ResultError: true, }, nil } - if resource, ok := config["resource"].(string); ok && resource == "statusError" { + if config.Resource == "statusError" { return &MockRefresher{ StatusError: true, }, nil diff --git a/pkg/controllers/namespaceresource/keymanagementprovider_controller.go b/pkg/controllers/namespaceresource/keymanagementprovider_controller.go index 6d746fd1c8..bc75032e39 100644 --- a/pkg/controllers/namespaceresource/keymanagementprovider_controller.go +++ b/pkg/controllers/namespaceresource/keymanagementprovider_controller.go @@ -87,15 +87,15 @@ func (r *KeyManagementProviderReconciler) ReconcileWithType(ctx context.Context, return ctrl.Result{}, err } - config := map[string]interface{}{ - "refresherType": refresherType, - "provider": provider, - "providerType": keyManagementProvider.Spec.Type, - "providerRefreshInterval": keyManagementProvider.Spec.RefreshInterval, - "resource": resource, + refresherConfig := refresh.RefresherConfig{ + RefresherType: refresherType, + Provider: provider, + ProviderType: keyManagementProvider.Spec.Type, + ProviderRefreshInterval: keyManagementProvider.Spec.RefreshInterval, + Resource: resource, } - refresher, err := refresh.CreateRefresherFromConfig(config) + refresher, err := refresh.CreateRefresherFromConfig(refresherConfig) if err != nil { writeKMProviderStatus(ctx, r, &keyManagementProvider, logger, isFetchSuccessful, err.Error(), lastFetchedTime, nil) return ctrl.Result{}, err diff --git a/pkg/controllers/namespaceresource/keymanagementprovider_controller_test.go b/pkg/controllers/namespaceresource/keymanagementprovider_controller_test.go index 21a2907537..f80b4a6b42 100644 --- a/pkg/controllers/namespaceresource/keymanagementprovider_controller_test.go +++ b/pkg/controllers/namespaceresource/keymanagementprovider_controller_test.go @@ -71,25 +71,24 @@ func (mr *MockRefresher) GetStatus() interface{} { return mr.Status } -func (mr *MockRefresher) Create(config map[string]interface{}) (refresh.Refresher, error) { - if resource, ok := config["resource"].(string); ok && resource == "refreshError/test" { +func (mr *MockRefresher) Create(config refresh.RefresherConfig) (refresh.Refresher, error) { + if config.Resource == "refreshError/test" { return &MockRefresher{ RefreshError: true, }, nil } - if resource, ok := config["resource"].(string); ok && resource == "resultError/test" { + if config.Resource == "resultError/test" { return &MockRefresher{ ResultError: true, }, nil } - if resource, ok := config["resource"].(string); ok && resource == "statusError/test" { + if config.Resource == "statusError/test" { return &MockRefresher{ StatusError: true, }, nil } return &MockRefresher{}, nil } - func TestKeyManagementProviderReconciler_ReconcileWithType(t *testing.T) { tests := []struct { name string diff --git a/pkg/keymanagementprovider/refresh/factory.go b/pkg/keymanagementprovider/refresh/factory.go index 30c8e833fb..eed41e119e 100644 --- a/pkg/keymanagementprovider/refresh/factory.go +++ b/pkg/keymanagementprovider/refresh/factory.go @@ -21,7 +21,8 @@ var refresherFactories = make(map[string]RefresherFactory) type RefresherFactory interface { // Create creates a new instance of the refresher using the provided configuration - Create(config map[string]interface{}) (Refresher, error) + // Create(config map[string]interface{}) (Refresher, error) + Create(config RefresherConfig) (Refresher, error) } // Refresher is an interface that defines methods to be implemented by a each refresher @@ -37,21 +38,10 @@ func Register(name string, factory RefresherFactory) { } // CreateRefresherFromConfig creates a new instance of the refresher using the provided configuration -func CreateRefresherFromConfig(refresherConfig map[string]interface{}) (Refresher, error) { - refresherTypeValue, exists := refresherConfig["refresherType"] - if !exists { - return nil, fmt.Errorf("refresherType not found in config") - } - refresherType, ok := refresherTypeValue.(string) - if !ok { - return nil, fmt.Errorf("refresherType is not a string") - } - if !ok || refresherType == "" { - return nil, fmt.Errorf("refresherType cannot be empty") - } - factory, ok := refresherFactories[refresherType] +func CreateRefresherFromConfig(refresherConfig RefresherConfig) (Refresher, error) { + factory, ok := refresherFactories[refresherConfig.RefresherType] if !ok { - return nil, fmt.Errorf("refresher factory with name %s not found", refresherType) + return nil, fmt.Errorf("refresher factory with name %s not found", refresherConfig.RefresherType) } return factory.Create(refresherConfig) } diff --git a/pkg/keymanagementprovider/refresh/factory_test.go b/pkg/keymanagementprovider/refresh/factory_test.go index 76d1f92761..e8762a4440 100644 --- a/pkg/keymanagementprovider/refresh/factory_test.go +++ b/pkg/keymanagementprovider/refresh/factory_test.go @@ -22,7 +22,7 @@ import ( type MockRefresher struct{} -func (f *MockRefresher) Create(_ map[string]interface{}) (Refresher, error) { +func (f *MockRefresher) Create(_ RefresherConfig) (Refresher, error) { return &MockRefresher{}, nil } @@ -40,8 +40,8 @@ func (f *MockRefresher) GetStatus() interface{} { func TestRefreshFactory_Create(t *testing.T) { Register("mockRefresher", &MockRefresher{}) - refresherConfig := map[string]interface{}{ - "refresherType": "mockRefresher", + refresherConfig := RefresherConfig{ + RefresherType: "mockRefresher", } factory := refresherFactories["mockRefresher"] refresher, err := factory.Create(refresherConfig) @@ -108,8 +108,8 @@ func TestCreateRefresherFromConfig(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - refresherConfig := map[string]interface{}{ - "refresherType": tt.refresherType, + refresherConfig := RefresherConfig{ + RefresherType: tt.refresherType, } _, err := CreateRefresherFromConfig(refresherConfig) if tt.expectedError && err == nil { diff --git a/pkg/keymanagementprovider/refresh/kubeRefresh.go b/pkg/keymanagementprovider/refresh/kubeRefresh.go index 8168e9137d..b1dafe7413 100644 --- a/pkg/keymanagementprovider/refresh/kubeRefresh.go +++ b/pkg/keymanagementprovider/refresh/kubeRefresh.go @@ -99,50 +99,11 @@ func (kr *KubeRefresher) GetStatus() interface{} { } // Create creates a new KubeRefresher instance -func (kr *KubeRefresher) Create(config map[string]interface{}) (Refresher, error) { - providerValue, exists := config["provider"] - if !exists { - return nil, fmt.Errorf("provider is required in config") - } - provider, ok := providerValue.(kmp.KeyManagementProvider) - if !ok { - return nil, fmt.Errorf("provider is not of type KeyManagementProvider") - } - - provierTypeValue, exists := config["providerType"] - if !exists { - return nil, fmt.Errorf("providerType is required in config") - } - - providerType, ok := provierTypeValue.(string) - if !ok { - return nil, fmt.Errorf("providerType is not of type string") - } - - providerRefreshIntervaleValue, exists := config["providerRefreshInterval"] - if !exists { - return nil, fmt.Errorf("providerRefreshInterval is required in config") - } - - providerRefreshInterval, ok := providerRefreshIntervaleValue.(string) - if !ok { - return nil, fmt.Errorf("providerRefreshInterval is not of type string") - } - - resourceValue, exists := config["resource"] - if !exists { - return nil, fmt.Errorf("resource is required in config") - } - - resource, ok := resourceValue.(string) - if !ok { - return nil, fmt.Errorf("resource is not of type string") - } - +func (kr *KubeRefresher) Create(config RefresherConfig) (Refresher, error) { return &KubeRefresher{ - Provider: provider, - ProviderType: providerType, - ProviderRefreshInterval: providerRefreshInterval, - Resource: resource, + Provider: config.Provider, + ProviderType: config.ProviderType, + ProviderRefreshInterval: config.ProviderRefreshInterval, + Resource: config.Resource, }, nil } diff --git a/pkg/keymanagementprovider/refresh/kubeRefresh_test.go b/pkg/keymanagementprovider/refresh/kubeRefresh_test.go index ccf6317252..9875098b82 100644 --- a/pkg/keymanagementprovider/refresh/kubeRefresh_test.go +++ b/pkg/keymanagementprovider/refresh/kubeRefresh_test.go @@ -144,125 +144,6 @@ func TestKubeRefresher_Refresh(t *testing.T) { } } -func TestKubeRefresher_Create(t *testing.T) { - factory := mock.TestKeyManagementProviderFactory{} - provider, _ := factory.Create("", config.KeyManagementProviderConfig{}, "") - - tests := []struct { - name string - config map[string]interface{} - expectedError bool - expectedErrMsg string - }{ - { - name: "Success", - config: map[string]interface{}{ - "provider": provider, - "providerType": "inline", - "providerRefreshInterval": "", - "resource": "kmpname", - }, - expectedError: false, - }, - { - name: "ProviderMissing", - config: map[string]interface{}{ - "providerRefreshInterval": "", - "providerType": "inline", - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "provider is required in config", - }, - { - name: "ProviderTypeMissing", - config: map[string]interface{}{ - "provider": provider, - "providerRefreshInterval": "", - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "providerType is required in config", - }, - { - name: "ProviderRefreshIntervalMissing", - config: map[string]interface{}{ - "provider": provider, - "providerType": "inline", - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "providerRefreshInterval is required in config", - }, - { - name: "ResourceMissing", - config: map[string]interface{}{ - "provider": provider, - "providerType": "inline", - "providerRefreshInterval": "", - }, - expectedError: true, - expectedErrMsg: "resource is required in config", - }, - { - name: "ProviderInvalid", - config: map[string]interface{}{ - "provider": 123, - "providerType": "inline", - "providerRefreshInterval": "", - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "provider is not of type KeyManagementProvider", - }, - { - name: "ProviderTypeInvalid", - config: map[string]interface{}{ - "provider": provider, - "providerType": 123, - "providerRefreshInterval": "", - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "providerType is not of type string", - }, - { - name: "ProviderRefreshIntervalInvalid", - config: map[string]interface{}{ - "provider": provider, - "providerType": "inline", - "providerRefreshInterval": 123, - "resource": "kmpname", - }, - expectedError: true, - expectedErrMsg: "providerRefreshInterval is not of type string", - }, - { - name: "ResourceInvalid", - config: map[string]interface{}{ - "provider": provider, - "providerType": "inline", - "providerRefreshInterval": "", - "resource": 123, - }, - expectedError: true, - expectedErrMsg: "resource is not of type string", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - kr := &KubeRefresher{} - _, err := kr.Create(tt.config) - if err != nil && err.Error() != tt.expectedErrMsg { - t.Errorf("expected error message '%s', got '%s'", tt.expectedErrMsg, err.Error()) - } - if err == nil && tt.expectedErrMsg != "" { - t.Errorf("expected error message '%s', got no error", tt.expectedErrMsg) - } - }) - } -} func TestKubeRefresher_GetResult(t *testing.T) { kr := &KubeRefresher{ Result: ctrl.Result{RequeueAfter: time.Minute}, @@ -293,3 +174,64 @@ func TestKubeRefresher_GetStatus(t *testing.T) { t.Fatalf("Expected status %v, but got %v", expectedStatus, status) } } +func TestKubeRefresher_Create(t *testing.T) { + tests := []struct { + name string + config RefresherConfig + expectedProviderType string + expectedRefreshInterval string + expectedResource string + }{ + { + name: "Valid Config", + config: RefresherConfig{ + Provider: &mock.TestKeyManagementProvider{}, + ProviderType: "test-kmp", + ProviderRefreshInterval: "1m", + Resource: "test-resource", + }, + expectedProviderType: "test-kmp", + expectedRefreshInterval: "1m", + expectedResource: "test-resource", + }, + { + name: "Empty Config", + config: RefresherConfig{ + Provider: nil, + ProviderType: "", + ProviderRefreshInterval: "", + Resource: "", + }, + expectedProviderType: "", + expectedRefreshInterval: "", + expectedResource: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kr := &KubeRefresher{} + refresher, err := kr.Create(tt.config) + if err != nil { + t.Fatalf("Expected no error, but got %v", err) + } + + kubeRefresher, ok := refresher.(*KubeRefresher) + if !ok { + t.Fatalf("Expected KubeRefresher type, but got %T", refresher) + } + + if kubeRefresher.ProviderType != tt.expectedProviderType { + t.Fatalf("Expected ProviderType %v, but got %v", tt.expectedProviderType, kubeRefresher.ProviderType) + } + + if kubeRefresher.ProviderRefreshInterval != tt.expectedRefreshInterval { + t.Fatalf("Expected ProviderRefreshInterval %v, but got %v", tt.expectedRefreshInterval, kubeRefresher.ProviderRefreshInterval) + } + + if kubeRefresher.Resource != tt.expectedResource { + t.Fatalf("Expected Resource %v, but got %v", tt.expectedResource, kubeRefresher.Resource) + } + }) + } +} diff --git a/pkg/keymanagementprovider/refresh/refresh.go b/pkg/keymanagementprovider/refresh/refresh.go index 462d38691c..e9f11f314a 100644 --- a/pkg/keymanagementprovider/refresh/refresh.go +++ b/pkg/keymanagementprovider/refresh/refresh.go @@ -18,6 +18,8 @@ package refresh import ( "context" + + "github.com/ratify-project/ratify/pkg/keymanagementprovider" ) const ( @@ -33,3 +35,11 @@ type Refresher interface { // GetStatus is a method that returns the status of the refresh GetStatus() interface{} } + +type RefresherConfig struct { + RefresherType string + Provider keymanagementprovider.KeyManagementProvider + ProviderType string + ProviderRefreshInterval string + Resource string +}