fix(settings): updateSettingsFromFlags only if dataStore is new [EE-2397] (#6475)

api/cli/cli.go

@@ -50,7 +50,7 @@ func (*Service) ParseFlags(version string) (*portainer.CLIFlags, error) {
 		SSLCert:                   kingpin.Flag("sslcert", "Path to the SSL certificate used to secure the Portainer instance").String(),
 		SSLKey:                    kingpin.Flag("sslkey", "Path to the SSL key used to secure the Portainer instance").String(),
 		Rollback:                  kingpin.Flag("rollback", "Rollback the database store to the previous version").Bool(),
-		SnapshotInterval:          kingpin.Flag("snapshot-interval", "Duration between each environment snapshot job").Default(defaultSnapshotInterval).String(),
+		SnapshotInterval:          kingpin.Flag("snapshot-interval", "Duration between each environment snapshot job").String(),
 		AdminPassword:             kingpin.Flag("admin-password", "Hashed admin password").String(),
 		AdminPasswordFile:         kingpin.Flag("admin-password-file", "Path to the file containing the password for the admin user").String(),
 		Labels:                    pairs(kingpin.Flag("hide-label", "Hide containers with a specific label in the UI").Short('l')),
@@ -129,7 +129,7 @@ func validateEndpointURL(endpointURL string) error {
 }
 
 func validateSnapshotInterval(snapshotInterval string) error {
-	if snapshotInterval != defaultSnapshotInterval {
+	if snapshotInterval != "" {
 		_, err := time.ParseDuration(snapshotInterval)
 		if err != nil {
 			return errInvalidSnapshotInterval

api/cli/defaults.go

@@ -20,7 +20,6 @@ const (
 	defaultSSL                 = "false"
 	defaultSSLCertPath         = "/certs/portainer.crt"
 	defaultSSLKeyPath          = "/certs/portainer.key"
-	defaultSnapshotInterval    = "5m"
 	defaultBaseURL             = "/"
 	defaultSecretKeyName       = "portainer"
 )

api/cmd/portainer/main.go

@@ -98,7 +98,7 @@ func initDataStore(flags *portainer.CLIFlags, secretKey []byte, fileService port
 		return nil
 	}
 
-	// Init sets some defaults - its basically a migration
+	// Init sets some defaults - it's basically a migration
 	err = store.Init()
 	if err != nil {
 		log.Fatalf("Failed initializing data store: %v", err)
@@ -228,11 +228,11 @@ func initKubernetesClientFactory(signatureService portainer.DigitalSignatureServ
 	return kubecli.NewClientFactory(signatureService, reverseTunnelService, instanceID, dataStore)
 }
 
-func initSnapshotService(snapshotInterval string, dataStore dataservices.DataStore, dockerClientFactory *docker.ClientFactory, kubernetesClientFactory *kubecli.ClientFactory, shutdownCtx context.Context) (portainer.SnapshotService, error) {
+func initSnapshotService(snapshotIntervalFromFlag string, dataStore dataservices.DataStore, dockerClientFactory *docker.ClientFactory, kubernetesClientFactory *kubecli.ClientFactory, shutdownCtx context.Context) (portainer.SnapshotService, error) {
 	dockerSnapshotter := docker.NewSnapshotter(dockerClientFactory)
 	kubernetesSnapshotter := kubernetes.NewSnapshotter(kubernetesClientFactory)
 
-	snapshotService, err := snapshot.NewService(snapshotInterval, dataStore, dockerSnapshotter, kubernetesSnapshotter, shutdownCtx)
+	snapshotService, err := snapshot.NewService(snapshotIntervalFromFlag, dataStore, dockerSnapshotter, kubernetesSnapshotter, shutdownCtx)
 	if err != nil {
 		return nil, err
 	}
@@ -253,11 +253,17 @@ func updateSettingsFromFlags(dataStore dataservices.DataStore, flags *portainer.
 		return err
 	}
 
-	settings.LogoURL = *flags.Logo
-	settings.SnapshotInterval = *flags.SnapshotInterval
-	settings.EnableEdgeComputeFeatures = *flags.EnableEdgeComputeFeatures
-	settings.EnableTelemetry = true
-	settings.OAuthSettings.SSO = true
+	if *flags.SnapshotInterval != "" {
+		settings.SnapshotInterval = *flags.SnapshotInterval
+	}
+
+	if *flags.Logo != "" {
+		settings.LogoURL = *flags.Logo
+	}
+
+	if *flags.EnableEdgeComputeFeatures {
+		settings.EnableEdgeComputeFeatures = *flags.EnableEdgeComputeFeatures
+	}
 
 	if *flags.Templates != "" {
 		settings.TemplatesURL = *flags.Templates

api/datastore/init.go

@@ -7,6 +7,30 @@ import (
 
 // Init creates the default data set.
 func (store *Store) Init() error {
+	err := store.checkOrCreateInstanceID()
+	if err != nil {
+		return err
+	}
+
+	err = store.checkOrCreateDefaultSettings()
+	if err != nil {
+		return err
+	}
+
+	err = store.checkOrCreateDefaultSSLSettings()
+	if err != nil {
+		return err
+	}
+
+	err = store.checkOrCreateDefaultData()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (store *Store) checkOrCreateInstanceID() error {
 	instanceID, err := store.VersionService.InstanceID()
 	if store.IsErrObjectNotFound(err) {
 		uid, err := uuid.NewV4()
@@ -15,18 +39,17 @@ func (store *Store) Init() error {
 		}
 
 		instanceID = uid.String()
-		err = store.VersionService.StoreInstanceID(instanceID)
-		if err != nil {
-			return err
-		}
-	} else if err != nil {
-		return err
+		return store.VersionService.StoreInstanceID(instanceID)
 	}
+	return err
+}
 
+func (store *Store) checkOrCreateDefaultSettings() error {
 	// TODO: these need to also be applied when importing
 	settings, err := store.SettingsService.Settings()
 	if store.IsErrObjectNotFound(err) {
 		defaultSettings := &portainer.Settings{
+			EnableTelemetry:      true,
 			AuthenticationMethod: portainer.AuthenticationInternal,
 			BlackListedLabels:    make([]portainer.Pair, 0),
 			LDAPSettings: portainer.LDAPSettings{
@@ -40,8 +63,10 @@ func (store *Store) Init() error {
 					{},
 				},
 			},
-			OAuthSettings: portainer.OAuthSettings{},
-
+			OAuthSettings: portainer.OAuthSettings{
+				SSO: true,
+			},
+			SnapshotInterval:         portainer.DefaultSnapshotInterval,
 			EdgeAgentCheckinInterval: portainer.DefaultEdgeAgentCheckinIntervalInSeconds,
 			TemplatesURL:             portainer.DefaultTemplatesURL,
 			HelmRepositoryURL:        portainer.DefaultHelmRepositoryURL,
@@ -50,35 +75,33 @@ func (store *Store) Init() error {
 			KubectlShellImage:        portainer.DefaultKubectlShellImage,
 		}
 
-		err = store.SettingsService.UpdateSettings(defaultSettings)
-		if err != nil {
-			return err
-		}
-	} else if err != nil {
+		return store.SettingsService.UpdateSettings(defaultSettings)
+	}
+	if err != nil {
 		return err
-	} else if err == nil {
-		if settings.UserSessionTimeout == "" {
-			settings.UserSessionTimeout = portainer.DefaultUserSessionTimeout
-			store.Settings().UpdateSettings(settings)
-		}
 	}
 
-	_, err = store.SSLSettings().Settings()
-	if err != nil {
-		if !store.IsErrObjectNotFound(err) {
-			return err
-		}
+	if settings.UserSessionTimeout == "" {
+		settings.UserSessionTimeout = portainer.DefaultUserSessionTimeout
+		return store.Settings().UpdateSettings(settings)
+	}
+	return nil
+}
 
+func (store *Store) checkOrCreateDefaultSSLSettings() error {
+	_, err := store.SSLSettings().Settings()
+
+	if store.IsErrObjectNotFound(err) {
 		defaultSSLSettings := &portainer.SSLSettings{
 			HTTPEnabled: true,
 		}
 
-		err = store.SSLSettings().UpdateSettings(defaultSSLSettings)
-		if err != nil {
-			return err
-		}
+		return store.SSLSettings().UpdateSettings(defaultSSLSettings)
 	}
+	return err
+}
 
+func (store *Store) checkOrCreateDefaultData() error {
 	groups, err := store.EndpointGroupService.EndpointGroups()
 	if err != nil {
 		return err
@@ -99,6 +122,5 @@ func (store *Store) Init() error {
 			return err
 		}
 	}
-
 	return nil
 }

api/internal/snapshot/snapshot.go

@@ -23,21 +23,39 @@ type Service struct {
 }
 
 // NewService creates a new instance of a service
-func NewService(snapshotInterval string, dataStore dataservices.DataStore, dockerSnapshotter portainer.DockerSnapshotter, kubernetesSnapshotter portainer.KubernetesSnapshotter, shutdownCtx context.Context) (*Service, error) {
-	snapshotFrequency, err := time.ParseDuration(snapshotInterval)
+func NewService(snapshotIntervalFromFlag string, dataStore dataservices.DataStore, dockerSnapshotter portainer.DockerSnapshotter, kubernetesSnapshotter portainer.KubernetesSnapshotter, shutdownCtx context.Context) (*Service, error) {
+	snapshotFrequency, err := parseSnapshotFrequency(snapshotIntervalFromFlag, dataStore)
 	if err != nil {
 		return nil, err
 	}
 
 	return &Service{
 		dataStore:                 dataStore,
-		snapshotIntervalInSeconds: snapshotFrequency.Seconds(),
+		snapshotIntervalInSeconds: snapshotFrequency,
 		dockerSnapshotter:         dockerSnapshotter,
 		kubernetesSnapshotter:     kubernetesSnapshotter,
 		shutdownCtx:               shutdownCtx,
 	}, nil
 }
 
+func parseSnapshotFrequency(snapshotInterval string, dataStore dataservices.DataStore) (float64, error) {
+	if snapshotInterval == "" {
+		settings, err := dataStore.Settings().Settings()
+		if err != nil {
+			return 0, err
+		}
+		snapshotInterval = settings.SnapshotInterval
+		if snapshotInterval == "" {
+			snapshotInterval = portainer.DefaultSnapshotInterval
+		}
+	}
+	snapshotFrequency, err := time.ParseDuration(snapshotInterval)
+	if err != nil {
+		return 0, err
+	}
+	return snapshotFrequency.Seconds(), nil
+}
+
 // Start will start a background routine to execute periodic snapshots of environments(endpoints)
 func (service *Service) Start() {
 	if service.refreshSignal != nil {

api/portainer.go

@@ -1376,6 +1376,8 @@ const (
 	// PortainerAgentSignatureMessage represents the message used to create a digital signature
 	// to be used when communicating with an agent
 	PortainerAgentSignatureMessage = "Portainer-App"
+	// DefaultSnapshotInterval represents the default interval between each environment snapshot job
+	DefaultSnapshotInterval = "5m"
 	// DefaultEdgeAgentCheckinIntervalInSeconds represents the default interval (in seconds) used by Edge agents to checkin with the Portainer instance
 	DefaultEdgeAgentCheckinIntervalInSeconds = 5
 	// DefaultTemplatesURL represents the URL to the official templates supported by Portainer