Skip to content

Add enhancement proposal for generic constraints feature #91

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.

Sign up for GitHub

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

Merged
dinhxuanvu merged 7 commits into from
Nov 18, 2021
Merged

Add enhancement proposal for generic constraints feature #91

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f979265
Add enhancement proposal for cluster runtime constraints feature
dinhxuanvu Sep 8, 2021
8ab48ec
Add compound constraint section
dinhxuanvu Sep 10, 2021
05be439
Update the EP to focus on generic constraint aspect only
dinhxuanvu Oct 11, 2021
260bf1b
Update the enhancement with details on syntax and alternative syntax
dinhxuanvu Nov 4, 2021
23c130f
Update the enhancement to address feedbacks
dinhxuanvu Nov 10, 2021
15a6e8d
Update the enhancement to select the final syntax as discussed
dinhxuanvu Nov 15, 2021
84335c9
Update some minor details on the syntax
dinhxuanvu Nov 17, 2021
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
181 changes: 181 additions & 0 deletions enhancements/generic-constraints.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
---
title: generic constraint
authors:
- "@dinhxuanvu"
reviewers:
- "@kevinrizza"
- "@joelandford"
approvers:
- "@kevinrizza"
creation-date: 2021-08-26
last-updated: 2020-10-08
status: provisional
---

# generic-constraint

## Release Signoff Checklist

- [ ] Enhancement is `implementable`
- [ ] Design details are appropriately documented from clear requirements
- [ ] Test plan is defined
- [ ] Graduation criteria for dev preview, tech preview, GA

## Summary

The dependency resolution in OLM needs to support constraints that are based on arbitrary properties from either bundles or clusters beyond the existing GVK or package properties.

