Updatse coalesce.go to use log/slog and update tests to pass

Signed-off-by: James Payne <jamoflaw@gmail.com>
pull/30774/head
James Payne 5 months ago
parent a842140ef6
commit 0714420bcf

@ -18,7 +18,7 @@ package util
import ( import (
"fmt" "fmt"
"log" "log/slog"
"github.com/mitchellh/copystructure" "github.com/mitchellh/copystructure"
"github.com/pkg/errors" "github.com/pkg/errors"
@ -47,7 +47,7 @@ func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, err
if err != nil { if err != nil {
return vals, err return vals, err
} }
return coalesce(log.Printf, chrt, valsCopy, "", false) return coalesce(chrt, valsCopy, "", false)
} }
// MergeValues is used to merge the values in a chart and its subcharts. This // MergeValues is used to merge the values in a chart and its subcharts. This
@ -69,7 +69,7 @@ func MergeValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error)
if err != nil { if err != nil {
return vals, err return vals, err
} }
return coalesce(log.Printf, chrt, valsCopy, "", true) return coalesce(chrt, valsCopy, "", true)
} }
func copyValues(vals map[string]interface{}) (Values, error) { func copyValues(vals map[string]interface{}) (Values, error) {
@ -96,13 +96,13 @@ type printFn func(format string, v ...interface{})
// Note, the merge argument specifies whether this is being used by MergeValues // Note, the merge argument specifies whether this is being used by MergeValues
// or CoalesceValues. Coalescing removes null values and their keys in some // or CoalesceValues. Coalescing removes null values and their keys in some
// situations while merging keeps the null values. // situations while merging keeps the null values.
func coalesce(printf printFn, ch *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) { func coalesce(ch *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) {
coalesceValues(printf, ch, dest, prefix, merge) coalesceValues(ch, dest, prefix, merge)
return coalesceDeps(printf, ch, dest, prefix, merge) return coalesceDeps(ch, dest, prefix, merge)
} }
// coalesceDeps coalesces the dependencies of the given chart. // coalesceDeps coalesces the dependencies of the given chart.
func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) { func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}, prefix string, merge bool) (map[string]interface{}, error) {
for _, subchart := range chrt.Dependencies() { for _, subchart := range chrt.Dependencies() {
if c, ok := dest[subchart.Name()]; !ok { if c, ok := dest[subchart.Name()]; !ok {
// If dest doesn't already have the key, create it. // If dest doesn't already have the key, create it.
@ -114,10 +114,10 @@ func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}
dvmap := dv.(map[string]interface{}) dvmap := dv.(map[string]interface{})
subPrefix := concatPrefix(prefix, chrt.Metadata.Name) subPrefix := concatPrefix(prefix, chrt.Metadata.Name)
// Get globals out of dest and merge them into dvmap. // Get globals out of dest and merge them into dvmap.
coalesceGlobals(printf, dvmap, dest, subPrefix, merge) coalesceGlobals(dvmap, dest, subPrefix, merge)
// Now coalesce the rest of the values. // Now coalesce the rest of the values.
var err error var err error
dest[subchart.Name()], err = coalesce(printf, subchart, dvmap, subPrefix, merge) dest[subchart.Name()], err = coalesce(subchart, dvmap, subPrefix, merge)
if err != nil { if err != nil {
return dest, err return dest, err
} }
@ -129,20 +129,20 @@ func coalesceDeps(printf printFn, chrt *chart.Chart, dest map[string]interface{}
// coalesceGlobals copies the globals out of src and merges them into dest. // coalesceGlobals copies the globals out of src and merges them into dest.
// //
// For convenience, returns dest. // For convenience, returns dest.
func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix string, _ bool) { func coalesceGlobals(dest, src map[string]interface{}, prefix string, _ bool) {
var dg, sg map[string]interface{} var dg, sg map[string]interface{}
if destglob, ok := dest[GlobalKey]; !ok { if destglob, ok := dest[GlobalKey]; !ok {
dg = make(map[string]interface{}) dg = make(map[string]interface{})
} else if dg, ok = destglob.(map[string]interface{}); !ok { } else if dg, ok = destglob.(map[string]interface{}); !ok {
printf("warning: skipping globals because destination %s is not a table.", GlobalKey) slog.Warn(fmt.Sprintf("warning: skipping globals because destination %s is not a table.", GlobalKey))
return return
} }
if srcglob, ok := src[GlobalKey]; !ok { if srcglob, ok := src[GlobalKey]; !ok {
sg = make(map[string]interface{}) sg = make(map[string]interface{})
} else if sg, ok = srcglob.(map[string]interface{}); !ok { } else if sg, ok = srcglob.(map[string]interface{}); !ok {
printf("warning: skipping globals because source %s is not a table.", GlobalKey) slog.Warn(fmt.Sprintf("warning: skipping globals because source %s is not a table.", GlobalKey))
return return
} }
@ -158,7 +158,7 @@ func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix st
dg[key] = vv dg[key] = vv
} else { } else {
if destvmap, ok := destv.(map[string]interface{}); !ok { if destvmap, ok := destv.(map[string]interface{}); !ok {
printf("Conflict: cannot merge map onto non-map for %q. Skipping.", key) slog.Warn(fmt.Sprintf("Conflict: cannot merge map onto non-map for %q. Skipping.", key))
} else { } else {
// Basically, we reverse order of coalesce here to merge // Basically, we reverse order of coalesce here to merge
// top-down. // top-down.
@ -166,13 +166,13 @@ func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix st
// In this location coalesceTablesFullKey should always have // In this location coalesceTablesFullKey should always have
// merge set to true. The output of coalesceGlobals is run // merge set to true. The output of coalesceGlobals is run
// through coalesce where any nils will be removed. // through coalesce where any nils will be removed.
coalesceTablesFullKey(printf, vv, destvmap, subPrefix, true) coalesceTablesFullKey(vv, destvmap, subPrefix, true)
dg[key] = vv dg[key] = vv
} }
} }
} else if dv, ok := dg[key]; ok && istable(dv) { } else if dv, ok := dg[key]; ok && istable(dv) {
// It's not clear if this condition can actually ever trigger. // It's not clear if this condition can actually ever trigger.
printf("key %s is table. Skipping", key) slog.Warn(fmt.Sprintf("key %s is table. Skipping", key))
} else { } else {
// TODO: Do we need to do any additional checking on the value? // TODO: Do we need to do any additional checking on the value?
dg[key] = val dg[key] = val
@ -192,7 +192,7 @@ func copyMap(src map[string]interface{}) map[string]interface{} {
// coalesceValues builds up a values map for a particular chart. // coalesceValues builds up a values map for a particular chart.
// //
// Values in v will override the values in the chart. // Values in v will override the values in the chart.
func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, prefix string, merge bool) { func coalesceValues(c *chart.Chart, v map[string]interface{}, prefix string, merge bool) {
subPrefix := concatPrefix(prefix, c.Metadata.Name) subPrefix := concatPrefix(prefix, c.Metadata.Name)
// Using c.Values directly when coalescing a table can cause problems where // Using c.Values directly when coalescing a table can cause problems where
@ -206,7 +206,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
// means there is a problem in the deep copying package or something // means there is a problem in the deep copying package or something
// wrong with c.Values. In this case we will use c.Values and report // wrong with c.Values. In this case we will use c.Values and report
// an error. // an error.
printf("warning: unable to copy values, err: %s", err) slog.Warn(fmt.Sprintf("warning: unable to copy values, err: %s", err))
vc = c.Values vc = c.Values
} else { } else {
vc, ok = valuesCopy.(map[string]interface{}) vc, ok = valuesCopy.(map[string]interface{})
@ -214,7 +214,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
// c.Values has a map[string]interface{} structure. If the copy of // c.Values has a map[string]interface{} structure. If the copy of
// it cannot be treated as map[string]interface{} there is something // it cannot be treated as map[string]interface{} there is something
// strangely wrong. Log it and use c.Values // strangely wrong. Log it and use c.Values
printf("warning: unable to convert values copy to values type") slog.Warn(fmt.Sprintf("warning: unable to convert values copy to values type"))
vc = c.Values vc = c.Values
} }
} }
@ -234,7 +234,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
// If the original value is nil, there is nothing to coalesce, so we don't print // If the original value is nil, there is nothing to coalesce, so we don't print
// the warning // the warning
if val != nil { if val != nil {
printf("warning: skipped value for %s.%s: Not a table.", subPrefix, key) slog.Warn(fmt.Sprintf("warning: skipped value for %s.%s: Not a table.", subPrefix, key))
} }
} else { } else {
// If the key is a child chart, coalesce tables with Merge set to true // If the key is a child chart, coalesce tables with Merge set to true
@ -242,7 +242,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
// Because v has higher precedence than nv, dest values override src // Because v has higher precedence than nv, dest values override src
// values. // values.
coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge) coalesceTablesFullKey(dest, src, concatPrefix(subPrefix, key), merge)
} }
} }
} else { } else {
@ -265,17 +265,17 @@ func childChartMergeTrue(chrt *chart.Chart, key string, merge bool) bool {
// //
// dest is considered authoritative. // dest is considered authoritative.
func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} { func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} {
return coalesceTablesFullKey(log.Printf, dst, src, "", false) return coalesceTablesFullKey(dst, src, "", false)
} }
func MergeTables(dst, src map[string]interface{}) map[string]interface{} { func MergeTables(dst, src map[string]interface{}) map[string]interface{} {
return coalesceTablesFullKey(log.Printf, dst, src, "", true) return coalesceTablesFullKey(dst, src, "", true)
} }
// coalesceTablesFullKey merges a source map into a destination map. // coalesceTablesFullKey merges a source map into a destination map.
// //
// dest is considered authoritative. // dest is considered authoritative.
func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, prefix string, merge bool) map[string]interface{} { func coalesceTablesFullKey(dst, src map[string]interface{}, prefix string, merge bool) map[string]interface{} {
// When --reuse-values is set but there are no modifications yet, return new values // When --reuse-values is set but there are no modifications yet, return new values
if src == nil { if src == nil {
return dst return dst
@ -293,12 +293,12 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref
dst[key] = val dst[key] = val
} else if istable(val) { } else if istable(val) {
if istable(dv) { if istable(dv) {
coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge) coalesceTablesFullKey(dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge)
} else { } else {
printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val) slog.Warn(fmt.Sprintf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val))
} }
} else if istable(dv) && val != nil { } else if istable(dv) && val != nil {
printf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val) slog.Warn(fmt.Sprintf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val))
} }
} }
return dst return dst

