Skip to content

Commit

Permalink
Add feature flag AllowUnrecognizedFeatures (#6056)
Browse files Browse the repository at this point in the history
By default, Boulder's feature flag code verifies that the list of flags
being set (from a JSON file) maps to actually-existing flags.

However, this gets in the way of a deployment strategy where feature
flags are added to config templates during a staging deploy with "true"
or "false" filled in depending on production or staging status - for
instance, when rolling out a deprecation to staging ahead of production.
If those configs get rolled to prod before the corresponding Boulder
deploy, Boulder will refuse to start up, even though it would be fine to
start up with the unrecognized flag ignored.

The envisioned deployment behavior here is that prod will have
AllowUnrecognizedFeatures: true while staging will have it set to false,
to ensure that misspellings of feature flag names are caught during
staging deploy. As a correlary, this assumes that the list of flags in
configs will be the same between staging and prod, with only their
values changing.
  • Loading branch information
jsha authored Apr 22, 2022
1 parent 9629c88 commit 29249b4
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 7 deletions.
5 changes: 3 additions & 2 deletions features/featureflag_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 17 additions & 5 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package features

import (
"fmt"
"strings"
"sync"
)

Expand Down Expand Up @@ -88,6 +89,9 @@ const (
// SHA1CSRs controls whether the /acme/finalize endpoint rejects CSRs that
// are self-signed using SHA1.
SHA1CSRs
// AllowUnrecognizedFeatures is internal to the features package: if true,
// skip error when unrecognized feature flag names are passed.
AllowUnrecognizedFeatures
)

// List of features and their default value, protected by fMu
Expand Down Expand Up @@ -118,6 +122,7 @@ var features = map[FeatureFlag]bool{
OldTLSOutbound: true,
OldTLSInbound: true,
SHA1CSRs: true,
AllowUnrecognizedFeatures: false,
}

var fMu = new(sync.RWMutex)
Expand All @@ -134,17 +139,24 @@ func init() {
}

// Set accepts a list of features and whether they should
// be enabled or disabled, it will return a error if passed
// a feature name that it doesn't know
// be enabled or disabled. In the presence of unrecognized
// flags, it will return an error or not depending on the
// value of AllowUnrecognizedFeatures.
func Set(featureSet map[string]bool) error {
fMu.Lock()
defer fMu.Unlock()
var unknown []string
for n, v := range featureSet {
f, present := nameToFeature[n]
if !present {
return fmt.Errorf("feature '%s' doesn't exist", n)
if present {
features[f] = v
} else {
unknown = append(unknown, n)
}
features[f] = v
}
if len(unknown) > 0 && !features[AllowUnrecognizedFeatures] {
return fmt.Errorf("unrecognized feature flag names: %s",
strings.Join(unknown, ", "))
}
return nil
}
Expand Down
26 changes: 26 additions & 0 deletions features/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,32 @@ import (
"github.com/letsencrypt/boulder/test"
)

func TestAllowUnrecognizedFeatures(t *testing.T) {
features = map[FeatureFlag]bool{
unused: false,
AllowUnrecognizedFeatures: false,
}
nameToFeature = map[string]FeatureFlag{
"unused": unused,
"AllowUnrecognizedFeatures": AllowUnrecognizedFeatures,
}
err := Set(map[string]bool{
"Z4lG0": true,
"AllowUnrecognizedFeatures": true,
})
test.AssertNotError(t, err, "expected no error when setting an unrecognized feature")

err = Set(map[string]bool{
"Z4lG0": true,
"Zombo": true,
"AllowUnrecognizedFeatures": false,
})
test.AssertError(t, err, "expected error when disallowing unrecognized features")
test.AssertContains(t, err.Error(), "unrecognized feature flag names: ")
test.AssertContains(t, err.Error(), "Z4lG0")
test.AssertContains(t, err.Error(), "Zombo")
}

func TestFeatures(t *testing.T) {
features = map[FeatureFlag]bool{
unused: false,
Expand Down

0 comments on commit 29249b4

Please sign in to comment.