From d67751ca469f512dc4f189edd9c67e58c5c108c5 Mon Sep 17 00:00:00 2001 From: linyguo Date: Fri, 6 Sep 2024 17:17:34 +0800 Subject: [PATCH 1/2] add traceValue when read catalog property in parent catalog --- .../managers/configs/configs-manager_test.go | 80 ++++++++++++++++++- .../config/catalog/catalogprovider.go | 30 +++---- 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/api/pkg/apis/v1alpha1/managers/configs/configs-manager_test.go b/api/pkg/apis/v1alpha1/managers/configs/configs-manager_test.go index da004d5ef..e45015686 100644 --- a/api/pkg/apis/v1alpha1/managers/configs/configs-manager_test.go +++ b/api/pkg/apis/v1alpha1/managers/configs/configs-manager_test.go @@ -539,7 +539,6 @@ func TestCircularCatalogReferences(t *testing.T) { os.Setenv(constants.SymphonyAPIUrlEnvName, ts.URL+"/") os.Setenv(constants.UseServiceAccountTokenEnvName, "false") - // TODO: here evalContext shares the same copy as in Get evalContext := utils.EvaluationContext{} vendorContext := coa_contexts.VendorContext{ EvaluationContext: &evalContext, @@ -575,7 +574,82 @@ func TestCircularCatalogReferences(t *testing.T) { _, err = manager.Get("config2:v1", "attribute", nil, evalContext) assert.Nil(t, err, "Detect correct attribute, expect no error") +} + +func TestParentConfigEvaluation(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var response interface{} + switch r.URL.Path { + case "/catalogs/registry/config-v-v1": + response = model.CatalogState{ + ObjectMeta: model.ObjectMeta{ + Name: "config-v-v1", + }, + Spec: &model.CatalogSpec{ + ParentName: "parent:v1", + Properties: map[string]interface{}{ + "attribute": "value", + }, + }, + } + case "/catalogs/registry/parent-v-v1": + response = model.CatalogState{ + ObjectMeta: model.ObjectMeta{ + Name: "parent-v-v1", + }, + Spec: &model.CatalogSpec{ + Properties: map[string]interface{}{ + "parentAttribute": "${{$config('config:v1','attribute')}}", + "parentCircular": "${{$config('config:v1','parentCircular')}}", + }, + }, + } + default: + response = AuthResponse{ + AccessToken: "test-token", + TokenType: "Bearer", + Username: "test-user", + Roles: []string{"role1", "role2"}, + } + } + + json.NewEncoder(w).Encode(response) + })) + defer ts.Close() + os.Setenv(constants.SymphonyAPIUrlEnvName, ts.URL+"/") + os.Setenv(constants.UseServiceAccountTokenEnvName, "false") + + evalContext := utils.EvaluationContext{} + vendorContext := coa_contexts.VendorContext{ + EvaluationContext: &evalContext, + } + provider := catalog.CatalogConfigProvider{} + + provider.Context = &coa_contexts.ManagerContext{ + VencorContext: &vendorContext, + } + err := provider.Init(catalog.CatalogConfigProviderConfig{}) + assert.Nil(t, err) + + manager := ConfigsManager{} + config := managers.ManagerConfig{ + Name: "config-name", + Type: "config-type", + Properties: map[string]string{ + "providers.config": "ConfigProvider", + }, + } + providers := make(map[string]providers.IProvider) + providers["ConfigProvider"] = &provider + err = manager.Init(&vendorContext, config, providers) + assert.Nil(t, err) + + evalContext.ConfigProvider = &manager + + val, err := manager.Get("config:v1", "parentAttribute", nil, evalContext) + assert.Equal(t, "value", val) + assert.Nil(t, err) - _, err = manager.Get("config1:v1", "parentAttribute", nil, evalContext) - assert.Error(t, err, "Detect circular dependency, override: parent-v-v1, field: parentAttribute") + _, err = manager.Get("config:v1", "parentCircular", nil, evalContext) + assert.Error(t, err, "Circular dependency in config should throw error") } diff --git a/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go b/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go index 9c363e3fa..495bfe840 100644 --- a/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go +++ b/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go @@ -91,7 +91,7 @@ func CatalogConfigProviderConfigFromMap(properties map[string]string) (CatalogCo return ret, nil } -func (m *CatalogConfigProvider) unwindOverrides(ctx context.Context, override string, field string, namespace string) (string, error) { +func (m *CatalogConfigProvider) unwindOverrides(ctx context.Context, override string, field string, namespace string, localcontext interface{}, dependencyList map[string]map[string]bool) (interface{}, error) { override = utils.ConvertReferenceToObjectName(override) catalog, err := m.ApiClient.GetCatalog(ctx, override, namespace, m.Config.User, m.Config.Password) if err != nil { @@ -99,13 +99,13 @@ func (m *CatalogConfigProvider) unwindOverrides(ctx context.Context, override st } if v, ok := catalog.Spec.Properties[field]; ok { if vstring, ok := v.(string); ok { - return vstring, nil + return m.traceValue(vstring, localcontext, dependencyList) } else { return "", v1alpha2.NewCOAError(nil, fmt.Sprintf("field '%s' doesn't has a valid value in configuration'%s'", field, override), v1alpha2.BadConfig) } } if catalog.Spec.ParentName != "" { - return m.unwindOverrides(ctx, catalog.Spec.ParentName, field, namespace) + return m.unwindOverrides(ctx, catalog.Spec.ParentName, field, namespace, localcontext, dependencyList) } return "", v1alpha2.NewCOAError(nil, fmt.Sprintf("field '%s' is not found in configuration '%s'", field, override), v1alpha2.NotFound) } @@ -125,25 +125,25 @@ func (m *CatalogConfigProvider) Read(ctx context.Context, object string, field s return "", err } - if v, ok := catalog.Spec.Properties[field]; ok { - // check circular dependency - var dependencyList map[string]map[string]bool = nil - if localcontext != nil { - if evalContext, ok := localcontext.(coa_utils.EvaluationContext); ok { - if coa_utils.HasCircularDependency(object, field, evalContext) { - clog.Errorf(" M (Catalog): Read detect circular dependency. Object: %v, field: %v, ", object, field) - return "", v1alpha2.NewCOAError(nil, fmt.Sprintf("Detect circular dependency, object: %s, field: %s", object, field), v1alpha2.BadConfig) - } - dependencyList = coa_utils.DeepCopyDependencyList(evalContext.ParentConfigs) - dependencyList = coa_utils.UpdateDependencyList(object, field, dependencyList) + // check circular dependency + var dependencyList map[string]map[string]bool = nil + if localcontext != nil { + if evalContext, ok := localcontext.(coa_utils.EvaluationContext); ok { + if coa_utils.HasCircularDependency(object, field, evalContext) { + clog.Errorf(" M (Catalog): Read detect circular dependency. Object: %v, field: %v, ", object, field) + return "", v1alpha2.NewCOAError(nil, fmt.Sprintf("Detect circular dependency, object: %s, field: %s", object, field), v1alpha2.BadConfig) } + dependencyList = coa_utils.DeepCopyDependencyList(evalContext.ParentConfigs) + dependencyList = coa_utils.UpdateDependencyList(object, field, dependencyList) } + } + if v, ok := catalog.Spec.Properties[field]; ok { return m.traceValue(v, localcontext, dependencyList) } if catalog.Spec.ParentName != "" { - overrid, err := m.unwindOverrides(ctx, catalog.Spec.ParentName, field, namespace) + overrid, err := m.unwindOverrides(ctx, catalog.Spec.ParentName, field, namespace, localcontext, dependencyList) if err != nil { return "", err } else { From fb906b134d96dd7b46165723de663cfdb1d8a60b Mon Sep 17 00:00:00 2001 From: linyguo Date: Fri, 6 Sep 2024 17:24:12 +0800 Subject: [PATCH 2/2] update trace value --- .../v1alpha1/providers/config/catalog/catalogprovider.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go b/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go index 495bfe840..742d036bc 100644 --- a/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go +++ b/api/pkg/apis/v1alpha1/providers/config/catalog/catalogprovider.go @@ -98,11 +98,7 @@ func (m *CatalogConfigProvider) unwindOverrides(ctx context.Context, override st return "", err } if v, ok := catalog.Spec.Properties[field]; ok { - if vstring, ok := v.(string); ok { - return m.traceValue(vstring, localcontext, dependencyList) - } else { - return "", v1alpha2.NewCOAError(nil, fmt.Sprintf("field '%s' doesn't has a valid value in configuration'%s'", field, override), v1alpha2.BadConfig) - } + return m.traceValue(v, localcontext, dependencyList) } if catalog.Spec.ParentName != "" { return m.unwindOverrides(ctx, catalog.Spec.ParentName, field, namespace, localcontext, dependencyList)