This enhancement proposes the adoption of [Common Expression Language
(CEL)](https://github.com/google/cel-go) to present the specification and usage of generic constraint that OLM can evaluate for dependency resolution at runtime.

CEL is a sufficiently lightweight expression language that has been adopted in multiple opensource projects. It allows users to express operator constraints in both straight-forward and advanced cases without OLM having to dictate the evaluation methodology.

## Motivation

At the moment, OLM only supports 2 built-in types of constraint:
- GVK constraint (`olm.gvk.required`)* which depends on a specific GroupVersionKind information of a CRD/API.
- Package constraint (`olm.package.required`)* which depends a specific package name and version (or a range of version)

Both types are specifically handled by the resolver in OLM and cannot be changed or expanded to support further use cases. In order to support more types of constraints, it is generically expected for OLM to add more specific built-in types to allow users to express those new constraints in the bundles. Over the time, it may become cumbersome and impractical to continue to add more specific types to the resolver. The need for OLM to support a generic constraint model becomes more apparent. With a generic constraint model, OLM no longer needs to carry any specific evaluation methodology besides the understanding of how to parse the constraint syntax. The users should be able to express declaratively the rules for a constraint that can be evaluated againts the dataset of arbitrary properties.

* Note: The current GVK and package constraints are specified in `dependencies.yaml` using `olm.gvk` and `olm.package` type and they are coverted to `olm.gvk.required` or `olm.package.required` property (which is required constraint) to differentiate from the `olm.package` and `olm.gvk` property in the bundle.
Copy link
Member

@ecordell ecordell Nov 4, 2021

Choose a reason for hiding this comment

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

Suggested change
* Note: The current GVK and package constraints are specified in `dependencies.yaml` using `olm.gvk` and `olm.package` type and they are coverted to `olm.gvk.required` or `olm.package.required` property (which is required constraint) to differentiate from the `olm.package` and `olm.gvk` property in the bundle.
* Note: The current GVK and package constraints are specified in `dependencies.yaml` using `olm.gvk` and `olm.package` type and they are converted to `olm.gvk.required` or `olm.package.required` property (which is required constraint) to differentiate from the `olm.package` and `olm.gvk` property in the bundle.


### Goals
- Provide specification and guideline to specify a constraint that contains declarative requirements/rules.
- Support the compound constraint (the constraint with mutliple requirements that is resolved into a single operator).
### Non-Goals
- Eliminate the existing built-in constraints.
- Support multiple expression languages other than CEL (though the syntax may allow the support for other languages for future use).
- Provide mechanism to provide cluster constraints. This feature can be addressed in a separate enhancement.


## Proposal

The [Common Expression Language (CEL)](https://github.com/google/cel-go) is a great inline expression language that can be used to express multiple rules within a single expression. The rules are fully customizable and constructed specifically for users' needs and requirements without OLM needing to know the design methodology. It is important to recoginize the dependent relationship between constraints and properties. As a constraint is evaluated against a dataset of properties from a bundle or a given source, in order to design a constraint, there must be an understanding on what properties are available in a given a bundle or source and what information they present in order for a constraint to be evaluated propertly.

The CEL library also supports [extension functions](https://github.com/google/cel-spec/blob/master/doc/langdef.md#extension-functions) which allows OLM to implement additional functions that may be necessary for certain operations. These functions then can be used within the CEL expression. For example, [semver](https://semver.org/) comparison is not built-in in CEL and it is often used in OLM as bundles are versioned in semver format. The semver functions can be added in the initial release and allow users to express semver evaluations in the constraint. However, extension functions should be treated and designed with care as they can be difficult to change in the future and potentially create version skew or breaking changes.

All CEL expressions are [parsed and typechecked](https://github.com/google/cel-go#parse-and-check) before evaluation. The insulting programs can be cached for later evaluation to reduce unnecesary computation. Also, the CEL evaluation is [thread-safe and side-effect free](https://github.com/google/cel-go#evaluate).
Copy link
Member

@ecordell ecordell Nov 4, 2021

Choose a reason for hiding this comment

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

Suggested change
All CEL expressions are [parsed and typechecked](https://github.com/google/cel-go#parse-and-check) before evaluation. The insulting programs can be cached for later evaluation to reduce unnecesary computation. Also, the CEL evaluation is [thread-safe and side-effect free](https://github.com/google/cel-go#evaluate).
All CEL expressions are [parsed and typechecked](https://github.com/google/cel-go#parse-and-check) before evaluation. The resulting programs can be cached for later evaluation to reduce unnecessary computation. Also, the CEL evaluation is [thread-safe and side-effect free](https://github.com/google/cel-go#evaluate).


### User Stories

#### Generic Constraint

As an operator author, I would like to specify a constraint that has my own rules/requirements that are appliable to a set of known properties are from other bundles or from the cluster itself without having to tell OLM how to evaluate those rules. For example, if I know there is a property named certified in some bundles, I can express a constraint with a rule that says my operator to only be installed along with those certified bundles without having to tell OLM to support a new type of constraint for certified property.

#### Compound Constraint

As an operator author, I would like to specify a constraint that has multiple rules and requirements that must be resolved into a single dependent operator. For example, I would like to have a constraint for a dependent operator that belongs to a specific package and provides a specific API/GVK.

### Design Details

#### Generic Constraint Syntax

The constraint syntax is designed to have similarity with the existing constraints that OLM supports which are GVK and package. It is intentional to keep the fundamental syntax with `type` and `value` the same so that it doesn't introduce any dramatic changes to the overall user experiences.

Each constraint is specified using the following syntax:

```json=
{
"type":"olm.constraint",
Copy link
Member

@njhale njhale Nov 4, 2021

Choose a reason for hiding this comment

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

why are all types prefixed by olm? shouldn't it be deppy or maybe nothing?

Copy link
Member

@joelanford joelanford Nov 5, 2021

Choose a reason for hiding this comment

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

I think we should define at least one reserved prefix and document that custom properties should also use different unique prefixes (maybe owned domain?) to ensure uniqueness across the global set of properties that might exist. Uniqueness is especially important now that we're introducing a constraint syntax that supports arbitrary expressions against arbitrary properties.

"value":{
"evaluator":{
"id":"cel"
},
"rule": "properties.exists(p, p.type == \"certified\")",
"message": "require to have certified property",
"action":{
"id":"require"
}
}
}
```

The constraint `type` is `olm.constraint` and the `value` is information associated with constraint. The `value` struct has 4 fields: `evaluator`, `rule`, `message` and `action`.

The `evaluator` is a struct with `id` field to represent the language library that will be used to evaluate the expression. At the moment, only CEL expression is supported using `cel` as the identifier.
Copy link
Member

@joelanford joelanford Nov 3, 2021

Choose a reason for hiding this comment

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

What's the purpose of evaluator being an object with an id field rather than having evaluator being the string ID directly?

Do we envision needing to add other fields to the evaluator object? If so, what are some examples of those other fields that we know we'll need in the future?

Copy link
Member

@ecordell ecordell Nov 4, 2021

Choose a reason for hiding this comment

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

I'd go further: why not omit the evaluator entirely and identify via the type?

"type":"olm.constraint.cel",

Same for action:

{
   "type":"olm.constraint.cel.required",
   "value": "properties.exists(p, p.type == \"certified\")",
}

Edit:

Just noticed this form in the section at the end. Any reason it's not the preferred form?

Copy link
Member

@joelanford joelanford Nov 4, 2021

Choose a reason for hiding this comment

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

My concern is that different root types will cause version skew concerns when we add a new constraint type.

If we:

  • add support for olm.constraint.cel.required in 0.20
  • add support for olm.constraint.foobar.required in 0.21

And then a bundle contains olm.constraint.foobar.required and is attempted to be installed in <=0.20, what does OLM do with that property?

In 0.20, support for that as a known constraint is not present, so OLM would treat it like an opaque property and ignore it entirely. There's no opportunity to fail resolution (if its actually required) or ignore it on purpose (because its optional) because OLM can't tell user intent since we're co-mingling constraints and properties.


The `rule` field is the string that presents a CEL expression that will be evaluated during resolution against . Only CEL expression is supported in the initial release.
Copy link
Member

@joelanford joelanford Nov 3, 2021

Choose a reason for hiding this comment

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

Does this field apply only when evaluator.id is "cel"? If not, it we should describe the general purpose of this field and have a few examples (one of which should be a cel rule).


Copy link
Member

@joelanford joelanford Nov 3, 2021

Choose a reason for hiding this comment

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

Separate thread about rule...

  • Can you go into some detail about what variables will be available in the context of an expression? Is it just properties?
  • Are we planning to enable macros? If so, all or a subset?
  • Are we planning to implement any custom functions? If so, what are they?

The `message` field is an optional field that is accommodating the rule field to surface information regarding what this CEL rule is about. The message will be surfaced in resolution output when the rule evaluates to false.

Finally, the `action` field is a struct with the `id` field to indicate the resolution action that this constraint will behave. For example, for `require` action, there must be at least one candidate that satisfies this constraint. This action can be used to indicate optional constraint in the future. For the initial release, `require` action is supported.
Copy link
Member

@joelanford joelanford Nov 3, 2021

Choose a reason for hiding this comment

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

Same question as I have for evaluator. Why do we need the sub id field? What are some examples of other fields that we know we'll need under the action object?

Will all actions be applicable to all future evaluators and all rules?


#### Compound Constraint
Copy link
Member

@joelanford joelanford Nov 3, 2021

Choose a reason for hiding this comment

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

Given that #97 focuses on general compound constraints, it seems like our stance should be to favor that as a best practice. While the approach described here is just as valid from a technical perspective, it seems like the approach in #97 integrates better into how the resolver works and will give users more insight into which aspect(s) of the compound constraint caused resolution failures.


The CEL [syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax) supports a wide range of operators including logic operator such as AND and OR. As a result, a single CEL expression can have multiple rules for multiple conditions that are linked together by logic operators. These rules are evaluated against a dataset of multiple different properties from a bundle or any given source and the output is solved into a single bundle or operator that sastifies all of those rules within a single constraint. For example:

```json=
{
"type": "olm.constraint",
"value": {
"evaluator": {
"id": "cel",
}
"rule": 'properties.exists(p, p.type == "certified") && properties.exists(p, p.type == "stable")',
"message": 'require to have "certified" and "stable" properties',
"action": {
"id": "require"
}
}
}
```

The expression in `rule` has two requirements that are linked in an AND (&&) logical operator. In order for that expression to be true, both requirements must be satisfied. Therefore, the resolved operator will be a single bundle that has both certified and stable properties. This concept represents the compound constraint.

### Risks and Mitigations

#### User experience concern

The CEL language is relatively new and requires a learning curve to understand. The complexity of the language may potentially create a difficult user experience (UX) especially when the users intend to write a complex expression for an advanced user case. Also, the current resolver doesn't surface the resolution information very clearly which leads to the difficulty on how the users can identify what goes wrong when the CEL expression is evaluated to `false`.

Mitigation: The feature can be implemented under a feature flag to allow feedbacks to be gathered and then the UX can be evaluated based on those feedbacks in order to improve the UX if needed in future releases.

Also, the constraint syntax can be simplified to reduce the complexity in order to improve the overall UX. (See Alternatives).

For majority of use cases, the CEL expression should be simple and straight-forward especially with well-constructed examples and guidelines where users can make a few plug-in changes and ready to be used.

The `message` field is introduced to allow specific information from the author who writes the expression to surface as resolution output.

#### CEL language maturity and support

The CEL language is relatively new and not yet at stable release (v1). There are potentially breaking changes that can be introduced.

Migration: The CEL library can be spinned to a specific release to avoid introducing breaking changes if occurs.

The language is being used in another well-known opensource projects including Tekton and Kubernetes ([apiserver/CRD](https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md)). This should improve the confidence on the usage of CEL language and these teams will potentially influence or impose against any major breaking changes that may come to CEL.

If CEL becomes too instable in the future due to breaking changes, it is possible to gradually deprecate the support of the language and introduce another language that is more stable. The constraint syntax allows the support of other languages in the future via `evaluator` field.

## Alternatives

### Other languages choices

While CEL is the selected language to be supoorted in the new OLM constraint type. It is not the intention to limit OLM to only support CEL expression. It possible to support multiple expression languages in the future releases if needed.

Besides CEL, there are other languages that can be used such as:

- [Rego](https://github.com/open-policy-agent/opa/tree/main/rego) (Open Policy Agent)
- [Expr](https://github.com/antonmedv/expr)
- [Starlark](https://github.com/bazelbuild/starlark)
- [Cue](https://github.com/cue-lang/cue)

All of these languages can be supported in constraint type. The `evaluator` field is designed to support multiple evaluators/languages if needed. However, intrducing a new language to the constraint type should be evaluated carefully to ensure there is a real need for it. Providing the support for multiple languages can be overwhelming and potentially creates a fragmented user experiences and unnecessary maintainance effort in a long run.

### Simplified constraint type

The current proposed constraint syntax has most of information nested under `value` field. The complexity of a struct with nested fields can create a bad user experience. It is possible to introduce a constraint type that is specified for CEL

```json=
{
"type": "olm.constraint.cel",
"value": {
"rule": 'properties.exists(p, p.type == "certified") && properties.exists(p, p.type == "stable")',
"message": 'require to have "certified" and "stable" properties',
Copy link
Member

@ecordell ecordell Nov 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This just seems like restating the constraint. Is this actually a field that should be optionally available on in the property metaschema?

{
  "type": "proptype",
  "value": <>, 
  "failureMessage": "custom message about this property that a user will see if it is unsatisfied"
}

Copy link
Member

@njhale njhale Nov 4, 2021

Choose a reason for hiding this comment

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

I'd buy "description" making sense for all properties.

Copy link
Member

@joelanford joelanford Nov 16, 2021

Choose a reason for hiding this comment

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

Based on the current state of this EP, I think I'd vote to start with failureMessage in the root of the olm.constraint value struct.

A general description and a constraint failureMessage seem different enough semantically to me that I don't think we should overload one onto the other.

}
}
```

The identifier `type` for this constraint is `olm.constraint.cel` which means this is a CEL constraint that only supports CEL expression. The `value` field contains the `rule` field which houses the CEL expression and the `message` field which is the resolution output if the expression is evaluated as `false`.

This simplified version of constraint type still satisfies the goals of having a generic constraint (though to lesser extend due to lacking of expandability that is provided by additional fields) and the compound constraint.