fix: GCP Config to use the config pkg similar to other configs

ravi-shankar-sap · Sep 7, 2024 · be9ffe8 · be9ffe8
1 parent f1ed036
commit be9ffe8
Show file tree

Hide file tree

Showing 58 changed files with 185 additions and 196 deletions.
diff --git a/cmd/main.go b/cmd/main.go
@@ -356,6 +356,7 @@ func loadConfig() config.Config {
 	quota.InitConfig(cfg)
 	skrruntimeconfig.InitConfig(cfg)
 	scope.InitConfig(cfg)
+	gcpclient.InitConfig(cfg)
 
 	cfg.Read()
 

diff --git a/config/config/gcpConfig.yaml b/config/config/gcpConfig.yaml
@@ -0,0 +1,3 @@
+retryWaitTime: 5s
+operationWaitTime: 5s
+apiTimeout: 8s
diff --git a/internal/controller/cloud-control/suite_test.go b/internal/controller/cloud-control/suite_test.go
@@ -68,12 +68,7 @@ var _ = BeforeSuite(func() {
 		NotTo(HaveOccurred(), "failed creating namespace %s in Garden", infra.Garden().Namespace())
 
 	// Setup environment variables
-	env := abstractions.NewMockedEnvironment(map[string]string{
-		"GCP_SA_JSON_KEY_PATH":        "test",
-		"GCP_RETRY_WAIT_DURATION":     "300ms",
-		"GCP_OPERATION_WAIT_DURATION": "300ms",
-		"GCP_API_TIMEOUT_DURATION":    "300ms",
-	})
+	env := abstractions.NewMockedEnvironment(map[string]string{})
 
 	// Setup controllers
 	// Scope

diff --git a/internal/controller/cloud-resources/suite_test.go b/internal/controller/cloud-resources/suite_test.go
@@ -80,12 +80,7 @@ var _ = BeforeSuite(func() {
 	quota.SkrQuota.Override(&cloudresourcesv1beta1.IpRange{}, infra.SKR().Scheme(), "", 1000)
 
 	// Setup environment variables
-	env := abstractions.NewMockedEnvironment(map[string]string{
-		"GCP_SA_JSON_KEY_PATH":        "test",
-		"GCP_RETRY_WAIT_DURATION":     "300ms",
-		"GCP_OPERATION_WAIT_DURATION": "300ms",
-		"GCP_API_TIMEOUT_DURATION":    "300ms",
-	})
+	env := abstractions.NewMockedEnvironment(map[string]string{})
 
 	testSetupLog := ctrl.Log.WithName("testSetup")
 

diff --git a/pkg/kcp/provider/gcp/client/gcpConstants.go b/pkg/kcp/provider/gcp/client/gcpConstants.go
@@ -2,10 +2,9 @@ package client
 
 import (
 	"fmt"
+	"github.com/kyma-project/cloud-manager/pkg/config"
 	"time"
 
-	"github.com/kyma-project/cloud-manager/pkg/common/abstractions"
-
 	"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 )
 
@@ -22,22 +21,50 @@ const GcpRetryWaitTime = time.Second * 3
 const GcpOperationWaitTime = time.Second * 5
 const GcpApiTimeout = time.Second * 3
 
-type GcpConfig struct {
+type GcpConfigStruct struct {
 	GcpRetryWaitTime     time.Duration
 	GcpOperationWaitTime time.Duration
 	GcpApiTimeout        time.Duration
+
+	//Config from files...
+	RetryWaitTime     string `yaml:"retryWaitTime,omitempty" json:"retryWaitTime,omitempty"`
+	OperationWaitTime string `yaml:"operationWaitTime,omitempty" json:"operationWaitTime,omitempty"`
+	ApiTimeout        string `yaml:"apiTimeout,omitempty" json:"apiTimeout,omitempty"`
 }
 
-func GetGcpConfig(env abstractions.Environment) *GcpConfig {
-	return &GcpConfig{
-		GcpRetryWaitTime:     GetConfigDuration(env, "GCP_RETRY_WAIT_DURATION", GcpRetryWaitTime),
-		GcpOperationWaitTime: GetConfigDuration(env, "GCP_OPERATION_WAIT_DURATION", GcpOperationWaitTime),
-		GcpApiTimeout:        GetConfigDuration(env, "GCP_API_TIMEOUT_DURATION", GcpApiTimeout),
-	}
+func (c *GcpConfigStruct) AfterConfigLoaded() {
+	c.GcpRetryWaitTime = GetDuration(c.RetryWaitTime, GcpRetryWaitTime)
+	c.GcpOperationWaitTime = GetDuration(c.OperationWaitTime, GcpOperationWaitTime)
+	c.GcpApiTimeout = GetDuration(c.ApiTimeout, GcpApiTimeout)
 }
 
-func GetConfigDuration(env abstractions.Environment, key string, defaultValue time.Duration) time.Duration {
-	duration, err := time.ParseDuration(env.Get(key))
+func InitConfig(cfg config.Config) {
+	cfg.Path(
+		"gcpConfig",
+		config.Path(
+			"retryWaitTime",
+			config.DefaultScalar("5s"),
+			config.SourceEnv("GCP_RETRY_WAIT_DURATION"),
+		),
+		config.Path(
+			"operationWaitTime",
+			config.DefaultScalar("5s"),
+			config.SourceEnv("GCP_OPERATION_WAIT_DURATION"),
+		),
+		config.Path(
+			"apiTimeout",
+			config.DefaultScalar("8s"),
+			config.SourceEnv("GCP_API_TIMEOUT_DURATION"),
+		),
+		config.SourceFile("gcpclient.GcpConfig.yaml"),
+		config.Bind(GcpConfig),
+	)
+}
+
+var GcpConfig = &GcpConfigStruct{}
+
+func GetDuration(value string, defaultValue time.Duration) time.Duration {
+	duration, err := time.ParseDuration(value)
 	if err != nil {
 		return defaultValue
 	}

diff --git a/pkg/kcp/provider/gcp/client/provider.go b/pkg/kcp/provider/gcp/client/provider.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/kyma-project/cloud-manager/pkg/common/abstractions"
 	"github.com/kyma-project/cloud-manager/pkg/composed"
 	"github.com/kyma-project/cloud-manager/pkg/metrics"
 	"google.golang.org/api/cloudresourcemanager/v1"
@@ -122,7 +121,7 @@ func newHttpClient(ctx context.Context, saJsonKeyPath string) (*http.Client, err
 		return nil, fmt.Errorf("error obtaining GCP HTTP client: [%w]", err)
 	}
 	CheckGcpAuthentication(ctx, saJsonKeyPath)
-	client.Timeout = GetGcpConfig(abstractions.NewOSEnvironment()).GcpApiTimeout
+	client.Timeout = GcpConfig.GcpApiTimeout
 	return client, nil
 }
 

diff --git a/pkg/kcp/provider/gcp/cloudclient/provider.go b/pkg/kcp/provider/gcp/cloudclient/provider.go
@@ -2,34 +2,6 @@ package cloudclient
 
 import (
 	"context"
-	"github.com/kyma-project/cloud-manager/pkg/common/abstractions"
-	"time"
 )
 
 type ClientProvider[T any] func(ctx context.Context, saJsonKeyPath string) (T, error)
-
-const GcpRetryWaitTime = time.Second * 3
-const GcpOperationWaitTime = time.Second * 5
-const GcpApiTimeout = time.Second * 3
-
-type GcpConfig struct {
-	GcpRetryWaitTime     time.Duration
-	GcpOperationWaitTime time.Duration
-	GcpApiTimeout        time.Duration
-}
-
-func GetGcpConfig(env abstractions.Environment) *GcpConfig {
-	return &GcpConfig{
-		GcpRetryWaitTime:     GetConfigDuration(env, "GCP_RETRY_WAIT_DURATION", GcpRetryWaitTime),
-		GcpOperationWaitTime: GetConfigDuration(env, "GCP_OPERATION_WAIT_DURATION", GcpOperationWaitTime),
-		GcpApiTimeout:        GetConfigDuration(env, "GCP_API_TIMEOUT_DURATION", GcpApiTimeout),
-	}
-}
-
-func GetConfigDuration(env abstractions.Environment, key string, defaultValue time.Duration) time.Duration {
-	duration, err := time.ParseDuration(env.Get(key))
-	if err != nil {
-		return defaultValue
-	}
-	return duration
-}
diff --git a/pkg/kcp/provider/gcp/iprange/v1/checkGcpOperation.go b/pkg/kcp/provider/gcp/iprange/v1/checkGcpOperation.go
@@ -7,6 +7,7 @@ import (
 	"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 	"github.com/kyma-project/cloud-manager/pkg/composed"
 	"github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
+	gcpclient "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
 	"google.golang.org/api/googleapi"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -52,7 +53,7 @@ func checkGcpOperation(ctx context.Context, st composed.State) (error, context.C
 
 		//Operation not completed yet.. requeue again.
 		if op != nil && !op.Done {
-			return composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), nil
+			return composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), nil
 		}
 
 		//If not able to find the operation or it is completed, reset OpIdentifier.
@@ -92,7 +93,7 @@ func checkGcpOperation(ctx context.Context, st composed.State) (error, context.C
 
 		//Operation not completed yet.. requeue again.
 		if op != nil && op.Status != "DONE" {
-			return composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), nil
+			return composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), nil
 		}
 
 		//If not able to find the operation or it is completed, reset OpIdentifier.

diff --git a/pkg/kcp/provider/gcp/iprange/v1/checkGcpOperation_test.go b/pkg/kcp/provider/gcp/iprange/v1/checkGcpOperation_test.go
@@ -7,6 +7,7 @@ import (
 	cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 	"github.com/kyma-project/cloud-manager/pkg/composed"
 	"github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
+	gcpclient "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 	"google.golang.org/api/compute/v1"
@@ -205,7 +206,7 @@ func (suite *checkGcpOperationSuite) TestWhenSvcNwOperationNotComplete() {
 	//Invoke the function under test
 	err, resCtx := checkGcpOperation(ctx, state)
 	assert.Nil(suite.T(), resCtx)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), err)
 }
 
 func (suite *checkGcpOperationSuite) TestWhenSvcNwOperationSuccessful() {
@@ -414,7 +415,7 @@ func (suite *checkGcpOperationSuite) TestWhenComputeOperationNotComplete() {
 	//Invoke the function under test
 	err, resCtx := checkGcpOperation(ctx, state)
 	assert.Nil(suite.T(), resCtx)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), err)
 }
 
 func (suite *checkGcpOperationSuite) TestWhenComputeOperationSuccessful() {

diff --git a/pkg/kcp/provider/gcp/iprange/v1/state.go b/pkg/kcp/provider/gcp/iprange/v1/state.go
@@ -29,9 +29,6 @@ type State struct {
 
 	serviceNetworkingClient client3.ServiceNetworkingClient
 	computeClient           client3.ComputeClient
-
-	//gcp config
-	gcpConfig *client2.GcpConfig
 }
 
 type StateFactory interface {
@@ -42,15 +39,13 @@ type stateFactory struct {
 	serviceNetworkingClientProvider client2.ClientProvider[client3.ServiceNetworkingClient]
 	computeClientProvider           client2.ClientProvider[client3.ComputeClient]
 	env                             abstractions.Environment
-	gcpConfig                       *client2.GcpConfig
 }
 
 func NewStateFactory(serviceNetworkingClientProvider client2.ClientProvider[client3.ServiceNetworkingClient], computeClientProvider client2.ClientProvider[client3.ComputeClient], env abstractions.Environment) StateFactory {
 	return &stateFactory{
 		serviceNetworkingClientProvider: serviceNetworkingClientProvider,
 		computeClientProvider:           computeClientProvider,
 		env:                             env,
-		gcpConfig:                       client2.GetGcpConfig(env),
 	}
 }
 
@@ -71,15 +66,14 @@ func (f *stateFactory) NewState(ctx context.Context, ipRangeState types.State) (
 		return nil, err
 	}
 
-	return newState(ipRangeState, snc, cc, f.gcpConfig), nil
+	return newState(ipRangeState, snc, cc), nil
 }
 
-func newState(ipRangeState types.State, snc client3.ServiceNetworkingClient, cc client3.ComputeClient, gcpConfig *client2.GcpConfig) *State {
+func newState(ipRangeState types.State, snc client3.ServiceNetworkingClient, cc client3.ComputeClient) *State {
 	return &State{
 		State:                   ipRangeState,
 		serviceNetworkingClient: snc,
 		computeClient:           cc,
-		gcpConfig:               gcpConfig,
 	}
 }
 

diff --git a/pkg/kcp/provider/gcp/iprange/v1/state_test.go b/pkg/kcp/provider/gcp/iprange/v1/state_test.go
@@ -58,7 +58,6 @@ type testStateFactory struct {
 	computeClientProvider           client.ClientProvider[iprangeclient.ComputeClient]
 	serviceNetworkingClientProvider client.ClientProvider[iprangeclient.ServiceNetworkingClient]
 	fakeHttpServer                  *httptest.Server
-	gcpConfig                       *client.GcpConfig
 }
 
 func newTestStateFactory(fakeHttpServer *httptest.Server) (*testStateFactory, error) {
@@ -90,7 +89,6 @@ func newTestStateFactory(fakeHttpServer *httptest.Server) (*testStateFactory, er
 		computeClientProvider:           computeClientProvider,
 		serviceNetworkingClientProvider: svcNwClientProvider,
 		fakeHttpServer:                  fakeHttpServer,
-		gcpConfig:                       client.GetGcpConfig(env),
 	}, nil
 
 }
@@ -127,7 +125,7 @@ func (f *testStateFactory) newStateWith(ctx context.Context, ipRange *cloudcontr
 		},
 	})
 
-	return newState(newTypesState(focalState), snc, cc, f.gcpConfig), nil
+	return newState(newTypesState(focalState), snc, cc), nil
 
 }
 

diff --git a/pkg/kcp/provider/gcp/iprange/v1/syncAddress.go b/pkg/kcp/provider/gcp/iprange/v1/syncAddress.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
+	gcpclient "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
@@ -45,15 +46,15 @@ func syncAddress(ctx context.Context, st composed.State) (error, context.Context
 				Reason:  v1beta1.ReasonGcpError,
 				Message: err.Error(),
 			}).
-			SuccessError(composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime)).
+			SuccessError(composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime)).
 			SuccessLogMsg(fmt.Sprintf("Error creating/deleting Address object in GCP :%s", err)).
 			Run(ctx, state)
 	}
 	if operation != nil {
 		ipRange.Status.OpIdentifier = operation.Name
 		return composed.UpdateStatus(ipRange).
-			SuccessError(composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime)).
+			SuccessError(composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime)).
 			Run(ctx, state)
 	}
