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

fix: Prevent operator panic on wrong Kamelet binding #3595

Closed
wants to merge 1 commit into from

Conversation

essobedo
Copy link
Contributor

@essobedo essobedo commented Aug 31, 2022

fixes #3586

Motivation

A Kamelet binding in the wrong format can lead to operator panic which needs to be avoided.

Modifications:

  • Adds a function to verify the Kamelet binding to return an error in case of an invalid format.

Release Note

fix: Prevent operator panic on wrong Kamelet binding

@essobedo essobedo force-pushed the 3586/avoid-panic-on-wrong-kamelet-binding branch from 1279535 to 87e1f90 Compare August 31, 2022 15:42
@essobedo essobedo changed the title fix: Prevent panic on wrong Kamelet binding fix: Prevent operator panic on wrong Kamelet binding Aug 31, 2022
@essobedo essobedo force-pushed the 3586/avoid-panic-on-wrong-kamelet-binding branch from 87e1f90 to cd1b08d Compare August 31, 2022 15:50
@essobedo
Copy link
Contributor Author

essobedo commented Sep 1, 2022

The builds are failing due to:

  • TestNativeIntegrations/automatic_rollout_deployment_from_fast-jar_to_native_kit
  • TestKitKnativeFullBuild

Both are not related to these changes

@essobedo essobedo force-pushed the 3586/avoid-panic-on-wrong-kamelet-binding branch from cd1b08d to dbed12a Compare September 2, 2022 07:50
@squakez squakez added this to the 1.11.0 milestone Sep 2, 2022
@squakez squakez added the kind/bug Something isn't working label Sep 2, 2022
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work. This PR solves certain specific cases (ie, a sink with a not nil endpoint), but still fails in case of a valid yaml specification that does not fit the expected ErrorHandler specification. A definitive fix should be done in the parseErrorHandler func, when we unmarshal the configuration. Not sure if there is an easy and elegant way to validate against the expected schema. In any case, I'd keep this work on hold, as we may need to apply some refactoring due to apache/camel-k-runtime#868

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Pasquale said, let's leave it open for now.

@squakez squakez added the status/wip Work in progress label Sep 7, 2022
@essobedo essobedo marked this pull request as draft September 7, 2022 08:38
@essobedo
Copy link
Contributor Author

essobedo commented Sep 7, 2022

Ok no problem once apache/camel-k-runtime#868 will be fixed, I will try to find a more elegant approach

@essobedo essobedo closed this Sep 25, 2022
@essobedo essobedo deleted the 3586/avoid-panic-on-wrong-kamelet-binding branch September 25, 2022 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working status/wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong KameletBinding definition leads to operator panic
4 participants