Skip to content
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
1 change: 1 addition & 0 deletions api/internal/managers/app/config/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func NewAppConfigManager(config kotsv1beta1.Config, opts ...AppConfigManagerOpti
apitemplate.WithIsAirgap(manager.isAirgap),
apitemplate.WithPrivateCACertConfigMapName(manager.privateCACertConfigMapName),
apitemplate.WithKubeClient(manager.kcli),
apitemplate.WithLogger(manager.logger),
)
}

Expand Down
1 change: 1 addition & 0 deletions api/internal/managers/app/release/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func NewAppReleaseManager(config kotsv1beta1.Config, opts ...AppReleaseManagerOp
template.WithIsAirgap(manager.isAirgap),
template.WithPrivateCACertConfigMapName(manager.privateCACertConfigMapName),
template.WithKubeClient(manager.kcli),
template.WithLogger(manager.logger),
)
}

Expand Down
26 changes: 18 additions & 8 deletions api/pkg/template/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ func (e *Engine) templateConfigItems() (*kotsv1beta1.Config, error) {
for j := range cfg.Spec.Groups[i].Items {
resolved, err := e.resolveConfigItem(cfg.Spec.Groups[i].Items[j].Name)
if err != nil {
return nil, err
e.logger.WithError(err).WithField("item", cfg.Spec.Groups[i].Items[j].Name).Warn("failed to resolve item, using empty values")
cfg.Spec.Groups[i].Items[j].Value = multitype.FromString("")
cfg.Spec.Groups[i].Items[j].Default = multitype.FromString("")
cfg.Spec.Groups[i].Items[j].Filename = ""
continue
}

// Apply user value if it exists, otherwise use the templated config value (but not the default)
Expand Down Expand Up @@ -78,7 +82,8 @@ func (e *Engine) configOption(name string) (string, error) {

resolved, err := e.resolveConfigItem(name)
if err != nil {
return "", fmt.Errorf("resolve config item: %w", err)
e.logger.WithError(err).WithField("item", name).Warn("failed to resolve item, returning empty string")
return "", nil
}
return resolved.Effective, nil
}
Expand All @@ -88,13 +93,15 @@ func (e *Engine) configOptionData(name string) (string, error) {

resolved, err := e.resolveConfigItem(name)
if err != nil {
return "", fmt.Errorf("resolve config item: %w", err)
e.logger.WithError(err).WithField("item", name).Warn("failed to resolve item, returning empty string")
return "", nil
}

// Base64 decode for file content
decoded, err := base64.StdEncoding.DecodeString(resolved.Effective)
if err != nil {
return "", fmt.Errorf("decode base64 value: %w", err)
e.logger.WithError(err).WithField("item", name).Warn("failed to decode base64 for item, returning empty string")
return "", nil
}
return string(decoded), nil
}
Expand All @@ -104,7 +111,8 @@ func (e *Engine) configOptionEquals(name, expected string) (bool, error) {

resolved, err := e.resolveConfigItem(name)
if err != nil {
return false, fmt.Errorf("resolve config item: %w", err)
e.logger.WithError(err).WithField("item", name).WithField("expected", expected).Warn("failed to resolve item, returning false")
return false, nil
}
return resolved.Effective == expected, nil
}
Expand All @@ -114,8 +122,9 @@ func (e *Engine) configOptionNotEquals(name, expected string) (bool, error) {

resolved, err := e.resolveConfigItem(name)
if err != nil {
// NOTE: this is parity from KOTS but I would expect this to return true
return false, fmt.Errorf("resolve config item: %w", err)
// NOTE: logically one might expect this to return true, but this matches KOTS behavior
e.logger.WithError(err).WithField("item", name).WithField("expected", expected).Warn("failed to resolve item, returning false")
return false, nil
}
return resolved.Effective != expected, nil
}
Expand All @@ -125,7 +134,8 @@ func (e *Engine) configOptionFilename(name string) (string, error) {

resolved, err := e.resolveConfigItem(name)
if err != nil {
return "", fmt.Errorf("resolve config item: %w", err)
e.logger.WithError(err).WithField("item", name).Warn("failed to resolve item, returning empty string")
return "", nil
}

// Only return user filename, not config filename for KOTS parity
Expand Down
138 changes: 138 additions & 0 deletions api/pkg/template/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,59 @@ func TestEngine_templateConfigItems(t *testing.T) {
},
},
},
{
name: "graceful failure on template error - returns empty values",
config: &kotsv1beta1.Config{
Spec: kotsv1beta1.ConfigSpec{
Groups: []kotsv1beta1.ConfigGroup{
{
Name: "test",
Items: []kotsv1beta1.ConfigItem{
{
Name: "valid_item",
Value: multitype.FromString("valid_value"),
},
{
Name: "invalid_template",
Value: multitype.FromString("repl{{ NonExistentFunc }}"),
},
{
Name: "another_valid",
Value: multitype.FromString("another_value"),
},
},
},
},
},
},
expected: &kotsv1beta1.Config{
Spec: kotsv1beta1.ConfigSpec{
Groups: []kotsv1beta1.ConfigGroup{
{
Name: "test",
Items: []kotsv1beta1.ConfigItem{
{
Name: "valid_item",
Value: multitype.FromString("valid_value"),
Default: multitype.FromString(""),
},
{
Name: "invalid_template",
Value: multitype.FromString(""),
Default: multitype.FromString(""),
Filename: "",
},
{
Name: "another_valid",
Value: multitype.FromString("another_value"),
Default: multitype.FromString(""),
},
},
},
},
},
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -544,3 +597,88 @@ func TestEngine_ShouldInvalidateItem(t *testing.T) {
engine.depsTree = map[string][]string{}
assert.False(t, engine.shouldInvalidateItem("item1"), "should not invalidate item1 as it doesn't exist in either config values")
}

// TestEngine_ConfigOption_HandlesFailuresGracefully tests that ConfigOption functions return empty
// strings/false instead of propagating errors, allowing templates to complete successfully
func TestEngine_ConfigOption_HandlesFailuresGracefully(t *testing.T) {
tests := []struct {
name string
configItem kotsv1beta1.ConfigItem
template string
expectResult string
}{
{
name: "template error in value - ConfigOption returns empty string",
configItem: kotsv1beta1.ConfigItem{
Name: "bad_template",
Value: multitype.FromString("repl{{ NonExistentFunc }}"),
},
template: "value:repl{{ ConfigOption \"bad_template\" }}",
expectResult: "value:",
},
{
name: "invalid base64 - ConfigOptionData returns empty string",
configItem: kotsv1beta1.ConfigItem{
Name: "invalid_base64",
Value: multitype.FromString("not-valid-base64!@#$"),
},
template: "data:repl{{ ConfigOptionData \"invalid_base64\" }}",
expectResult: "data:",
},
{
name: "nonexistent item in middle of template",
configItem: kotsv1beta1.ConfigItem{
Name: "existing",
Value: multitype.FromString("exists"),
},
template: "prefix-repl{{ ConfigOption \"nonexistent\" }}-suffix",
expectResult: "prefix--suffix",
},
{
name: "ConfigOptionEquals with nonexistent returns false",
configItem: kotsv1beta1.ConfigItem{
Name: "item",
Value: multitype.FromString("value"),
},
template: "repl{{ if ConfigOptionEquals \"nonexistent\" \"test\" }}yes repl{{ else }}no repl{{ end }}",
expectResult: "no ",
},
{
name: "ConfigOptionNotEquals with nonexistent returns false",
configItem: kotsv1beta1.ConfigItem{
Name: "item",
Value: multitype.FromString("value"),
},
template: "repl{{ if ConfigOptionNotEquals \"nonexistent\" \"test\" }}yes repl{{ else }}no repl{{ end }}",
expectResult: "no ",
},
{
name: "multiline YAML with failed ConfigOption",
configItem: kotsv1beta1.ConfigItem{
Name: "existing",
Value: multitype.FromString("value1"),
},
template: "line1: repl{{ ConfigOption \"existing\" }}\nline2: repl{{ ConfigOption \"nonexistent\" }}\nline3: value3",
expectResult: "line1: value1\nline2: \nline3: value3",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
config := &kotsv1beta1.Config{
Spec: kotsv1beta1.ConfigSpec{
Groups: []kotsv1beta1.ConfigGroup{
{
Name: "test",
Items: []kotsv1beta1.ConfigItem{tt.configItem},
},
},
},
}
engine := NewEngine(config)
result, err := engine.processTemplate(tt.template)
require.NoError(t, err, "template execution should not fail")
assert.Equal(t, tt.expectResult, result)
})
}
}
13 changes: 13 additions & 0 deletions api/pkg/template/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"text/template"

"github.com/Masterminds/sprig/v3"
"github.com/replicatedhq/embedded-cluster/api/pkg/logger"
"github.com/replicatedhq/embedded-cluster/api/types"
ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
"github.com/replicatedhq/embedded-cluster/pkg/release"
kotsv1beta1 "github.com/replicatedhq/kotskinds/apis/kots/v1beta1"
"github.com/sirupsen/logrus"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -31,6 +33,7 @@ type Engine struct {
privateCACertConfigMapName string // ConfigMap name for private CA certificates, empty string if not available
isAirgapInstallation bool // Whether the installation is an airgap installation
kubeClient client.Client
logger logrus.FieldLogger

// ExecOptions
proxySpec *ecv1beta1.ProxySpec // Proxy spec for the proxy template functions, if applicable
Expand Down Expand Up @@ -85,6 +88,12 @@ func WithKubeClient(kcli client.Client) EngineOption {
}
}

func WithLogger(logger logrus.FieldLogger) EngineOption {
return func(e *Engine) {
e.logger = logger
}
}

func NewEngine(config *kotsv1beta1.Config, opts ...EngineOption) *Engine {
engine := &Engine{
mode: ModeGeneric, // default to generic mode
Expand All @@ -101,6 +110,10 @@ func NewEngine(config *kotsv1beta1.Config, opts ...EngineOption) *Engine {
opt(engine)
}

if engine.logger == nil {
engine.logger = logger.NewDiscardLogger()
}

// Initialize funcMap once
engine.funcMap = sprig.TxtFuncMap()
maps.Copy(engine.funcMap, engine.getFuncMap())
Expand Down
40 changes: 22 additions & 18 deletions api/pkg/template/execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,12 @@ func TestEngine_ConfigOptionEquals(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Snapshot Backup", result)

// Test with an unknown item - an error is returned and false is returned
// Test with an unknown item - returns false with no error (template execution continues)
err = engine.Parse("{{repl if ConfigOptionEquals \"notfound\" \"filesystem\" }}Filesystem Storage{{repl else }}Other Storage{{repl end }}")
require.NoError(t, err)
result, err = engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
require.Error(t, err)
assert.Equal(t, "", result)
require.NoError(t, err)
assert.Equal(t, "Other Storage", result)
}

func TestEngine_ConfigOptionNotEquals(t *testing.T) {
Expand Down Expand Up @@ -300,12 +300,12 @@ func TestEngine_ConfigOptionNotEquals(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Other Backup", result)

// Test with an unknown item - an error is returned and false is returned
// Test with an unknown item - returns false with no error (template execution continues)
err = engine.Parse("{{repl if ConfigOptionNotEquals \"notfound\" \"filesystem\" }}Filesystem Storage{{repl else }}Other Storage{{repl end }}")
require.NoError(t, err)
result, err = engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
require.Error(t, err)
assert.Equal(t, "", result)
require.NoError(t, err)
assert.Equal(t, "Other Storage", result)
}

func TestEngine_ConfigOptionData(t *testing.T) {
Expand Down Expand Up @@ -419,11 +419,11 @@ func TestEngine_ConfigOptionFilename(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "", result)

// Test with an unknown item - an error is returned and empty string is returned
// Test with an unknown item - returns empty string with no error (template execution continues)
err = engine.Parse("{{repl ConfigOptionFilename \"notfound\" }}")
require.NoError(t, err)
result, err = engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
require.Error(t, err)
require.NoError(t, err)
assert.Equal(t, "", result)
}

Expand Down Expand Up @@ -495,9 +495,10 @@ func TestEngine_CircularDependency(t *testing.T) {

err := engine.Parse("{{repl ConfigOption \"item_a\" }}")
require.NoError(t, err)
_, err = engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
require.Error(t, err)
assert.Contains(t, err.Error(), "circular dependency detected for item_a")
result, err := engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
// Circular dependencies return empty string with no error (template execution continues)
require.NoError(t, err)
assert.Equal(t, "", result)
}

func TestEngine_DeepDependencyChain(t *testing.T) {
Expand Down Expand Up @@ -764,9 +765,10 @@ func TestEngine_UnknownConfigItem(t *testing.T) {

err := engine.Parse("{{repl ConfigOption \"nonexistent\" }}")
require.NoError(t, err)
_, err = engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
require.Error(t, err)
assert.Contains(t, err.Error(), "config item nonexistent not found")
result, err := engine.Execute(nil, WithProxySpec(&ecv1beta1.ProxySpec{}))
// Unknown config items return empty string with no error (template execution continues)
require.NoError(t, err)
assert.Equal(t, "", result)
}

func TestEngine_DependencyTreeAndCaching(t *testing.T) {
Expand Down Expand Up @@ -1361,10 +1363,12 @@ func TestEngine_ConfigMode_CircularDependency(t *testing.T) {

engine := NewEngine(config, WithMode(ModeConfig))

// Should detect circular dependency and return error
_, err := engine.Execute(nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "circular dependency detected")
// Circular dependencies in config are gracefully handled by returning empty strings
result, err := engine.Execute(nil)
require.NoError(t, err)
// Both items should have empty values since they depend on each other
assert.Contains(t, result, "item_a")
assert.Contains(t, result, "item_b")
}

func TestEngine_ConfigMode_ComplexDependencyChain(t *testing.T) {
Expand Down
Loading