Skip to content
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

Add support for specifying the API Group and API Version of Custom Resource dyamically #4876

Closed
penev-ff opened this issue Feb 14, 2023 · 10 comments

Comments

@penev-ff
Copy link

Is your enhancement related to a problem? Please describe

In the current mechanism for defining a CustomResource,
the @Version and @Group annotations are used to specify the CRD's apiGroup and apiVersion:

@Group("groupName")
@Version("v1")
@ShortNames("sn")
public class MyCustomResource extends CustomResource<MyCustomResourceSpec, MyCustomResourceStatus> {

}

However, these annotations require a hard-coded, compile-time processed string literal.

While this approach is convenient, it can be also limiting in certain scenarios,
such as when we want to provide the values dynamically.

In our case, we want to provide the group/version using the environment variables (e.g. GROUP, VERSION).

Describe the solution you'd like

  • A possible solution might be to add support for parsing environment variable placeholders in the annotation values:
  @Group("${bar}") 

Describe alternatives you've considered

An alternative would be to somehow provide a mechanism for overriding the methods, whose return value would take precedence over the value of the annotation.

We tried to override some of the getGroup/getCRDName methods in the CustomResource<S, T> class.

However, this didn't change the behavior.

The reason is that the static versions of these methods are used when constructing the CustomResrouce or its definition context built later. Consequently, the values from the annotations are used to generate the appropriate K8s API endpoint.

Additional context

No response

@shawkins
Copy link
Contributor

shawkins commented Feb 14, 2023

It seems like there are two scenarios where this comes into play - supporting multiple versions using the same object model when most of the resources do not change (that may require knowing about the multiple mappings at the same time). And supporting multiple operators for the same logical type by changing the group name (should be only a single mapping at a time).

The closest support we have today is using the generic api, then using an objectmapper to convert from the generic value objects to your typed one. There is also KubenetesDeserializer.registerCustomKind(apiVersion, kind, class) which could help with the load method - but that is not ideal as it's a static method on an internal api, so we'll like remove / refine that support later.

Do you need this to be fully dynamic, or is it possible to support annotations like:

@GroupVersion("something", "v1", "v2")
@GroupVersion("somethingelse", "v1")
public class ...

That would of course need to be coupled with changes is the resource method, and add a resources method that also accepted the api version - but it would handle both cases.

@penev-ff
Copy link
Author

penev-ff commented Feb 14, 2023

We need to support multiple operators for the same logical type by changing the group name.

Unfortunately, it is not possible to hardcode the group name using a literal like this:

@GroupVersion("group", "v1")
public class ...

The reason is that the group name won't be fixed, but uniquely generated and loaded via environment variable, and then we need to provided it to the CustomResource model.

So, it seems to be fully dynamic.

@metacosm
Copy link
Collaborator

Could you explain your use case in greater detail, please, as I have to admit that I'm struggling to see how this could be useful? In both cases described by @shawkins, I would argue that things would probably be better handled using inheritance to deal with the common parts but I may be missing some context.

@penev-ff
Copy link
Author

penev-ff commented Feb 21, 2023

Yes, I will try to provide more detailed explanation and clarify our use-case.

We have a helm chart used to install K8s application with CRD and dedicated per namespace operator deployment.

In our situation, we need the capability to manage several installations of the same application within a cluster.
But, if we attempt to install more than one CRD with the same name and group (during the helm installation), we will see an error message stating that the cluster already contains a CRD with the same name and group:

"Error: INSTALLATION FAILED: rendered manifests contain a resource that already exists. Unable to continue with install: CustomResourceDefinition ..."

To resolve this problem and prevent the aforementioned conflicts, we decided to provide uniquely generated group name per installation for the corresponding CRD.

Throughout the deployment process, the operator will additionally be made aware of the uniquely formed group name of the CRD via an environmental variable. In this manner, the operator would be aware of which CRD to watch.

So we have different namespaced installations with the application and the operator. Each operator must be assigned to a single CRD with a specifically given name or group.

However, the current version of the fabric8 client maps the group name of the CustomResource using the @Group("fixed") annotation, which only allows for a fixed string literal to be used.

This limits the ability to map the uniquely generated group name, exposed using the environment variable, in the Java operator.

Therefore, enabling more flexible way to set or override the name / group name or the other annotation-based values in the CustomResource POJO, would be helpful in such situations.

Also I managed to find a similar issue that might be helpful: #2738

@metacosm
Copy link
Collaborator

I'm really confused… why do you need to install the same CRD multiple times? It seems that your operator should only watch resources from its target namespaces rather than trying to trick the cluster into deploying multiple versions of the same CRD under different apiVersions. Maybe a more concrete example would help understand better?

@shawkins
Copy link
Contributor

shawkins commented Feb 22, 2023

This comes down to the isolation needed and who "owns" the crd. One approach we're trying in a similar situation is to pull a common crd out of multiple operator bundles to be managed as its own dependent bundle. This is because we know that there won't be any conflicts with responsibility over the resources and we're ok with centralized management.

If you want full isolation though - such that you have the ability to roll in crd changes on a per namespace basis, then you need separate crds.

@metacosm
Copy link
Collaborator

Or something else, like kcp… I don't think it's the role of the client to substitute to the platform's limitation, but that's just my opinion.

@ivanangelov
Copy link

Hello,

Our exact use case is the following:

  1. We are using java-operator-sdk in order to build a K8s operator.
  2. For creating the CustomResource in the Java code we are using io.fabric8.kubernetes.client.CustomResource.
  3. The attributes for the CustomResource (Group, Version, ShortNames, Kind, Plural, etc.) could be set only via a Java annotation, which requires a hardcoded string.
  4. In our solution, the values for the attributes are generated in deploy time, so we pass them as parameters to our operator app and then would need to set them to the CustomResource. The current approach with the annotations is not sufficient for us.
  5. So, we would like to be able to set the attributes of the CustomResource based on parameters passed to the Operator app and not as hardcoded strings.

Regards,
Ivan

@shawkins
Copy link
Contributor

shawkins commented Mar 24, 2023

In rough terms everywhere the relevant annotations are consulted would need to through common methods - that is currently not quite the case, at least KubernetesDeserializer is not using HasMetadata.getGroup / getVersion. JOSDK is probably using the right methods, and there's some on CustomResource as well. After making sure all existing paths use the appropriate methods, a static resolver would need to be installable into both HasMetadata and CustomResource to allow for the user to override the annotation values in whatever way makes sense for them. This isn't a great solution as it will be reliant on more static state.

Another option that I haven't tried is to add your own bootstrapping logic that manipulates the annotation state: https://itecnote.com/tecnote/java-modify-a-class-definitions-annotation-string-parameter-at-runtime/ - of course that feels hackish.

A final thought is that this may be something a custom interceptor could handle - that is leave the group/version in the annotation, and let logic similar to the backwards compatibility interceptor rewrite that to the desired group/version. What other annotations are you looking to modify?

@metacosm this will come up enough that we should give more thought as to how to handle it.

@stale
Copy link

stale bot commented Jun 22, 2023

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale label Jun 22, 2023
@stale stale bot closed this as completed Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants