feat(api): fix kotsv1beta1 config values need defaults (#2457)

* feat(api): fix kotsv1beta1 config values need defaults

* f

* f

* f

* f

* f

* f

Author: emosbaugh

.github/workflows/ci.yaml

@@ -904,7 +904,7 @@ jobs:
       - check-images
       - check-operator-crds
       - check-swagger-docs
-    if: always()
+    if: ${{ !cancelled() }}
     steps:
       # https://docs.github.com/en/actions/learn-github-actions/contexts#needs-context
       - name: fail if e2e job was not successful

.github/workflows/kinds.yaml

@@ -59,7 +59,7 @@ jobs:
     needs:
       - lint
       - test
-    if: always()
+    if: ${{ !cancelled() }}
     steps:
       - name: succeed if everything passed
         run: echo "Validation succeeded"

.github/workflows/release-prod.yaml

@@ -622,7 +622,7 @@ jobs:
       - e2e-docker
       - release
       - release-app
-    if: always()
+    if: ${{ !cancelled() }}
     steps:
       # https://docs.github.com/en/actions/learn-github-actions/contexts#needs-context
       - name: fail if e2e job was not successful

.github/workflows/utils.yaml

@@ -56,7 +56,7 @@ jobs:
     needs:
       - lint
       - test
-    if: always()
+    if: ${{ !cancelled() }}
     steps:
       - name: succeed if everything passed
         run: echo "Validation succeeded"

api/controllers/kubernetes/install/controller.go

@@ -46,7 +46,7 @@ type InstallController struct {
 	tlsConfig           types.TLSConfig
 	license             []byte
 	airgapBundle        string
-	configValues        string
+	configValues        map[string]string
 	endUserConfig       *ecv1beta1.Config
 	store               store.Store
 	ki                  kubernetesinstallation.Installation
@@ -111,7 +111,7 @@ func WithAirgapBundle(airgapBundle string) InstallControllerOption {
 	}
 }
 
-func WithConfigValues(configValues string) InstallControllerOption {
+func WithConfigValues(configValues map[string]string) InstallControllerOption {
 	return func(c *InstallController) {
 		c.configValues = configValues
 	}
@@ -164,6 +164,13 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 		opt(controller)
 	}
 
+	if controller.configValues != nil {
+		err := controller.store.AppConfigStore().SetConfigValues(controller.configValues)
+		if err != nil {
+			return nil, fmt.Errorf("set app config values: %w", err)
+		}
+	}
+
 	// If none is provided, use the default env settings from helm to create a RESTClientGetter
 	if controller.restClientGetter == nil {
 		controller.restClientGetter = helmcli.New().RESTClientGetter()
@@ -185,7 +192,6 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 			infra.WithTLSConfig(controller.tlsConfig),
 			infra.WithLicense(controller.license),
 			infra.WithAirgapBundle(controller.airgapBundle),
-			infra.WithConfigValues(controller.configValues),
 			infra.WithReleaseData(controller.releaseData),
 			infra.WithEndUserConfig(controller.endUserConfig),
 		)

api/controllers/kubernetes/install/controller_test.go

@@ -9,13 +9,18 @@ import (
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
+	appconfig "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/config"
 	"github.com/replicatedhq/embedded-cluster/api/internal/managers/kubernetes/infra"
 	"github.com/replicatedhq/embedded-cluster/api/internal/managers/kubernetes/installation"
 	"github.com/replicatedhq/embedded-cluster/api/internal/statemachine"
 	"github.com/replicatedhq/embedded-cluster/api/internal/store"
 	"github.com/replicatedhq/embedded-cluster/api/types"
+	ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
 	"github.com/replicatedhq/embedded-cluster/pkg/kubernetesinstallation"
 	"github.com/replicatedhq/embedded-cluster/pkg/metrics"
+	"github.com/replicatedhq/embedded-cluster/pkg/release"
+	kotsv1beta1 "github.com/replicatedhq/kotskinds/apis/kots/v1beta1"
+	"github.com/replicatedhq/kotskinds/multitype"
 )
 
 func TestGetInstallationConfig(t *testing.T) {
@@ -253,20 +258,49 @@ func TestGetInstallationStatus(t *testing.T) {
 }
 
 func TestSetupInfra(t *testing.T) {
+	// Create an app config
+	appConfig := kotsv1beta1.Config{
+		Spec: kotsv1beta1.ConfigSpec{
+			Groups: []kotsv1beta1.ConfigGroup{
+				{
+					Name:  "test-group",
+					Title: "Test Group",
+					Items: []kotsv1beta1.ConfigItem{
+						{
+							Name:    "test-item",
+							Type:    "text",
+							Title:   "Test Item",
+							Default: multitype.BoolOrString{StrVal: "default"},
+							Value:   multitype.BoolOrString{StrVal: "value"},
+						},
+					},
+				},
+			},
+		},
+	}
+	configValues := kotsv1beta1.ConfigValues{
+		Spec: kotsv1beta1.ConfigValuesSpec{
+			Values: map[string]kotsv1beta1.ConfigValue{
+				"test-item": {Default: "default", Value: "value"},
+			},
+		},
+	}
+
 	tests := []struct {
 		name          string
 		currentState  statemachine.State
 		expectedState statemachine.State
-		setupMocks    func(kubernetesinstallation.Installation, *installation.MockInstallationManager, *infra.MockInfraManager, *metrics.MockReporter, *store.MockStore)
+		setupMocks    func(kubernetesinstallation.Installation, *installation.MockInstallationManager, *infra.MockInfraManager, *metrics.MockReporter, *store.MockStore, *appconfig.MockAppConfigManager)
 		expectedErr   error
 	}{
 		{
 			name:          "successful setup",
 			currentState:  StateInstallationConfigured,
 			expectedState: StateSucceeded,
-			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore) {
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
-					fm.On("Install", mock.Anything, ki).Return(nil),
+					am.On("GetKotsadmConfigValues", appConfig).Return(configValues, nil),
+					fm.On("Install", mock.Anything, ki, configValues).Return(nil),
 					// TODO: we are not yet reporting
 					// mr.On("ReportInstallationSucceeded", mock.Anything),
 				)
@@ -277,9 +311,10 @@ func TestSetupInfra(t *testing.T) {
 			name:          "install infra error",
 			currentState:  StateInstallationConfigured,
 			expectedState: StateInfrastructureInstallFailed,
-			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore) {
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
-					fm.On("Install", mock.Anything, ki).Return(errors.New("install error")),
+					am.On("GetKotsadmConfigValues", appConfig).Return(configValues, nil),
+					fm.On("Install", mock.Anything, ki, configValues).Return(errors.New("install error")),
 					st.LinuxInfraMockStore.On("GetStatus").Return(types.Status{Description: "install error"}, nil),
 					// TODO: we are not yet reporting
 					// mr.On("ReportInstallationFailed", mock.Anything, errors.New("install error")),
@@ -291,9 +326,10 @@ func TestSetupInfra(t *testing.T) {
 			name:          "install infra error without report if infra store fails",
 			currentState:  StateInstallationConfigured,
 			expectedState: StateInfrastructureInstallFailed,
-			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore) {
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
-					fm.On("Install", mock.Anything, ki).Return(errors.New("install error")),
+					am.On("GetKotsadmConfigValues", appConfig).Return(configValues, nil),
+					fm.On("Install", mock.Anything, ki, configValues).Return(errors.New("install error")),
 					st.LinuxInfraMockStore.On("GetStatus").Return(nil, assert.AnError),
 				)
 			},
@@ -303,9 +339,10 @@ func TestSetupInfra(t *testing.T) {
 			name:          "install infra panic",
 			currentState:  StateInstallationConfigured,
 			expectedState: StateInfrastructureInstallFailed,
-			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore) {
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
-					fm.On("Install", mock.Anything, ki).Panic("this is a panic"),
+					am.On("GetKotsadmConfigValues", appConfig).Return(configValues, nil),
+					fm.On("Install", mock.Anything, ki, configValues).Panic("this is a panic"),
 					st.LinuxInfraMockStore.On("GetStatus").Return(types.Status{Description: "this is a panic"}, nil),
 					// TODO: we are not yet reporting
 					// mr.On("ReportInstallationFailed", mock.Anything, errors.New("this is a panic")),
@@ -317,10 +354,21 @@ func TestSetupInfra(t *testing.T) {
 			name:          "invalid state transition",
 			currentState:  StateNew,
 			expectedState: StateNew,
-			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore) {
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 			},
 			expectedErr: assert.AnError, // Just check that an error occurs, don't care about exact message
 		},
+		{
+			name:          "config values error",
+			currentState:  StateInstallationConfigured,
+			expectedState: StateInstallationConfigured,
+			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
+				mock.InOrder(
+					am.On("GetKotsadmConfigValues", appConfig).Return(kotsv1beta1.ConfigValues{}, assert.AnError),
+				)
+			},
+			expectedErr: assert.AnError,
+		},
 	}
 
 	for _, tt := range tests {
@@ -334,14 +382,17 @@ func TestSetupInfra(t *testing.T) {
 			mockInfraManager := &infra.MockInfraManager{}
 			mockMetricsReporter := &metrics.MockReporter{}
 			mockStore := &store.MockStore{}
-			tt.setupMocks(ki, mockInstallationManager, mockInfraManager, mockMetricsReporter, mockStore)
+			mockAppConfigManager := &appconfig.MockAppConfigManager{}
+			tt.setupMocks(ki, mockInstallationManager, mockInfraManager, mockMetricsReporter, mockStore, mockAppConfigManager)
 
 			controller, err := NewInstallController(
 				WithInstallation(ki),
 				WithStateMachine(sm),
 				WithInstallationManager(mockInstallationManager),
 				WithInfraManager(mockInfraManager),
+				WithAppConfigManager(mockAppConfigManager),
 				WithMetricsReporter(mockMetricsReporter),
+				WithReleaseData(getTestReleaseData(&appConfig)),
 				WithStore(mockStore),
 			)
 			require.NoError(t, err)
@@ -451,3 +502,11 @@ func TestGetInfra(t *testing.T) {
 		})
 	}
 }
+
+func getTestReleaseData(appConfig *kotsv1beta1.Config) *release.ReleaseData {
+	return &release.ReleaseData{
+		EmbeddedClusterConfig: &ecv1beta1.Config{},
+		ChannelRelease:        &release.ChannelRelease{},
+		AppConfig:             appConfig,
+	}
+}

api/controllers/kubernetes/install/infra.go

@@ -2,13 +2,18 @@ package install
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"runtime/debug"
 
 	"github.com/replicatedhq/embedded-cluster/api/types"
 )
 
 func (c *InstallController) SetupInfra(ctx context.Context) (finalErr error) {
+	if c.releaseData == nil || c.releaseData.AppConfig == nil {
+		return errors.New("app config not found")
+	}
+
 	lock, err := c.stateMachine.AcquireLock()
 	if err != nil {
 		return types.NewConflictError(err)
@@ -23,6 +28,11 @@ func (c *InstallController) SetupInfra(ctx context.Context) (finalErr error) {
 		}
 	}()
 
+	configValues, err := c.appConfigManager.GetKotsadmConfigValues(*c.releaseData.AppConfig)
+	if err != nil {
+		return fmt.Errorf("failed to get kotsadm config values: %w", err)
+	}
+
 	err = c.stateMachine.Transition(lock, StateInfrastructureInstalling)
 	if err != nil {
 		return types.NewConflictError(err)
@@ -51,7 +61,7 @@ func (c *InstallController) SetupInfra(ctx context.Context) (finalErr error) {
 			}
 		}()
 
-		if err := c.infraManager.Install(ctx, c.ki); err != nil {
+		if err := c.infraManager.Install(ctx, c.ki, configValues); err != nil {
 			return fmt.Errorf("failed to install infrastructure: %w", err)
 		}
 

api/controllers/linux/install/controller.go

@@ -2,6 +2,7 @@ package install
 
 import (
 	"context"
+	"fmt"
 	"sync"
 
 	appconfig "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/config"
@@ -56,7 +57,7 @@ type InstallController struct {
 	tlsConfig                 types.TLSConfig
 	license                   []byte
 	airgapBundle              string
-	configValuesFile          string
+	configValues              map[string]string
 	endUserConfig             *ecv1beta1.Config
 	clusterID                 string
 	store                     store.Store
@@ -129,9 +130,9 @@ func WithAirgapBundle(airgapBundle string) InstallControllerOption {
 	}
 }
 
-func WithConfigValuesFile(configValuesFile string) InstallControllerOption {
+func WithConfigValues(configValues map[string]string) InstallControllerOption {
 	return func(c *InstallController) {
-		c.configValuesFile = configValuesFile
+		c.configValues = configValues
 	}
 }
 
@@ -200,6 +201,13 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 		opt(controller)
 	}
 
+	if controller.configValues != nil {
+		err := controller.store.AppConfigStore().SetConfigValues(controller.configValues)
+		if err != nil {
+			return nil, fmt.Errorf("set app config values: %w", err)
+		}
+	}
+
 	if controller.stateMachine == nil {
 		controller.stateMachine = NewStateMachine(WithStateMachineLogger(controller.logger))
 	}
@@ -248,7 +256,6 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 			infra.WithTLSConfig(controller.tlsConfig),
 			infra.WithLicense(controller.license),
 			infra.WithAirgapBundle(controller.airgapBundle),
-			infra.WithConfigValuesFile(controller.configValuesFile), // CLI file path
 			infra.WithReleaseData(controller.releaseData),
 			infra.WithEndUserConfig(controller.endUserConfig),
 			infra.WithClusterID(controller.clusterID),