Repository: vmware-tanzu/tanzu-framework (language: go)

Issue:
`~/.config/tanzu/config.yaml` is getting corrupted after running unit tests
**Bug description**
The CLI commands `tanzu login` and `tanzu config server list` not working after running tests in grpc_test.go and token_test.go, it seems test cases are corrupting the ~/.config/tanzu/config.yaml file.

**Affected product area (please put an X in all that apply)**

- ( ) APIs
- ( ) Addons
- (x) CLI
- ( ) Docs
- ( ) IAM
- ( ) Installation
- ( ) Plugin
- ( ) Security
- ( ) Test and Release
- (x) User Experience
- ( ) Developer Experience

**Expected behavior**
The `tanzu login`, `tanzu config server list` and other CLI commands should work fine even after running any unit test suites.
**Steps to reproduce the bug**
Get latest tanzu-framework code, delete existing ~/.config/tanzu/config.yaml file, run `make build-install-local` and `make test`, then run tests in grpc_test.go and token_test.go, now run commands `tanzu login` and `tanzu config server list`, you can see error.
**Version** (include the SHA if the version is not obvious)
❯ tanzu version
version: v0.28.0-dev
buildDate: 2022-11-16
sha: 1d60a188
**Environment where the bug was observed (cloud, OS, etc)**
Mac
**Relevant Debug Output (Logs, manifests, etc)**
[cli-commands-output.txt](https://github.com/vmware-tanzu/tanzu-framework/files/10026373/cli-commands-output.txt)
[config.yml.txt](https://github.com/vmware-tanzu/tanzu-framework/files/10026377/config.yml.txt)



Discussion / hints:

Fix patch:
diff --git a/cli/core/pkg/auth/csp/grpc.go b/cli/core/pkg/auth/csp/grpc.go
index 0f63872ee0..77e518eb90 100644
--- a/cli/core/pkg/auth/csp/grpc.go
+++ b/cli/core/pkg/auth/csp/grpc.go
@@ -15,6 +15,7 @@ import (
 	grpc_oauth "google.golang.org/grpc/credentials/oauth"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/vmware-tanzu/tanzu-framework/cli/core/pkg/interfaces"
 	configapi "github.com/vmware-tanzu/tanzu-framework/cli/runtime/apis/config/v1alpha1"
 	"github.com/vmware-tanzu/tanzu-framework/cli/runtime/config"
 )
@@ -26,6 +27,14 @@ const (
 	apiToken         = "api-token"
 )
 
+var (
+	configClientWrapper interfaces.ConfigClientWrapper
+)
+
+func init() {
+	configClientWrapper = interfaces.NewConfigClient()
+}
+
 // WithCredentialDiscovery returns a grpc.CallOption that adds credentials into gRPC calls.
 // The credentials are loaded from the auth context found on the machine.
 func WithCredentialDiscovery() (grpc.CallOption, error) {
@@ -83,12 +92,12 @@ func (c *configSource) Token() (*oauth2.Token, error) {
 	g.GlobalOpts.Auth.IDToken = token.IDToken
 
 	// Acquire tanzu config lock
-	config.AcquireTanzuConfigLock()
-	defer config.ReleaseTanzuConfigLock()
+	configClientWrapper.AcquireTanzuConfigLock()
+	defer configClientWrapper.ReleaseTanzuConfigLock()
 
 	// TODO: Add Read/Write locking mechanism before updating the configuration
 	// Currently we are only acquiring the lock while updating the configuration
-	if err := config.StoreClientConfig(c.ClientConfig); err != nil {
+	if err := configClientWrapper.StoreClientConfig(c.ClientConfig); err != nil {
 		return nil, err
 	}
 
diff --git a/cli/core/pkg/config/config.go b/cli/core/pkg/config/config.go
index b432c5c159..9cc2b4b18c 100644
--- a/cli/core/pkg/config/config.go
+++ b/cli/core/pkg/config/config.go
@@ -11,7 +11,7 @@ import (
 )
 
 var (
-	configClient interfaces.ConfigClient
+	configClient interfaces.ConfigClientWrapper
 )
 
 func init() {
diff --git a/cli/core/pkg/fakes/config_client_fake.go b/cli/core/pkg/fakes/config_client_fake.go
deleted file mode 100644
index 3530740e41..0000000000
--- a/cli/core/pkg/fakes/config_client_fake.go
+++ /dev/null
@@ -1,102 +0,0 @@
-// Code generated by counterfeiter. DO NOT EDIT.
-package fakes
-
-import (
-	"sync"
-
-	"github.com/vmware-tanzu/tanzu-framework/cli/core/pkg/interfaces"
-)
-
-type FakeConfigClient struct {
-	GetEnvConfigurationsStub        func() map[string]string
-	getEnvConfigurationsMutex       sync.RWMutex
-	getEnvConfigurationsArgsForCall []struct {
-	}
-	getEnvConfigurationsReturns struct {
-		result1 map[string]string
-	}
-	getEnvConfigurationsReturnsOnCall map[int]struct {
-		result1 map[string]string
-	}
-	invocations      map[string][][]interface{}
-	invocationsMutex sync.RWMutex
-}
-
-func (fake *FakeConfigClient) GetEnvConfigurations() map[string]string {
-	fake.getEnvConfigurationsMutex.Lock()
-	ret, specificReturn := fake.getEnvConfigurationsReturnsOnCall[len(fake.getEnvConfigurationsArgsForCall)]
-	fake.getEnvConfigurationsArgsForCall = append(fake.getEnvConfigurationsArgsForCall, struct {
-	}{})
-	stub := fake.GetEnvConfigurationsStub
-	fakeReturns := fake.getEnvConfigurationsReturns
-	fake.recordInvocation("GetEnvConfigurations", []interface{}{})
-	fake.getEnvConfigurationsMutex.Unlock()
-	if stub != nil {
-		return stub()
-	}
-	if specificReturn {
-		return ret.result1
-	}
-	return fakeReturns.result1
-}
-
-func (fake *FakeConfigClient) GetEnvConfigurationsCallCount() int {
-	fake.getEnvConfigurationsMutex.RLock()
-	defer fake.getEnvConfigurationsMutex.RUnlock()
-	return len(fake.getEnvConfigurationsArgsForCall)
-}
-
-func (fake *FakeConfigClient) GetEnvConfigurationsCalls(stub func() map[string]string) {
-	fake.getEnvConfigurationsMutex.Lock()
-	defer fake.getEnvConfigurationsMutex.Unlock()
-	fake.GetEnvConfigurationsStub = stub
-}
-
-func (fake *FakeConfigClient) GetEnvConfigurationsReturns(result1 map[string]string) {
-	fake.getEnvConfigurationsMutex.Lock()
-	defer fake.getEnvConfigurationsMutex.Unlock()
-	fake.GetEnvConfigurationsStub = nil
-	fake.getEnvConfigurationsReturns = struct {
-		result1 map[string]string
-	}{result1}
-}
-
-func (fake *FakeConfigClient) GetEnvConfigurationsReturnsOnCall(i int, result1 map[string]string) {
-	fake.getEnvConfigurationsMutex.Lock()
-	defer fake.getEnvConfigurationsMutex.Unlock()
-	fake.GetEnvConfigurationsStub = nil
-	if fake.getEnvConfigurationsReturnsOnCall == nil {
-		fake.getEnvConfigurationsReturnsOnCall = make(map[int]struct {
-			result1 map[string]string
-		})
-	}
-	fake.getEnvConfigurationsReturnsOnCall[i] = struct {
-		result1 map[string]string
-	}{result1}
-}
-
-func (fake *FakeConfigClient) Invocations() map[string][][]interface{} {
-	fake.invocationsMutex.RLock()
-	defer fake.invocationsMutex.RUnlock()
-	fake.getEnvConfigurationsMutex.RLock()
-	defer fake.getEnvConfigurationsMutex.RUnlock()
-	copiedInvocations := map[string][][]interface{}{}
-	for key, value := range fake.invocations {
-		copiedInvocations[key] = value
-	}
-	return copiedInvocations
-}
-
-func (fake *FakeConfigClient) recordInvocation(key string, args []interface{}) {
-	fake.invocationsMutex.Lock()
-	defer fake.invocationsMutex.Unlock()
-	if fake.invocations == nil {
-		fake.invocations = map[string][][]interface{}{}
-	}
-	if fake.invocations[key] == nil {
-		fake.invocations[key] = [][]interface{}{}
-	}
-	fake.invocations[key] = append(fake.invocations[key], args)
-}
-
-var _ interfaces.ConfigClient = new(FakeConfigClient)
diff --git a/cli/core/pkg/fakes/fake_config_client_wrapper.go b/cli/core/pkg/fakes/fake_config_client_wrapper.go
new file mode 100644
index 0000000000..f8243fd2d9
--- /dev/null
+++ b/cli/core/pkg/fakes/fake_config_client_wrapper.go
@@ -0,0 +1,237 @@
+// Code generated by counterfeiter. DO NOT EDIT.
+package fakes
+
+import (
+	"sync"
+
+	"github.com/vmware-tanzu/tanzu-framework/cli/core/pkg/interfaces"
+	"github.com/vmware-tanzu/tanzu-framework/cli/runtime/apis/config/v1alpha1"
+)
+
+type FakeConfigClientWrapper struct {
+	AcquireTanzuConfigLockStub        func()
+	acquireTanzuConfigLockMutex       sync.RWMutex
+	acquireTanzuConfigLockArgsForCall []struct {
+	}
+	GetEnvConfigurationsStub        func() map[string]string
+	getEnvConfigurationsMutex       sync.RWMutex
+	getEnvConfigurationsArgsForCall []struct {
+	}
+	getEnvConfigurationsReturns struct {
+		result1 map[string]string
+	}
+	getEnvConfigurationsReturnsOnCall map[int]struct {
+		result1 map[string]string
+	}
+	ReleaseTanzuConfigLockStub        func()
+	releaseTanzuConfigLockMutex       sync.RWMutex
+	releaseTanzuConfigLockArgsForCall []struct {
+	}
+	StoreClientConfigStub        func(*v1alpha1.ClientConfig) error
+	storeClientConfigMutex       sync.RWMutex
+	storeClientConfigArgsForCall []struct {
+		arg1 *v1alpha1.ClientConfig
+	}
+	storeClientConfigReturns struct {
+		result1 error
+	}
+	storeClientConfigReturnsOnCall map[int]struct {
+		result1 error
+	}
+	invocations      map[string][][]interface{}
+	invocationsMutex sync.RWMutex
+}
+
+func (fake *FakeConfigClientWrapper) AcquireTanzuConfigLock() {
+	fake.acquireTanzuConfigLockMutex.Lock()
+	fake.acquireTanzuConfigLockArgsForCall = append(fake.acquireTanzuConfigLockArgsForCall, struct {
+	}{})
+	stub := fake.AcquireTanzuConfigLockStub
+	fake.recordInvocation("AcquireTanzuConfigLock", []interface{}{})
+	fake.acquireTanzuConfigLockMutex.Unlock()
+	if stub != nil {
+		fake.AcquireTanzuConfigLockStub()
+	}
+}
+
+func (fake *FakeConfigClientWrapper) AcquireTanzuConfigLockCallCount() int {
+	fake.acquireTanzuConfigLockMutex.RLock()
+	defer fake.acquireTanzuConfigLockMutex.RUnlock()
+	return len(fake.acquireTanzuConfigLockArgsForCall)
+}
+
+func (fake *FakeConfigClientWrapper) AcquireTanzuConfigLockCalls(stub func()) {
+	fake.acquireTanzuConfigLockMutex.Lock()
+	defer fake.acquireTanzuConfigLockMutex.Unlock()
+	fake.AcquireTanzuConfigLockStub = stub
+}
+
+func (fake *FakeConfigClientWrapper) GetEnvConfigurations() map[string]string {
+	fake.getEnvConfigurationsMutex.Lock()
+	ret, specificReturn := fake.getEnvConfigurationsReturnsOnCall[len(fake.getEnvConfigurationsArgsForCall)]
+	fake.getEnvConfigurationsArgsForCall = append(fake.getEnvConfigurationsArgsForCall, struct {
+	}{})
+	stub := fake.GetEnvConfigurationsStub
+	fakeReturns := fake.getEnvConfigurationsReturns
+	fake.recordInvocation("GetEnvConfigurations", []interface{}{})
+	fake.getEnvConfigurationsMutex.Unlock()
+	if stub != nil {
+		return stub()
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *FakeConfigClientWrapper) GetEnvConfigurationsCallCount() int {
+	fake.getEnvConfigurationsMutex.RLock()
+	defer fake.getEnvConfigurationsMutex.RUnlock()
+	return len(fake.getEnvConfigurationsArgsForCall)
+}
+
+func (fake *FakeConfigClientWrapper) GetEnvConfigurationsCalls(stub func() map[string]string) {
+	fake.getEnvConfigurationsMutex.Lock()
+	defer fake.getEnvConfigurationsMutex.Unlock()
+	fake.GetEnvConfigurationsStub = stub
+}
+
+func (fake *FakeConfigClientWrapper) GetEnvConfigurationsReturns(result1 map[string]string) {
+	fake.getEnvConfigurationsMutex.Lock()
+	defer fake.getEnvConfigurationsMutex.Unlock()
+	fake.GetEnvConfigurationsStub = nil
+	fake.getEnvConfigurationsReturns = struct {
+		result1 map[string]string
+	}{result1}
+}
+
+func (fake *FakeConfigClientWrapper) GetEnvConfigurationsReturnsOnCall(i int, result1 map[string]string) {
+	fake.getEnvConfigurationsMutex.Lock()
+	defer fake.getEnvConfigurationsMutex.Unlock()
+	fake.GetEnvConfigurationsStub = nil
+	if fake.getEnvConfigurationsReturnsOnCall == nil {
+		fake.getEnvConfigurationsReturnsOnCall = make(map[int]struct {
+			result1 map[string]string
+		})
+	}
+	fake.getEnvConfigurationsReturnsOnCall[i] = struct {
+		result1 map[string]string
+	}{result1}
+}
+
+func (fake *FakeConfigClientWrapper) ReleaseTanzuConfigLock() {
+	fake.releaseTanzuConfigLockMutex.Lock()
+	fake.releaseTanzuConfigLockArgsForCall = append(fake.releaseTanzuConfigLockArgsForCall, struct {
+	}{})
+	stub := fake.ReleaseTanzuConfigLockStub
+	fake.recordInvocation("ReleaseTanzuConfigLock", []interface{}{})
+	fake.releaseTanzuConfigLockMutex.Unlock()
+	if stub != nil {
+		fake.ReleaseTanzuConfigLockStub()
+	}
+}
+
+func (fake *FakeConfigClientWrapper) ReleaseTanzuConfigLockCallCount() int {
+	fake.releaseTanzuConfigLockMutex.RLock()
+	defer fake.releaseTanzuConfigLockMutex.RUnlock()
+	return len(fake.releaseTanzuConfigLockArgsForCall)
+}
+
+func (fake *FakeConfigClientWrapper) ReleaseTanzuConfigLockCalls(stub func()) {
+	fake.releaseTanzuConfigLockMutex.Lock()
+	defer fake.releaseTanzuConfigLockMutex.Unlock()
+	fake.ReleaseTanzuConfigLockStub = stub
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfig(arg1 *v1alpha1.ClientConfig) error {
+	fake.storeClientConfigMutex.Lock()
+	ret, specificReturn := fake.storeClientConfigReturnsOnCall[len(fake.storeClientConfigArgsForCall)]
+	fake.storeClientConfigArgsForCall = append(fake.storeClientConfigArgsForCall, struct {
+		arg1 *v1alpha1.ClientConfig
+	}{arg1})
+	stub := fake.StoreClientConfigStub
+	fakeReturns := fake.storeClientConfigReturns
+	fake.recordInvocation("StoreClientConfig", []interface{}{arg1})
+	fake.storeClientConfigMutex.Unlock()
+	if stub != nil {
+		return stub(arg1)
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfigCallCount() int {
+	fake.storeClientConfigMutex.RLock()
+	defer fake.storeClientConfigMutex.RUnlock()
+	return len(fake.storeClientConfigArgsForCall)
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfigCalls(stub func(*v1alpha1.ClientConfig) error) {
+	fake.storeClientConfigMutex.Lock()
+	defer fake.storeClientConfigMutex.Unlock()
+	fake.StoreClientConfigStub = stub
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfigArgsForCall(i int) *v1alpha1.ClientConfig {
+	fake.storeClientConfigMutex.RLock()
+	defer fake.storeClientConfigMutex.RUnlock()
+	argsForCall := fake.storeClientConfigArgsForCall[i]
+	return argsForCall.arg1
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfigReturns(result1 error) {
+	fake.storeClientConfigMutex.Lock()
+	defer fake.storeClientConfigMutex.Unlock()
+	fake.StoreClientConfigStub = nil
+	fake.storeClientConfigReturns = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeConfigClientWrapper) StoreClientConfigReturnsOnCall(i int, result1 error) {
+	fake.storeClientConfigMutex.Lock()
+	defer fake.storeClientConfigMutex.Unlock()
+	fake.StoreClientConfigStub = nil
+	if fake.storeClientConfigReturnsOnCall == nil {
+		fake.storeClientConfigReturnsOnCall = make(map[int]struct {
+			result1 error
+		})
+	}
+	fake.storeClientConfigReturnsOnCall[i] = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeConfigClientWrapper) Invocations() map[string][][]interface{} {
+	fake.invocationsMutex.RLock()
+	defer fake.invocationsMutex.RUnlock()
+	fake.acquireTanzuConfigLockMutex.RLock()
+	defer fake.acquireTanzuConfigLockMutex.RUnlock()
+	fake.getEnvConfigurationsMutex.RLock()
+	defer fake.getEnvConfigurationsMutex.RUnlock()
+	fake.releaseTanzuConfigLockMutex.RLock()
+	defer fake.releaseTanzuConfigLockMutex.RUnlock()
+	fake.storeClientConfigMutex.RLock()
+	defer fake.storeClientConfigMutex.RUnlock()
+	copiedInvocations := map[string][][]interface{}{}
+	for key, value := range fake.invocations {
+		copiedInvocations[key] = value
+	}
+	return copiedInvocations
+}
+
+func (fake *FakeConfigClientWrapper) recordInvocation(key string, args []interface{}) {
+	fake.invocationsMutex.Lock()
+	defer fake.invocationsMutex.Unlock()
+	if fake.invocations == nil {
+		fake.invocations = map[string][][]interface{}{}
+	}
+	if fake.invocations[key] == nil {
+		fake.invocations[key] = [][]interface{}{}
+	}
+	fake.invocations[key] = append(fake.invocations[key], args)
+}
+
+var _ interfaces.ConfigClientWrapper = new(FakeConfigClientWrapper)
diff --git a/cli/core/pkg/interfaces/config_client_interface.go b/cli/core/pkg/interfaces/config_client_interface.go
index cdfd640875..f67ccfc134 100644
--- a/cli/core/pkg/interfaces/config_client_interface.go
+++ b/cli/core/pkg/interfaces/config_client_interface.go
@@ -5,20 +5,36 @@
 package interfaces
 
 import (
+	configapi "github.com/vmware-tanzu/tanzu-framework/cli/runtime/apis/config/v1alpha1"
 	"github.com/vmware-tanzu/tanzu-framework/cli/runtime/config"
 )
 
 //go:generate counterfeiter -o ../fakes/config_client_fake.go . ConfigClient
-type ConfigClient interface {
+type ConfigClientWrapper interface {
 	GetEnvConfigurations() map[string]string
+	StoreClientConfig(clientConfig *configapi.ClientConfig) error
+	AcquireTanzuConfigLock()
+	ReleaseTanzuConfigLock()
 }
 
-type configClientImpl struct{}
+type configClientWrapperImpl struct{}
 
-func NewConfigClient() ConfigClient {
-	return &configClientImpl{}
+func NewConfigClient() ConfigClientWrapper {
+	return &configClientWrapperImpl{}
 }
 
-func (cc *configClientImpl) GetEnvConfigurations() map[string]string {
+func (cc *configClientWrapperImpl) GetEnvConfigurations() map[string]string {
 	return config.GetEnvConfigurations()
 }
+
+func (cc *configClientWrapperImpl) AcquireTanzuConfigLock() {
+	config.AcquireTanzuConfigLock()
+}
+
+func (cc *configClientWrapperImpl) ReleaseTanzuConfigLock() {
+	config.ReleaseTanzuConfigLock()
+}
+
+func (cc *configClientWrapperImpl) StoreClientConfig(clientConfig *configapi.ClientConfig) error {
+	return config.StoreClientConfig(clientConfig)
+}


Test patch:
diff --git a/cli/core/pkg/auth/csp/grpc_test.go b/cli/core/pkg/auth/csp/grpc_test.go
index 52d4f2c7ed..abdda843db 100644
--- a/cli/core/pkg/auth/csp/grpc_test.go
+++ b/cli/core/pkg/auth/csp/grpc_test.go
@@ -47,6 +47,11 @@ var _ = Describe("Unit tests for grpc", func() {
 				IDToken:     idToken,
 			}
 			confSource = initializeConfigSource(gsa)
+
+			cc := &fakes.FakeConfigClientWrapper{}
+			configClientWrapper = cc
+			cc.StoreClientConfigReturns(nil)
+			cc.AcquireTanzuConfigLock()
 		})
 		It("should return current token", func() {
 			token, err := confSource.Token()
@@ -75,20 +80,24 @@ var _ = Describe("Unit tests for grpc", func() {
 				"token_type": "Test",
 				"expires_in": 86400,
 				"scope": "Test",
-				"access_token": "LetMeIn",
-				"refresh_token": "LetMeInAgain"}`)))
+				"access_token": "LetMeInGrpc1",
+				"refresh_token": "LetMeInAgainGrpc1"}`)))
 
 			fakeHTTPClient.DoReturns(&http.Response{
 				StatusCode: 200,
 				Body:       responseBody,
 			}, nil)
 
+			cc := &fakes.FakeConfigClientWrapper{}
+			configClientWrapper = cc
+			cc.StoreClientConfigReturns(nil)
+			cc.AcquireTanzuConfigLock()
 		})
 		It("should return token from server", func() {
 			token, err := confSource.Token()
 			Expect(err).NotTo(HaveOccurred())
-			Expect(token.AccessToken).To(Equal("LetMeIn"))
-			Expect(token.RefreshToken).To(Equal("LetMeInAgain"))
+			Expect(token.AccessToken).To(Equal("LetMeInGrpc1"))
+			Expect(token.RefreshToken).To(Equal("LetMeInAgainGrpc1"))
 		})
 	})
 })
diff --git a/cli/core/pkg/config/config_test.go b/cli/core/pkg/config/config_test.go
index 5c0fd64cd5..7cde5a5549 100644
--- a/cli/core/pkg/config/config_test.go
+++ b/cli/core/pkg/config/config_test.go
@@ -21,7 +21,7 @@ const envVar = "test-conf-env"
 var _ = Describe("config env variables", func() {
 	Context("get env from config", func() {
 		BeforeEach(func() {
-			cc := &fakes.FakeConfigClient{}
+			cc := &fakes.FakeConfigClientWrapper{}
 			configClient = cc
 			prevValue = os.Getenv(envVar)
 			confEnvMap := map[string]string{envVar: envVar}
@@ -35,7 +35,7 @@ var _ = Describe("config env variables", func() {
 	})
 	Context("config return nil map", func() {
 		BeforeEach(func() {
-			cc := &fakes.FakeConfigClient{}
+			cc := &fakes.FakeConfigClientWrapper{}
 			configClient = cc
 			cc.GetEnvConfigurationsReturns(nil)
 		})