-	return composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime), nil
+	return composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime), nil
 }
diff --git a/pkg/kcp/provider/gcp/iprange/v1/syncAddress_test.go b/pkg/kcp/provider/gcp/iprange/v1/syncAddress_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 	"github.com/kyma-project/cloud-manager/pkg/composed"
 	"github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
+	gcpclient "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 	"google.golang.org/api/compute/v1"
@@ -68,7 +69,7 @@ func (suite *syncAddressSuite) TestCreateSuccess() {
 
 	//Invoke the function under test
 	err, _ = syncAddress(ctx, state)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime), err)
 
 	//Load updated object
 	err = state.LoadObj(ctx)
@@ -109,7 +110,7 @@ func (suite *syncAddressSuite) TestCreateFailure() {
 
 	//Invoke the function under test
 	err, _ = syncAddress(ctx, state)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), err)
 
 	//Load updated object
 	err = state.LoadObj(ctx)
@@ -188,7 +189,7 @@ func (suite *syncAddressSuite) TestDeleteSuccess() {
 
 	//Invoke the function under test
 	err, _ = syncAddress(ctx, state)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime), err)
 
 	//Load updated object
 	err = state.LoadObj(ctx)
@@ -238,7 +239,7 @@ func (suite *syncAddressSuite) TestDeleteFailure() {
 
 	//Invoke the function under test
 	err, _ = syncAddress(ctx, state)
-	assert.Equal(suite.T(), composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime), err)
+	assert.Equal(suite.T(), composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime), err)
 
 	//Load updated object
 	err = state.LoadObj(ctx)

