From bafb0e63a0ac667323b34f12e4d34c10bcd8c3f9 Mon Sep 17 00:00:00 2001
From: Yury Kovalev <ykovalev@redhat.com>
Date: Tue, 13 Oct 2026 16:40:05 +0200
Subject: [PATCH] Apply built-in defaults when reusing an existing cluster
 manifest

When deploying onto a cluster that already had a stored manifest secret,
assembleConfigForCommand used that manifest directly as the base config,
bypassing deployer.DefaultConfig(). Manifests written by an older roxie
(predating the imageRegistry field) left roxie.imageRegistry empty, which
UsesCustomRegistry() treats as a custom registry, causing deploy to fail
validation with "roxie.imageRegistry must include a registry host and org
path (e.g. quay.io/rhacs-eng), got:".

Always start from the built-in defaults and merge the manifest (and user
config, then args) on top, so defaults are preserved when the manifest
omits them. Non-empty manifest values still take precedence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 cmd/config.go      | 17 +++++++++++------
 cmd/deploy_test.go | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/cmd/config.go b/cmd/config.go
index 8888dd4..fb5a1ef 100644
--- a/cmd/config.go
+++ b/cmd/config.go
@@ -8,12 +8,17 @@ import (
 )
 
 func assembleConfigForCommand(configBase *deployer.Config, configFromArgs deployer.Config, skipUserConfig bool) (deployer.Config, error) {
-	var config deployer.Config
-	if configBase == nil {
-		// Start with default configuration.
-		config = deployer.DefaultConfig()
-	} else {
-		config = *configBase
+	// Always start from the built-in defaults so that defaults (e.g.
+	// roxie.imageRegistry) are applied even when reusing an existing cluster
+	// manifest as the base configuration. Otherwise a manifest written by an
+	// older roxie (which predates such defaults) would leave those fields empty.
+	config := deployer.DefaultConfig()
+	if configBase != nil {
+		// Merge the provided base (e.g. an existing cluster manifest) on top of
+		// the defaults; non-empty values in the base take precedence.
+		if err := mergo.Merge(&config, configBase, mergo.WithOverride, mergo.WithoutDereference); err != nil {
+			return deployer.Config{}, fmt.Errorf("merging base config over defaults: %w", err)
+		}
 	}
 
 	// Apply user config on top (overriding defaults).
diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go
index d0536c8..eb96dc7 100644
--- a/cmd/deploy_test.go
+++ b/cmd/deploy_test.go
@@ -364,6 +364,42 @@ func TestValidateImageRegistry(t *testing.T) {
 	}
 }
 
+func TestAssembleConfigForCommand(t *testing.T) {
+	// Isolate the user config lookup so it cannot pick up a real file on disk.
+	tmpDir := t.TempDir()
+	t.Setenv("XDG_CONFIG_HOME", tmpDir)
+	t.Setenv("HOME", tmpDir) // For non-Unix systems.
+
+	t.Run("applies defaults when no base config is provided", func(t *testing.T) {
+		config, err := assembleConfigForCommand(nil, deployer.Config{}, true)
+		require.NoError(t, err)
+		assert.Equal(t, constants.DefaultRegistry, config.Roxie.ImageRegistry)
+	})
+
+	t.Run("applies defaults even when base config (e.g. cluster manifest) omits them", func(t *testing.T) {
+		// Simulate a manifest stored by an older roxie that predates the
+		// imageRegistry field: ImageRegistry is empty. Defaults must still apply
+		// so an empty registry is not treated as a custom registry.
+		base := &deployer.Config{
+			Roxie: deployer.RoxieConfig{Version: "4.5.0"},
+		}
+		config, err := assembleConfigForCommand(base, deployer.Config{}, true)
+		require.NoError(t, err)
+		assert.Equal(t, constants.DefaultRegistry, config.Roxie.ImageRegistry)
+		// Values present in the base config are preserved.
+		assert.Equal(t, imagetag.MainTag("4.5.0"), config.Roxie.Version)
+	})
+
+	t.Run("base config overrides defaults", func(t *testing.T) {
+		base := &deployer.Config{
+			Roxie: deployer.RoxieConfig{ImageRegistry: "example.com/custom"},
+		}
+		config, err := assembleConfigForCommand(base, deployer.Config{}, true)
+		require.NoError(t, err)
+		assert.Equal(t, "example.com/custom", config.Roxie.ImageRegistry)
+	})
+}
+
 func TestApplyUserDefaults(t *testing.T) {
 	tests := []struct {
 		name     string