From 858b04a88e2c09f20bc609d737f3455a31d77284 Mon Sep 17 00:00:00 2001 From: Panchajanya Mysarla Date: Mon, 24 Jul 2023 19:28:32 -0500 Subject: [PATCH] By default; patch additional metadata field using replace strategy --- config/contexts.go | 144 ++-------------- config/contexts_additionalmetadata_test.go | 188 +++++++-------------- config/contexts_it_test.go | 6 +- 3 files changed, 80 insertions(+), 258 deletions(-) diff --git a/config/contexts.go b/config/contexts.go index 417fa1dc..13ee63c4 100644 --- a/config/contexts.go +++ b/config/contexts.go @@ -14,33 +14,6 @@ import ( configtypes "github.com/vmware-tanzu/tanzu-plugin-runtime/config/types" ) -// ContextOpts is used to construct options for context apis -type ContextOpts struct { - DeleteAllAdditionalMetadata bool // To delete the entire additional metadata property of context - AdditionalMetadataKey string // key string to identify the additional metadata property -} - -// ContextOptions are used to pass options for context apis -type ContextOptions func(c *ContextOpts) - -// WithDeleteAllAdditionalMetadata to delete all additional metadata of a context -func WithDeleteAllAdditionalMetadata() ContextOptions { - return func(c *ContextOpts) { - c.DeleteAllAdditionalMetadata = true - } -} - -// WithAdditionalMetadataKey to specify additional metadata key -func WithAdditionalMetadataKey(key string) ContextOptions { - return func(c *ContextOpts) { - c.AdditionalMetadataKey = key - } -} - -func NewContextOpts() *ContextOpts { - return &ContextOpts{} -} - // GetContext retrieves the context by name func GetContext(name string) (*configtypes.Context, error) { // Retrieve client config node @@ -273,44 +246,6 @@ func EndpointFromContext(s *configtypes.Context) (endpoint string, err error) { } } -// DeleteContextAdditionalMetadata deletes additionalMetadata of a specific context in the Tanzu client configuration. -// The contextName parameter specifies the name of the context from which additionalMetadata is to be deleted. -// The options parameter allows providing additional context configuration options. -func DeleteContextAdditionalMetadata(contextName string, options ...ContextOptions) error { - // Create a new contextOptions with default values - contextOptions := NewContextOpts() - - // Apply any provided options to the contextOptions - for _, opt := range options { - opt(contextOptions) - } - - // Acquire a lock on the Tanzu client configuration to ensure exclusive access during modifications. - AcquireTanzuConfigLock() - defer ReleaseTanzuConfigLock() - - // Retrieve the YAML node representing the client configuration. - node, err := getClientConfigNodeNoLock() - if err != nil { - return err - } - - // Get the context node for the specified contextName from the client configuration. - ctx, err := getContext(node, contextName) - if err != nil { - return err - } - - // Delete the additionalMetadata of the context by key or delete all, based on the provided contextOptions. - err = removeContextAdditionalMetadata(node, ctx.Name, contextOptions.AdditionalMetadataKey, contextOptions.DeleteAllAdditionalMetadata) - if err != nil { - return err - } - - // Persist the modified client configuration back to storage. - return persistConfig(node) -} - func getContext(node *yaml.Node, name string) (*configtypes.Context, error) { // check if context name is empty if name == "" { @@ -361,11 +296,8 @@ func setContext(node *yaml.Node, ctx *configtypes.Context) (persist bool, err er return false, errors.New("context name cannot be empty") } - // Get Patch Strategies from config metadata - patchStrategies, err := GetConfigMetadataPatchStrategy() - if err != nil { - patchStrategies = make(map[string]string) - } + // Get Patch Strategies + patchStrategies := constructPatchStrategies() var persistDiscoverySources bool @@ -425,6 +357,22 @@ func setContext(node *yaml.Node, ctx *configtypes.Context) (persist bool, err er return persistDiscoverySources || persist, err } +// Get Patch Strategies from config metadata +// By default; AdditionalMetadata field will be patched in replace strategy if there are no patch strategies +func constructPatchStrategies() map[string]string { + patchStrategies, err := GetConfigMetadataPatchStrategy() + if err != nil { + patchStrategies = map[string]string{ + "contexts.additionalMetadata": "replace", + } + } + // Verify if there are patch strategies defined for `contexts.additionalMetadata` if not set replace by default + if patchStrategies != nil && patchStrategies["contexts.additionalMetadata"] != "merge" { + patchStrategies["contexts.additionalMetadata"] = "replace" + } + return patchStrategies +} + func setCurrentContext(node *yaml.Node, ctx *configtypes.Context) (persist bool, err error) { // Find current context node in the yaml node keys := []nodeutils.Key{ @@ -493,59 +441,3 @@ func removeContext(node *yaml.Node, name string) error { contextsNode.Content = contexts return nil } - -// removeContextAdditionalMetadata removes the additionalMetadata property of a specific context in a YAML node. -// If removeAll is true, it removes the entire additionalMetadata property. -// If removeAll is false, it removes a specific key from the additionalMetadata property. -func removeContextAdditionalMetadata(node *yaml.Node, name, key string, removeAll bool) error { - // check if context name is empty - if name == "" { - return errors.New("context name cannot be empty") - } - - // check if key is empty when not removing all - if !removeAll && key == "" { - return errors.New("key cannot be empty") - } - - // Find the contexts node in the YAML node - keys := []nodeutils.Key{ - {Name: KeyContexts}, - } - contextsNode := nodeutils.FindNode(node.Content[0], nodeutils.WithKeys(keys)) - if contextsNode == nil { - // If no contexts node is found, there is nothing to remove. - return nil - } - - // Create a new list to hold modified context nodes - var contexts []*yaml.Node - for _, contextNode := range contextsNode.Content { - if index := nodeutils.GetNodeIndex(contextNode.Content, "name"); index != -1 && contextNode.Content[index].Value == name { - if removeAll { - // If removeAll is true, remove the entire additionalMetadata property. - index := nodeutils.GetNodeIndex(contextNode.Content, KeyAdditionalMetadata) - if index != -1 { - contextNode.Content = append(contextNode.Content[:index-1], contextNode.Content[index+1:]...) - } - } else { - // If removeAll is false, remove the specified key from additionalMetadata property. - additionalMetadataKey := []nodeutils.Key{{Name: KeyAdditionalMetadata}} - additionalMetadataNode := nodeutils.FindNode(contextNode, nodeutils.WithKeys(additionalMetadataKey)) - if additionalMetadataNode != nil && additionalMetadataNode.Content != nil { - index := nodeutils.GetNodeIndex(additionalMetadataNode.Content, key) - if index != -1 { - // Remove the specific key from additionalMetadata. - additionalMetadataNode.Content = append(additionalMetadataNode.Content[:index-1], additionalMetadataNode.Content[index+1:]...) - } - } - } - } - // Add the modified context node to the new list. - contexts = append(contexts, contextNode) - } - - // Update the contexts node with the modified list of contexts. - contextsNode.Content = contexts - return nil -} diff --git a/config/contexts_additionalmetadata_test.go b/config/contexts_additionalmetadata_test.go index 7f7db3c3..5c55cf13 100644 --- a/config/contexts_additionalmetadata_test.go +++ b/config/contexts_additionalmetadata_test.go @@ -11,7 +11,7 @@ import ( configtypes "github.com/vmware-tanzu/tanzu-plugin-runtime/config/types" ) -func TestDeleteContextAdditionalMetadata(t *testing.T) { +func TestContextAdditionalMetadataStringToString(t *testing.T) { // Setup config data _, cleanUp := setupTestConfig(t, &CfgTestData{cfgNextGen: ``, cfg: ``, cfgMetadata: ``}) @@ -20,151 +20,90 @@ func TestDeleteContextAdditionalMetadata(t *testing.T) { }() var testcases = []struct { - name string - ctx *configtypes.Context - ctxOptions ContextOptions - out map[string]interface{} - errStr string + name string + ctx *configtypes.Context + out map[string]interface{} + errStr string }{ { - name: "should add additional metadata", + name: "should add additional metadata \"issuer1\": \"vmw1\"", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw1", - "issuer2": "vmw1", - "issuer3": "vmw1", + "issuer1": "vmw1", }, }, - ctxOptions: WithAdditionalMetadataKey("issuer4"), out: map[string]interface{}{ - "issuer": "vmw1", - "issuer2": "vmw1", - "issuer3": "vmw1", + "issuer1": "vmw1", }, }, - { - name: "should update additional metadata", + name: "should update additional metadata \"issuer2\": \"vmw2\",", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw2", + "issuer2": "vmw2", }, }, - ctxOptions: WithAdditionalMetadataKey("issuer"), out: map[string]interface{}{ - "issuer2": "vmw1", - "issuer3": "vmw1", - }, - }, - { - name: "should delete all additional metadata", - ctx: &configtypes.Context{ - Name: "test-mc", - Target: configtypes.TargetK8s, - AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw2", - "issuer2": "vmw2", - "issuer3": "vmw2", - "issuer4": "vmw2", - }, + "issuer2": "vmw2", }, - ctxOptions: WithDeleteAllAdditionalMetadata(), - out: nil, }, - } - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - // perform test - err := SetContext(tc.ctx, false) - if tc.errStr == "" { - assert.NoError(t, err) - } else { - assert.EqualError(t, err, tc.errStr) - } - - err = DeleteContextAdditionalMetadata(tc.ctx.Name, tc.ctxOptions) - assert.NoError(t, err) - - ctx, err := GetContext(tc.ctx.Name) - assert.NoError(t, err) - - assert.Equal(t, tc.out, ctx.AdditionalMetadata) - }) - } -} - -func TestContextAdditionalMetadataStringToString(t *testing.T) { - // Setup config data - _, cleanUp := setupTestConfig(t, &CfgTestData{cfgNextGen: ``, cfg: ``, cfgMetadata: ``}) - - defer func() { - cleanUp() - }() - - var testcases = []struct { - name string - ctx *configtypes.Context - out map[string]interface{} - errStr string - }{ { - name: "should add additional metadata tags:tag1", + name: "should clear additional metadata \"issuer1\": \"\",", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw1", + "issuer1": "", }, }, - }, - { - name: "should update additional metadata tags:tag2", - ctx: &configtypes.Context{ - Name: "test-mc", - Target: configtypes.TargetK8s, - AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw2", - }, + out: map[string]interface{}{ + "issuer1": "", }, }, { - name: "should clear additional metadata string:string", + name: "should update additional metadata \"issuer1\": \"vmw1\",", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, AdditionalMetadata: map[string]interface{}{ - "issuer": "", + "issuer1": "vmw1", }, }, + out: map[string]interface{}{ + "issuer1": "vmw1", + }, }, - { - name: "should update additional metadata tags:tag1", + name: "should update additional metadata \"issuer2\": \"vmw\",\n\t\t\t\t\t\"issuer1\": \"vmw\",", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw1", + "issuer2": "vmw", + "issuer1": "vmw", }, }, + out: map[string]interface{}{ + "issuer1": "vmw", + "issuer2": "vmw", + }, }, + { - name: "should update additional metadata tags:tag2", + name: "should delete all additional metadata", ctx: &configtypes.Context{ - Name: "test-mc", - Target: configtypes.TargetK8s, - AdditionalMetadata: map[string]interface{}{ - "issuer": "vmw2", - }, + Name: "test-mc", + Target: configtypes.TargetK8s, + AdditionalMetadata: map[string]interface{}{}, }, + out: nil, }, } @@ -180,11 +119,7 @@ func TestContextAdditionalMetadataStringToString(t *testing.T) { ctx, err := GetContext(tc.ctx.Name) assert.NoError(t, err) - if tc.out != nil { - assert.Equal(t, tc.out, ctx.AdditionalMetadata) - } else { - assert.Equal(t, tc.ctx.AdditionalMetadata, ctx.AdditionalMetadata) - } + assert.Equal(t, tc.out, ctx.AdditionalMetadata) }) } } @@ -205,9 +140,8 @@ func TestContextAdditionalMetadataStringToInt(t *testing.T) { }{ // Additional metadata of map[string]int - { - name: "should add additional metadata tags:0", + name: "should add additional metadata \"contextId\": 0,", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -220,7 +154,7 @@ func TestContextAdditionalMetadataStringToInt(t *testing.T) { }, }, { - name: "should update additional metadata tags:1", + name: "should update additional metadata \"contextId\": 1,", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -271,7 +205,7 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { }{ { - name: "should add additional metadata \"tags\": []string{},", + name: "should add additional metadata \"issuers\": []string{},", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -284,7 +218,7 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { }, }, { - name: "should update additional metadata \"tags\": []string{\"tag1\"},", + name: "should update additional metadata \t\"issuers\": []interface{}{\"vmw1\"},", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -292,9 +226,12 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { "issuers": []interface{}{"vmw1"}, }, }, + out: map[string]interface{}{ + "issuers": []interface{}{"vmw1"}, + }, }, { - name: "should update additional metadata \"tags\": []string{\"tag2\"},", + name: "should update additional metadata \"issuers\": []interface{}{\"vmw1\", \"vmw2\"},", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -302,9 +239,12 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { "issuers": []interface{}{"vmw1", "vmw2"}, }, }, + out: map[string]interface{}{ + "issuers": []interface{}{"vmw1", "vmw2"}, + }, }, { - name: "should update additional metadata \"tags\": []string{\"tag1\", \"tag2\"},", + name: "should update additional metadata \"issuers\": []interface{}{\"vmw1\", \"vmw2\"},", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -312,9 +252,12 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { "issuers": []interface{}{"vmw1", "vmw2"}, }, }, + out: map[string]interface{}{ + "issuers": []interface{}{"vmw1", "vmw2"}, + }, }, { - name: "should update additional metadata \"tags\": []string{\"tag3\", \"tag4\"},\n", + name: "should update additional metadata \"issuers\": []interface{}{\"vmw1\", \"vmw2\", \"vmw3\", \"vmw4\"},", ctx: &configtypes.Context{ Name: "test-mc", Target: configtypes.TargetK8s, @@ -322,16 +265,18 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { "issuers": []interface{}{"vmw1", "vmw2", "vmw3", "vmw4"}, }, }, + out: map[string]interface{}{ + "issuers": []interface{}{"vmw1", "vmw2", "vmw3", "vmw4"}, + }, }, { - name: "should update additional metadata \"tags\": []string{\"tag3\", \"tag4\", \"tag1\"},", + name: "should delete all additional metadata", ctx: &configtypes.Context{ - Name: "test-mc", - Target: configtypes.TargetK8s, - AdditionalMetadata: map[string]interface{}{ - "issuers": []interface{}{"vmw1", "vmw2", "vmw3", "vmw4"}, - }, + Name: "test-mc", + Target: configtypes.TargetK8s, + AdditionalMetadata: map[string]interface{}{}, }, + out: nil, }, } @@ -347,11 +292,7 @@ func TestContextAdditionalMetadataStringToStringArray(t *testing.T) { ctx, err := GetContext(tc.ctx.Name) assert.NoError(t, err) - if tc.out != nil { - assert.Equal(t, tc.out, ctx.AdditionalMetadata) - } else { - assert.Equal(t, tc.ctx.AdditionalMetadata, ctx.AdditionalMetadata) - } + assert.Equal(t, tc.out, ctx.AdditionalMetadata) }) } } @@ -401,7 +342,6 @@ func TestContextAdditionalMetadataStringToMap(t *testing.T) { }, out: map[string]interface{}{ "auth": map[string]interface{}{ - "a1": "x", "a2": "y", }, }, @@ -420,7 +360,6 @@ func TestContextAdditionalMetadataStringToMap(t *testing.T) { out: map[string]interface{}{ "auth": map[string]interface{}{ "a1": "y", - "a2": "y", }, }, }, @@ -440,7 +379,6 @@ func TestContextAdditionalMetadataStringToMap(t *testing.T) { "auth": map[string]interface{}{ "a1": "z", "a3": "z", - "a2": "y", }, }, }, @@ -656,9 +594,6 @@ func TestContextAdditionalMetadataStringToStructArray(t *testing.T) { "accessToken": "token1", "IDToken": "id-1", }, - map[string]interface{}{ - "accessToken": "token2", - }, }, }, }, @@ -685,13 +620,8 @@ func TestContextAdditionalMetadataStringToStructArray(t *testing.T) { out: map[string]interface{}{ "globalAuth": []interface{}{ map[string]interface{}{ - "accessToken": "token1", - "IDToken": "id-1", "permissions": []interface{}{"p1"}, }, - map[string]interface{}{ - "accessToken": "token2", - }, }, "globalAuth2": []interface{}{ map[string]interface{}{ diff --git a/config/contexts_it_test.go b/config/contexts_it_test.go index 20ef5ddd..c2778f17 100644 --- a/config/contexts_it_test.go +++ b/config/contexts_it_test.go @@ -260,14 +260,14 @@ currentContext: path: test-path-updated context: test-context-updated isManagementCluster: true - additionalMetadata: - metaToken: updated-token1 - newToken: optional discoverySources: - gcp: name: test bucket: test-bucket-updated manifestPath: test-manifest-path-updated + additionalMetadata: + metaToken: updated-token1 + newToken: optional currentContext: kubernetes: test-mc2 `