diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f3ec2a70..e02c85e7e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#6775](https://github.com/apache/trafficcontrol/issues/6775) *Traffic Ops* Invalid "orgServerFqdn" in Delivery Service creation/update causes Internal Server Error - [#6695](https://github.com/apache/trafficcontrol/issues/6695) *Traffic Control Cache Config (t3c)* Directory creation was erroneously reporting an error when actually succeeding. - [#7411](https://github.com/apache/trafficcontrol/pull/7411) *Traffic Control Cache Config (t3c)* Fixed issue with wrong parent ordering with MSO non-topology delivery services. +- [#7425](https://github.com/apache/trafficcontrol/pull/7425) *Traffic Control Cache Config (t3c)* Fixed issue with layered profile iteration being done in the wrong order. ## [7.0.0] - 2022-07-19 ### Added diff --git a/lib/go-atscfg/atscfg.go b/lib/go-atscfg/atscfg.go index 91dbdde4cf..457b662455 100644 --- a/lib/go-atscfg/atscfg.go +++ b/lib/go-atscfg/atscfg.go @@ -848,13 +848,13 @@ func layerProfilesFromMap(profileNames []string, params []parameterWithProfilesM } layeredParamMap := map[ParamKey]tc.Parameter{} - - for _, profileName := range profileNames { - profileParams := allProfileParams[profileName] + // profileNames is ordered, we need to iterate through this backwards + // because we need subsequent params on other profiles to override previous ones, + // this will provide the proper "layering" that we want. + for i := len(profileNames) - 1; i >= 0; i-- { + profileParams := allProfileParams[profileNames[i]] for _, param := range profileParams { paramkey := getParamKey(param) - // because profileNames is ordered, this will cause subsequent params - // on other profiles to override previous ones, "layering" like we want. layeredParamMap[paramkey] = param } } diff --git a/lib/go-atscfg/atscfg_test.go b/lib/go-atscfg/atscfg_test.go index db0f6bb523..4075b3f41d 100644 --- a/lib/go-atscfg/atscfg_test.go +++ b/lib/go-atscfg/atscfg_test.go @@ -185,18 +185,21 @@ func TestLayerProfiles(t *testing.T) { } // per the params, on the layered profiles "FOO,BAR,BAZ" (in that order): - // (1) beta in BAR should override alpha FOO, - // because they share the ConfigFile+Name key and BAR is later in the layering - // (2) gamma should be added, but not override + // we iterate through params in reverse order because FOO will have the highest priority + // then BAR then BAZ, any duplicate params that exist with the same ConfigFile+Name + // in subsequent profiles can be overwritten by the next higher profile + // (1) alpha in FOO, should override beta in BAR + // because they share the ConfigFile+Name key and FOO is later in the layering + // (2) gamma should be added, but not overridden // because the key is ConfigFile+Name, which isn't on any previous profile - // (3) epsilon in BAZ should override delta in BAR - // because they share the ConfigFile+Name key and BAZ is later in the layering + // (3) delta in BAR should override epsilon in BAZ + // because they share the ConfigFile+Name key and BAR is later in the layering // - this tests the parameters being in a different order than (1) // (4) zeta should be added, but not override // because the key is ConfigFile+Name, which isn't on any previous profile // - this tests the name matching but not config file, reverse of (2). - // (5) theta in BAZ should override eta and iota in FOO and BAR - // because they share the ConfigFile+Name key and BAZ is last in the layering + // (5) iota in FOO should override theta in BAZ and eta and BAR + // because they share the ConfigFile+Name key and FOO is last in the layering // - this tests multiple overrides layeredParams, err := LayerProfiles(profileNames, allParams) @@ -209,32 +212,32 @@ func TestLayerProfiles(t *testing.T) { vals[param.Value] = struct{}{} } - if _, ok := vals["alpha"]; ok { - t.Errorf("expected: param 'beta' to override 'alpha', actual: alpha in layered parameters") + if _, ok := vals["beta"]; ok { + t.Errorf("expected: param 'alpha' to override 'beta', actual: beta in layered parameters") } - if _, ok := vals["beta"]; !ok { - t.Errorf("expected: param 'beta' to override 'alpha', actual: beta not in layered parameters") + if _, ok := vals["alpha"]; !ok { + t.Errorf("expected: param 'alpha' to override 'beta', actual: alpha not in layered parameters") } if _, ok := vals["gamma"]; !ok { t.Errorf("expected: param 'gamma' with no ConfigFile+Name in another profile to be in layered parameters, actual: gamma not in layered parameters") } - if _, ok := vals["delta"]; ok { - t.Errorf("expected: param 'epsilon' to override 'delta' in prior profile, actual: delta in layered parameters") + if _, ok := vals["epsilon"]; ok { + t.Errorf("expected: param 'delta' to override 'epsilon' in prior profile, actual: epsilon in layered parameters") } - if _, ok := vals["epsilon"]; !ok { - t.Errorf("expected: param 'epsilon' to override 'delta' in prior profile, actual: epsilon not in layered parameters") + if _, ok := vals["delta"]; !ok { + t.Errorf("expected: param 'delta' to override 'epsilon' in prior profile, actual: delta not in layered parameters") } if _, ok := vals["zeta"]; !ok { t.Errorf("expected: param 'zeta' with no ConfigFile+Name in another profile to be in layered parameters, actual: zeta not in layered parameters") } - if _, ok := vals["theta"]; !ok { - t.Errorf("expected: param 'theta' to override 'eta' and 'iota' in prior profile, actual: theta not in layered parameters") + if _, ok := vals["iota"]; !ok { + t.Errorf("expected: param 'iota' to override 'eta' and 'theta' in prior profile, actual: iota not in layered parameters") } if _, ok := vals["eta"]; ok { - t.Errorf("expected: param 'theta' to override 'eta' and 'iota' in prior profile, actual: eta in layered parameters") + t.Errorf("expected: param 'iota' to override 'eta' and 'theta' in prior profile, actual: eta in layered parameters") } - if _, ok := vals["iota"]; ok { - t.Errorf("expected: param 'theta' to override 'eta' and 'iota' in prior profile, actual: iota in layered parameters") + if _, ok := vals["theta"]; ok { + t.Errorf("expected: param 'iota' to override 'eta' and 'theta' in prior profile, actual: theta in layered parameters") } } diff --git a/lib/go-atscfg/parentdotconfig_test.go b/lib/go-atscfg/parentdotconfig_test.go index a37a9c885b..6dc100dbd9 100644 --- a/lib/go-atscfg/parentdotconfig_test.go +++ b/lib/go-atscfg/parentdotconfig_test.go @@ -3734,13 +3734,13 @@ func TestMakeParentDotConfigTopologiesServerMultipleProfileParams(t *testing.T) Name: ParentConfigCacheParamWeight, ConfigFile: "parent.config", Value: "100", - Profiles: []byte(`["serverprofile0"]`), + Profiles: []byte(`["serverprofile1"]`), }, tc.Parameter{ Name: ParentConfigCacheParamWeight, ConfigFile: "parent.config", Value: "200", - Profiles: []byte(`["serverprofile1"]`), + Profiles: []byte(`["serverprofile0"]`), }, }