Skip to content

Allow the DefaultCondition on events to be string or object #163

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

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

spolti
Copy link
Member

@spolti spolti commented Mar 15, 2023

fixes #157

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

@spolti spolti requested a review from ricardozanini as a code owner March 15, 2023 19:38
@spolti spolti force-pushed the defaultCondition branch 5 times, most recently from 1f97a97 to 3df0a24 Compare March 16, 2023 01:06
@spolti
Copy link
Member Author

spolti commented Mar 16, 2023

@ricardozanini it is ready to merge, the parse from the sdk go is working, however while generating the csv some objects goes with the different type, e.g.:

* spec.states[1].transition: Invalid value: "string": spec.states[1].transition in body must be of type object: "string"

By creating a customType e.g stringOrObject as we discussed earlier might not fix the issue since we would need to instruct the kubernetes controller-tools to inject our type and override that JSon schema, e.g. replace oneOf with anyOf, the same that is done with the intStr from apimachinery as seen here kubernetes-sigs/controller-tools@8fbbd2e

From the operator side we might have two options:

  • Use the strict workflow object
  • find a way to relax the objects (maybe using the model.Object typed as string) and unmarshall it to the desired object.

@davidesalerno please take a look here as well.

@spolti
Copy link
Member Author

spolti commented Mar 16, 2023

codecov again =/

@spolti spolti force-pushed the defaultCondition branch from 3df0a24 to 2f12a7d Compare March 16, 2023 01:17
@spolti spolti force-pushed the defaultCondition branch from 2f12a7d to 5f62687 Compare March 16, 2023 12:43
@spolti
Copy link
Member Author

spolti commented Mar 16, 2023

There is e KEP opened for supporting the oneOf tag, but it seems not to be implemented yet.
This is a big discussion around this topic kubernetes-sigs/controller-tools#298

the KEP: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1027-api-unions/README.md
There is another feature that might be helpful for us in the kubebuilder that will available on k8s 1.27 only: kubernetes/enhancements#2876

@spolti spolti merged commit d4781e3 into serverlessworkflow:main Mar 20, 2023
@spolti spolti deleted the defaultCondition branch March 20, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow the DefaultCondition on events to be string or object
3 participants