@ -18,7 +18,7 @@ package util
import ( import (
"encoding/json" "encoding/json"
"fmt" "log/slog"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -702,22 +702,26 @@ func TestCoalesceValuesWarnings(t *testing.T) {
}, },
} }
warnings := make([]string, 0) // Capture logs emitted from slog
printf := func(format string, v ...interface{}) { defaultLogger := slog.Default()
t.Logf(format, v...) logCaptureHandler := NewLogCaptureHandler(nil)
warnings = append(warnings, fmt.Sprintf(format, v...)) slog.SetDefault(slog.New(logCaptureHandler))
}
_, err := coalesce(printf, c, vals, "", false) _, err := coalesce(c, vals, "", false)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
logOutput := logCaptureHandler.Records().AsMessageSlice()
t.Logf("vals: %v", vals) t.Logf("vals: %v", vals)
assert.Contains(t, warnings, "warning: skipped value for level1.level2.level3.boat: Not a table.") assert.Contains(t, logOutput, "warning: skipped value for level1.level2.level3.boat: Not a table.")
assert.Contains(t, warnings, "warning: destination for level1.level2.level3.spear.tip is a table. Ignoring non-table value (true)") assert.Contains(t, logOutput, "warning: destination for level1.level2.level3.spear.tip is a table. Ignoring non-table value (true)")
assert.Contains(t, warnings, "warning: cannot overwrite table with non table for level1.level2.level3.spear.sail (map[cotton:true])") assert.Contains(t, logOutput, "warning: cannot overwrite table with non table for level1.level2.level3.spear.sail (map[cotton:true])")
// Reset and set the default logger back to its original state
logCaptureHandler.Reset()
slog.SetDefault(defaultLogger)
} }
func TestConcatPrefix(t *testing.T) { func TestConcatPrefix(t *testing.T) {

@ -0,0 +1,131 @@
/*
Copyright The Helm Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package util
import (
"context"
"log/slog"
"sync"
)
// A simple handler to record the log messages emitted by slog.
// Intended to be used only by tests
type LogCaptureHandler struct {
mu *sync.Mutex
records []slog.Record
opts slog.HandlerOptions
}
type LogCaptureSlice []slog.Record
func NewLogCaptureHandler(opts *slog.HandlerOptions) *LogCaptureHandler {
if opts == nil {
opts = &slog.HandlerOptions{}
}
return &LogCaptureHandler{
mu: &sync.Mutex{},
opts: *opts,
}
}
func (h *LogCaptureHandler) Enabled(_ context.Context, _ slog.Level) bool {
// Handle all levels
return true
}
func (h *LogCaptureHandler) Handle(_ context.Context, r slog.Record) error {
h.mu.Lock()
defer h.mu.Unlock()
h.records = append(h.records, r)
return nil
}
// This is intentionally not following "standard" usage of WithAttrs and WithGroup with regard
// to handlers since we want to be able to capture all messages emitted from slog sources rather
// than each sub-logger generating its own slice of records
func (h *LogCaptureHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
return &LogCaptureHandler{
// Copy the mutex as we could be writing to the same records from multiple threads
mu: h.mu,
opts: slog.HandlerOptions{
Level: h.opts.Level,
AddSource: h.opts.AddSource,
ReplaceAttr: h.opts.ReplaceAttr,
},
// Copy the same records as we want to be able to collate _all_ the log messages
// emitted by slog sources
records: h.records,
}
}
// See comment on WithAttrs regarding reusing the mutex + records slice
func (h *LogCaptureHandler) WithGroup(name string) slog.Handler {
return &LogCaptureHandler{
mu: h.mu,
opts: slog.HandlerOptions{
Level: h.opts.Level,
AddSource: h.opts.AddSource,
ReplaceAttr: h.opts.ReplaceAttr,
},
// Copy the same records as we want to be able to collate _all_ the log messages
// emitted by slog sources
records: h.records,
}
}
// Records returns a copy of the captured log records
func (h *LogCaptureHandler) Records() *LogCaptureSlice {
h.mu.Lock()
defer h.mu.Unlock()
records := make([]slog.Record, len(h.records))
copy(records, h.records)
result := LogCaptureSlice(records)
return &result
}
// Converts the captured log records into a map of log message and log level
func (l *LogCaptureSlice) AsMessageLevelMap() map[string]slog.Level {
result := make(map[string]slog.Level)
for _, record := range *l {
result[record.Message] = record.Level
}
return result
}
// Converts the captured log records into a slice of log messages
func (l *LogCaptureSlice) AsMessageSlice() []string {
result := make([]string, 0)
for _, record := range *l {
result = append(result, record.Message)
}
return result
}
// Reset clears the captured log messages.
func (h *LogCaptureHandler) Reset() {
h.mu.Lock()
defer h.mu.Unlock()
h.records = make([]slog.Record, 0)
}
Loading…
Cancel
Save