+++33333

cmd/addon-operator/TODO_README.md

@@ -0,0 +1,3 @@
+1. Хранить openapi values в values storage, потому что могут переопределиться дефолты
+2. AppendValuesPatch для хранения с compaction
+3. values хуков приоритетнее чем values из configMap

pkg/addon-operator/debug_server.go

@@ -2,11 +2,12 @@ package addon_operator
 
 import (
 	"fmt"
-	"github.com/flant/addon-operator/pkg/module_manager/models/modules"
 	"net/http"
 	"sort"
 	"strconv"
 
+	"github.com/flant/addon-operator/pkg/module_manager/models/modules"
+
 	"github.com/go-chi/chi/v5"
 
 	"github.com/flant/addon-operator/pkg/app"
@@ -59,7 +60,7 @@ func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) {
 
 		m := op.ModuleManager.GetModule(modName)
 		if m == nil {
-			return nil, fmt.Errorf("Module not found")
+			return nil, fmt.Errorf("module not found")
 		}
 
 		switch valType {

pkg/module_manager/loader/fs/fs.go

@@ -82,6 +82,7 @@ func (fl *FileSystemLoader) LoadModules() ([]*modules.BasicModule, error) {
 			}
 
 			// 3. from openapi defaults
+			//TODO(yalosev): think about openapi
 			cb, vb, err := fl.readOpenAPIFiles(filepath.Join(module.Path, "openapi"))
 			if err != nil {
 				return nil, err
@@ -93,18 +94,18 @@ func (fl *FileSystemLoader) LoadModules() ([]*modules.BasicModule, error) {
 					return nil, err
 				}
 
-				s := fl.valuesValidator.SchemaStorage.ModuleValuesSchema(valuesModuleName, validation.ModuleSchema)
-				if s != nil {
-					validation.ApplyDefaults(initialValues, s)
-				}
-
-				if moduleEnabled {
-					// we don't need to validate values for disabled modules
-					err = fl.valuesValidator.ValidateModuleValues(valuesModuleName, initialValues)
-					if err != nil {
-						return nil, fmt.Errorf("validation failed: %w", err)
-					}
-				}
+				//s := fl.valuesValidator.SchemaStorage.ModuleValuesSchema(valuesModuleName, validation.ModuleSchema)
+				//if s != nil {
+				//	validation.ApplyDefaults(initialValues, s)
+				//}
+				//
+				//if moduleEnabled {
+				//	// we don't need to validate values for disabled modules
+				//	err = fl.valuesValidator.ValidateModuleValues(valuesModuleName, initialValues)
+				//	if err != nil {
+				//		return nil, fmt.Errorf("validation failed: %w", err)
+				//	}
+				//}
 			}
 
 			//
@@ -115,7 +116,8 @@ func (fl *FileSystemLoader) LoadModules() ([]*modules.BasicModule, error) {
 				panic("Here")
 			}
 
-			bm := modules.NewBasicModule(module.Name, module.Path, module.Order, moduleEnabled, moduleValues, fl.valuesValidator)
+			vs := modules.NewValuesStorage(moduleValues, fl.valuesValidator)
+			bm := modules.NewBasicModule(module.Name, module.Path, module.Order, moduleEnabled, vs, fl.valuesValidator)
 
 			result = append(result, bm)
 		}

pkg/module_manager/models/modules/basic.go

@@ -57,12 +57,15 @@ type BasicModule struct {
 	dc *hooks.HookExecutionDependencyContainer
 }
 
-func NewBasicModule(name, path string, order uint32, enabled bool, initialValues utils.Values, validator validator) *BasicModule {
+func NewBasicModule(name, path string, order uint32, enabled bool, vs *ValuesStorage, validator validator) *BasicModule {
+	if vs == nil {
+		vs = NewValuesStorage(utils.Values{}, validator)
+	}
 	return &BasicModule{
 		Name:               name,
 		Order:              order,
 		Path:               path,
-		valuesStorage:      NewValuesStorage(initialValues, validator),
+		valuesStorage:      vs,
 		dynamicValuesPatch: nil,
 		enabledByConfig:    enabled,
 		state: &moduleState{
@@ -90,6 +93,10 @@ func (bm *BasicModule) GetName() string {
 	return bm.Name
 }
 
+func (bm *BasicModule) GetPath() string {
+	return bm.Path
+}
+
 func (bm *BasicModule) GetHooks(bt ...sh_op_types.BindingType) []*hooks.ModuleHook {
 	return bm.hooks.getHooks(bt...)
 }
@@ -694,7 +701,7 @@ func (bm *BasicModule) executeHook(h *hooks.ModuleHook, bindingType sh_op_types.
 		if configValuesPatchResult.ValuesChanged {
 			logEntry.Debugf("Module hook '%s': validate module config values before update", h.GetName())
 			// Validate merged static and new values.
-			validationErr := bm.valuesStorage.SetNewConfigValues(bm.Name, configValuesPatchResult.Values)
+			validationErr := bm.valuesStorage.PreCommitConfigValues(bm.Name, configValuesPatchResult.Values)
 			if validationErr != nil {
 				return multierror.Append(
 					fmt.Errorf("cannot apply config values patch for module values"),
@@ -708,7 +715,7 @@ func (bm *BasicModule) executeHook(h *hooks.ModuleHook, bindingType sh_op_types.
 				return fmt.Errorf("module hook '%s': set kube module config failed: %s", h.GetName(), err)
 			}
 
-			bm.valuesStorage.ApplyDirtyConfigValues()
+			bm.valuesStorage.CommitConfigValues()
 
 			logEntry.Debugf("Module hook '%s': kube module '%s' config values updated:\n%s", h.GetName(), bm.Name, bm.valuesStorage.GetConfigValues().DebugString())
 		}
@@ -726,7 +733,7 @@ func (bm *BasicModule) executeHook(h *hooks.ModuleHook, bindingType sh_op_types.
 		if valuesPatchResult.ValuesChanged {
 			logEntry.Debugf("Module hook '%s': validate module values before update", h.GetName())
 			// Validate schema for updated module values
-			validationErr := bm.valuesStorage.SetNewValues(bm.Name, valuesPatchResult.Values)
+			validationErr := bm.valuesStorage.PreCommitValues(bm.Name, valuesPatchResult.Values)
 			if validationErr != nil {
 				return multierror.Append(
 					fmt.Errorf("cannot apply values patch for module values"),
@@ -736,7 +743,7 @@ func (bm *BasicModule) executeHook(h *hooks.ModuleHook, bindingType sh_op_types.
 
 			// Save patch set if everything is ok.
 			//bm.dynamicValuesPatch = utils.AppendValuesPatch(bm.dynamicValuesPatch, valuesPatchResult.ValuesPatch)
-			bm.valuesStorage.ApplyDirtyValues()
+			bm.valuesStorage.CommitValues()
 			//newValues := bm.GetValues()
 
 			logEntry.Debugf("Module hook '%s': dynamic module '%s' values updated:\n%s", h.GetName(), bm.Name, bm.valuesStorage.GetValues().DebugString())
@@ -771,40 +778,41 @@ func (bm *BasicModule) handleModuleValuesPatch(currentValues utils.Values, value
 		return nil, fmt.Errorf("merge module '%s' values failed: %s", bm.Name, err)
 	}
 
+	// module values doesn't contain moduleName key as top level one
+	// but patches are still have, we have to use temporary map
+	// TODO: maybe we have to change ApplyValuesPatch function
+	tmpValues := utils.Values{
+		moduleValuesKey: currentValues,
+	}
+
 	// Apply new patches in Strict mode. Hook should not return 'remove' with nonexistent path.
-	newValues, valuesChanged, err := utils.ApplyValuesPatch(currentValues, valuesPatch, utils.Strict)
+	newValues, valuesChanged, err := utils.ApplyValuesPatch(tmpValues, valuesPatch, utils.Strict)
 	if err != nil {
 		return nil, fmt.Errorf("merge module '%s' values failed: %s", bm.Name, err)
 	}
 
+	newValues = newValues[moduleValuesKey].(map[string]interface{})
+
 	result := &moduleValuesMergeResult{
 		ModuleValuesKey: moduleValuesKey,
-		Values:          utils.Values{moduleValuesKey: make(map[string]interface{})},
+		Values:          newValues,
 		ValuesChanged:   valuesChanged,
 		ValuesPatch:     valuesPatch,
 	}
 
-	if newValues.HasKey(moduleValuesKey) {
-		_, ok := newValues[moduleValuesKey].(map[string]interface{})
-		if !ok {
-			return nil, fmt.Errorf("expected map at key '%s', got:\n%s", result.ModuleValuesKey, newValues.SectionByKey(moduleValuesKey).DebugString())
-		}
-		result.Values = newValues.SectionByKey(moduleValuesKey)
-	}
-
 	return result, nil
 }
 
 func (bm *BasicModule) ValidateAndSaveConfigValues(v utils.Values) error {
-	return bm.valuesStorage.SetNewValues(bm.Name, v)
+	return bm.valuesStorage.PreCommitConfigValues(bm.Name, v)
 }
 
 func (bm *BasicModule) ConfigValuesHaveChanges() bool {
 	return bm.valuesStorage.dirtyConfigValuesHasDiff()
 }
 
 func (bm *BasicModule) CommitConfigValuesChange() {
-	bm.valuesStorage.ApplyDirtyConfigValues()
+	bm.valuesStorage.CommitConfigValues()
 }
 
 func (bm *BasicModule) GetValues() utils.Values {
@@ -816,7 +824,7 @@ func (bm *BasicModule) GetConfigValues() utils.Values {
 }
 
 // Synchronization xxx
-// TODO: don't like this honestly, i think we can remove it
+// TODO: don't like this honestly, i think we can remake it
 func (bm *BasicModule) Synchronization() *SynchronizationState {
 	return bm.state.synchronizationState
 }

pkg/module_manager/models/modules/basic_test.go

@@ -0,0 +1,43 @@
+package modules
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/flant/addon-operator/pkg/utils"
+	"github.com/flant/addon-operator/pkg/values/validation"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestHandleModulePatch(t *testing.T) {
+	valuesStr := `
+foo: 
+  bar: baz
+`
+	value, err := utils.NewValuesFromBytes([]byte(valuesStr))
+	require.NoError(t, err)
+	vv := validation.NewValuesValidator()
+	vs := NewValuesStorage(value, vv)
+	bm := NewBasicModule("test-1", "/tmp/test", 100, true, vs, vv)
+
+	patch := utils.ValuesPatch{Operations: []*utils.ValuesPatchOperation{
+		{
+			Op:    "add",
+			Path:  "/test1/foo/xxx",
+			Value: json.RawMessage(`"yyy"`),
+		},
+		{
+			Op:    "remove",
+			Path:  "/test1/foo/bar",
+			Value: json.RawMessage(`"zxc"`),
+		},
+	}}
+	res, err := bm.handleModuleValuesPatch(bm.GetValues(), patch)
+	assert.True(t, res.ValuesChanged)
+	assert.YAMLEq(t, `
+foo: 
+  xxx: yyy
+`,
+		res.Values.AsString("yaml"))
+}

pkg/module_manager/models/modules/global.go

@@ -35,12 +35,15 @@ type GlobalModule struct {
 	dc              *hooks.HookExecutionDependencyContainer
 }
 
-func NewGlobalModule(hooksDir string, validator validator, dc *hooks.HookExecutionDependencyContainer) *GlobalModule {
+func NewGlobalModule(hooksDir string, vs *ValuesStorage, validator validator, dc *hooks.HookExecutionDependencyContainer) *GlobalModule {
+	if vs == nil {
+		vs = NewValuesStorage(utils.Values{}, validator)
+	}
 	return &GlobalModule{
 		hooksDir:      hooksDir,
 		byBinding:     make(map[sh_op_types.BindingType][]*hooks.GlobalHook),
 		byName:        make(map[string]*hooks.GlobalHook),
-		valuesStorage: NewValuesStorage(nil, validator), // TODO(yalosev): initial
+		valuesStorage: vs,
 		dc:            dc,
 	}
 }
@@ -185,7 +188,7 @@ func (gm *GlobalModule) executeHook(h *hooks.GlobalHook, bindingType sh_op_types
 		if configValuesPatchResult != nil && configValuesPatchResult.ValuesChanged {
 			logEntry.Debugf("Global hook '%s': validate global config values before update", h.GetName())
 			// Validate merged static and new values.
-			validationErr := gm.valuesStorage.SetNewConfigValues(gm.GetName(), configValuesPatchResult.Values)
+			validationErr := gm.valuesStorage.PreCommitConfigValues(gm.GetName(), configValuesPatchResult.Values)
 			//mergedValues := h.moduleManager.GlobalStaticAndNewValues(configValuesPatchResult.Values)
 			//validationErr := h.moduleManager.ValuesValidator.ValidateGlobalConfigValues(mergedValues)
 			if validationErr != nil {
@@ -198,7 +201,7 @@ func (gm *GlobalModule) executeHook(h *hooks.GlobalHook, bindingType sh_op_types
 				return fmt.Errorf("global hook '%s': set kube config failed: %s", h.GetName(), err)
 			}
 
-			gm.valuesStorage.ApplyDirtyConfigValues()
+			gm.valuesStorage.CommitConfigValues()
 
 			//h.moduleManager.UpdateGlobalConfigValues(configValuesPatchResult.Values)
 			// TODO(yalosev): save patches - UpdateGlobalDynamicValuesPatches
@@ -231,12 +234,12 @@ func (gm *GlobalModule) executeHook(h *hooks.GlobalHook, bindingType sh_op_types
 		// and no patches for 'global' section — valuesPatchResult will be nil in this case.
 		if valuesPatchResult != nil && valuesPatchResult.ValuesChanged {
 			logEntry.Debugf("Global hook '%s': validate global values before update", h.GetName())
-			validationErr := gm.valuesStorage.SetNewValues(gm.GetName(), valuesPatchResult.Values)
+			validationErr := gm.valuesStorage.PreCommitValues(gm.GetName(), valuesPatchResult.Values)
 			if validationErr != nil {
 				return fmt.Errorf("cannot apply values patch for global values: %w", validationErr)
 			}
 
-			gm.valuesStorage.ApplyDirtyValues()
+			gm.valuesStorage.CommitValues()
 
 			logEntry.Debugf("Global hook '%s': kube global values updated", h.GetName())
 			logEntry.Debugf("New global values:\n%s", gm.valuesStorage.GetValues().DebugString())
@@ -256,15 +259,15 @@ func (gm *GlobalModule) executeHook(h *hooks.GlobalHook, bindingType sh_op_types
 
 // TODO(yalosev): change name, because we don't save values here, just store them as dirty
 func (gm *GlobalModule) ValidateAndSaveConfigValues(v utils.Values) error {
-	return gm.valuesStorage.SetNewConfigValues(gm.GetName(), v)
+	return gm.valuesStorage.PreCommitConfigValues(gm.GetName(), v)
 }
 
 func (gm *GlobalModule) ConfigValuesHaveChanges() bool {
 	return gm.valuesStorage.dirtyConfigValuesHasDiff()
 }
 
 func (gm *GlobalModule) CommitConfigValuesChange() {
-	gm.valuesStorage.ApplyDirtyConfigValues()
+	gm.valuesStorage.CommitConfigValues()
 }
 func (gm *GlobalModule) GetValues() utils.Values {
 	return gm.valuesStorage.GetValues()
@@ -297,27 +300,27 @@ func (gm *GlobalModule) handlePatch(currentValues utils.Values, valuesPatch util
 		return nil, nil
 	}
 
+	// global values doesn't contain global key as top level one
+	// but patches are still have, we have to use temporary map
+	// TODO: maybe we have to change ApplyValuesPatch function
+	tmpValues := utils.Values{
+		utils.GlobalValuesKey: currentValues,
+	}
+
 	// Apply new patches in Strict mode. Hook should not return 'remove' with nonexistent path.
-	newValues, valuesChanged, err := utils.ApplyValuesPatch(currentValues, globalValuesPatch, utils.Strict)
+	newValues, valuesChanged, err := utils.ApplyValuesPatch(tmpValues, globalValuesPatch, utils.Strict)
 	if err != nil {
 		return nil, fmt.Errorf("merge global values failed: %s", err)
 	}
 
+	newValues = newValues[utils.GlobalValuesKey].(map[string]interface{})
+
 	result := &globalValuesPatchResult{
-		Values:        utils.Values{utils.GlobalValuesKey: make(map[string]interface{})},
+		Values:        newValues,
 		ValuesChanged: valuesChanged,
 		ValuesPatch:   globalValuesPatch,
 	}
 
-	if newValues.HasGlobal() {
-		_, ok := newValues[utils.GlobalValuesKey].(map[string]interface{})
-		if !ok {
-			return nil, fmt.Errorf("expected map at key '%s', got:\n%s", utils.GlobalValuesKey, newValues.Global().DebugString())
-		}
-
-		result.Values = newValues.Global()
-	}
-
 	return result, nil
 }
 

pkg/module_manager/models/modules/helm.go

@@ -59,15 +59,15 @@ type HelmModuleDependencies struct {
 func NewHelmModule(globalValues utils.Values, bm *BasicModule, tmpDir string, deps *HelmModuleDependencies) (*HelmModule, error) {
 	moduleValues := bm.GetValues()
 
-	resultValues := map[string]interface{}{
+	chartValues := map[string]interface{}{
 		"global": globalValues,
 		utils.ModuleNameToValuesKey(bm.GetName()): moduleValues,
 	}
 
 	hm := &HelmModule{
 		name:         bm.Name,
 		path:         bm.Path,
-		values:       resultValues,
+		values:       chartValues,
 		tmpDir:       tmpDir,
 		dependencies: deps,
 	}