-
Notifications
You must be signed in to change notification settings - Fork 236
feat: ResourceIDMapper for external event sources, external dependents and bulk dependents #3020
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.
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
base: next
Are you sure you want to change the base?
Changes from all commits
1bc89e1
4b0fbbf
4de9b85
26fb256
7626c48
1d335f8
d923e55
ddef4b0
f9afd93
bb02c98
7e9d43e
991be80
3549883
30f3a36
8158925
1712f8e
e952b08
680c355
26a9850
f957efb
a71dd7f
2b41b84
c34e7ab
3c7da23
6885b50
100a09d
f69ec1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| --- | ||
| title: Migrating from v5.1 to v5.2 | ||
| description: Migrating from v5.1 to v5.2 | ||
| --- | ||
|
|
||
| Version 5.2 brings some breaking changes to certain components. This document provides | ||
| a migration guide for these changes. For all the new features, see the release notes. | ||
|
|
||
| ## Custom ID types across multiple components using ResourceIDMapper and ResourceIDProvider | ||
|
|
||
| Working with the id of a resource is needed across various components in the framework. | ||
| Until this version, the components provided by the framework assumed that you could easily | ||
| convert the id of a resource into a String representation. For example, | ||
| [BulkDependentResources](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java#L46) | ||
| worked with a `Map<String,R>` of resources, where the id was always of type String. | ||
|
|
||
| Mainly because of the need to manage external dependent resources more elegantly, | ||
| we introduced a cross-cutting concept: [`ResourceIDMapper`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java), | ||
| which gets the ID of a resource. This is used across various components, see: | ||
|
|
||
| - [`ExternalResourceCachingEventSource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSource.java#L66) | ||
| - [`ExternalBulkDependentResource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/ExternalBulkDependentResource.java) | ||
| - [`AbstractExternalDependentResource`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java#L39) | ||
| and its subclasses. | ||
|
|
||
| We also added [`ResourceIDProvider`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDProvider.java), | ||
| which you can implement in your Pojo representing a resource. | ||
|
|
||
| The easiest way to migrate to this new approach is to implement this interface for your (external) resource | ||
| and set the ID type generics for the components above. The default implementation of the `ResourceIDMapper` | ||
| works with `ResourceIDProvider` (see [related implementation](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ResourceIDMapper.java#L52)). | ||
|
|
||
| If you cannot implement `ResourceIDProvider` because, for example, the class that represents the external resource is generated and final, | ||
| you can always set a custom `ResourceIDMapper` on the components above. | ||
|
|
||
| See also: | ||
| - related issue: [link](https://github.com/operator-framework/java-operator-sdk/issues/2972) | ||
| - related pull requests: | ||
| - [2970](https://github.com/operator-framework/java-operator-sdk/pull/2970) | ||
| - [3020](https://github.com/operator-framework/java-operator-sdk/pull/3020) | ||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.processing; | ||
|
|
||
| import io.fabric8.kubernetes.api.model.HasMetadata; | ||
| import io.javaoperatorsdk.operator.processing.event.ResourceID; | ||
| import io.javaoperatorsdk.operator.processing.event.source.ExternalResourceCachingEventSource; | ||
|
|
||
| /** | ||
| * Provides id for the target resource. This mapper is used across multiple components of the | ||
| * framework, like: | ||
| * | ||
| * <ul> | ||
| * <li>{@link io.javaoperatorsdk.operator.processing.dependent.AbstractExternalDependentResource} | ||
| * <li>{@link ExternalResourceCachingEventSource} | ||
| * <li>{@link io.javaoperatorsdk.operator.processing.dependent.KubernetesBulkDependentResource} | ||
| * </ul> | ||
| * | ||
| * @see ResourceIDProvider<ID> | ||
| */ | ||
| public interface ResourceIDMapper<R, ID> { | ||
|
|
||
| /** | ||
| * @return id for the target resource. | ||
| */ | ||
| ID idFor(R resource); | ||
csviri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Can be used if a polling event source handles only single secondary resource and the id is | ||
| * String. See also docs for: {@link ExternalResourceCachingEventSource}; or in {@link | ||
| * io.javaoperatorsdk.operator.processing.dependent.AbstractExternalDependentResource} when there | ||
| * is always only one secondary resource for that dependent resource. By definition cannot be used | ||
| * for a BulkDependent resources. | ||
| * | ||
| * @return static id mapper, all resources are mapped for same id. | ||
| * @param <R> secondary resource type | ||
| */ | ||
| static <R> ResourceIDMapper<R, String> singleResourceResourceIDMapper() { | ||
| // the result could be any string, by definition would work with any value | ||
| return r -> "id"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The value should be irrelevant and we should rather use a value that has not risk of conflicting with an identifier a user would create.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will have to upgrade upgrade the javadocs, there is no situation where there can be a collision. Assuming that this is by definition not usable for BulkDependentResources - will add javadoc. If you see some situation where it actually can lead to a collision pls describe it.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the harm of choosing a value that we're sure won't cause a collision, regardless of whether one such collision can happen? I mean if the value doesn't matter, then why not choose one that doesn't look like it is meaningful? For that matter, it could be a totally random one.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I belive in this cases we should focus:
So to fulfill those two as you said, any string that is simple enough would do. I choose simply
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But added also a comment that it could be any values, feel free to choose any simple and expressive string literal, it is fine by me. |
||
| } | ||
|
|
||
| @SuppressWarnings({"rawtypes", "unchecked"}) | ||
| static <R, ID> ResourceIDMapper<R, ID> resourceIdProviderMapper() { | ||
| return r -> { | ||
| if (r instanceof ResourceIDProvider resourceIDProvider) { | ||
| return (ID) resourceIDProvider.resourceId(); | ||
| } else { | ||
| throw new IllegalStateException( | ||
| "Resource does not implement ExternalDependentIDProvider: " + r.getClass()); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| static <R extends HasMetadata> ResourceIDMapper<R, ResourceID> kubernetesResourceIdMapper() { | ||
| return ResourceID::fromResource; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.processing; | ||
|
|
||
| /** | ||
| * Provides the identifier for an object that represents a resource. This ID is used: | ||
| * | ||
| * <ul> | ||
| * <li>to select the target external resource for a dependent resource from the resources returned | ||
| * by {@link io.javaoperatorsdk.operator.api.reconciler.Context#getSecondaryResources(Class)}, | ||
| * <li>used in {@link ResourceIDMapper} for event sources in external resources. But also for bulk | ||
| * dependent resource see {@link | ||
| * io.javaoperatorsdk.operator.processing.dependent.ExternalBulkDependentResource}, | ||
| * <li>and external event sources, see {@link | ||
| * io.javaoperatorsdk.operator.processing.event.source.ExternalResourceCachingEventSource} | ||
| * </ul> | ||
| * | ||
| * @see ResourceIDMapper | ||
| * @param <ID> type of the id | ||
| */ | ||
| public interface ResourceIDProvider<ID> { | ||
|
|
||
| /** ID for the resource POJO that implement this interface. */ | ||
| ID resourceId(); | ||
csviri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.