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

Disable default source when using user-supplied gem source #265

Merged
merged 2 commits into from
Aug 8, 2017

Conversation

adamleff
Copy link
Contributor

@adamleff adamleff commented Aug 8, 2017

The clear_sources property does not remove the default gem source when the RubyGems package provider does its work. We need to also set the include_default_source property to false to ensure users' requesting gem source is the only gem source used (instead of rubygems.org, for example).

The `clear_sources` property does not remove the default gem source when the
RubyGems package provider does its work. We need to also set the
`include_default_source` property to false to ensure users' requesting gem
source is the only gem source used (instead of rubygems.org, for example).

Signed-off-by: Adam Leff <adam@leff.co>
Copy link
Contributor

@alexpop alexpop left a comment

Choose a reason for hiding this comment

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

👍

@jeremymv2
Copy link
Contributor

jeremymv2 commented Aug 8, 2017

🤔 the include_default_source is only a recent addition to chef-client:
chef/chef@28be4df

Likely would have to be handled differently to continue backwards client compatibility.

@adamleff
Copy link
Contributor Author

adamleff commented Aug 8, 2017

@jeremymv2 true, so this would be a question for @chris-rock how far back our backward-compat policy is for the audit cookbook and Chef version.

I'll add some guards around it in the meantime.

@adamleff
Copy link
Contributor Author

adamleff commented Aug 8, 2017

For posterity, this property has been in Chef stable since 13.0

@jeremymv2
Copy link
Contributor

In the past we've decided on minimum version 12.5.1

https://github.com/chef-cookbooks/audit/blob/master/metadata.rb#L13

I think the guards are the way to go.

This will protect against backward compat issues, allowing the old
`clear_sources` behavior dating back to Chef 12.x to continue to work.

Signed-off-by: Adam Leff <adam@leff.co>
@adamleff
Copy link
Contributor Author

adamleff commented Aug 8, 2017

And it is now appropriate guarded, @jeremymv2 :)

Copy link
Contributor

@arlimus arlimus left a comment

Choose a reason for hiding this comment

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

Looks great thank you Adam!

@adamleff
Copy link
Contributor Author

adamleff commented Aug 8, 2017

The Travis tests are due to a breakage between berkshelf/solve and molinillo which cases cookbooks in the dependency chain to go missing. I've tested everything locally using an older ChefDK and it tests well, so I'm going to merge this.

@adamleff adamleff merged commit 3b2c340 into master Aug 8, 2017
@adamleff adamleff deleted the adamleff/gem-source-fix branch August 8, 2017 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants