add a mechanism to deprecate AppConfig settings w/ backwards-compat (#…

…7353)

Related to #7312, the motivation behind these changes is to introduce a way to deprecate configurations in `AppConfig`, while still preserving backwards compatibility.

From the Epic:

> NOTE: `host_settings` is now replaced by `features`. We should still support `host_settings` as an alias to `features` for backwards compatibility, but in our communications, we should use and recommend features as the canonical way forward.

server/fleet/app.go

@@ -1,8 +1,11 @@
 package fleet
 
 import (
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"time"
 
 	"github.com/fleetdm/fleet/v4/server/config"
@@ -106,6 +109,9 @@ type VulnerabilitySettings struct {
 }
 
 // AppConfig holds server configuration that can be changed via the API.
+//
+// Note: management of deprecated fields is done on JSON-marshalling and uses
+// the legacyConfig struct to list them.
 type AppConfig struct {
 	OrgInfo            OrgInfo            `json:"org_info"`
 	ServerSettings     ServerSettings     `json:"server_settings"`
@@ -125,20 +131,48 @@ type AppConfig struct {
 
 	WebhookSettings WebhookSettings `json:"webhook_settings"`
 	Integrations    Integrations    `json:"integrations"`
+
+	strictDecoding bool
+}
+
+// legacyConfig holds settings that have been replaced, superceded or
+// deprecated by other AppConfig settings.
+type legacyConfig struct {
 }
 
 // EnrichedAppConfig contains the AppConfig along with additional fleet
 // instance configuration settings as returned by the
 // "GET /api/latest/fleet/config" API endpoint (and fleetctl get config).
 type EnrichedAppConfig struct {
 	AppConfig
+	enrichedAppConfigFields
+}
 
+// enrichedAppConfigFields are grouped separately to aid with JSON unmarshaling
+type enrichedAppConfigFields struct {
 	UpdateInterval  *UpdateIntervalConfig  `json:"update_interval,omitempty"`
 	Vulnerabilities *VulnerabilitiesConfig `json:"vulnerabilities,omitempty"`
 	License         *LicenseInfo           `json:"license,omitempty"`
 	Logging         *Logging               `json:"logging,omitempty"`
 }
 
+// UnmarshalJSON implements the json.Unmarshaler interface to make sure we serialize
+// both AppConfig and enrichedAppConfigFields properly:
+//
+// - If this function is not defined, AppConfig.UnmarshalJSON gets promoted and
+// will be called instead.
+// - If we try to unmarshal everything in one go, AppConfig.UnmarshalJSON doesn't get
+// called.
+func (e *EnrichedAppConfig) UnmarshalJSON(data []byte) error {
+	if err := json.Unmarshal(data, &e.AppConfig); err != nil {
+		return err
+	}
+	if err := json.Unmarshal(data, &e.enrichedAppConfigFields); err != nil {
+		return err
+	}
+	return nil
+}
+
 type Duration struct {
 	time.Duration
 }
@@ -239,6 +273,44 @@ func (c *AppConfig) ApplyDefaults() {
 	c.WebhookSettings.Interval.Duration = 24 * time.Hour
 }
 
+// EnableStrictDecoding enables strict decoding of the AppConfig struct.
+func (c *AppConfig) EnableStrictDecoding() { c.strictDecoding = true }
+
+// UnmarshalJSON implements the json.Unmarshaler interface.
+func (c *AppConfig) UnmarshalJSON(b []byte) error {
+	// Define a new type, this is to prevent infinite recursion when
+	// unmarshalling the AppConfig struct.
+	type cfgStructUnmarshal AppConfig
+	compatConfig := struct {
+		*legacyConfig
+		*cfgStructUnmarshal
+	}{
+		&legacyConfig{},
+		(*cfgStructUnmarshal)(c),
+	}
+
+	decoder := json.NewDecoder(bytes.NewReader(b))
+	if c.strictDecoding {
+		decoder.DisallowUnknownFields()
+	}
+	if err := decoder.Decode(&compatConfig); err != nil {
+		return err
+	}
+	if _, err := decoder.Token(); err != io.EOF {
+		return errors.New("unexpected extra tokens found in config")
+	}
+
+	// TODO(roperzh): define and assign legacy settings to new fields. This has
+	// the drawback of legacy fields taking precedence over new fields if both
+	// are defined. Eg:
+	//
+	//	if compatConfig.legacyConfig.HostSettings != nil {
+	//		c.Features = *compatConfig.legacyConfig.HostSettings
+	//	}
+
+	return nil
+}
+
 // OrgInfo contains general info about the organization using Fleet.
 type OrgInfo struct {
 	OrgName    string `json:"org_name"`

server/service/appconfig.go

@@ -26,7 +26,11 @@ import (
 
 type appConfigResponse struct {
 	fleet.AppConfig
+	appConfigResponseFields
+}
 
+// appConfigResponseFields are grouped separately to aid with JSON unmarshaling
+type appConfigResponseFields struct {
 	UpdateInterval  *fleet.UpdateIntervalConfig  `json:"update_interval"`
 	Vulnerabilities *fleet.VulnerabilitiesConfig `json:"vulnerabilities"`
 
@@ -40,6 +44,23 @@ type appConfigResponse struct {
 	Err error `json:"error,omitempty"`
 }
 
+// UnmarshalJSON implements the json.Unmarshaler interface to make sure we serialize
+// both AppConfig and appConfigResponseFields properly:
+//
+// - If this function is not defined, AppConfig.UnmarshalJSON gets promoted and
+// will be called instead.
+// - If we try to unmarshal everything in one go, AppConfig.UnmarshalJSON doesn't get
+// called.
+func (r *appConfigResponse) UnmarshalJSON(data []byte) error {
+	if err := json.Unmarshal(data, &r.AppConfig); err != nil {
+		return err
+	}
+	if err := json.Unmarshal(data, &r.appConfigResponseFields); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (r appConfigResponse) error() error { return r.Err }
 
 func getAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (interface{}, error) {
@@ -105,11 +126,13 @@ func getAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet.Se
 			WebhookSettings: config.WebhookSettings,
 			Integrations:    config.Integrations,
 		},
-		UpdateInterval:  updateIntervalConfig,
-		Vulnerabilities: vulnConfig,
-		License:         license,
-		Logging:         loggingConfig,
-		SandboxEnabled:  svc.SandboxEnabled(),
+		appConfigResponseFields: appConfigResponseFields{
+			UpdateInterval:  updateIntervalConfig,
+			Vulnerabilities: vulnConfig,
+			License:         license,
+			Logging:         loggingConfig,
+			SandboxEnabled:  svc.SandboxEnabled(),
+		},
 	}
 	return response, nil
 }
@@ -157,7 +180,7 @@ func modifyAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet
 	req := request.(*modifyAppConfigRequest)
 	config, err := svc.ModifyAppConfig(ctx, req.RawMessage)
 	if err != nil {
-		return appConfigResponse{Err: err}, nil
+		return appConfigResponse{appConfigResponseFields: appConfigResponseFields{Err: err}}, nil
 	}
 	license, err := svc.License(ctx)
 	if err != nil {
@@ -169,8 +192,10 @@ func modifyAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet
 	}
 	response := appConfigResponse{
 		AppConfig: *config,
-		License:   license,
-		Logging:   loggingConfig,
+		appConfigResponseFields: appConfigResponseFields{
+			License: license,
+			Logging: loggingConfig,
+		},
 	}
 
 	if response.SMTPSettings.SMTPPassword != "" {
@@ -223,7 +248,7 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte) (*fleet.AppCo
 	invalid := &fleet.InvalidArgumentError{}
 	var newAppConfig fleet.AppConfig
 	if err := json.Unmarshal(p, &newAppConfig); err != nil {
-		return nil, ctxerr.Wrap(ctx, err)
+		return nil, ctxerr.Wrap(ctx, &badRequestError{message: err.Error()})
 	}
 
 	if newAppConfig.FleetDesktop.TransparencyURL != "" {
@@ -243,9 +268,8 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte) (*fleet.AppCo
 	}
 
 	// We apply the config that is incoming to the old one
-	decoder := json.NewDecoder(bytes.NewReader(p))
-	decoder.DisallowUnknownFields()
-	if err := decoder.Decode(&appConfig); err != nil {
+	appConfig.EnableStrictDecoding()
+	if err := json.Unmarshal(p, &appConfig); err != nil {
 		return nil, ctxerr.Wrap(ctx, &badRequestError{message: err.Error()})
 	}