diff --git a/pkg/kcp/provider/gcp/iprange/v1/syncPsaConnection.go b/pkg/kcp/provider/gcp/iprange/v1/syncPsaConnection.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 	"github.com/kyma-project/cloud-manager/pkg/composed"
+	gcpclient "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/client"
 	"google.golang.org/api/servicenetworking/v1"
 )
 
@@ -42,15 +43,15 @@ func syncPsaConnection(ctx context.Context, st composed.State) (error, context.C
 				Reason:  v1beta1.ReasonGcpError,
 				Message: err.Error(),
 			}).
-			SuccessError(composed.StopWithRequeueDelay(state.gcpConfig.GcpRetryWaitTime)).
+			SuccessError(composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpRetryWaitTime)).
 			SuccessLogMsg(fmt.Sprintf("Error creating/deleting Service Connection object in GCP :%s", err)).
 			Run(ctx, state)
 	}
 	if operation != nil {
 		ipRange.Status.OpIdentifier = operation.Name
 		return composed.UpdateStatus(ipRange).
-			SuccessError(composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime)).
+			SuccessError(composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime)).
 			Run(ctx, state)
 	}
-	return composed.StopWithRequeueDelay(state.gcpConfig.GcpOperationWaitTime), nil
+	return composed.StopWithRequeueDelay(gcpclient.GcpConfig.GcpOperationWaitTime), nil
 }