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

Define content rules over CRD #23

Open
dweber019 opened this issue Dec 1, 2021 · 9 comments
Open

Define content rules over CRD #23

dweber019 opened this issue Dec 1, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@dweber019
Copy link

It would be nice to define the content rules described on https://www.apicur.io/registry/docs/apicurio-registry/2.1.x/getting-started/assembly-intro-to-registry-rules.html in https://github.com/Apicurio/apicurio-registry-content-sync-operator/blob/main/api/src/main/java/io/apicurio/sync/api/ArtifactSpec.java.

Use case: We have AVRO definitions which have different compatibility configurations.

@famarting
Copy link
Contributor

famarting commented Dec 1, 2021

great suggestion, this is a natural step.

Apicurio Registry supports configuring rules that apply globally to all artifacts and rules specific to one artifact (so that rule will be applied to all artifact versions)

I think this will need a new CRD, possible we can cover the two usecases with the same CRD

@famarting famarting added the enhancement New feature or request label Dec 1, 2021
@dweber019
Copy link
Author

@famartinrh I can do a PR. Would implement the rules in the same CRD but as optional fields.
Ok?

@famarting
Copy link
Contributor

@dweber019 I think this makes more sense to be implemented into a separated CRD. Rules are an entity of it's own in Apicurio Registry.

Something like

apiVersion: rule.apicur.io/v1alpha1
kind: Rule
metadata:
  name: validity-global
spec:
  type: VALIDITY
  config: SYNTAXT_ONLY

and

apiVersion: rule.apicur.io/v1alpha1
kind: Rule
metadata:
  name: compat-artifact-foo
spec:
  type: COMPATIBILITY
  config: BACKWARD
  groupId: test-group
  artifactId: foo

@dweber019
Copy link
Author

Looks good to me.
Would have the same CRD for artifact and global rules? => the fields groupId and artifactId are optional and if not set the rule is for the global behavior?

@famarting
Copy link
Contributor

famarting commented Dec 14, 2021

Looks good to me. Would have the same CRD for artifact and global rules? => the fields groupId and artifactId are optional and if not set the rule is for the global behavior?

that was my thinking :)
but I'm ok for another approach if you prefer it

@eshepelyuk
Copy link

eshepelyuk commented Jan 12, 2022

I would suggest not to use the same CRD to define global rules and rules for an artifact.
But to use the new CRD only for declaring a global rules. And add new field to artifact CRD.

In fact a global rule can is better to be defined via apicurio registry config , not a CRD of this operator.

https://www.apicur.io/registry/docs/apicurio-registry/2.1.x/getting-started/assembly-intro-to-registry-rules.html#registry-rules-config_registry

Actually an issue was raised to be able to define rules per artifact, so it makes sense to extend Artifact CRD only and leave global content rules xonfig to Apicurio itself.

Wdyt ?

@famarting
Copy link
Contributor

I think having a CRD for configuring global rules makes total sense. This project could be seen as a declarative API to manage and configure Apicurio Registry, and therefore most of the Apicurio Registry APIs will need to be implemented to have full control over the registry in a declarative way. Once someone is onboarded into declarative configurations... having to call an API or go to a UI to do some specific configuration manually seems like a poor user experience.

About having the CRD only for globalRules and adding a new field to artifact CRD... The problem I see to managing artifact rules in the artifact CRD is that an instance of artifact CRD represents an artifact version in Apicurio Registry, so if an artifact has multiple versions there are going to be multiple instances of the artifact CRD. That way the artifact rule configuration will be replicated accross all the instances of the artifact CRD. It will be confusing to users which artifact rule config in which artifact CRD is the correct one. Again seems a poor user experience. That's why I suggested a separated CRD that points to an artifact (groupId and artifactId) and there the content rules for that specific artifact are uniquely defined.

@eshepelyuk
Copy link

Well, I see there is a sort of functionality duplication.

There is an issue in another project Apicurio/apicurio-registry-operator#61

That operator is dedicated to setup and confifure apicurio registry, including global rules.

This operator is only about working with artifacts (including artifact specific rules)

Am I missing smth regarding responsibilities of those 2 projects ?

@famarting
Copy link
Contributor

regarding responsibilities for this two projects, that's something we can shape over time...

Currently apicurio-registry-opeator does nothing about managing global rules, that issue you linked was fixed by implementing a new directly in apicurio-registry.

To clarify we could consider this project (apicurio-registry-content-sync-operator) is a tool for management of artifacts in apicurio-registry. In the future it could make sense to integrate this project inside apicurio-registry-operator, that way the artifact controller would be deployed by apicurio-registry-operator.

Said so, apicurio-registry-content-sync-operator can also be seen as the gitops enabler for apicurio-registry, that way it could make sense to also provide support for configuring global and artifact rules, because if a user gets into gitops it's likely they will want to do manage everything with kubernetes CRDs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants