[Sync] main branch to release-0.12.0 for pr186 #188
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Label is used in the AuthConfig templates and used as a label selector on the Authorino instance. * Activated some AuthConfig handling unit tests that were previously missing a suite level Signed-off-by: Aslak Knutsen <aslak@4fs.no>
This way we can stick to the contract introduced by `Reconciler` interface and keep the code simpler. Additionally adds an idiom to ensure that each type conforms to the interface definition at compile time. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
This improvement is built on top of `Reconciler` interface to provide a unified approach for handling dependent resources. For this purpose, from the original `Reconciler` interface, `SubResourceReconciler` interface has been derived. It includes the `Reconcile` function from the parent and in addition provides two additional contracts for handling dependent resources: > `Cleanup` This is expected to be called when there is no relevant ISVC in the namespace. This allows handling "singleton" resources used for all ISVCs across the namespace, e.g. `PodMonitor` or `PeerAuthentication`. > `Delete` Allows to remove resources no longer needed when associated ISVC is deleted. This is currently only used by AuthConfig and Route reconcilers. For the latter, it is needed as the route is in another namespace, therefore we cannot use ISVC as the owner reference. > Enhancements - All reconcilers now include the "compile-time assertion of interface implementation" idiom to ensure consistency across the reconcilers (and to actually leverage the idea of interface contracts). - To simplify and reduce code duplication there are two default implementations (called traits) - `NoResourceRemoval` and `SingleResourcePerNamespace` which can be included in a concrete `SubResourceReconciler`. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
As it now holds more than a single interface Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
When constructing a manager the defined scheme instance is passed to manager's client. Therefore calling manager.GetClient().Scheme(), or in fact client.Scheme() on injected one, makes passing scheme to reconcilers redundant. This simplifies the code by removing explicit coupling with the scheme instance. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
With this approach there is no need for extract code in main reconcilers (i.e. KServe and ModelMesh) to handle reconcile/delete/cleanup logic of related resources. The only requirement now is to add instance of the subresource reconciler to the slice, the rest is handled uniformly. It also process all subreconcilers now instead of failing fast on the first occured error, following the eventual consistency approach. This is handled by using go-multierror library which is also found in odh-operator. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
…mponentIsEnabled Signed-off-by: Spolti <fspolti@redhat.com>
feat: dynamically include authorino label selector
chore(reconciler): refactors Reconciler to simplify handling dependent resources
[RHOAIENG-5125] - Wrong err assignment at controller utils VerifyIfCo…
This change handles "soft opt-in" for authorization capability handled by Authorino. The solution is based on Conditions exposed in DSCInitialization resource, as provided in opendatahub-io/opendatahub-operator#949. When the condition of type `CapabilityServiceMeshAuthorization` has a reason `MissingOperator` it will assume authorino/authorization module is not present and therefore does not handle AuthConfigs - leaving models not secured as it was for the previous versions. This approach allows to softly opt-in for Authorization without blocking the upgrade. Any other reason means that ODH Operator is in not happy state and will eventually reconcile so that Authorino will start working. The logic consists of the following steps: * during the controller bootstrap it will check separately if ServiceMesh is enabled and Authorino is present (latter by inspecting the status.condition) * when defining watches (using builder instance for OpenshiftReconciler) it will conditionally set a watch for `AuthConfig` * it guards AuthConfig reconciler from executing based on the "authorization capability". This prevents constant failures due to `AuthConfig` not being defined for the cluster. > [!NOTE] > As it is the case with original logic brought in #181, it will require restart of the controller pod to set a watch on AuthConfig resource when Authorino is installed in the cluster. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
when Authorino operator is missing in the cluster no AuthConfig should be created. Signed-off-by: bartoszmajsak <bartosz.majsak@gmail.com>
feat(authz): soft opt-in for authz capability
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jooho, terrytangyuan 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#186
This PR need several PR from April 2nd ~ 5th so it would be better to sync than cherry pick more than 10 commits.
Description
How Has This Been Tested?
Merge criteria: