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(#3956): Add maven settings-security.xml to maven command #4099

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

gansheer
Copy link
Contributor

@gansheer gansheer commented Mar 3, 2023

Resolves: #3956

Description

In case the settings-security.xml file is declared in the integration platform as described in the documentation it needs to be explicitly specified as part of the maven commands, else maven default to ${user.home}/.m2/settings-security.xml instead of /tmp/kit-xxxx/maven/settings-security.xml.

Release Note

fix: Add maven settings-security.xml to maven command

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.

Nice one. Do you think you can work on some test to cover such scenario?

@squakez squakez added the kind/bug Something isn't working label Mar 3, 2023
@gansheer
Copy link
Contributor Author

gansheer commented Mar 3, 2023

Nice one. Do you think you can work on some test to cover such scenario?

I don't think I can make an e2e test, I would need a private repository for it. Except if one already exists in the existing tests I don't see a simple way to do it. I made my local tests deploying a nexus server and it was not that simple.

As for a Unit test I am trying to write one on the Do() method but it's kind of a mess.

@squakez
Copy link
Contributor

squakez commented Mar 3, 2023

@gansheer no worry then, I'm merging, thanks!

@squakez squakez merged commit 18161e9 into apache:main Mar 3, 2023
@squakez
Copy link
Contributor

squakez commented Mar 3, 2023

Btw, please backport to both 1.10 and 1.12 if possible.

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.

Camel-K maven security
3 participants