From e3d90cd323c08b34cf7fe2828b453bd75a8cf84b Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Thu, 30 Jan 2025 09:12:01 -0700 Subject: [PATCH] Prefer parent profile values over child profile values. - Fixes: #1672 Signed-off-by: Jonathon Anderson --- CHANGELOG.md | 1 + internal/pkg/node/mergo.go | 83 ++++++++++++++++++--------------- internal/pkg/node/mergo_test.go | 60 ++++++++++++++++++++---- 3 files changed, 96 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b4b495..4918c797 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Show each overlay only once, even when both site and distribution versions exist. #1675 - Remove a redundant "Building image" log message after image exec. #1694 - Don't populate NetDevs[].Type or NetDevs[].Netmask during upgrade. #1661 +- Prefer parent profile values over child profile values. #1672 ## v4.6.0rc1, 2025-01-29 diff --git a/internal/pkg/node/mergo.go b/internal/pkg/node/mergo.go index 79e20d11..7f0b3cce 100644 --- a/internal/pkg/node/mergo.go +++ b/internal/pkg/node/mergo.go @@ -8,57 +8,64 @@ import ( "dario.cat/mergo" "github.com/mohae/deepcopy" - "github.com/warewulf/warewulf/internal/pkg/util" "github.com/warewulf/warewulf/internal/pkg/wwlog" ) -// getNodeProfiles retrieves a list of profile IDs associated with a specific node ID. -// It retrives nested profiles and ensures the list is cleaned of duplicates -// and negations (denoted with a '~' prefix). +// getNodeProfiles returns a deduplicated list of profile identifiers associated with the given node. +// It expands nested profiles from the node's profile list and then cleans the result by removing +// duplicate entries and any profiles prefixed with '~' (which denote negated profiles). +// +// If the specified node is not found in the configuration, the function returns an empty slice. // // Parameters: -// - id: The identifier of the node whose profiles are to be retrieved. +// +// id - the identifier of the node whose profiles are to be retrieved. // // Returns: -// - A slice of profile IDs associated with the given node ID. +// +// A slice of profile IDs associated with the given node, with nested profiles expanded and cleaned. func (config *NodesYaml) getNodeProfiles(id string) (profiles []string) { - if node, ok := config.Nodes[id]; ok { - for _, profileID := range node.Profiles { - profiles = cleanList(append(profiles, profileID)) - if !strings.HasPrefix(profileID, "~") { - profiles = config.appendProfileProfiles(profiles, profileID) - } - } - } - return cleanList(profiles) -} + visited := make(map[string]bool) -// appendProfileProfiles recursively appends profile IDs associated with a given profile ID -// to the provided list of profile IDs. It recursively processes nested profiles and ensures -// the list is cleaned of duplicates and negations (denoted with a '~' prefix). -// -// Profiles are only added if they do not already exist in the list. -// -// Parameters: -// - profiles: A slice of strings representing the current list of profiles by ID. -// - id: The identifier of the profile whose associated profiles are to be appended. -// -// Returns: -// - A slice of strings containing the updated list of profile IDs. -func (config *NodesYaml) appendProfileProfiles(profiles []string, id string) []string { - if profile, ok := config.NodeProfiles[id]; ok { - for _, subID := range profile.Profiles { - if !util.InSlice(profiles, subID) { - profiles = cleanList(append(profiles, subID)) - if !strings.HasPrefix(subID, "~") { - profiles = config.appendProfileProfiles(profiles, subID) - } - } - } + if node, ok := config.Nodes[id]; ok { + profiles = cleanList(config.getProfilesProfiles(node.Profiles, visited)) } return profiles } +// getProfilesProfiles recursively expands a list of profile identifiers by processing their nested profiles. +// For each profile ID in the input slice: +// - If the profile ID begins with '~', it is treated as a negation and appended directly to the output. +// - If the profile ID has already been processed (as tracked by the visited map), it is skipped to prevent duplication and infinite recursion. +// - Otherwise, the profile ID is marked as visited. If a corresponding profile exists in the configuration's NodeProfiles, +// its nested profiles are recursively processed and appended to the output before appending the profile ID itself. +// +// Parameters: +// +// input - A slice of profile IDs to be expanded. +// visited - A map tracking profile IDs that have already been processed to avoid duplicates and cycles. +// +// Returns: +// +// A slice of profile IDs that includes the original and all recursively expanded nested profiles. +func (config *NodesYaml) getProfilesProfiles(input []string, visited map[string]bool) (output []string) { + for _, id := range input { + if strings.HasPrefix(id, "~") { + output = append(output, id) + continue + } else if visited[id] { + continue + } else { + visited[id] = true + if profile, ok := config.NodeProfiles[id]; ok { + output = append(output, config.getProfilesProfiles(profile.Profiles, visited)...) + } + output = append(output, id) + } + } + return output +} + type Transformer struct{} func (t Transformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { diff --git a/internal/pkg/node/mergo_test.go b/internal/pkg/node/mergo_test.go index ea3bf3c4..9b507d39 100644 --- a/internal/pkg/node/mergo_test.go +++ b/internal/pkg/node/mergo_test.go @@ -82,7 +82,7 @@ nodeprofiles: profiles: - p2`, node: "n1", - profiles: []string{"p1", "p2"}, + profiles: []string{"p2", "p1"}, }, "double nested profile": { nodesConf: ` @@ -98,7 +98,25 @@ nodeprofiles: profiles: - p3`, node: "n1", - profiles: []string{"p1", "p2", "p3"}, + profiles: []string{"p3", "p2", "p1"}, + }, + "double negated profile": { + nodesConf: ` +nodes: + n1: + profiles: + - p1 + - p2 + - p3 +nodeprofiles: + p2: + profiles: + - ~p1 + p3: + profiles: + - ~p1`, + node: "n1", + profiles: []string{"p2", "p3"}, }, "negated nested profile": { nodesConf: ` @@ -106,18 +124,22 @@ nodes: n1: profiles: - p1 + - p4 nodeprofiles: p1: profiles: - p2 p2: profiles: - - "~p2" - - p3`, + - p3 + p3: {} + p4: + profiles: + - ~p2`, node: "n1", - profiles: []string{"p1", "p3"}, + profiles: []string{"p3", "p1", "p4"}, }, - "cicular nested profile": { + "circular nested profile": { nodesConf: ` nodes: n1: @@ -134,9 +156,9 @@ nodeprofiles: profiles: - p1`, node: "n1", - profiles: []string{"p1", "p2", "p3"}, + profiles: []string{"p3", "p2", "p1"}, }, - "cicular nested profile negation": { + "circular nested profile negation": { nodesConf: ` nodes: n1: @@ -154,7 +176,7 @@ nodeprofiles: profiles: - p1`, node: "n1", - profiles: []string{"p2", "p3", "p1"}, + profiles: []string{"p3", "p2"}, }, "repeated nested profile": { nodesConf: ` @@ -175,7 +197,7 @@ nodeprofiles: profiles: - pa2`, node: "n1", - profiles: []string{"pa1", "pb1", "pb2", "pa2"}, + profiles: []string{"pa1", "pb2", "pb1"}, }, } @@ -281,6 +303,24 @@ nodeprofiles: source: "SUPERSEDED", value: "n1 comment", }, + "nested profile comments": { + nodesConf: ` +nodes: + n1: + profiles: + - p2 +nodeprofiles: + p1: + comment: p1 comment + p2: + profiles: + - p1 + comment: p2 comment`, + node: "n1", + field: "Comment", + source: "p2", + value: "p2 comment", + }, "node kernel args": { nodesConf: ` nodes: