fix(engine): propagate and handle chart accessor errors

Ensure that errors returned by NewAccessor are propagated and handled correctly rather than ignored, which prevents a potential nil-pointer dereference panic in recAllTpls. Added TestRenderInvalidChartType to prevent future regressions.

Signed-off-by: Ankit Pramanik <59945244+ankit98040@users.noreply.github.com>
pull/32167/head
Ankit Pramanik 4 months ago
parent 4dec37abd2
commit 0211dc877b

@ -77,7 +77,10 @@ func New(config *rest.Config) Engine {
// section contains a value named "bar", that value will be passed on to the // section contains a value named "bar", that value will be passed on to the
// bar chart during render time. // bar chart during render time.
func (e Engine) Render(chrt ci.Charter, values common.Values) (map[string]string, error) { func (e Engine) Render(chrt ci.Charter, values common.Values) (map[string]string, error) {
tmap := allTemplates(chrt, values) tmap, err := allTemplates(chrt, values)
if err != nil {
return nil, err
}
return e.render(tmap) return e.render(tmap)
} }
@ -521,22 +524,24 @@ func (p byPathLen) Less(i, j int) bool {
// allTemplates returns all templates for a chart and its dependencies. // allTemplates returns all templates for a chart and its dependencies.
// //
// As it goes, it also prepares the values in a scope-sensitive manner. // As it goes, it also prepares the values in a scope-sensitive manner.
func allTemplates(c ci.Charter, vals common.Values) map[string]renderable { func allTemplates(c ci.Charter, vals common.Values) (map[string]renderable, error) {
templates := make(map[string]renderable) templates := make(map[string]renderable)
recAllTpls(c, templates, vals) if _, err := recAllTpls(c, templates, vals); err != nil {
return templates return nil, err
}
return templates, nil
} }
// recAllTpls recurses through the templates in a chart. // recAllTpls recurses through the templates in a chart.
// //
// As it recurses, it also sets the values to be appropriate for the template // As it recurses, it also sets the values to be appropriate for the template
// scope. // scope.
func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values) map[string]any { func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values) (map[string]any, error) {
vals := values.AsMap() vals := values.AsMap()
subCharts := make(map[string]any) subCharts := make(map[string]any)
accessor, err := ci.NewAccessor(c) accessor, err := ci.NewAccessor(c)
if err != nil { if err != nil {
slog.Error("error accessing chart", "error", err) return nil, fmt.Errorf("error accessing chart: %w", err)
} }
chartMetaData := accessor.MetadataAsMap() chartMetaData := accessor.MetadataAsMap()
chartMetaData["IsRoot"] = accessor.IsRoot() chartMetaData["IsRoot"] = accessor.IsRoot()
@ -559,9 +564,15 @@ func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Val
} }
for _, child := range accessor.Dependencies() { for _, child := range accessor.Dependencies() {
// TODO: Handle error sub, err := ci.NewAccessor(child)
sub, _ := ci.NewAccessor(child) if err != nil {
subCharts[sub.Name()] = recAllTpls(child, templates, next) return nil, fmt.Errorf("error accessing dependency chart: %w", err)
}
subTpls, err := recAllTpls(child, templates, next)
if err != nil {
return nil, err
}
subCharts[sub.Name()] = subTpls
} }
newParentID := accessor.ChartFullPath() newParentID := accessor.ChartFullPath()
@ -579,7 +590,7 @@ func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Val
} }
} }
return next return next, nil
} }
// isTemplateValid returns true if the template is valid for the chart type // isTemplateValid returns true if the template is valid for the chart type

@ -590,12 +590,24 @@ func TestAllTemplates(t *testing.T) {
} }
dep1.AddDependency(dep2) dep1.AddDependency(dep2)
tpls := allTemplates(ch1, common.Values{}) tpls, err := allTemplates(ch1, common.Values{})
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}
if len(tpls) != 5 { if len(tpls) != 5 {
t.Errorf("Expected 5 charts, got %d", len(tpls)) t.Errorf("Expected 5 charts, got %d", len(tpls))
} }
} }
func TestRenderInvalidChartType(t *testing.T) {
_, err := Render(struct{}{}, common.Values{})
if err == nil {
t.Error("Expected error when rendering invalid chart type, got nil")
} else if !strings.Contains(err.Error(), "unsupported chart type") {
t.Errorf("Expected error to contain 'unsupported chart type', got: %v", err)
}
}
func TestChartValuesContainsIsRoot(t *testing.T) { func TestChartValuesContainsIsRoot(t *testing.T) {
modTime := time.Now() modTime := time.Now()
ch1 := &chart.Chart{ ch1 := &chart.Chart{

Loading…
Cancel
Save