feat(featureflags): improved feature flag handling [EE-4609] (#8222)

* updated and improved feature flags using new module

* merge init into parse

* update the package documentation

* better docs

* minor tidy
pull/8477/head
Matt Hook 2 years ago committed by GitHub
parent 51b9804fab
commit bfc610c192
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -36,7 +36,7 @@ func (*Service) ParseFlags(version string) (*portainer.CLIFlags, error) {
Data: kingpin.Flag("data", "Path to the folder where the data is stored").Default(defaultDataDirectory).Short('d').String(),
DemoEnvironment: kingpin.Flag("demo", "Demo environment").Bool(),
EndpointURL: kingpin.Flag("host", "Environment URL").Short('H').String(),
FeatureFlags: BoolPairs(kingpin.Flag("feat", "List of feature flags").Hidden()),
FeatureFlags: kingpin.Flag("feat", "List of feature flags").Strings(),
EnableEdgeComputeFeatures: kingpin.Flag("edge-compute", "Enable Edge Compute features").Bool(),
NoAnalytics: kingpin.Flag("no-analytics", "Disable Analytics in app (deprecated)").Bool(),
TLS: kingpin.Flag("tlsverify", "TLS support").Default(defaultTLS).Bool(),

@ -3,11 +3,9 @@ package main
import (
"context"
"crypto/sha256"
"fmt"
"math/rand"
"os"
"path"
"strconv"
"strings"
"time"
@ -47,6 +45,7 @@ import (
"github.com/portainer/portainer/api/oauth"
"github.com/portainer/portainer/api/scheduler"
"github.com/portainer/portainer/api/stacks/deployments"
"github.com/portainer/portainer/pkg/featureflags"
"github.com/portainer/portainer/pkg/libhelm"
"github.com/gofrs/uuid"
@ -318,45 +317,6 @@ func updateSettingsFromFlags(dataStore dataservices.DataStore, flags *portainer.
return dataStore.SSLSettings().UpdateSettings(sslSettings)
}
// enableFeaturesFromFlags turns on or off feature flags
// e.g. portainer --feat open-amt --feat fdo=true ... (defaults to true)
// note, settings are persisted to the DB. To turn off `--feat open-amt=false`
func enableFeaturesFromFlags(dataStore dataservices.DataStore, flags *portainer.CLIFlags) error {
settings, err := dataStore.Settings().Settings()
if err != nil {
return err
}
if settings.FeatureFlagSettings == nil {
settings.FeatureFlagSettings = make(map[portainer.Feature]bool)
}
// loop through feature flags to check if they are supported
for _, feat := range *flags.FeatureFlags {
var correspondingFeature *portainer.Feature
for i, supportedFeat := range portainer.SupportedFeatureFlags {
if strings.EqualFold(feat.Name, string(supportedFeat)) {
correspondingFeature = &portainer.SupportedFeatureFlags[i]
}
}
if correspondingFeature == nil {
return fmt.Errorf("unknown feature flag '%s'", feat.Name)
}
featureState, err := strconv.ParseBool(feat.Value)
if err != nil {
return fmt.Errorf("feature flag's '%s' value should be true or false", feat.Name)
}
log.Info().Str("feature", string(*correspondingFeature)).Bool("state", featureState).Msg("")
settings.FeatureFlagSettings[*correspondingFeature] = featureState
}
return dataStore.Settings().UpdateSettings(settings)
}
func loadAndParseKeyPair(fileService portainer.FileService, signatureService portainer.DigitalSignatureService) error {
private, public, err := fileService.LoadKeyPair()
if err != nil {
@ -547,6 +507,10 @@ func loadEncryptionSecretKey(keyfilename string) []byte {
func buildServer(flags *portainer.CLIFlags) portainer.Server {
shutdownCtx, shutdownTrigger := context.WithCancel(context.Background())
if flags.FeatureFlags != nil {
featureflags.Parse(*flags.FeatureFlags, portainer.SupportedFeatureFlags)
}
fileService := initFileService(*flags.Data)
encryptionKey := loadEncryptionSecretKey(*flags.SecretKeyName)
if encryptionKey == nil {
@ -576,11 +540,6 @@ func buildServer(flags *portainer.CLIFlags) portainer.Server {
log.Fatal().Err(err).Msg("failed initializing JWT service")
}
err = enableFeaturesFromFlags(dataStore, flags)
if err != nil {
log.Fatal().Err(err).Msg("failed enabling feature flag")
}
ldapService := initLDAPService()
oauthService := initOAuthService()

@ -1,111 +0,0 @@
package main
import (
"fmt"
"testing"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/cli"
"github.com/portainer/portainer/api/dataservices"
"github.com/portainer/portainer/api/datastore"
"github.com/stretchr/testify/assert"
"gopkg.in/alecthomas/kingpin.v2"
)
type mockKingpinSetting string
func (m mockKingpinSetting) SetValue(value kingpin.Value) {
value.Set(string(m))
}
func Test_enableFeaturesFromFlags(t *testing.T) {
is := assert.New(t)
_, store, teardown := datastore.MustNewTestStore(t, true, true)
defer teardown()
tests := []struct {
featureFlag string
isSupported bool
}{
{"test", false},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%s succeeds:%v", test.featureFlag, test.isSupported), func(t *testing.T) {
mockKingpinSetting := mockKingpinSetting(test.featureFlag)
flags := &portainer.CLIFlags{FeatureFlags: cli.BoolPairs(mockKingpinSetting)}
err := enableFeaturesFromFlags(store, flags)
if test.isSupported {
is.NoError(err)
} else {
is.Error(err)
}
})
}
t.Run("passes for all supported feature flags", func(t *testing.T) {
for _, flag := range portainer.SupportedFeatureFlags {
mockKingpinSetting := mockKingpinSetting(flag)
flags := &portainer.CLIFlags{FeatureFlags: cli.BoolPairs(mockKingpinSetting)}
err := enableFeaturesFromFlags(store, flags)
is.NoError(err)
}
})
}
const FeatTest portainer.Feature = "optional-test"
func optionalFunc(dataStore dataservices.DataStore) string {
// TODO: this is a code smell - finding out if a feature flag is enabled should not require having access to the store, and asking for a settings obj.
// ideally, the `if` should look more like:
// if featureflags.FlagEnabled(FeatTest) {}
settings, err := dataStore.Settings().Settings()
if err != nil {
return err.Error()
}
if settings.FeatureFlagSettings[FeatTest] {
return "enabled"
}
return "disabled"
}
func Test_optionalFeature(t *testing.T) {
portainer.SupportedFeatureFlags = append(portainer.SupportedFeatureFlags, FeatTest)
is := assert.New(t)
_, store, teardown := datastore.MustNewTestStore(t, true, true)
defer teardown()
// Enable the test feature
t.Run(fmt.Sprintf("%s succeeds:%v", FeatTest, true), func(t *testing.T) {
mockKingpinSetting := mockKingpinSetting(FeatTest)
flags := &portainer.CLIFlags{FeatureFlags: cli.BoolPairs(mockKingpinSetting)}
err := enableFeaturesFromFlags(store, flags)
is.NoError(err)
is.Equal("enabled", optionalFunc(store))
})
// Same store, so the feature flag should still be enabled
t.Run(fmt.Sprintf("%s succeeds:%v", FeatTest, true), func(t *testing.T) {
is.Equal("enabled", optionalFunc(store))
})
// disable the test feature
t.Run(fmt.Sprintf("%s succeeds:%v", FeatTest, true), func(t *testing.T) {
mockKingpinSetting := mockKingpinSetting(FeatTest + "=false")
flags := &portainer.CLIFlags{FeatureFlags: cli.BoolPairs(mockKingpinSetting)}
err := enableFeaturesFromFlags(store, flags)
is.NoError(err)
is.Equal("disabled", optionalFunc(store))
})
// Same store, so feature flag should still be disabled
t.Run(fmt.Sprintf("%s succeeds:%v", FeatTest, true), func(t *testing.T) {
is.Equal("disabled", optionalFunc(store))
})
}

@ -205,7 +205,6 @@ type (
SettingsService interface {
Settings() (*portainer.Settings, error)
UpdateSettings(settings *portainer.Settings) error
IsFeatureFlagEnabled(feature portainer.Feature) bool
BucketName() string
}

@ -47,17 +47,3 @@ func (service *Service) Settings() (*portainer.Settings, error) {
func (service *Service) UpdateSettings(settings *portainer.Settings) error {
return service.connection.UpdateObject(BucketName, []byte(settingsKey), settings)
}
func (service *Service) IsFeatureFlagEnabled(feature portainer.Feature) bool {
settings, err := service.Settings()
if err != nil {
return false
}
featureFlagSetting, ok := settings.FeatureFlagSettings[feature]
if ok {
return featureFlagSetting
}
return false
}

@ -7,6 +7,7 @@ import (
httperror "github.com/portainer/libhttp/error"
"github.com/portainer/libhttp/response"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/pkg/featureflags"
)
type publicSettingsResponse struct {
@ -21,7 +22,7 @@ type publicSettingsResponse struct {
// Whether edge compute features are enabled
EnableEdgeComputeFeatures bool `json:"EnableEdgeComputeFeatures" example:"true"`
// Supported feature flags
Features map[portainer.Feature]bool `json:"Features"`
Features map[featureflags.Feature]bool `json:"Features"`
// The URL used for oauth login
OAuthLoginURI string `json:"OAuthLoginURI" example:"https://gitlab.com/oauth"`
// The URL used for oauth logout
@ -80,7 +81,7 @@ func generatePublicSettings(appSettings *portainer.Settings) *publicSettingsResp
ShowKomposeBuildOption: appSettings.ShowKomposeBuildOption,
EnableTelemetry: appSettings.EnableTelemetry,
KubeconfigExpiry: appSettings.KubeconfigExpiry,
Features: appSettings.FeatureFlagSettings,
Features: featureflags.FeatureFlags(),
IsFDOEnabled: appSettings.EnableEdgeComputeFeatures && appSettings.FDOConfiguration.Enabled,
IsAMTEnabled: appSettings.EnableEdgeComputeFeatures && appSettings.OpenAMTConfiguration.Enabled,
}

@ -5,14 +5,14 @@ import (
"github.com/gorilla/mux"
httperror "github.com/portainer/libhttp/error"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/dataservices"
"github.com/portainer/portainer/pkg/featureflags"
)
func FeatureFlag(settingsService dataservices.SettingsService, feature portainer.Feature) mux.MiddlewareFunc {
func FeatureFlag(settingsService dataservices.SettingsService, feature featureflags.Feature) mux.MiddlewareFunc {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, request *http.Request) {
enabled := settingsService.IsFeatureFlagEnabled(feature)
enabled := featureflags.IsEnabled(feature)
if !enabled {
httperror.WriteError(rw, http.StatusForbidden, "This feature is not enabled", nil)

@ -115,9 +115,6 @@ func (s *stubSettingsService) UpdateSettings(settings *portainer.Settings) error
s.settings = settings
return nil
}
func (s *stubSettingsService) IsFeatureFlagEnabled(feature portainer.Feature) bool {
return false
}
func WithSettingsService(settings *portainer.Settings) datastoreOption {
return func(d *testDatastore) {
d.settings = &stubSettingsService{

@ -9,6 +9,7 @@ import (
"github.com/docker/docker/api/types/volume"
gittypes "github.com/portainer/portainer/api/git/types"
models "github.com/portainer/portainer/api/http/models/kubernetes"
"github.com/portainer/portainer/pkg/featureflags"
v1 "k8s.io/api/core/v1"
)
@ -104,7 +105,7 @@ type (
AdminPasswordFile *string
Assets *string
Data *string
FeatureFlags *[]Pair
FeatureFlags *[]string
DemoEnvironment *bool
EnableEdgeComputeFeatures *bool
EndpointURL *string
@ -1531,8 +1532,11 @@ const (
WebSocketKeepAlive = 1 * time.Hour
)
// List of supported features
var SupportedFeatureFlags = []Feature{}
// SupportFeatureFlags is a list of supported features. They should all be lower case
// e.g. "microk8s","kaas"
var SupportedFeatureFlags = []featureflags.Feature{
"microk8s",
}
const (
_ AuthenticationMethod = iota

@ -2,6 +2,7 @@ go 1.19
use (
./api
./pkg/featureflags
./pkg/libhelm
./third_party/digest
)

@ -0,0 +1,17 @@
Copyright (c) 2022 Portainer.io
This software is provided 'as-is', without any express or implied
warranty. In no event will the authors be held liable for any damages
arising from the use of this software.
Permission is granted to anyone to use this software for any purpose,
including commercial applications, and to alter it and redistribute it
freely, subject to the following restrictions:
1. The origin of this software must not be misrepresented; you must not
claim that you wrote the original software. If you use this software
in a product, an acknowledgment in the product documentation would be
appreciated but is not required.
2. Altered source versions must be plainly marked as such, and must not be
misrepresented as being the original software.
3. This notice may not be removed or altered from any source distribution.

@ -0,0 +1,98 @@
/*
Package featureflags implements feature flags for Portainer projects
Feature flags are used to turn on features that are not production ready.
Use the Parse function to enable feature flags and also the pass a list of
available flags
e.g.
var SupportedFeatureFlags = []featureflags.Feature{
"my-feature",
}
func main() {
// parse cli flags
// pass cli flags and supported feature flags to featureflags.Parse
featureflags.Parse([]string{"my-feature"}, SupportedFeatureFlags)
}
...
if featureflags.IsEnabled("my-feature") {
// do something
}
*/
package featureflags
import (
"os"
"strings"
"github.com/rs/zerolog/log"
)
// Feature represents a feature that can be enabled or disabled via feature flags
type Feature string
var featureFlags map[Feature]bool
// String returns the string representation of a feature flag
func (f Feature) String() string {
return string(f)
}
// IsEnabled returns true if the feature flag is enabled
func IsEnabled(feat Feature) bool {
return featureFlags[feat]
}
// IsSupported returns true if the feature is supported
func IsSupported(feat Feature) bool {
_, ok := featureFlags[feat]
return ok
}
// FeatureFlags returns a map of all feature flags.
// this is useful in situations where you need to pass all feature flags to a REST handler
// function
func FeatureFlags() map[Feature]bool {
return featureFlags
}
func initSupportedFeatures(supportedFeatures []Feature) {
featureFlags = make(map[Feature]bool)
for _, feat := range supportedFeatures {
featureFlags[feat] = false
}
}
// Parse turns on feature flags
// It accepts a list of feature flags as strings and a list of supported features.
// It will also check for feature flags in the PORTAINER_FEATURE_FLAGS environment variable.
// Multiple feature flags can be specified with the PORTAINER_FEATURE_FLAGS environment.
// variable using a comma separated list. e.g. "PORTAINER_FEATURE_FLAGS=feature1,feature2".
// If a feature flag is not supported, it will be logged and ignored.
// If a feature flag is supported, it will be logged and enabled.
func Parse(features []string, supportedFeatures []Feature) {
initSupportedFeatures(supportedFeatures)
env := os.Getenv("PORTAINER_FEATURE_FLAGS")
envFeatures := []string{}
if env != "" {
envFeatures = strings.Split(env, ",")
}
features = append(features, envFeatures...)
// loop through feature flags to check if they are supported
for _, feat := range features {
f := Feature(strings.ToLower(feat))
_, ok := featureFlags[f]
if !ok {
log.Warn().Str("feature", f.String()).Msgf("unknown feature flag")
continue
}
featureFlags[f] = true
log.Info().Str("feature", f.String()).Msg("enabling feature")
}
}

@ -0,0 +1,75 @@
package featureflags
import (
"os"
"strings"
"testing"
"github.com/stretchr/testify/assert"
)
func Test_enableFeaturesFromFlags(t *testing.T) {
is := assert.New(t)
supportedFeatures := []Feature{"supported", "supported2", "supported3", "supported4", "supported5"}
t.Run("supported features should be supported", func(t *testing.T) {
Init(supportedFeatures)
for _, featureFlag := range supportedFeatures {
is.True(IsSupported(featureFlag))
}
})
t.Run("unsupported features should not be supported", func(t *testing.T) {
Init(supportedFeatures)
is.False(IsSupported("unsupported"))
})
tests := []struct {
cliFeatureFlags []string
envFeatureFlags []string
}{
{[]string{"supported", "supported2"}, []string{"supported3", "supported4"}},
}
for _, test := range tests {
Init(supportedFeatures)
os.Unsetenv("PORTAINER_FEATURE_FLAGS")
os.Setenv("PORTAINER_FEATURE_FLAGS", strings.Join(test.envFeatureFlags, ","))
t.Run("testing", func(t *testing.T) {
Parse(test.cliFeatureFlags)
supported := toFeatureMap(test.cliFeatureFlags, test.envFeatureFlags)
// add env flags to supported flags
for _, featureFlag := range test.envFeatureFlags {
supported[Feature(featureFlag)] = true
}
for _, featureFlag := range supportedFeatures {
if _, ok := supported[featureFlag]; ok {
is.True(IsEnabled(featureFlag))
} else {
is.False(IsEnabled(featureFlag))
}
}
})
}
}
// helper
func toFeatureMap(cliFeatures []string, envFeatures []string) map[Feature]bool {
m := map[Feature]bool{}
for _, s := range cliFeatures {
m[Feature(s)] = true
}
for _, s := range envFeatures {
m[Feature(s)] = true
}
return m
}

@ -0,0 +1,17 @@
module github.com/portainer/portainer/pkg/featureflags
go 1.19
require (
github.com/rs/zerolog v1.28.0
github.com/stretchr/testify v1.8.1
)
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/mattn/go-colorable v0.1.12 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

@ -0,0 +1,30 @@
github.com/coreos/go-systemd/v22 v22.3.3-0.20220203105225-a9a7ef127534/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rs/xid v1.4.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
github.com/rs/zerolog v1.28.0 h1:MirSo27VyNi7RJYP3078AA1+Cyzd2GB66qy3aUHvsWY=
github.com/rs/zerolog v1.28.0/go.mod h1:NILgTygv/Uej1ra5XxGf82ZFSLk58MFGAUS2o6usyD0=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 h1:foEbQz/B0Oz6YIqu/69kfXPYeFQAuuMYFkjaqXzl5Wo=
golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Loading…
Cancel
Save