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(trait): force a volume path when key is set #3544

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Conversation

essobedo
Copy link
Contributor

@essobedo essobedo commented Aug 12, 2022

fixes #3543

Motivation

When we try to run an integration that needs to mount a volume with filtering but without a path, it fails with `[spec.template.spec.volumes[2].secret.items[0].path: Required``

Modifications:

  • Use the key when no mount path is proposed

Release Note

NONE

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.

LGTM. Only one request is to cover the case with an E2E test.
Could you please add test runs with key filtering scenarios after the following ones for both configmap and secret?
https://github.com/apache/camel-k/blob/main/e2e/global/common/config/config_test.go#L84
https://github.com/apache/camel-k/blob/main/e2e/global/common/config/config_test.go#L143

@tadayosi tadayosi added the kind/bug Something isn't working label Aug 22, 2022
@tadayosi tadayosi added this to the 1.10.0 milestone Aug 22, 2022
@essobedo
Copy link
Contributor Author

LGTM. Only one request is to cover the case with an E2E test. Could you please add test runs with key filtering scenarios after the following ones for both configmap and secret? https://github.com/apache/camel-k/blob/main/e2e/global/common/config/config_test.go#L84 https://github.com/apache/camel-k/blob/main/e2e/global/common/config/config_test.go#L143

Yes, of course, thx for the feedback, however, please note that I won't be able to do that before next week.

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.

Yes, of course, thx for the feedback, however, please note that I won't be able to do that before next week.

OK, then let's merge this one as it's good that this bug is fixed in 1.10.0. I'll file another issue for you for the follow-up task. Thanks!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot deploy an integration using configmap/secret key filtering
3 participants