Feat: Config Logic Enhancement #655
Conversation
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
daniel-hutao
changed the title
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
|
@aFlyBird0 Thanks for your very detailed code review work, I've made the appropriate changes and retested locally to make certain no new bugs were introduced. Please take a review again at this pr. |
| func LoadConfig(configFileName string) (*Config, error) { | ||
| // 1. read the original config file | ||
| originalConfigFileBytes, err := loadOriginalConfigFile(configFileName) | ||
| if err != nil { | ||
| log.Error(err) | ||
| log.Info("Maybe the default file doesn't exist or you forgot to pass your config file to the \"-f\" option?") | ||
| log.Info("See \"dtm help\" for more information.") | ||
| return nil, err | ||
| } | ||
| log.Debugf("Original general config: \n%s\n", string(configFileBytes)) | ||
|
|
||
| var gConfig ConfigFile | ||
| err = yaml.Unmarshal(configFileBytes, &gConfig) | ||
| // 2. split original config | ||
| coreConfigBytes, variablesConfigBytes, toolsConfigBytes, err := SplitConfigFileBytes(originalConfigFileBytes) | ||
| if err != nil { | ||
| log.Error("Please verify the format of your general config file.") | ||
| log.Errorf("Reading general config file failed. %s.", err) | ||
| return nil, err | ||
| } | ||
|
|
||
| errs := validateConfigFile(&gConfig) | ||
| if len(errs) != 0 { | ||
| for _, e := range errs { | ||
| log.Errorf("Config file validation failed: %s.", e) | ||
| } | ||
| return nil, nil | ||
| } | ||
| return renderConfigs(coreConfigBytes, variablesConfigBytes, toolsConfigBytes) | ||
| } |
| state: | ||
| --- | ||
| # core config | ||
| varFile: "" # If not empty, use the specified external variables config file |
There was a problem hiding this comment.
Maybe we should tell user that toolFile should be not empty when varFile is not empty. In docs or this yaml file. We can also do it later.
There was a problem hiding this comment.
The logic here is not that toolFile must be non-empty if varFile is not empty, but that if toolFile exists and var is used inside, varFile needs to exist. If toolFile and toolConfig does not exist, it does not matter whether varFile exists or not. We can "educate" the user later in one of the documents.
There was a problem hiding this comment.
The logic here is not that
toolFilemust be non-empty ifvarFileis not empty, but that iftoolFileexists and var is used inside,varFileneeds to exist. IftoolFileandtoolConfigdoes not exist, it does not matter whethervarFileexists or not. We can "educate" the user later in one of the documents.
Got it, thank you for explanation.
| func SplitConfigFileBytes(fileBytes []byte) (coreConfig []byte, varConfig []byte, toolConfig []byte, err error) { | ||
| splitBytes := bytes.Split(bytes.TrimPrefix(fileBytes, []byte("---")), []byte("---")) | ||
|
|
||
| switch len(splitBytes) { | ||
| case 1: | ||
| coreConfig = splitBytes[0] | ||
| case 2: | ||
| coreConfig = splitBytes[0] | ||
| toolConfig = splitBytes[1] | ||
| case 3: | ||
| coreConfig = splitBytes[0] | ||
| varConfig = splitBytes[1] | ||
| toolConfig = splitBytes[2] | ||
| default: | ||
| err = fmt.Errorf("invalid config format") | ||
| } | ||
|
|
||
| if len(coreConfig) == 0 { | ||
| err = fmt.Errorf("core config is empty") | ||
| } | ||
|
|
||
| return |
There was a problem hiding this comment.
A nice practice to use default value! It's very "Go"!
Signed-off-by: Daniel Hu tao.hu@merico.dev
Pre-Checklist
Note: please complete ALL items in the following checklist.
Description
configloader packageRelated Issues
close #596
related closed pr: #604
New Behavior (screenshots if needed)
See the test below.
Test
1. quickstart with single config
quickstart.yamlapplydelete2. quickstart with 2 config files
quickstart.yamltools-qs.yamlapplydelete3. gitops with single config file
gitops.yamlapplydelete4. gitops with 3 config files
gitops.yamlvariables-gitops.yamltools-gitops.yamlapplydelete