Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support for feature gates #1146

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/config/wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ executables
executoroptions
executorspecification
extensibility
featuregates
filepath
filesystem
filesytem
Expand Down
4 changes: 2 additions & 2 deletions api/config/internal/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

var _ = Describe("setup", func() {
It("creates initial", func() {
Expect(len(config.DefaultContext().ConfigTypes().KnownTypeNames())).To(Equal(6))
Expect(len(internal.DefaultConfigTypeScheme.KnownTypeNames())).To(Equal(6))
Expect(len(config.DefaultContext().ConfigTypes().KnownTypeNames())).To(Equal(8))
Expect(len(internal.DefaultConfigTypeScheme.KnownTypeNames())).To(Equal(8))
})
})
199 changes: 199 additions & 0 deletions api/datacontext/attrs/featuregatesattr/attr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package featuregatesattr

import (
"encoding/json"
"fmt"
"sync"

"github.com/mandelsoft/goutils/general"
"sigs.k8s.io/yaml"

"ocm.software/ocm/api/datacontext"
"ocm.software/ocm/api/utils/runtime"
)

const (
ATTR_KEY = "ocm.software/ocm/feature-gates"
ATTR_SHORT = "featuregates"
hilmarf marked this conversation as resolved.
Show resolved Hide resolved
)

func init() {
datacontext.RegisterAttributeType(ATTR_KEY, AttributeType{}, ATTR_SHORT)
}

type AttributeType struct{}

func (a AttributeType) Name() string {
return ATTR_KEY
}

func (a AttributeType) Description() string {
return `
*featuregates* Enable/Disable optional features of the OCM library.
Optionally, particular features modes and attributes can be configured, if
supported by the feature implementation.
`
}

func (a AttributeType) Encode(v interface{}, marshaller runtime.Marshaler) ([]byte, error) {
switch t := v.(type) {
case *Attribute:
return json.Marshal(v)
case string:
_, err := a.Decode([]byte(t), runtime.DefaultYAMLEncoding)
if err != nil {
return nil, err
}
return []byte(t), nil
case []byte:
_, err := a.Decode(t, runtime.DefaultYAMLEncoding)
if err != nil {
return nil, err
}
return t, nil
default:
return nil, fmt.Errorf("feature gate config required")
}
}

func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (interface{}, error) {
var c Attribute
err := yaml.Unmarshal(data, &c)
if err != nil {
return nil, err
}
return &c, nil
}

////////////////////////////////////////////////////////////////////////////////

const FEATURE_DISABLED = "off"

type Attribute struct {
lock sync.Mutex

Features map[string]*FeatureGate `json:"features"`
}

