Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(reset): Add cycle detector in reset.go #703

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions loader/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ import (
)

type ResetProcessor struct {
target interface{}
paths []tree.Path
target interface{}
paths []tree.Path
visitedNodes map[*yaml.Node]string
}

// UnmarshalYAML implement yaml.Unmarshaler
func (p *ResetProcessor) UnmarshalYAML(value *yaml.Node) error {
resolved, err := p.resolveReset(value, tree.NewPath())
p.visitedNodes = nil
if err != nil {
return err
}
Expand All @@ -41,10 +43,28 @@ func (p *ResetProcessor) UnmarshalYAML(value *yaml.Node) error {

// resolveReset detects `!reset` tag being set on yaml nodes and record position in the yaml tree
func (p *ResetProcessor) resolveReset(node *yaml.Node, path tree.Path) (*yaml.Node, error) {
pathStr := path.String()
// If the path contains "<<", removing the "<<" element and merging the path
if strings.Contains(path.String(), ".<<") {
path = tree.NewPath(strings.Replace(path.String(), ".<<", "", 1))
if strings.Contains(pathStr, ".<<") {
path = tree.NewPath(strings.Replace(pathStr, ".<<", "", 1))
}

// Check for cycle
if p.visitedNodes == nil {
p.visitedNodes = make(map[*yaml.Node]string)
}

// Check for cycle by seeing if the node has already been visited at this path
if previousPath, found := p.visitedNodes[node]; found {
// If the current node has been visited, we have a cycle if the previous path is a prefix
if strings.HasPrefix(pathStr, previousPath) {
return nil, fmt.Errorf("cycle detected at path: %s", pathStr)
}
}

// Mark the current node as visited
p.visitedNodes[node] = pathStr

// If the node is an alias, We need to process the alias field in order to consider the !override and !reset tags
if node.Kind == yaml.AliasNode {
return p.resolveReset(node.Alias, path)
Expand Down
21 changes: 21 additions & 0 deletions loader/reset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,24 @@ networks:
assert.NilError(t, err)
assert.Check(t, p.Networks["test"].External == false)
}

func TestResetCycle(t *testing.T) {
_, err := Load(
types.ConfigDetails{
ConfigFiles: []types.ConfigFile{
{
Filename: "(inline)",
Content: []byte(`
x-healthcheck: &healthcheck
egress-service:
<<: *healthcheck
`),
},
},
}, func(options *Options) {
options.SkipNormalization = true
options.SkipConsistencyCheck = true
},
)
assert.Error(t, err, "cycle detected at path: x-healthcheck.egress-service")
}