-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix issue 186 - default is ignored (#420)
## Description <!-- Please do not leave this blank This PR [adds/removes/fixes/replaces] the [feature/bug/etc]. --> Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [x] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑💻 Code Refactor - [ ] 🔥 Performance Improvements - [x] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Tickets & Documents <!-- Please use this format link issue numbers: Fixes #123 https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword --> - Related Issue # (issue) - Closes # 186 - Fixes # 186 > Remove if not applicable ## Screenshots <!-- Visual changes require screenshots --> ## Added tests? - [x ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ## Added to documentation? - [ ] 📜 README.md - [x ] 🙅 no documentation needed ## Checklist: - [x ] My code follows the style guidelines of this project - [ x] I have performed a self-review of my code - [ x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x ] My changes generate no new warnings - [ x] I have added tests that prove my fix is effective or that my feature works - [ x] New and existing unit tests pass locally with my changes ** - [ ] Any dependent changes have been merged and published in downstream modules ** NOTE: Tests in screenshot below are failing on the main branch as well. So while not all tests are passing with my change it is not the case that I added any new failures.  --------- Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
- Loading branch information
Showing
7 changed files
with
257 additions
and
40 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| // SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package controllers | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "container/list" | ||
| "fmt" | ||
|
|
||
| "github.com/mikefarah/yq/v4/pkg/yqlib" | ||
| "github.com/open-component-model/ocm/pkg/contexts/ocm/utils/localize" | ||
| ocmruntime "github.com/open-component-model/ocm/pkg/runtime" | ||
| ) | ||
|
|
||
| // Key we store spiff rules under within spiff template doc. | ||
| const ocmAdjustmentsTemplateKey = "ocmAdjustmentsTemplateKey" | ||
|
|
||
| // makeYqNode unmarshalls bytes into a CandidateNode | ||
| // In order to make debugging easier later during yaml processing we take in | ||
| // file name, document index and file index values which are then set on | ||
| // the CandidateNode. | ||
| func makeYqNode(docBytes []byte, fileName string, docIndex, fileIndex uint) (*yqlib.CandidateNode, error) { | ||
| yamlPreferences := yqlib.NewDefaultYamlPreferences() | ||
| yamlDecoder := yqlib.NewYamlDecoder(yamlPreferences) | ||
| rdr := bytes.NewBuffer(docBytes) | ||
|
|
||
| if err := yamlDecoder.Init(rdr); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| ret, err := yamlDecoder.Decode() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| ret.SetDocument(docIndex) | ||
| ret.SetFilename(fileName) | ||
| ret.SetFileIndex(int(fileIndex)) | ||
|
|
||
| return ret, nil | ||
| } | ||
|
|
||
| // spiffTemplateDoc is a wrapper around CandidateNode. Its purpose is to make the code in | ||
| // MutationReconcileLooper::generateSubstitutions easier to read. | ||
| // | ||
| // spiffTemplateDoc also represents a spiff++ template with a particular structure : | ||
| // ( defaults yaml merged with config values yaml ) | ||
| // + sequence of ocm controller conifg rules stored under the key 'ocmAdjustmentsTemplateKey'. | ||
| type spiffTemplateDoc yqlib.CandidateNode | ||
|
|
||
| // Store subst, in json form, under key ocmAdjustmentsTemplateKey. | ||
| func (s *spiffTemplateDoc) addSpiffRules(subst []localize.Substitution) error { | ||
| var err error | ||
|
|
||
| var adjustmentBytes []byte | ||
| if adjustmentBytes, err = ocmruntime.DefaultJSONEncoding.Marshal(subst); err != nil { | ||
| return fmt.Errorf("failed to marshal substitutions: %w", err) | ||
| } | ||
|
|
||
| var adjustmentsDoc *yqlib.CandidateNode | ||
|
|
||
| const fileIndex = 2 | ||
| if adjustmentsDoc, err = makeYqNode(adjustmentBytes, "adjustments", 0, fileIndex); err != nil { | ||
| return fmt.Errorf("failed to marshal adjustments: %w", err) | ||
| } | ||
|
|
||
| ((*yqlib.CandidateNode)(s)).AddKeyValueChild( | ||
| &yqlib.CandidateNode{ | ||
| Kind: yqlib.ScalarNode, | ||
| Value: ocmAdjustmentsTemplateKey, | ||
| }, | ||
| adjustmentsDoc) | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (s *spiffTemplateDoc) marshal() ([]byte, error) { | ||
| encoder := yqlib.NewYamlEncoder(yqlib.NewDefaultYamlPreferences()) | ||
| buf := bytes.NewBuffer([]byte{}) | ||
| pw := yqlib.NewSinglePrinterWriter(buf) | ||
| p := yqlib.NewPrinter(encoder, pw) | ||
|
|
||
| // Often yq is working with list of nodes because matches what | ||
| // yaml files are. Yaml files are sequences of yaml documents. | ||
| // This is one of those case. | ||
| nodes := list.New() | ||
| nodes.PushBack(((*yqlib.CandidateNode)(s))) | ||
|
|
||
| if err := p.PrintResults(nodes); err != nil { | ||
| return nil, fmt.Errorf("failed to print results: %w", err) | ||
| } | ||
|
|
||
| return buf.Bytes(), nil | ||
| } | ||
|
|
||
| // mergeDefaultsAndConfigValues unmarshals defaults and configValues to yaml | ||
| // and then returns the deep merge result produced by yqlib. | ||
| func mergeDefaultsAndConfigValues(defaults, configValues []byte) (*spiffTemplateDoc, error) { | ||
| var err error | ||
| var defaultsDoc, configValuesDoc *yqlib.CandidateNode | ||
|
|
||
| if defaultsDoc, err = makeYqNode(defaults, "defaults", 0, 0); err != nil { | ||
| return nil, fmt.Errorf("failed to make default document: %w", err) | ||
| } | ||
|
|
||
| if configValuesDoc, err = makeYqNode(configValues, "values", 0, 1); err != nil { | ||
| return nil, fmt.Errorf("failed to parse values values: %w", err) | ||
| } | ||
|
|
||
| evaluator := yqlib.NewAllAtOnceEvaluator() | ||
| var mergeResult *list.List | ||
| const mergeExpression = ". as $item ireduce({}; . * $item )" | ||
|
|
||
| // We get back a list because yq supports expressions that produce | ||
| // multiple yaml documents. | ||
| // And unfortunately golang lists aren't type safe. | ||
| // So now we have all these assertions to make sure we got back a single | ||
| // yaml document like we expect. | ||
| if mergeResult, err = evaluator.EvaluateNodes(mergeExpression, defaultsDoc, configValuesDoc); err != nil { | ||
| return nil, fmt.Errorf("failed to evaluate nodes: %w", err) | ||
| } | ||
|
|
||
| if docCount := mergeResult.Len(); docCount != 1 { | ||
| return nil, fmt.Errorf("merge of defaults and config has incorrect doc count. docCount: %v expression: '%v' defaults: '%v' config values '%v", | ||
| docCount, mergeExpression, defaultsDoc, configValuesDoc) | ||
| } | ||
|
|
||
| if value := mergeResult.Front().Value; value == nil { | ||
| return nil, fmt.Errorf("merge of defaults and config values produced nil result. expression: '%v' defaults: '%v' config values '%v", | ||
| mergeExpression, defaultsDoc, configValuesDoc) | ||
| } else if node, ok := value.(*yqlib.CandidateNode); ok { | ||
| return ((*spiffTemplateDoc)(node)), nil | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("merge of defaults and config values did not produce *CandidateNode. expression: '%v' defaults: '%v' config values '%v", | ||
| mergeExpression, defaultsDoc, configValuesDoc) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| //go:build test | ||
|
|
||
| // SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package controllers | ||
|
|
||
| import ( | ||
| glog "gopkg.in/op/go-logging.v1" | ||
| ) | ||
|
|
||
| // In production main func will take care of setting the log yq-lib log level. | ||
| // In test we count on this to set the default log level for all tests. | ||
| // We do this because yqlib log is very chatty even if it is sometimes very useful. | ||
| // Of course in an individual test one could change the level and reset to the default | ||
| // via a defer call. | ||
| func init() { | ||
| glog.SetLevel(glog.WARNING, "yq-lib") | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.