From ed767716285d783673b0ac11aacf36ac3783ce4e Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 9 Dec 2021 18:33:16 -0800 Subject: [PATCH 1/2] feat(constraints): add compound constraints and olm.constraint value parser Signed-off-by: Eric Stroczynski --- pkg/constraints/cel.go | 11 -- pkg/constraints/constraint.go | 86 +++++++++ pkg/constraints/constraint_test.go | 274 +++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+), 11 deletions(-) create mode 100644 pkg/constraints/constraint.go create mode 100644 pkg/constraints/constraint_test.go diff --git a/pkg/constraints/cel.go b/pkg/constraints/cel.go index 89d43c186..1fb10b5dd 100644 --- a/pkg/constraints/cel.go +++ b/pkg/constraints/cel.go @@ -16,17 +16,6 @@ import ( // PropertiesKey is the key for bundle properties map (input data for CEL evaluation) const PropertiesKey = "properties" -// Constraint is a struct representing the new generic constraint type -type Constraint struct { - // Constraint message that surfaces in resolution - // This field is optional - Message string `json:"message" yaml:"message"` - - // The cel struct that contraints CEL expression - // This field is required - Cel *Cel `json:"cel" yaml:"cel"` -} - // Cel is a struct representing CEL expression information type Cel struct { // The CEL expression diff --git a/pkg/constraints/constraint.go b/pkg/constraints/constraint.go new file mode 100644 index 000000000..bbce6b188 --- /dev/null +++ b/pkg/constraints/constraint.go @@ -0,0 +1,86 @@ +package constraints + +import ( + "bytes" + "encoding/json" + "fmt" +) + +// OLMConstraintType is the schema "type" key for all constraints known to OLM +// (except for legacy types). +const OLMConstraintType = "olm.constraint" + +// Constraint holds parsed, potentially nested dependency constraints. +type Constraint struct { + // Constraint message that surfaces in resolution + // This field is optional + Message string `json:"message,omitempty" yaml:"message,omitempty"` + + // The cel struct that contraints CEL expression + Cel *Cel `json:"cel,omitempty" yaml:"cel,omitempty"` + + // Package defines a constraint for a package within a version range. + Package *PackageConstraint `json:"package,omitempty" yaml:"package,omitempty"` + + // GVK defines a constraint for a GVK. + GVK *GVKConstraint `json:"gvk,omitempty" yaml:"gvk,omitempty"` + + // All, Any, and None are compound constraints. See this enhancement for details: + // https://github.com/operator-framework/enhancements/blob/master/enhancements/compound-bundle-constraints.md + All *CompoundConstraint `json:"all,omitempty" yaml:"all,omitempty"` + Any *CompoundConstraint `json:"any,omitempty" yaml:"any,omitempty"` + // A note on None: this constraint is not particularly useful by itself. + // It should be used within an All constraint alongside some other constraint type + // since saying "none of these GVKs/packages/etc." without an alternative doesn't make sense. + None *CompoundConstraint `json:"none,omitempty" yaml:"none,omitempty"` +} + +// CompoundConstraint holds a list of potentially nested constraints +// over which a boolean operation is applied. +type CompoundConstraint struct { + Constraints []Constraint `json:"constraints"` +} + +// GVKConstraint defines a GVK constraint. +type GVKConstraint struct { + Group string `json:"group"` + Kind string `json:"kind"` + Version string `json:"version"` +} + +// PackageConstraint defines a package constraint. +type PackageConstraint struct { + // Name of the package. + Name string `json:"name"` + // VersionRange required for the package. + VersionRange string `json:"versionRange"` +} + +// maxConstraintSize defines the maximum raw size in bytes of an olm.constraint. +// 64Kb seems reasonable, since this number allows for long description strings +// and either few deep nestings or shallow nestings and long constraints lists, +// but not both. +// QUESTION: make this configurable? +const maxConstraintSize = 2 << 16 + +// ErrMaxConstraintSizeExceeded is returned when a constraint's size > maxConstraintSize. +var ErrMaxConstraintSizeExceeded = fmt.Errorf("olm.constraint value is greater than max constraint size %d bytes", maxConstraintSize) + +// Parse parses an olm.constraint property's value recursively into a Constraint. +// Unknown value schemas result in an error. Constraints that exceed the number of bytes +// defined by maxConstraintSize result results in an error. +func Parse(v json.RawMessage) (c Constraint, err error) { + // There is no way to explicitly limit nesting depth. + // From https://github.com/golang/go/issues/31789#issuecomment-538134396, + // the recommended approach is to error out if raw input size + // is greater than some threshold. + if len(v) > maxConstraintSize { + return c, ErrMaxConstraintSizeExceeded + } + + d := json.NewDecoder(bytes.NewBuffer(v)) + d.DisallowUnknownFields() + err = d.Decode(&c) + + return +} diff --git a/pkg/constraints/constraint_test.go b/pkg/constraints/constraint_test.go new file mode 100644 index 000000000..c2bc0138a --- /dev/null +++ b/pkg/constraints/constraint_test.go @@ -0,0 +1,274 @@ +package constraints + +import ( + "encoding/json" + "fmt" + "math/rand" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParse(t *testing.T) { + type spec struct { + name string + input json.RawMessage + expConstraint Constraint + expError string + } + + specs := []spec{ + { + name: "Valid/BasicGVK", + input: json.RawMessage(inputBasicGVK), + expConstraint: Constraint{ + Message: "blah", + GVK: &GVKConstraint{Group: "example.com", Version: "v1", Kind: "Foo"}, + }, + }, + { + name: "Valid/BasicPackage", + input: json.RawMessage(inputBasicPackage), + expConstraint: Constraint{ + Message: "blah", + Package: &PackageConstraint{Name: "foo", VersionRange: ">=1.0.0"}, + }, + }, + { + name: "Valid/BasicAll", + input: json.RawMessage(fmt.Sprintf(inputBasicCompoundTmpl, "all")), + expConstraint: Constraint{ + Message: "blah", + All: &CompoundConstraint{ + Constraints: []Constraint{ + { + Message: "blah blah", + Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + }, + }, + }, + }, + }, + { + name: "Valid/BasicAny", + input: json.RawMessage(fmt.Sprintf(inputBasicCompoundTmpl, "any")), + expConstraint: Constraint{ + Message: "blah", + Any: &CompoundConstraint{ + Constraints: []Constraint{ + { + Message: "blah blah", + Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + }, + }, + }, + }, + }, + { + name: "Valid/BasicNone", + input: json.RawMessage(fmt.Sprintf(inputBasicCompoundTmpl, "none")), + expConstraint: Constraint{ + Message: "blah", + None: &CompoundConstraint{ + Constraints: []Constraint{ + { + Message: "blah blah", + Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + }, + }, + }, + }, + }, + { + name: "Valid/Complex", + input: json.RawMessage(inputComplex), + expConstraint: Constraint{ + Message: "blah", + All: &CompoundConstraint{ + Constraints: []Constraint{ + {Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}}, + {GVK: &GVKConstraint{Group: "fals.example.com", Kind: "Fal", Version: "v1"}}, + { + Message: "foo and buf must be stable versions", + All: &CompoundConstraint{ + Constraints: []Constraint{ + {Package: &PackageConstraint{Name: "foo", VersionRange: ">=1.0.0"}}, + {Package: &PackageConstraint{Name: "buf", VersionRange: ">=1.0.0"}}, + {GVK: &GVKConstraint{Group: "foos.example.com", Kind: "Foo", Version: "v1"}}, + }, + }, + }, + { + Message: "blah blah", + Any: &CompoundConstraint{ + Constraints: []Constraint{ + {GVK: &GVKConstraint{Group: "foos.example.com", Kind: "Foo", Version: "v1beta1"}}, + {GVK: &GVKConstraint{Group: "foos.example.com", Kind: "Foo", Version: "v1beta2"}}, + {GVK: &GVKConstraint{Group: "foos.example.com", Kind: "Foo", Version: "v1"}}, + }, + }, + }, + { + None: &CompoundConstraint{ + Constraints: []Constraint{ + {GVK: &GVKConstraint{Group: "bazs.example.com", Kind: "Baz", Version: "v1alpha1"}}, + }, + }, + }, + }, + }, + }, + }, + { + name: "Invalid/TooLarge", + input: func(t *testing.T) json.RawMessage { + p := make([]byte, maxConstraintSize+1) + _, err := rand.Read(p) + require.NoError(t, err) + return json.RawMessage(p) + }(t), + expError: ErrMaxConstraintSizeExceeded.Error(), + }, + { + name: "Invalid/UnknownField", + input: json.RawMessage( + `{"message": "something", "arbitrary": {"key": "value"}}`, + ), + expError: `json: unknown field "arbitrary"`, + }, + } + + for _, s := range specs { + t.Run(s.name, func(t *testing.T) { + constraint, err := Parse(s.input) + if s.expError == "" { + require.NoError(t, err) + require.Equal(t, s.expConstraint, constraint) + } else { + require.EqualError(t, err, s.expError) + } + }) + } +} + +const ( + inputBasicGVK = `{ + "message": "blah", + "gvk": { + "group": "example.com", + "version": "v1", + "kind": "Foo" + } + }` + + inputBasicPackage = `{ + "message": "blah", + "package": { + "name": "foo", + "versionRange": ">=1.0.0" + } + }` + + inputBasicCompoundTmpl = `{ +"message": "blah", +"%s": { + "constraints": [ + { + "message": "blah blah", + "package": { + "name": "fuz", + "versionRange": ">=1.0.0" + } + } + ] +}} +` + + inputComplex = `{ +"message": "blah", +"all": { + "constraints": [ + { + "package": { + "name": "fuz", + "versionRange": ">=1.0.0" + } + }, + { + "gvk": { + "group": "fals.example.com", + "version": "v1", + "kind": "Fal" + } + }, + { + "message": "foo and buf must be stable versions", + "all": { + "constraints": [ + { + "package": { + "name": "foo", + "versionRange": ">=1.0.0" + } + }, + { + "package": { + "name": "buf", + "versionRange": ">=1.0.0" + } + }, + { + "gvk": { + "group": "foos.example.com", + "version": "v1", + "kind": "Foo" + } + } + ] + } + }, + { + "message": "blah blah", + "any": { + "constraints": [ + { + "gvk": { + "group": "foos.example.com", + "version": "v1beta1", + "kind": "Foo" + } + }, + { + "gvk": { + "group": "foos.example.com", + "version": "v1beta2", + "kind": "Foo" + } + }, + { + "gvk": { + "group": "foos.example.com", + "version": "v1", + "kind": "Foo" + } + } + ] + } + }, + { + "none": { + "constraints": [ + { + "gvk": { + "group": "bazs.example.com", + "version": "v1alpha1", + "kind": "Baz" + } + } + ] + } + } + ] +}} +` +) From cb519f122201cda22f645c1b06a2588fcd3c0a0f Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 10 Dec 2021 12:02:26 -0800 Subject: [PATCH 2/2] renaming and add yaml tags Signed-off-by: Eric Stroczynski --- pkg/constraints/constraint.go | 14 +++++++------- pkg/constraints/constraint_test.go | 24 ++++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/constraints/constraint.go b/pkg/constraints/constraint.go index bbce6b188..d59d04542 100644 --- a/pkg/constraints/constraint.go +++ b/pkg/constraints/constraint.go @@ -38,22 +38,22 @@ type Constraint struct { // CompoundConstraint holds a list of potentially nested constraints // over which a boolean operation is applied. type CompoundConstraint struct { - Constraints []Constraint `json:"constraints"` + Constraints []Constraint `json:"constraints" yaml:"constraints"` } // GVKConstraint defines a GVK constraint. type GVKConstraint struct { - Group string `json:"group"` - Kind string `json:"kind"` - Version string `json:"version"` + Group string `json:"group" yaml:"group"` + Kind string `json:"kind" yaml:"kind"` + Version string `json:"version" yaml:"version"` } // PackageConstraint defines a package constraint. type PackageConstraint struct { - // Name of the package. - Name string `json:"name"` + // PackageName is the name of the package. + PackageName string `json:"packageName" yaml:"packageName"` // VersionRange required for the package. - VersionRange string `json:"versionRange"` + VersionRange string `json:"versionRange" yaml:"versionRange"` } // maxConstraintSize defines the maximum raw size in bytes of an olm.constraint. diff --git a/pkg/constraints/constraint_test.go b/pkg/constraints/constraint_test.go index c2bc0138a..b35e36fa0 100644 --- a/pkg/constraints/constraint_test.go +++ b/pkg/constraints/constraint_test.go @@ -31,7 +31,7 @@ func TestParse(t *testing.T) { input: json.RawMessage(inputBasicPackage), expConstraint: Constraint{ Message: "blah", - Package: &PackageConstraint{Name: "foo", VersionRange: ">=1.0.0"}, + Package: &PackageConstraint{PackageName: "foo", VersionRange: ">=1.0.0"}, }, }, { @@ -43,7 +43,7 @@ func TestParse(t *testing.T) { Constraints: []Constraint{ { Message: "blah blah", - Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + Package: &PackageConstraint{PackageName: "fuz", VersionRange: ">=1.0.0"}, }, }, }, @@ -58,7 +58,7 @@ func TestParse(t *testing.T) { Constraints: []Constraint{ { Message: "blah blah", - Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + Package: &PackageConstraint{PackageName: "fuz", VersionRange: ">=1.0.0"}, }, }, }, @@ -73,7 +73,7 @@ func TestParse(t *testing.T) { Constraints: []Constraint{ { Message: "blah blah", - Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}, + Package: &PackageConstraint{PackageName: "fuz", VersionRange: ">=1.0.0"}, }, }, }, @@ -86,14 +86,14 @@ func TestParse(t *testing.T) { Message: "blah", All: &CompoundConstraint{ Constraints: []Constraint{ - {Package: &PackageConstraint{Name: "fuz", VersionRange: ">=1.0.0"}}, + {Package: &PackageConstraint{PackageName: "fuz", VersionRange: ">=1.0.0"}}, {GVK: &GVKConstraint{Group: "fals.example.com", Kind: "Fal", Version: "v1"}}, { Message: "foo and buf must be stable versions", All: &CompoundConstraint{ Constraints: []Constraint{ - {Package: &PackageConstraint{Name: "foo", VersionRange: ">=1.0.0"}}, - {Package: &PackageConstraint{Name: "buf", VersionRange: ">=1.0.0"}}, + {Package: &PackageConstraint{PackageName: "foo", VersionRange: ">=1.0.0"}}, + {Package: &PackageConstraint{PackageName: "buf", VersionRange: ">=1.0.0"}}, {GVK: &GVKConstraint{Group: "foos.example.com", Kind: "Foo", Version: "v1"}}, }, }, @@ -164,7 +164,7 @@ const ( inputBasicPackage = `{ "message": "blah", "package": { - "name": "foo", + "packageName": "foo", "versionRange": ">=1.0.0" } }` @@ -176,7 +176,7 @@ const ( { "message": "blah blah", "package": { - "name": "fuz", + "packageName": "fuz", "versionRange": ">=1.0.0" } } @@ -190,7 +190,7 @@ const ( "constraints": [ { "package": { - "name": "fuz", + "packageName": "fuz", "versionRange": ">=1.0.0" } }, @@ -207,13 +207,13 @@ const ( "constraints": [ { "package": { - "name": "foo", + "packageName": "foo", "versionRange": ">=1.0.0" } }, { "package": { - "name": "buf", + "packageName": "buf", "versionRange": ">=1.0.0" } },