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 1 commit
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
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"
)
2 changes: 2 additions & 0 deletions api/datacontext/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ func (c *_attributes) SetAttribute(name string, value interface{}) error {
if *c.updater != nil {
(*c.updater).Update()
}
} else {
ocmlog.Logger().LogError(err, "cannot set context attribute", "attr", name, "value", value)
}
return err
}
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/ocm.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ The value can be a simple type or a JSON/YAML string for complex values
the backend and descriptor updated will be persisted on AddVersion
or closing a provided existing component version.

- <code>ocm.software/ocm/feature-gates</code> [<code>featuregates</code>]: *featuregates* Enable/Disable optional features of the OCM library.

Optionally, particular features modes and attributes can be configured, if
supported by the feature implementation.

- <code>ocm.software/signing/sigstore</code> [<code>sigstore</code>]: *sigstore config* Configuration to use for sigstore based signing.

The following fields are used.
Expand Down
Loading
Loading