// FeatureGate store settings for a particular feature gate.
// To be extended by additional config possibility.
// Default behavior is to be enabled if entry is given
// for a feature name and mode is not equal *off*.
type FeatureGate struct {
Mode string `json:"mode"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally in favor of this, but why choose to have a Mode setting for the Gate if you already have attributes? Shouldn't the mere presence of the gate determine if its enabled or disabled?

Copy link
Contributor Author

@mandelsoft mandelsoft Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the mode is to have a fixed/non-optional config field usable to enable/disable a feature and control an implementation mode (if required). The value off is always used to enforce disabling the feature (in case it is enabled by default).

The attributes are completely optional and intended to pass arbitrary information to the feature. (An on/off switch does not use attributes at all (omitempty))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the mode is to have a fixed/non-optional config field usable to enable/disable a feature and control an implementation mode (if required). The value off is always used to enforce disabling the feature (in case it is enabled by default).

Could you give me an example for an "implementation mode" that is not on/off? Ive never seen it anywhere in production and was assuming that even if there was a non on/off, it would be part of the attributes.

Attributes map[string]json.RawMessage `json:"attributes,omitempty"`
}

func New() *Attribute {
return &Attribute{Features: map[string]*FeatureGate{}}
}

func (a *Attribute) EnableFeature(name string, state *FeatureGate) {
a.lock.Lock()
defer a.lock.Unlock()

if state == nil {
state = &FeatureGate{}
}
if state.Mode == FEATURE_DISABLED {
state.Mode = ""
}
a.Features[name] = state
}

func (a *Attribute) SetFeature(name string, state *FeatureGate) {
a.lock.Lock()
defer a.lock.Unlock()

if state == nil {
state = &FeatureGate{}
}
a.Features[name] = state
}

func (a *Attribute) DisableFeature(name string) {
a.lock.Lock()
defer a.lock.Unlock()

a.Features[name] = &FeatureGate{Mode: "off"}
}

func (a *Attribute) DefaultFeature(name string) {
a.lock.Lock()
defer a.lock.Unlock()

delete(a.Features, name)
}

func (a *Attribute) IsEnabled(name string, def ...bool) bool {
return a.GetFeature(name, def...).Mode != FEATURE_DISABLED
}

func (a *Attribute) GetFeature(name string, def ...bool) *FeatureGate {
a.lock.Lock()
defer a.lock.Unlock()

g, ok := a.Features[name]
if !ok {
g = &FeatureGate{}
if !general.Optional(def...) {
g.Mode = FEATURE_DISABLED
}
}
return g
}

////////////////////////////////////////////////////////////////////////////////

func Get(ctx datacontext.Context) *Attribute {
v := ctx.GetAttributes().GetAttribute(ATTR_KEY)
if v == nil {
v = New()
}
return v.(*Attribute)
}

func Set(ctx datacontext.Context, c *Attribute) {
ctx.GetAttributes().SetAttribute(ATTR_KEY, c)
}

var lock sync.Mutex

func get(ctx datacontext.Context) *Attribute {
attrs := ctx.GetAttributes()
v := attrs.GetAttribute(ATTR_KEY)

if v == nil {
v = New()
attrs.SetAttribute(ATTR_KEY, v)
}
return v.(*Attribute)
}

func SetFeature(ctx datacontext.Context, name string, state *FeatureGate) {
lock.Lock()
defer lock.Unlock()

get(ctx).SetFeature(name, state)
}

func EnableFeature(ctx datacontext.Context, name string, state *FeatureGate) {
lock.Lock()
defer lock.Unlock()

get(ctx).EnableFeature(name, state)
}

func DisableFeature(ctx datacontext.Context, name string) {
lock.Lock()
defer lock.Unlock()

get(ctx).DisableFeature(name)
}

func DefaultFeature(ctx datacontext.Context, name string) {
lock.Lock()
defer lock.Unlock()

get(ctx).DefaultFeature(name)
}
1 change: 1 addition & 0 deletions api/datacontext/attrs/init.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package attrs

import (
_ "ocm.software/ocm/api/datacontext/attrs/featuregatesattr"
_ "ocm.software/ocm/api/datacontext/attrs/logforward"
_ "ocm.software/ocm/api/datacontext/attrs/rootcertsattr"
_ "ocm.software/ocm/api/datacontext/attrs/tmpcache"
Expand Down
76 changes: 76 additions & 0 deletions api/datacontext/config/featuregates/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package featuregates_test

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"ocm.software/ocm/api/datacontext/attrs/featuregatesattr"
"ocm.software/ocm/api/datacontext/config/attrs"
"ocm.software/ocm/api/datacontext/config/featuregates"

"ocm.software/ocm/api/config"
"ocm.software/ocm/api/datacontext"
)

var _ = Describe("feature gates", func() {
var ctx config.Context

BeforeEach(func() {
ctx = config.WithSharedAttributes(datacontext.New(nil)).New()
})

Context("applies", func() {
It("handles default", func() {
a := featuregatesattr.Get(ctx)

Expect(a.IsEnabled("test")).To(BeFalse())
Expect(a.IsEnabled("test", true)).To(BeTrue())
g := a.GetFeature("test", true)
Expect(g).NotTo(BeNil())
Expect(g.Mode).To(Equal(""))
})

It("enables feature", func() {
cfg := featuregates.New()
cfg.EnableFeature("test", &featuregates.FeatureGate{Mode: "on"})
ctx.ApplyConfig(cfg, "manual")

a := featuregatesattr.Get(ctx)

Expect(a.IsEnabled("test")).To(BeTrue())
Expect(a.IsEnabled("test", true)).To(BeTrue())
g := a.GetFeature("test")
Expect(g).NotTo(BeNil())
Expect(g.Mode).To(Equal("on"))
})

It("disables feature", func() {
cfg := featuregates.New()
cfg.DisableFeature("test")
ctx.ApplyConfig(cfg, "manual")

a := featuregatesattr.Get(ctx)

Expect(a.IsEnabled("test")).To(BeFalse())
Expect(a.IsEnabled("test", true)).To(BeFalse())
})

It("handle attribute config", func() {
cfg := featuregatesattr.New()
cfg.EnableFeature("test", &featuregates.FeatureGate{Mode: "on"})

spec := attrs.New()
Expect(spec.AddAttribute(featuregatesattr.ATTR_KEY, cfg)).To(Succeed())
Expect(ctx.ApplyConfig(spec, "test")).To(Succeed())

ctx.ApplyConfig(spec, "manual")

a := featuregatesattr.Get(ctx)

Expect(a.IsEnabled("test")).To(BeTrue())
g := a.GetFeature("test")
Expect(g).NotTo(BeNil())
Expect(g.Mode).To(Equal("on"))
})

})
})
13 changes: 13 additions & 0 deletions api/datacontext/config/featuregates/suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package featuregates_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestConfig(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Feature Gates Config Test Suite")
}
69 changes: 69 additions & 0 deletions api/datacontext/config/featuregates/type.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package featuregates

import (
cfgcpi "ocm.software/ocm/api/config/cpi"
"ocm.software/ocm/api/datacontext/attrs/featuregatesattr"
"ocm.software/ocm/api/utils/runtime"
)

const (
ConfigType = "featuregates" + cfgcpi.OCM_CONFIG_TYPE_SUFFIX
ConfigTypeV1 = ConfigType + runtime.VersionSeparator + "v1"
)

func init() {
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*Config](ConfigType, usage))
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*Config](ConfigTypeV1, usage))
}

type FeatureGate = featuregatesattr.FeatureGate

// Config describes a memory based repository interface.
type Config struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the reason to have a config and an attribute set, shouldnt attributes alone be enough?

runtime.ObjectVersionedType `json:",inline"`
featuregatesattr.Attribute `json:",inline"`
}

// New creates a new memory ConfigSpec.
func New() *Config {
return &Config{
ObjectVersionedType: runtime.NewVersionedTypedObject(ConfigType),
Attribute: *featuregatesattr.New(),
}
}

func (a *Config) GetType() string {
return ConfigType
}

func (a *Config) ApplyTo(ctx cfgcpi.Context, target interface{}) error {
t, ok := target.(cfgcpi.Context)
if !ok {
return cfgcpi.ErrNoContext(ConfigType)
}
if len(a.Features) == 0 {
return nil
}
for n, g := range a.Features {
featuregatesattr.SetFeature(t, n, g)
}
return nil
}

const usage = `
The config type <code>` + ConfigType + `</code> can be used to define a list
of feature gate settings:

<pre>
type: ` + ConfigType + `
features:
&lt;name>: {
mode: off | &lt;any key to enable>
attributes: {
&lt;name>: &lt;any yaml value>
...
}
}
...
</pre>
`
1 change: 1 addition & 0 deletions api/datacontext/config/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ package config

import (
_ "ocm.software/ocm/api/datacontext/config/attrs"
_ "ocm.software/ocm/api/datacontext/config/featuregates"
_ "ocm.software/ocm/api/datacontext/config/logging"
)
Loading
Loading