Merge pull request #72 from safing/feature/options

Extend config options

api/config.go

@@ -42,13 +42,15 @@ func registerConfig() error {
 		Name:            "API Address",
 		Key:             CfgDefaultListenAddressKey,
 		Description:     "Define on which IP and port the API should listen on.",
-		Order:           128,
 		OptType:         config.OptTypeString,
 		ExpertiseLevel:  config.ExpertiseLevelDeveloper,
 		ReleaseLevel:    config.ReleaseLevelStable,
 		DefaultValue:    getDefaultListenAddress(),
 		ValidationRegex: "^([0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}:[0-9]{1,5}|\\[[:0-9A-Fa-f]+\\]:[0-9]{1,5})$",
 		RequiresRestart: true,
+		Annotations: config.Annotations{
+			config.DisplayOrderAnnotation: 128,
+		},
 	})
 	if err != nil {
 		return err

config/database.go

@@ -110,7 +110,7 @@ func (s *StorageInterface) Query(q *query.Query, local, internal bool) (*iterato
 
 func (s *StorageInterface) processQuery(it *iterator.Iterator, opts []*Option) {
 
-	sort.Sort(sortableOptions(opts))
+	sort.Sort(sortByKey(opts))
 
 	for _, opt := range opts {
 		r, err := opt.Export()

config/expertise.go

@@ -3,17 +3,22 @@
 package config
 
 import (
-	"fmt"
 	"sync/atomic"
 
 	"github.com/tevino/abool"
 )
 
+// ExpertiseLevel allows to group settings by user expertise.
+// It's useful if complex or technical settings should be hidden
+// from the average user while still allowing experts and developers
+// to change deep configuration settings.
+type ExpertiseLevel uint8
+
 // Expertise Level constants
 const (
-	ExpertiseLevelUser      uint8 = 0
+	ExpertiseLevelUser      ExpertiseLevel = 0
-	ExpertiseLevelExpert    uint8 = 1
+	ExpertiseLevelExpert    ExpertiseLevel = 1
-	ExpertiseLevelDeveloper uint8 = 2
+	ExpertiseLevelDeveloper ExpertiseLevel = 2
 
 	ExpertiseLevelNameUser      = "user"
 	ExpertiseLevelNameExpert    = "expert"
@@ -23,33 +28,44 @@ const (
 )
 
 var (
-	expertiseLevel           *int32
 	expertiseLevelOption     *Option
+	expertiseLevel           = new(int32)
 	expertiseLevelOptionFlag = abool.New()
 )
 
 func init() {
-	var expertiseLevelVal int32
-	expertiseLevel = &expertiseLevelVal
-
 	registerExpertiseLevelOption()
 }
 
 func registerExpertiseLevelOption() {
 	expertiseLevelOption = &Option{
-		Name:        "Expertise Level",
+		Name:           "Expertise Level",
-		Key:         expertiseLevelKey,
+		Key:            expertiseLevelKey,
-		Description: "The Expertise Level controls the perceived complexity. Higher settings will show you more complex settings and information. This might also affect various other things relying on this setting. Modified settings in higher expertise levels stay in effect when switching back. (Unlike the Release Level)",
+		Description:    "The Expertise Level controls the perceived complexity. Higher settings will show you more complex settings and information. This might also affect various other things relying on this setting. Modified settings in higher expertise levels stay in effect when switching back. (Unlike the Release Level)",
-
 		OptType:        OptTypeString,
 		ExpertiseLevel: ExpertiseLevelUser,
-		ReleaseLevel:   ExpertiseLevelUser,
+		ReleaseLevel:   ReleaseLevelStable,
-
+		DefaultValue:   ExpertiseLevelNameUser,
-		RequiresRestart: false,
+		Annotations: Annotations{
-		DefaultValue:    ExpertiseLevelNameUser,
+			DisplayHintAnnotation: DisplayHintOneOf,
-
+		},
-		ExternalOptType: "string list",
+		PossibleValues: []PossibleValue{
-		ValidationRegex: fmt.Sprintf("^(%s|%s|%s)$", ExpertiseLevelNameUser, ExpertiseLevelNameExpert, ExpertiseLevelNameDeveloper),
+			{
+				Name:        "Easy",
+				Value:       ExpertiseLevelNameUser,
+				Description: "Easy application mode by hidding complex settings.",
+			},
+			{
+				Name:        "Expert",
+				Value:       ExpertiseLevelNameExpert,
+				Description: "Expert application mode. Allows access to almost all configuration options.",
+			},
+			{
+				Name:        "Developer",
+				Value:       ExpertiseLevelNameDeveloper,
+				Description: "Developer mode. Please be careful!",
+			},
+		},
 	}
 
 	err := Register(expertiseLevelOption)

config/get.go

@@ -15,7 +15,7 @@ type (
 	BoolOption func() bool
 )
 
-func getValueCache(name string, option *Option, requestedType uint8) (*Option, *valueCache) {
+func getValueCache(name string, option *Option, requestedType OptionType) (*Option, *valueCache) {
 	// get option
 	if option == nil {
 		var ok bool

config/get_test.go

@@ -25,7 +25,7 @@ func parseAndSetDefaultConfig(jsonData string) error {
 	return SetDefaultConfig(m)
 }
 
-func quickRegister(t *testing.T, key string, optType uint8, defaultValue interface{}) {
+func quickRegister(t *testing.T, key string, optType OptionType, defaultValue interface{}) {
 	err := Register(&Option{
 		Name:           key,
 		Key:            key,

config/option.go

@@ -11,15 +11,18 @@ import (
 	"github.com/safing/portbase/database/record"
 )
 
+// OptionType defines the value type of an option.
+type OptionType uint8
+
 // Various attribute options. Use ExternalOptType for extended types in the frontend.
 const (
-	OptTypeString      uint8 = 1
+	OptTypeString      OptionType = 1
-	OptTypeStringArray uint8 = 2
+	OptTypeStringArray OptionType = 2
-	OptTypeInt         uint8 = 3
+	OptTypeInt         OptionType = 3
-	OptTypeBool        uint8 = 4
+	OptTypeBool        OptionType = 4
 )
 
-func getTypeName(t uint8) string {
+func getTypeName(t OptionType) string {
 	switch t {
 	case OptTypeString:
 		return "string"
@@ -34,25 +37,140 @@ func getTypeName(t uint8) string {
 	}
 }
 
+// PossibleValue defines a value that is possible for
+// a configuration setting.
+type PossibleValue struct {
+	// Name is a human readable name of the option.
+	Name string
+	// Description is a human readable description of
+	// this value.
+	Description string
+	// Value is the actual value of the option. The type
+	// must match the option's value type.
+	Value interface{}
+}
+
+// Annotations can be attached to configuration options to
+// provide hints for user interfaces or other systems working
+// or setting configuration options.
+// Annotation keys should follow the below format to ensure
+// future well-known annotation additions do not conflict
+// with vendor/product/package specific annoations.
+//
+// Format: <vendor/package>:<scope>:<identifier>
+type Annotations map[string]interface{}
+
+// Well known annotations defined by this package.
+const (
+	// DisplayHintAnnotation provides a hint for the user
+	// interface on how to render an option.
+	// The value of DisplayHintAnnotation is expected to
+	// be a string. See DisplayHintXXXX constants below
+	// for a list of well-known display hint annotations.
+	DisplayHintAnnotation = "safing/portbase:ui:display-hint"
+	// DisplayOrderAnnotation provides a hint for the user
+	// interface in which order settings should be displayed.
+	// The value of DisplayOrderAnnotations is expected to be
+	// an number (int).
+	DisplayOrderAnnotation = "safing/portbase:ui:order"
+	// UnitAnnotations defines the SI unit of an option (if any).
+	UnitAnnotation = "safing/portbase:ui:unit"
+	// CategoryAnnotations can provide an additional category
+	// to each settings. This category can be used by a user
+	// interface to group certain options together.
+	// User interfaces should treat a CategoryAnnotation, if
+	// supported, with higher priority as a DisplayOrderAnnotation.
+	CategoryAnnotation = "safing/portbase:ui:category"
+	// SubsystemAnnotation can be used to mark an option as part
+	// of a module subsystem.
+	SubsystemAnnotation = "safing/portbase:module:subsystem"
+)
+
+// Values for the DisplayHintAnnotation
+const (
+	// DisplayHintOneOf is used to mark an option
+	// as a "select"-style option. That is, only one of
+	// the supported values may be set. This option makes
+	// only sense together with the PossibleValues property
+	// of Option.
+	DisplayHintOneOf = "one-of"
+	// DisplayHintOrdered Used to mark a list option as ordered.
+	// That is, the order of items is important and a user interface
+	// is encouraged to provide the user with re-ordering support
+	// (like drag'n'drop).
+	DisplayHintOrdered = "ordered"
+)
+
 // Option describes a configuration option.
 type Option struct {
 	sync.Mutex
-
+	// Name holds the name of the configuration options.
-	Name        string
+	// It should be human readable and is mainly used for
-	Key         string // in path format: category/sub/key
+	// presentation purposes.
+	// Name is considered immutable after the option has
+	// been created.
+	Name string
+	// Key holds the database path for the option. It should
+	// follow the path format `category/sub/key`.
+	// Key is considered immutable after the option has
+	// been created.
+	Key string
+	// Description holds a human readable description of the
+	// option and what is does. The description should be short.
+	// Use the Help property for a longer support text.
+	// Description is considered immutable after the option has
+	// been created.
 	Description string
-	Help        string
+	// Help may hold a long version of the description providing
-	Order       int
+	// assistence with the configuration option.
-
+	// Help is considered immutable after the option has
-	OptType        uint8
+	// been created.
-	ExpertiseLevel uint8
+	Help string
-	ReleaseLevel   uint8
+	// OptType defines the type of the option.
-
+	// OptType is considered immutable after the option has
+	// been created.
+	OptType OptionType
+	// ExpertiseLevel can be used to set the required expertise
+	// level for the option to be displayed to a user.
+	// ExpertiseLevel is considered immutable after the option has
+	// been created.
+	ExpertiseLevel ExpertiseLevel
+	// ReleaseLevel is used to mark the stability of the option.
+	// ReleaseLevel is considered immutable after the option has
+	// been created.
+	ReleaseLevel ReleaseLevel
+	// RequiresRestart should be set to true if a modification of
+	// the options value requires a restart of the whole application
+	// to take effect.
+	// RequiresRestart is considered immutable after the option has
+	// been created.
 	RequiresRestart bool
-	DefaultValue    interface{}
+	// DefaultValue holds the default value of the option. Note that
-
+	// this value can be overwritten during runtime (see activeDefaultValue
-	ExternalOptType string
+	// and activeFallbackValue).
+	// DefaultValue is considered immutable after the option has
+	// been created.
+	DefaultValue interface{}
+	// ValidationRegex may contain a regular expression used to validate
+	// the value of option. If the option type is set to OptTypeStringArray
+	// the validation regex is applied to all entries of the string slice.
+	// Note that it is recommended to keep the validation regex simple so
+	// it can also be used in other languages (mainly JavaScript) to provide
+	// a better user-experience by pre-validating the expression.
+	// ValidationRegex is considered immutable after the option has
+	// been created.
 	ValidationRegex string
+	// PossibleValues may be set to a slice of values that are allowed
+	// for this configuration setting. Note that PossibleValues makes most
+	// sense when ExternalOptType is set to HintOneOf
+	// PossibleValues is considered immutable after the option has
+	// been created.
+	PossibleValues []PossibleValue `json:",omitempty"`
+	// Annotations adds additional annotations to the configuration options.
+	// See documentation of Annotations for more information.
+	// Annotations is considered mutable and setting/reading annotation keys
+	// must be performed while the option is locked.
+	Annotations Annotations
 
 	activeValue         *valueCache // runtime value (loaded from config file or set by user)
 	activeDefaultValue  *valueCache // runtime default value (may be set internally)
@@ -60,6 +178,45 @@ type Option struct {
 	compiledRegex       *regexp.Regexp
 }
 
+// AddAnnotation adds the annotation key to option if it's not already set.
+func (option *Option) AddAnnotation(key string, value interface{}) {
+	option.Lock()
+	defer option.Unlock()
+
+	if option.Annotations == nil {
+		option.Annotations = make(Annotations)
+	}
+
+	if _, ok := option.Annotations[key]; ok {
+		return
+	}
+	option.Annotations[key] = value
+}
+
+// SetAnnotation sets the value of the annotation key overwritting an
+// existing value if required.
+func (option *Option) SetAnnotation(key string, value interface{}) {
+	option.Lock()
+	defer option.Unlock()
+
+	if option.Annotations == nil {
+		option.Annotations = make(Annotations)
+	}
+	option.Annotations[key] = value
+}
+
+// GetAnnotation returns the value of the annotation key
+func (option *Option) GetAnnotation(key string) (interface{}, bool) {
+	option.Lock()
+	defer option.Unlock()
+
+	if option.Annotations == nil {
+		return nil, false
+	}
+	val, ok := option.Annotations[key]
+	return val, ok
+}
+
 // Export expors an option to a Record.
 func (option *Option) Export() (record.Record, error) {
 	option.Lock()
@@ -93,20 +250,8 @@ func (option *Option) Export() (record.Record, error) {
 	return r, nil
 }
 
-type sortableOptions []*Option
+type sortByKey []*Option
 
-// Len is the number of elements in the collection.
+func (opts sortByKey) Len() int           { return len(opts) }
-func (opts sortableOptions) Len() int {
+func (opts sortByKey) Less(i, j int) bool { return opts[i].Key < opts[j].Key }
-	return len(opts)
+func (opts sortByKey) Swap(i, j int)      { opts[i], opts[j] = opts[j], opts[i] }
-}
-
-// Less reports whether the element with
-// index i should sort before the element with index j.
-func (opts sortableOptions) Less(i, j int) bool {
-	return opts[i].Key < opts[j].Key
-}
-
-// Swap swaps the elements with indexes i and j.
-func (opts sortableOptions) Swap(i, j int) {
-	opts[i], opts[j] = opts[j], opts[i]
-}

config/perspective.go

@@ -63,7 +63,7 @@ optionsLoop:
 	return perspective, nil
 }
 
-func (p *Perspective) getPerspectiveValueCache(name string, requestedType uint8) *valueCache {
+func (p *Perspective) getPerspectiveValueCache(name string, requestedType OptionType) *valueCache {
 	// get option
 	pOption, ok := p.config[name]
 	if !ok {

config/registry.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"regexp"
+	"strings"
 	"sync"
 )
 
@@ -11,6 +12,23 @@ var (
 	options     = make(map[string]*Option)
 )
 
+// ForEachOption calls fn for each defined option. If fn returns
+// and error the iteration is stopped and the error is returned.
+// Note that ForEachOption does not guarantee a stable order of
+// iteration between multiple calles. ForEachOption does NOT lock
+// opt when calling fn.
+func ForEachOption(fn func(opt *Option) error) error {
+	optionsLock.Lock()
+	defer optionsLock.Unlock()
+
+	for _, opt := range options {
+		if err := fn(opt); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Register registers a new configuration option.
 func Register(option *Option) error {
 	if option.Name == "" {
@@ -26,8 +44,15 @@ func Register(option *Option) error {
 		return fmt.Errorf("failed to register option: please set option.OptType")
 	}
 
-	var err error
+	if option.ValidationRegex == "" && option.PossibleValues != nil {
+		values := make([]string, len(option.PossibleValues))
+		for idx, val := range option.PossibleValues {
+			values[idx] = fmt.Sprintf("%v", val.Value)
+		}
+		option.ValidationRegex = fmt.Sprintf("^(%s)$", strings.Join(values, "|"))
+	}
 
+	var err error
 	if option.ValidationRegex != "" {
 		option.compiledRegex, err = regexp.Compile(option.ValidationRegex)
 		if err != nil {

config/release.go

@@ -3,17 +3,20 @@
 package config
 
 import (
-	"fmt"
 	"sync/atomic"
 
 	"github.com/tevino/abool"
 )
 
+// ReleaseLevel is used to define the maturity of a
+// configuration setting.
+type ReleaseLevel uint8
+
 // Release Level constants
 const (
-	ReleaseLevelStable       uint8 = 0
+	ReleaseLevelStable       ReleaseLevel = 0
-	ReleaseLevelBeta         uint8 = 1
+	ReleaseLevelBeta         ReleaseLevel = 1
-	ReleaseLevelExperimental uint8 = 2
+	ReleaseLevelExperimental ReleaseLevel = 2
 
 	ReleaseLevelNameStable       = "stable"
 	ReleaseLevelNameBeta         = "beta"
@@ -23,33 +26,44 @@ const (
 )
 
 var (
-	releaseLevel           *int32
+	releaseLevel           = new(int32)
 	releaseLevelOption     *Option
 	releaseLevelOptionFlag = abool.New()
 )
 
 func init() {
-	var releaseLevelVal int32
-	releaseLevel = &releaseLevelVal
-
 	registerReleaseLevelOption()
 }
 
 func registerReleaseLevelOption() {
 	releaseLevelOption = &Option{
-		Name:        "Release Level",
+		Name:           "Release Level",
-		Key:         releaseLevelKey,
+		Key:            releaseLevelKey,
-		Description: "The Release Level changes which features are available to you. Some beta or experimental features are also available in the stable release channel. Unavailable settings are set to the default value.",
+		Description:    "The Release Level changes which features are available to you. Some beta or experimental features are also available in the stable release channel. Unavailable settings are set to the default value.",
-
 		OptType:        OptTypeString,
 		ExpertiseLevel: ExpertiseLevelExpert,
 		ReleaseLevel:   ReleaseLevelStable,
-
+		DefaultValue:   ReleaseLevelNameStable,
-		RequiresRestart: false,
+		Annotations: Annotations{
-		DefaultValue:    ReleaseLevelNameStable,
+			DisplayHintAnnotation: DisplayHintOneOf,
-
+		},
-		ExternalOptType: "string list",
+		PossibleValues: []PossibleValue{
-		ValidationRegex: fmt.Sprintf("^(%s|%s|%s)$", ReleaseLevelNameStable, ReleaseLevelNameBeta, ReleaseLevelNameExperimental),
+			{
+				Name:        "Stable",
+				Value:       ReleaseLevelNameStable,
+				Description: "Only show stable features.",
+			},
+			{
+				Name:        "Beta",
+				Value:       ReleaseLevelNameBeta,
+				Description: "Show stable and beta features.",
+			},
+			{
+				Name:        "Experimental",
+				Value:       ReleaseLevelNameExperimental,
+				Description: "Show experimental features",
+			},
+		},
 	}
 
 	err := Register(releaseLevelOption)
@@ -86,6 +100,6 @@ func updateReleaseLevel() {
 	}
 }
 
-func getReleaseLevel() uint8 {
+func getReleaseLevel() ReleaseLevel {
-	return uint8(atomic.LoadInt32(releaseLevel))
+	return ReleaseLevel(atomic.LoadInt32(releaseLevel))
 }

config/validate.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"reflect"
 )
 
 type valueCache struct {
@@ -28,7 +29,48 @@ func (vc *valueCache) getData(opt *Option) interface{} {
 	}
 }
 
+// isAllowedPossibleValue checks if value is defined as a PossibleValue
+// in opt. If there are not possible values defined value is considered
+// allowed and nil is returned. isAllowedPossibleValue ensure the actual
+// value is an allowed primitiv value by using reflection to convert
+// value and each PossibleValue to a comparable primitiv if possible.
+// In case of complex value types isAllowedPossibleValue uses
+// reflect.DeepEqual as a fallback.
+func isAllowedPossibleValue(opt *Option, value interface{}) error {
+	if opt.PossibleValues == nil {
+		return nil
+	}
+
+	for _, val := range opt.PossibleValues {
+		compareAgainst := val.Value
+		valueType := reflect.TypeOf(value)
+
+		// loading int's from the configuration JSON does not perserve the correct type
+		// as we get float64 instead. Make sure to convert them before.
+		if reflect.TypeOf(val.Value).ConvertibleTo(valueType) {
+			compareAgainst = reflect.ValueOf(val.Value).Convert(valueType).Interface()
+		}
+		if compareAgainst == value {
+			return nil
+		}
+
+		if reflect.DeepEqual(val.Value, value) {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("value is not allowed")
+}
+
 func validateValue(option *Option, value interface{}) (*valueCache, error) { //nolint:gocyclo
+	if option.OptType != OptTypeStringArray {
+		if err := isAllowedPossibleValue(option, value); err != nil {
+			return nil, fmt.Errorf("validation of option %s failed for %v: %w", option.Key, value, err)
+		}
+	}
+
+	reflect.TypeOf(value).ConvertibleTo(reflect.TypeOf(""))
+
 	switch v := value.(type) {
 	case string:
 		if option.OptType != OptTypeString {
@@ -61,6 +103,10 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
 				if !option.compiledRegex.MatchString(entry) {
 					return nil, fmt.Errorf("validation of option %s failed: string \"%s\" at index %d did not match validation regex", option.Key, entry, pos)
 				}
+
+				if err := isAllowedPossibleValue(option, entry); err != nil {
+					return nil, fmt.Errorf("validation of option %s failed: string %q at index %d is not allowed", option.Key, entry, pos)
+				}
 			}
 		}
 		return &valueCache{stringArrayVal: v}, nil

modules/subsystems/module.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/safing/portbase/config"
 	"github.com/safing/portbase/database"
 	_ "github.com/safing/portbase/database/dbmodule" // database module is required
 	"github.com/safing/portbase/modules"
@@ -25,10 +26,14 @@ var (
 )
 
 func init() {
-	// enable partial starting
+	// The subsystem layer takes over module management. Note that
+	// no one must have called EnableModuleManagement. Otherwise
+	// the subsystem layer will silently fail managing module
+	// dependencies!
+	// TODO(ppacher): we SHOULD panic here!
+	// TASK(#1431)
 	modules.EnableModuleManagement(handleModuleChanges)
 
-	// register module and enable it for starting
 	module = modules.Register("subsystems", prep, start, nil, "config", "database", "base")
 	module.Enable()
 
@@ -44,29 +49,59 @@ func prep() error {
 		return modules.ErrCleanExit
 	}
 
-	return module.RegisterEventHook("config", configChangeEvent, "control subsystems", handleConfigChanges)
+	// We need to listen for configuration changes so we can
+	// start/stop dependend modules in case a subsystem is
+	// (de-)activated.
+	if err := module.RegisterEventHook(
+		"config",
+		configChangeEvent,
+		"control subsystems",
+		handleConfigChanges,
+	); err != nil {
+		return fmt.Errorf("register event hook: %w", err)
+	}
+
+	return nil
 }
 
 func start() error {
-	// lock registration
+	// Registration of subsystems is only allowed during
+	// preperation. Make sure any further call to Register()
+	// panics.
 	subsystemsLocked.Set()
 
-	// lock slice and map
 	subsystemsLock.Lock()
-	// go through all dependencies
+	defer subsystemsLock.Unlock()
-	seen := make(map[string]struct{})
+
+	seen := make(map[string]struct{}, len(subsystems))
+	configKeyPrefixes := make(map[string]*Subsystem, len(subsystems))
+	// mark all sub-systems as seen. This prevents sub-systems
+	// from being added as a sub-systems dependency in addAndMarkDependencies.
 	for _, sub := range subsystems {
-		// mark subsystem module as seen
 		seen[sub.module.Name] = struct{}{}
+		configKeyPrefixes[sub.ConfigKeySpace] = sub
 	}
+
+	// aggregate all modules dependencies (and the subsystem module itself)
+	// into the Modules slice. Configuration options form dependened modules
+	// will be marked using config.SubsystemAnnotation if not already set.
 	for _, sub := range subsystems {
-		// add main module
 		sub.Modules = append(sub.Modules, statusFromModule(sub.module))
-		// add dependencies
 		sub.addDependencies(sub.module, seen)
 	}
-	// unlock
+
-	subsystemsLock.Unlock()
+	// Annotate all configuration options with their respective subsystem.
+	config.ForEachOption(func(opt *config.Option) error {
+		subsys, ok := configKeyPrefixes[opt.Key]
+		if !ok {
+			return nil
+		}
+
+		// Add a new subsystem annotation is it is not already set!
+		opt.AddAnnotation(config.SubsystemAnnotation, subsys.ID)
+
+		return nil
+	})
 
 	// apply config
 	module.StartWorker("initial subsystem configuration", func(ctx context.Context) error {
@@ -77,13 +112,10 @@ func start() error {
 
 func (sub *Subsystem) addDependencies(module *modules.Module, seen map[string]struct{}) {
 	for _, module := range module.Dependencies() {
-		_, ok := seen[module.Name]
+		if _, ok := seen[module.Name]; !ok {
-		if !ok {
-			// add dependency to modules
-			sub.Modules = append(sub.Modules, statusFromModule(module))
-			// mark as seen
 			seen[module.Name] = struct{}{}
-			// add further dependencies
+
+			sub.Modules = append(sub.Modules, statusFromModule(module))
 			sub.addDependencies(module, seen)
 		}
 	}
@@ -109,7 +141,7 @@ func printGraph() {
 	for _, sub := range subsystems {
 		sub.module.Enable() // mark as tree root
 	}
-	// print
+
 	for _, sub := range subsystems {
 		printModuleGraph("", sub.module, true)
 	}

modules/subsystems/subsystem.go

@@ -9,26 +9,46 @@ import (
 	"github.com/safing/portbase/modules"
 )
 
-// Subsystem describes a subset of modules that represent a part of a service or program to the user.
+// Subsystem describes a subset of modules that represent a part of a
+// service or program to the user. Subsystems can be (de-)activated causing
+// all related modules to be brought down or up.
 type Subsystem struct { //nolint:maligned // not worth the effort
 	record.Base
 	sync.Mutex
-
+	// ID is a unique identifier for the subsystem.
-	ID          string
+	ID string
-	Name        string
+	// Name holds a human readable name of the subsystem.
+	Name string
+	// Description may holds an optional description of
+	// the subsystem's purpose.
 	Description string
-	module      *modules.Module
+	// Modules contains all modules that are related to the subsystem.
-
+	// Note that this slice also contains a reference to the subsystem
-	Modules       []*ModuleStatus
+	// module itself.
-	FailureStatus uint8 // summary: worst status
+	Modules []*ModuleStatus
-
+	// FailureStatus is the worst failure status that is currently
+	// set in one of the subsystem's dependencies.
+	FailureStatus uint8
+	// ToggleOptionKey holds the key of the configuraiton option
+	// that is used to completely enable or disable this subsystem.
 	ToggleOptionKey string
-	toggleOption    *config.Option
+	// ExpertiseLevel defines the complexity of the subsystem and is
-	toggleValue     func() bool
+	// copied from the subsystem's toggleOption.
-	ExpertiseLevel  uint8 // copied from toggleOption
+	ExpertiseLevel config.ExpertiseLevel
-	ReleaseLevel    uint8 // copied from toggleOption
+	// ReleaseLevel defines the stability of the subsystem and is
-
+	// copied form the subsystem's toggleOption.
+	ReleaseLevel config.ReleaseLevel
+	// ConfigKeySpace defines the database key prefix that all
+	// options that belong to this subsystem have. Note that this
+	// value is mainly used to mark all related options with a
+	// config.SubsystemAnnotation. Options that are part of
+	// this subsystem but don't start with the correct prefix can
+	// still be marked by manually setting the appropriate annotation.
 	ConfigKeySpace string
+
+	module       *modules.Module
+	toggleOption *config.Option
+	toggleValue  config.BoolOption
 }
 
 // ModuleStatus describes the status of a module.
@@ -48,14 +68,17 @@ type ModuleStatus struct {
 
 // Save saves the Subsystem Status to the database.
 func (sub *Subsystem) Save() {
-	if databaseKeySpace != "" {
+	if databaseKeySpace == "" {
-		if !sub.KeyIsSet() {
+		return
-			sub.SetKey(databaseKeySpace + sub.ID)
+	}
-		}
+
-		err := db.Put(sub)
+	if !sub.KeyIsSet() {
-		if err != nil {
+		sub.SetKey(databaseKeySpace + sub.ID)
-			log.Errorf("subsystems: could not save subsystem status to database: %s", err)
+	}
-		}
+
+	err := db.Put(sub)
+	if err != nil {
+		log.Errorf("subsystems: could not save subsystem status to database: %s", err)
 	}
 }
 
@@ -90,6 +113,7 @@ func compareAndUpdateStatus(module *modules.Module, status *ModuleStatus) (chang
 	failureStatus, failureID, failureMsg := module.FailureStatus()
 	if status.FailureStatus != failureStatus ||
 		status.FailureID != failureID {
+
 		status.FailureStatus = failureStatus
 		status.FailureID = failureID
 		status.FailureMsg = failureMsg

modules/subsystems/subsystems.go

@@ -13,15 +13,18 @@ import (
 )
 
 var (
+	subsystemsLock   sync.Mutex
 	subsystems       []*Subsystem
 	subsystemsMap    = make(map[string]*Subsystem)
-	subsystemsLock   sync.Mutex
 	subsystemsLocked = abool.New()
 
 	handlingConfigChanges = abool.New()
 )
 
-// Register registers a new subsystem. The given option must be a bool option. Should be called in init() directly after the modules.Register() function. The config option must not yet be registered and will be registered for you. Pass a nil option to force enable.
+// Register registers a new subsystem. The given option must be a bool option.
+// Should be called in init() directly after the modules.Register() function.
+// The config option must not yet be registered and will be registered for
+// you. Pass a nil option to force enable.
 func Register(id, name, description string, module *modules.Module, configKeySpace string, option *config.Option) {
 	// lock slice and map
 	subsystemsLock.Lock()
@@ -33,8 +36,7 @@ func Register(id, name, description string, module *modules.Module, configKeySpa
 	}
 
 	// check if already registered
-	_, ok := subsystemsMap[name]
+	if _, ok := subsystemsMap[name]; ok {
-	if ok {
 		panic(fmt.Sprintf(`subsystem "%s" already registered`, name))
 	}
 
@@ -65,7 +67,6 @@ func Register(id, name, description string, module *modules.Module, configKeySpa
 		new.toggleValue = func() bool { return true }
 	}
 
-	// add to lists
 	subsystemsMap[name] = new
 	subsystems = append(subsystems, new)
 }
@@ -113,9 +114,10 @@ subsystemLoop:
 	}
 }
 
-func handleConfigChanges(ctx context.Context, data interface{}) error {
+func handleConfigChanges(ctx context.Context, _ interface{}) error {
-	// check if ready
+	// bail out early if we haven't started yet or are already
-	if !subsystemsLocked.IsSet() {
+	// shutting down
+	if !subsystemsLocked.IsSet() || modules.IsShuttingDown() {
 		return nil
 	}
 
@@ -127,11 +129,6 @@ func handleConfigChanges(ctx context.Context, data interface{}) error {
 		return nil
 	}
 
-	// don't do anything if we are already shutting down globally
-	if modules.IsShuttingDown() {
-		return nil
-	}
-
 	// only run one instance at any time
 	subsystemsLock.Lock()
 	defer subsystemsLock.Unlock()
@@ -143,18 +140,18 @@ func handleConfigChanges(ctx context.Context, data interface{}) error {
 			changed = true
 		}
 	}
+	if !changed {
+		return nil
+	}
 
-	// trigger module management if any setting was changed
+	err := modules.ManageModules()
-	if changed {
+	if err != nil {
-		err := modules.ManageModules()
+		module.Error(
-		if err != nil {
+			"modulemgmt-failed",
-			module.Error(
+			fmt.Sprintf("The subsystem framework failed to start or stop one or more modules.\nError: %s\nCheck logs for more information.", err),
-				"modulemgmt-failed",
+		)
-				fmt.Sprintf("The subsystem framework failed to start or stop one or more modules.\nError: %s\nCheck logs for more information.", err),
+	} else {
-			)
+		module.Resolve("modulemgmt-failed")
-		} else {
-			module.Resolve("modulemgmt-failed")
-		}
 	}
 
 	return nil

template/module.go

@@ -46,7 +46,7 @@ func prep() error {
 	// register options
 	err := config.Register(&config.Option{
 		Name:            "language",
-		Key:             "config:template/language",
+		Key:             "template/language",
 		Description:     "Sets the language for the template [TEMPLATE]",
 		OptType:         config.OptTypeString,
 		ExpertiseLevel:  config.ExpertiseLevelUser, // default