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: Validate maven artifacts #774

Merged
merged 1 commit into from
Jun 26, 2019

Conversation

jamesnetherton
Copy link
Contributor

fixes #742

@lburgazzoli we talked a little about this. I made the Maven repository checksumPolicy configurable when doing kamel install. By default the checksumPolicy is 'fail'.

E.g:

kamel install --maven-repository http://foobar.com/repo/@checksumpolicy=warn

I also tweaked things slightly so that there is always repository config set up for Maven central in order to enforce the checksumPolicy for it. Users can override this if they want to.


func containsMvnCentral(repositories []maven.Repository) bool {
for _, r := range repositories {
if r.ID == "central" {
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if we should check by ip ? as the id can be set by the user, then it can setanother non maven central repo as central

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I did ponder whether to make the check more complicated. Trouble is, there's a bunch of different host name combinations that you could potentially use for central, so checking against them isn't very reliable. Even trying to compare IPs is maybe not 100% reliable either because of load balancers.

So I figured if someone explicitly configures central as the repo ID, then assume it's the defacto config for Maven central.

@lburgazzoli
Copy link
Contributor

lburgazzoli commented Jun 26, 2019

Thinking a little bit more about this, I think we should move the default maven set-up in the platform controller as kamel install is not the only possible way to set-up camel-k thus in case one set it up manually, we'll lose the default repository set-up.

Not sure how simple it is so I will merge this one but would be nice if we could log an additional issue and think a little bit about it.

@jamesnetherton, would you lake to take it ?

@jamesnetherton
Copy link
Contributor Author

Yes, no problem. I'm happy to take a look at any follow up issues 👍

@lburgazzoli lburgazzoli merged commit 1de2681 into apache:master Jun 26, 2019
@jamesnetherton jamesnetherton deleted the 742-mvn-validate branch June 26, 2019 12:09
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.

validate maven artifacts
2 participants