From f1d33fe976ac6a7fc71769921bd208481feb349b Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 5 May 2026 10:47:08 -0700 Subject: [PATCH 1/4] feat: implement Go OpenFeature provider for feature flag evaluation Add pkg/featureflags package providing an OpenFeature-compatible FeatureProvider backed by the Milo AllowanceBucket API. A feature flag is enabled for an org when an AllowanceBucket with status.available > 0 exists for the (org, resourceType) pair where resourceType is "features.miloapis.com/". Key design decisions: - AllowanceBucketLister interface accepts any controller-runtime client, enabling easy injection and testing without a live API server - BooleanEvaluation queries by spec.consumerRef.name and spec.resourceType field selectors (both are indexed in the quota system) - Non-boolean evaluations return TYPE_MISMATCH; feature flags are boolean entitlements only - API errors return defaultValue with DEFAULT reason (no panics) - Missing or empty targetingKey returns defaultValue with DEFAULT reason Also fix fmt.Errorf with non-literal format strings in project webhook, surfaced as a vet error after upgrading to Go 1.25 (required by the openfeature SDK's go.mod minimum version). Closes #577 --- go.mod | 20 +- go.sum | 36 +-- .../v1alpha1/project_webhook.go | 12 +- pkg/featureflags/provider.go | 221 +++++++++++++++ pkg/featureflags/provider_test.go | 268 ++++++++++++++++++ 5 files changed, 526 insertions(+), 31 deletions(-) create mode 100644 pkg/featureflags/provider.go create mode 100644 pkg/featureflags/provider_test.go diff --git a/go.mod b/go.mod index d9ab77e2..6c23597c 100644 --- a/go.mod +++ b/go.mod @@ -1,18 +1,19 @@ module go.miloapis.com/milo -go 1.23.1 +go 1.25.0 require ( github.com/blang/semver/v4 v4.0.0 github.com/go-logr/logr v1.4.3 github.com/google/cel-go v0.22.0 + github.com/open-feature/go-sdk v1.17.2 github.com/spf13/cobra v1.9.1 - github.com/spf13/pflag v1.0.7 + github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0 go.opentelemetry.io/otel v1.35.0 go.opentelemetry.io/otel/trace v1.35.0 - golang.org/x/sync v0.16.0 + golang.org/x/sync v0.20.0 golang.org/x/time v0.12.0 gopkg.in/square/go-jose.v2 v2.6.0 k8s.io/api v0.32.3 @@ -103,16 +104,17 @@ require ( go.opentelemetry.io/otel/metric v1.35.0 // indirect go.opentelemetry.io/otel/sdk v1.34.0 // indirect go.opentelemetry.io/proto/otlp v1.3.1 // indirect + go.uber.org/mock v0.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect - golang.org/x/crypto v0.37.0 // indirect + golang.org/x/crypto v0.48.0 // indirect golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect - golang.org/x/net v0.39.0 // indirect + golang.org/x/net v0.50.0 // indirect golang.org/x/oauth2 v0.27.0 // indirect - golang.org/x/sys v0.32.0 // indirect - golang.org/x/term v0.31.0 // indirect - golang.org/x/text v0.24.0 // indirect - golang.org/x/tools v0.30.0 // indirect + golang.org/x/sys v0.41.0 // indirect + golang.org/x/term v0.40.0 // indirect + golang.org/x/text v0.35.0 // indirect + golang.org/x/tools v0.42.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/genproto v0.0.0-20241015192408-796eee8c2d53 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect diff --git a/go.sum b/go.sum index a919bbaa..398ef5fb 100644 --- a/go.sum +++ b/go.sum @@ -138,6 +138,8 @@ github.com/onsi/ginkgo/v2 v2.22.0 h1:Yed107/8DjTr0lKCNt7Dn8yQ6ybuDRQoMGrNFKzMfHg github.com/onsi/ginkgo/v2 v2.22.0/go.mod h1:7Du3c42kxCUegi0IImZ1wUQzMBVecgIHjR1C+NkhLQo= github.com/onsi/gomega v1.36.1 h1:bJDPBO7ibjxcbHMgSCoo4Yj18UWbKDlLwX1x9sybDcw= github.com/onsi/gomega v1.36.1/go.mod h1:PvZbdDc8J6XJEpDK4HCuRBm8a6Fzp9/DmhC9C7yFlog= +github.com/open-feature/go-sdk v1.17.2 h1:pTdeNks/hgnPrlqdgtFwltnIron1oOxqg4FmLlirJlY= +github.com/open-feature/go-sdk v1.17.2/go.mod h1:kTMCquVtck18XdSCI6rBoNFEBLvkOy4Tphu2pV8bq34= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/selinux v1.11.1 h1:nHFvthhM0qY8/m+vfhJylliSshm8G1jJ2jDMcgULaH8= @@ -169,8 +171,8 @@ github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M= -github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -231,6 +233,8 @@ go.opentelemetry.io/proto/otlp v1.3.1 h1:TrMUixzpM0yuc/znrFTP9MMRh8trP93mkCiDVeX go.opentelemetry.io/proto/otlp v1.3.1/go.mod h1:0X1WI4de4ZsLrrJNLAQbFeLCm3T7yBkR0XqQ7niQU+8= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= +go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= @@ -238,8 +242,8 @@ go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= -golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts= +golang.org/x/crypto v0.48.0/go.mod h1:r0kV5h3qnFPlQnBSrULhlsRfryS2pmewsg+XfMgkVos= golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= @@ -248,35 +252,35 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= -golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/net v0.50.0 h1:ucWh9eiCGyDR3vtzso0WMQinm2Dnt8cFMuQa9K33J60= +golang.org/x/net v0.50.0/go.mod h1:UgoSli3F/pBgdJBHCTc+tp3gmrU4XswgGRgtnwWTfyM= golang.org/x/oauth2 v0.27.0 h1:da9Vo7/tDv5RH/7nZDz1eMGS/q1Vv1N/7FCrBhI9I3M= golang.org/x/oauth2 v0.27.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= -golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= -golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= -golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= -golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= +golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= +golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/term v0.40.0 h1:36e4zGLqU4yhjlmxEaagx2KuYbJq3EwY8K943ZsHcvg= +golang.org/x/term v0.40.0/go.mod h1:w2P8uVp06p2iyKKuvXIm7N/y0UCRt3UfJTfZ7oOpglM= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= -golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= +golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= +golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.30.0 h1:BgcpHewrV5AUp2G9MebG4XPFI1E2W41zU1SaqVA9vJY= -golang.org/x/tools v0.30.0/go.mod h1:c347cR/OJfw5TI+GfX7RUPNMdDRRbjvYTS0jPyvsVtY= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go b/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go index 35caf644..129b2db7 100644 --- a/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go +++ b/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go @@ -65,15 +65,15 @@ func (m *ProjectMutator) Default(ctx context.Context, obj runtime.Object) error parentAPIGroup, parentAPIGroupOk := req.UserInfo.Extra[iamv1alpha1.ParentAPIGroupExtraKey] if !parentNameOk || !parentKindOk || !parentAPIGroupOk { - errMsg := "request context does not have the required parent information" - projectlog.Error(fmt.Errorf(errMsg), errMsg) - return fmt.Errorf(errMsg) + const errMsg = "request context does not have the required parent information" + projectlog.Error(fmt.Errorf("%s", errMsg), errMsg) + return fmt.Errorf("%s", errMsg) } if len(parentKind) != 1 || parentKind[0] != "Organization" || parentAPIGroup[0] != v1alpha1.GroupVersion.Group { - errMsg := "request context has invalid parent information, must be Organization from the resourcemanager.miloapis.com API group" - projectlog.Error(fmt.Errorf(errMsg), errMsg) - return fmt.Errorf(errMsg) + const errMsg = "request context has invalid parent information, must be Organization from the resourcemanager.miloapis.com API group" + projectlog.Error(fmt.Errorf("%s", errMsg), errMsg) + return fmt.Errorf("%s", errMsg) } requestContextOrgID := parentName[0] diff --git a/pkg/featureflags/provider.go b/pkg/featureflags/provider.go new file mode 100644 index 00000000..00cb16c7 --- /dev/null +++ b/pkg/featureflags/provider.go @@ -0,0 +1,221 @@ +// Package featureflags provides an OpenFeature provider backed by the Milo +// AllowanceBucket API. +// +// Feature flags are org-level boolean entitlements. A flag is enabled for an +// org when an AllowanceBucket exists with status.available > 0 for that +// (org, resourceType) pair. The resourceType is +// "features.miloapis.com/". +package featureflags + +import ( + "context" + "fmt" + + "github.com/open-feature/go-sdk/openfeature" + "sigs.k8s.io/controller-runtime/pkg/client" + + quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1" +) + +const ( + // ProviderName is the name reported in OpenFeature metadata. + ProviderName = "milo-allowance-bucket" + + // featureResourceTypePrefix is prepended to a flag key to form the + // AllowanceBucket spec.resourceType used for feature entitlements. + featureResourceTypePrefix = "features.miloapis.com/" +) + +// AllowanceBucketLister is the subset of the controller-runtime client needed +// by Provider. It is satisfied by any sigs.k8s.io/controller-runtime/pkg/client.Client. +type AllowanceBucketLister interface { + List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error +} + +// Provider implements the OpenFeature FeatureProvider interface. It resolves +// boolean flags by querying AllowanceBuckets: a flag is enabled when an +// AllowanceBucket for (org, features.miloapis.com/) has +// status.available > 0. +// +// All non-boolean flag types return a TYPE_MISMATCH error because feature flags +// are exclusively boolean entitlements. +type Provider struct { + lister AllowanceBucketLister +} + +// NewProvider returns a Provider that uses the given AllowanceBucketLister to +// query flag entitlements. Pass any controller-runtime client as the lister. +func NewProvider(lister AllowanceBucketLister) *Provider { + return &Provider{lister: lister} +} + +// Metadata returns the OpenFeature provider metadata. +func (p *Provider) Metadata() openfeature.Metadata { + return openfeature.Metadata{Name: ProviderName} +} + +// Hooks returns no hooks; the provider does not require lifecycle callbacks. +func (p *Provider) Hooks() []openfeature.Hook { + return []openfeature.Hook{} +} + +// BooleanEvaluation resolves a feature flag for an organisation. +// +// Resolution rules: +// - evalCtx[targetingKey] is the organisation name. An empty or missing +// targeting key returns defaultValue with reason DEFAULT. +// - The flag is enabled (true, TARGETING_MATCH) when an AllowanceBucket +// exists for (org, features.miloapis.com/) with status.available > 0. +// - Any API error returns defaultValue with reason DEFAULT (no panic). +// - A missing bucket, or a bucket with status.available == 0, returns +// defaultValue with reason DEFAULT. +func (p *Provider) BooleanEvaluation( + ctx context.Context, + flag string, + defaultValue bool, + evalCtx openfeature.FlattenedContext, +) openfeature.BoolResolutionDetail { + org, ok := targetingKey(evalCtx) + if !ok || org == "" { + return boolDefault(defaultValue) + } + + resourceType := featureResourceTypePrefix + flag + + var bucketList quotav1alpha1.AllowanceBucketList + if err := p.lister.List(ctx, &bucketList, + client.MatchingFields{ + "spec.consumerRef.name": org, + "spec.resourceType": resourceType, + }, + ); err != nil { + // Return the default value on API errors; do not propagate the error + // as a panic or a hard failure — the caller must decide whether the + // feature should be on or off by default. + return boolDefaultWithError(defaultValue, fmt.Errorf("failed to list AllowanceBuckets for org %q flag %q: %w", org, flag, err)) + } + + for i := range bucketList.Items { + if bucketList.Items[i].Status.Available > 0 { + return openfeature.BoolResolutionDetail{ + Value: true, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + Reason: openfeature.TargetingMatchReason, + }, + } + } + } + + return boolDefault(defaultValue) +} + +// StringEvaluation returns a TYPE_MISMATCH error. Feature flags are boolean +// entitlements and cannot be resolved as strings. +func (p *Provider) StringEvaluation( + _ context.Context, + _ string, + defaultValue string, + _ openfeature.FlattenedContext, +) openfeature.StringResolutionDetail { + return openfeature.StringResolutionDetail{ + Value: defaultValue, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + ResolutionError: openfeature.NewTypeMismatchResolutionError( + "feature flags are boolean entitlements; use BooleanEvaluation", + ), + Reason: openfeature.ErrorReason, + }, + } +} + +// FloatEvaluation returns a TYPE_MISMATCH error. Feature flags are boolean +// entitlements and cannot be resolved as floats. +func (p *Provider) FloatEvaluation( + _ context.Context, + _ string, + defaultValue float64, + _ openfeature.FlattenedContext, +) openfeature.FloatResolutionDetail { + return openfeature.FloatResolutionDetail{ + Value: defaultValue, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + ResolutionError: openfeature.NewTypeMismatchResolutionError( + "feature flags are boolean entitlements; use BooleanEvaluation", + ), + Reason: openfeature.ErrorReason, + }, + } +} + +// IntEvaluation returns a TYPE_MISMATCH error. Feature flags are boolean +// entitlements and cannot be resolved as integers. +func (p *Provider) IntEvaluation( + _ context.Context, + _ string, + defaultValue int64, + _ openfeature.FlattenedContext, +) openfeature.IntResolutionDetail { + return openfeature.IntResolutionDetail{ + Value: defaultValue, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + ResolutionError: openfeature.NewTypeMismatchResolutionError( + "feature flags are boolean entitlements; use BooleanEvaluation", + ), + Reason: openfeature.ErrorReason, + }, + } +} + +// ObjectEvaluation returns a TYPE_MISMATCH error. Feature flags are boolean +// entitlements and cannot be resolved as objects. +func (p *Provider) ObjectEvaluation( + _ context.Context, + _ string, + defaultValue any, + _ openfeature.FlattenedContext, +) openfeature.InterfaceResolutionDetail { + return openfeature.InterfaceResolutionDetail{ + Value: defaultValue, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + ResolutionError: openfeature.NewTypeMismatchResolutionError( + "feature flags are boolean entitlements; use BooleanEvaluation", + ), + Reason: openfeature.ErrorReason, + }, + } +} + +// targetingKey extracts the targeting key from a flattened evaluation context. +// The second return value is false when the key is absent or not a string. +func targetingKey(evalCtx openfeature.FlattenedContext) (string, bool) { + v, ok := evalCtx[openfeature.TargetingKey] + if !ok { + return "", false + } + s, ok := v.(string) + return s, ok +} + +// boolDefault returns a BoolResolutionDetail with DEFAULT reason. +func boolDefault(value bool) openfeature.BoolResolutionDetail { + return openfeature.BoolResolutionDetail{ + Value: value, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + Reason: openfeature.DefaultReason, + }, + } +} + +// boolDefaultWithError returns a BoolResolutionDetail with DEFAULT reason and a +// general resolution error. The default value is returned so callers can +// continue operating; the error is surfaced via ResolutionError for +// observability. +func boolDefaultWithError(value bool, err error) openfeature.BoolResolutionDetail { + return openfeature.BoolResolutionDetail{ + Value: value, + ProviderResolutionDetail: openfeature.ProviderResolutionDetail{ + ResolutionError: openfeature.NewGeneralResolutionError(err.Error()), + Reason: openfeature.DefaultReason, + }, + } +} diff --git a/pkg/featureflags/provider_test.go b/pkg/featureflags/provider_test.go new file mode 100644 index 00000000..8d7a2e21 --- /dev/null +++ b/pkg/featureflags/provider_test.go @@ -0,0 +1,268 @@ +package featureflags_test + +import ( + "context" + "errors" + "testing" + + "github.com/open-feature/go-sdk/openfeature" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1" + "go.miloapis.com/milo/pkg/featureflags" +) + +// fakeLister is a stub AllowanceBucketLister for unit tests. It returns a +// fixed set of AllowanceBuckets from its List call, filtered by the +// spec.consumerRef.name and spec.resourceType MatchingFields options when +// present, mirroring the field-selector behaviour of a real kube API server. +type fakeLister struct { + buckets []quotav1alpha1.AllowanceBucket + err error +} + +func (f *fakeLister) List(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + if f.err != nil { + return f.err + } + + lo := &client.ListOptions{} + for _, o := range opts { + o.ApplyToList(lo) + } + + bucketList, ok := list.(*quotav1alpha1.AllowanceBucketList) + if !ok { + return errors.New("fakeLister only supports AllowanceBucketList") + } + + // Apply MatchingFields filtering — mirrors the indexed field selectors + // (spec.consumerRef.name, spec.resourceType) that the real API server uses. + var wantConsumer, wantResourceType string + if lo.FieldSelector != nil { + for _, r := range lo.FieldSelector.Requirements() { + switch r.Field { + case "spec.consumerRef.name": + wantConsumer = r.Value + case "spec.resourceType": + wantResourceType = r.Value + } + } + } + + var matched []quotav1alpha1.AllowanceBucket + for _, b := range f.buckets { + if wantConsumer != "" && b.Spec.ConsumerRef.Name != wantConsumer { + continue + } + if wantResourceType != "" && b.Spec.ResourceType != wantResourceType { + continue + } + matched = append(matched, b) + } + bucketList.Items = matched + return nil +} + +// bucket is a test helper that constructs an AllowanceBucket with the given +// consumer name, resourceType, and available quota. +func bucket(consumerName, resourceType string, available int64) quotav1alpha1.AllowanceBucket { + return quotav1alpha1.AllowanceBucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: consumerName + "-" + resourceType, + }, + Spec: quotav1alpha1.AllowanceBucketSpec{ + ConsumerRef: quotav1alpha1.ConsumerRef{Name: consumerName, Kind: "Organization"}, + ResourceType: resourceType, + }, + Status: quotav1alpha1.AllowanceBucketStatus{ + Available: available, + }, + } +} + +func evalCtxWithOrg(org string) openfeature.FlattenedContext { + return openfeature.FlattenedContext{ + openfeature.TargetingKey: org, + } +} + +func TestBooleanEvaluation(t *testing.T) { + const ( + org = "acme-corp" + flagKey = "some-feature" + resType = "features.miloapis.com/" + flagKey + otherOrg = "other-org" + ) + + tests := []struct { + name string + buckets []quotav1alpha1.AllowanceBucket + listerErr error + evalCtx openfeature.FlattenedContext + defaultValue bool + wantValue bool + wantReason openfeature.Reason + }{ + { + name: "flag enabled when bucket exists with available > 0", + buckets: []quotav1alpha1.AllowanceBucket{ + bucket(org, resType, 1), + }, + evalCtx: evalCtxWithOrg(org), + wantValue: true, + wantReason: openfeature.TargetingMatchReason, + }, + { + name: "flag disabled when no bucket matches org and flag", + buckets: []quotav1alpha1.AllowanceBucket{ + bucket(otherOrg, resType, 1), + }, + evalCtx: evalCtxWithOrg(org), + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "flag disabled when bucket exists but available is zero", + buckets: []quotav1alpha1.AllowanceBucket{ + bucket(org, resType, 0), + }, + evalCtx: evalCtxWithOrg(org), + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "returns defaultValue on API error without panic", + listerErr: errors.New("api server unavailable"), + evalCtx: evalCtxWithOrg(org), + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "returns defaultValue on API error with non-default default", + listerErr: errors.New("api server unavailable"), + evalCtx: evalCtxWithOrg(org), + defaultValue: true, + wantValue: true, + wantReason: openfeature.DefaultReason, + }, + { + name: "returns defaultValue when targetingKey is absent", + evalCtx: openfeature.FlattenedContext{}, + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "returns defaultValue when targetingKey is empty string", + evalCtx: openfeature.FlattenedContext{ + openfeature.TargetingKey: "", + }, + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "bucket for wrong org does not enable flag", + buckets: []quotav1alpha1.AllowanceBucket{ + bucket(otherOrg, resType, 5), + }, + evalCtx: evalCtxWithOrg(org), + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + { + name: "bucket for correct org but wrong resourceType does not enable flag", + buckets: []quotav1alpha1.AllowanceBucket{ + bucket(org, "features.miloapis.com/different-feature", 5), + }, + evalCtx: evalCtxWithOrg(org), + wantValue: false, + wantReason: openfeature.DefaultReason, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + lister := &fakeLister{buckets: tc.buckets, err: tc.listerErr} + p := featureflags.NewProvider(lister) + + got := p.BooleanEvaluation(context.Background(), flagKey, tc.defaultValue, tc.evalCtx) + + if got.Value != tc.wantValue { + t.Errorf("Value: got %v, want %v", got.Value, tc.wantValue) + } + if got.Reason != tc.wantReason { + t.Errorf("Reason: got %q, want %q", got.Reason, tc.wantReason) + } + }) + } +} + +func TestNonBooleanEvaluationsReturnTypeMismatch(t *testing.T) { + lister := &fakeLister{} + p := featureflags.NewProvider(lister) + ctx := context.Background() + evalCtx := evalCtxWithOrg("acme-corp") + + t.Run("StringEvaluation returns TYPE_MISMATCH", func(t *testing.T) { + got := p.StringEvaluation(ctx, "flag", "default", evalCtx) + if got.ResolutionError.Error() == "" { + t.Error("expected a non-empty ResolutionError for StringEvaluation") + } + if got.Reason != openfeature.ErrorReason { + t.Errorf("Reason: got %q, want %q", got.Reason, openfeature.ErrorReason) + } + if got.Value != "default" { + t.Errorf("Value: got %q, want \"default\"", got.Value) + } + }) + + t.Run("FloatEvaluation returns TYPE_MISMATCH", func(t *testing.T) { + got := p.FloatEvaluation(ctx, "flag", 3.14, evalCtx) + if got.ResolutionError.Error() == "" { + t.Error("expected a non-empty ResolutionError for FloatEvaluation") + } + if got.Reason != openfeature.ErrorReason { + t.Errorf("Reason: got %q, want %q", got.Reason, openfeature.ErrorReason) + } + }) + + t.Run("IntEvaluation returns TYPE_MISMATCH", func(t *testing.T) { + got := p.IntEvaluation(ctx, "flag", int64(42), evalCtx) + if got.ResolutionError.Error() == "" { + t.Error("expected a non-empty ResolutionError for IntEvaluation") + } + if got.Reason != openfeature.ErrorReason { + t.Errorf("Reason: got %q, want %q", got.Reason, openfeature.ErrorReason) + } + }) + + t.Run("ObjectEvaluation returns TYPE_MISMATCH", func(t *testing.T) { + got := p.ObjectEvaluation(ctx, "flag", map[string]any{"k": "v"}, evalCtx) + if got.ResolutionError.Error() == "" { + t.Error("expected a non-empty ResolutionError for ObjectEvaluation") + } + if got.Reason != openfeature.ErrorReason { + t.Errorf("Reason: got %q, want %q", got.Reason, openfeature.ErrorReason) + } + }) +} + +func TestProviderMetadata(t *testing.T) { + p := featureflags.NewProvider(&fakeLister{}) + meta := p.Metadata() + if meta.Name != featureflags.ProviderName { + t.Errorf("Metadata.Name: got %q, want %q", meta.Name, featureflags.ProviderName) + } +} + +func TestProviderHooksEmpty(t *testing.T) { + p := featureflags.NewProvider(&fakeLister{}) + if hooks := p.Hooks(); len(hooks) != 0 { + t.Errorf("Hooks: expected empty slice, got %d hooks", len(hooks)) + } +} + +// TestProviderImplementsFeatureProvider verifies at compile time that *Provider +// satisfies the openfeature.FeatureProvider interface. +var _ openfeature.FeatureProvider = (*featureflags.Provider)(nil) From 40fb6992afee2620616a805439aead6de111d2ed Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 5 May 2026 10:52:26 -0700 Subject: [PATCH 2/4] fix: bump Dockerfile base image to golang:1.25 go.mod requires go 1.25 (introduced by the openfeature go-sdk v1.17.2 dependency); update the builder stage to match. --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 4c9cf872..22f87820 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ # and cross-compiles to $TARGETOS/$TARGETARCH. This makes multi-arch builds # (linux/amd64, linux/arm64) fast under buildx without requiring QEMU emulation # for the build itself. -FROM --platform=$BUILDPLATFORM golang:1.24 AS builder +FROM --platform=$BUILDPLATFORM golang:1.25 AS builder # Provided automatically by BuildKit when using buildx with --platform. ARG TARGETOS From 4181f81aaf17b3bd2f536b8273770ec6ebd78515 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 5 May 2026 11:46:54 -0700 Subject: [PATCH 3/4] fix: upgrade golang.org/x/net to v0.51.0 to patch CVE http2 nil panic golang.org/x/net/http2 < v0.51.0 is vulnerable to an uncaught exception via missing nil check on HTTP/2 frames with values 0x0a-0x0f (CVSS 6.9). Upgrade resolves the Snyk finding introduced by the openfeature SDK adding a fresh transitive path to the affected version. --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 6c23597c..1cffc2a4 100644 --- a/go.mod +++ b/go.mod @@ -109,7 +109,7 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/crypto v0.48.0 // indirect golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect - golang.org/x/net v0.50.0 // indirect + golang.org/x/net v0.51.0 // indirect golang.org/x/oauth2 v0.27.0 // indirect golang.org/x/sys v0.41.0 // indirect golang.org/x/term v0.40.0 // indirect diff --git a/go.sum b/go.sum index 398ef5fb..870689fd 100644 --- a/go.sum +++ b/go.sum @@ -254,6 +254,8 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.50.0 h1:ucWh9eiCGyDR3vtzso0WMQinm2Dnt8cFMuQa9K33J60= golang.org/x/net v0.50.0/go.mod h1:UgoSli3F/pBgdJBHCTc+tp3gmrU4XswgGRgtnwWTfyM= +golang.org/x/net v0.51.0 h1:94R/GTO7mt3/4wIKpcR5gkGmRLOuE/2hNGeWq/GBIFo= +golang.org/x/net v0.51.0/go.mod h1:aamm+2QF5ogm02fjy5Bb7CQ0WMt1/WVM7FtyaTLlA9Y= golang.org/x/oauth2 v0.27.0 h1:da9Vo7/tDv5RH/7nZDz1eMGS/q1Vv1N/7FCrBhI9I3M= golang.org/x/oauth2 v0.27.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= From ac0317eeb1d09a5bbfbb786af42eb11780f3e4aa Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 5 May 2026 17:09:34 -0700 Subject: [PATCH 4/4] feat: add cloud-portal-usage-metering feature flag Add the first ResourceRegistration of type Feature to milo, establishing the config/services/features/ Kustomize scaffold for future feature flags. The cloud-portal-usage-metering registration controls visibility of the Usage & Metering section in the Datum Cloud portal navigation. It is scoped to Organizations as the consumer type and uses FeatureGrant as the claiming resource sentinel (satisfying minItems:1 without enabling admission enforcement for arbitrary grants). Closes datum-cloud/enhancements#711 --- config/services/features/kustomization.yaml | 5 +++++ .../cloud-portal-usage-metering.yaml | 21 +++++++++++++++++++ .../features/registrations/kustomization.yaml | 4 ++++ config/services/kustomization.yaml | 1 + 4 files changed, 31 insertions(+) create mode 100644 config/services/features/kustomization.yaml create mode 100644 config/services/features/registrations/cloud-portal-usage-metering.yaml create mode 100644 config/services/features/registrations/kustomization.yaml diff --git a/config/services/features/kustomization.yaml b/config/services/features/kustomization.yaml new file mode 100644 index 00000000..b260b9a6 --- /dev/null +++ b/config/services/features/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +components: + - registrations diff --git a/config/services/features/registrations/cloud-portal-usage-metering.yaml b/config/services/features/registrations/cloud-portal-usage-metering.yaml new file mode 100644 index 00000000..126faa89 --- /dev/null +++ b/config/services/features/registrations/cloud-portal-usage-metering.yaml @@ -0,0 +1,21 @@ +apiVersion: quota.miloapis.com/v1alpha1 +kind: ResourceRegistration +metadata: + name: feature-cloud-portal-usage-metering + labels: + app.kubernetes.io/name: milo + app.kubernetes.io/component: feature-flags + features.miloapis.com/feature-name: cloud-portal-usage-metering +spec: + consumerType: + apiGroup: resourcemanager.miloapis.com + kind: Organization + type: Feature + resourceType: features.miloapis.com/cloud-portal-usage-metering + description: "Controls visibility of the Usage tab in the Datum Cloud portal. When enabled for an organization, the Usage & Metering section is shown in their portal navigation." + baseUnit: feature + displayUnit: features + unitConversionFactor: 1 + claimingResources: + - apiGroup: features.miloapis.com + kind: FeatureGrant diff --git a/config/services/features/registrations/kustomization.yaml b/config/services/features/registrations/kustomization.yaml new file mode 100644 index 00000000..0115f52c --- /dev/null +++ b/config/services/features/registrations/kustomization.yaml @@ -0,0 +1,4 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component +resources: + - cloud-portal-usage-metering.yaml diff --git a/config/services/kustomization.yaml b/config/services/kustomization.yaml index 6eead7b0..290a0c56 100644 --- a/config/services/kustomization.yaml +++ b/config/services/kustomization.yaml @@ -8,6 +8,7 @@ components: - iam - resourcemanager - quota + - features # identity is intentionally excluded here — it contains ActivityPolicy resources # which require the activity.miloapis.com CRDs to be installed. Those CRDs are # provided by the activity service, which is not deployed in the milo test cluster.