feat(constraints): add compound constraints and olm.constraint value parser #203
Conversation
openshift-ci
bot
added
the
approved
pkg/constraints/constraint.go
Outdated
| // CompoundConstraint holds a list of potentially nested constraints | ||
| // over which a boolean operation is applied. | ||
| type CompoundConstraint struct { | ||
| Constraints []Constraint `json:"constraints"` |
There was a problem hiding this comment.
While I understand CompoundConstraint is a list of constraints but then Constraint itself can be a All, Any, None constraint which is a CompoundConstraint itself. So we potentially have a CompoundConstraint as a list of CompoundConstraint. While it is not a problem programmatically but logically it is convoluted and frankly difficult to test.
There was a problem hiding this comment.
Not quite, since CompoundConstraint does not alias Constraint; they're two distinct types. I don't see how you would get nice recursion that leverages std serialization libs otherwise. The encoded form is readable and comprehensible imo. Plus this is what the EP proposed.
| // 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"` | ||
| } | ||
|
|
There was a problem hiding this comment.
Note: This was just moved to constraints.go and is still available within the package.
…parser Signed-off-by: Eric Stroczynski <[email protected]>
estroz
force-pushed
the
feat/compound-constraints
branch
from
b075448 to
ed76771
Compare
estroz
force-pushed
the
feat/compound-constraints
branch
from
eb20dfd to
cb519f1
Compare
Signed-off-by: Eric Stroczynski <[email protected]>
estroz
added
the
tide/merge-method-squash
|
/approve I'm happy with the changes introduced in this PR, but I will wait for @dinhxuanvu to lgtm as he has been involved. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awgreene, estroz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| 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), |
|
/lgtm |
Follow-up on #202 for https://github.com/operator-framework/enhancements/blob/master/enhancements/compound-bundle-constraints.md
Signed-off-by: Eric Stroczynski [email protected]