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

Make yum repo GPG key an attribute #326

Merged
merged 3 commits into from Aug 3, 2016

Conversation

Projects
None yet
3 participants
@iancward
Copy link
Contributor

iancward commented Jul 13, 2016

This fixes GH-311.

@olivielpeau

This comment has been minimized.

Copy link
Member

olivielpeau commented Jul 13, 2016

Thanks @iancward, this looks good! We'll include it in the next minor release.

@olivielpeau olivielpeau added this to the 2.5.0 milestone Jul 13, 2016

@olivielpeau olivielpeau added ready feature and removed ready labels Jul 13, 2016

@@ -62,10 +62,14 @@
}
architecture_map.default = 'x86_64'

# Older versions of yum embed M2Crypto with SSL that doesn't support TLS1.2
prefix = node['platform_version'].to_i < 6 ? 'http' : 'https'

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Jul 15, 2016

Member

After giving it some thought, I think this deserves a more accurate condition since it will run on all platforms. Could you change this to:

yum_protocol = 'https'
if node['platform_family'] == 'rhel' && node['platform_version'].to_i < 6
  # Older versions of yum embed M2Crypto with SSL that doesn't support TLS1.2
  yum_protocol = 'http'
end

(I've also renamed prefix yum_protocol since that name makes more sense here IMO)

This comment has been minimized.

Copy link
@iancward

iancward Jul 15, 2016

Author Contributor

I merely migrated the code from the repository recipe, but I just noticed that it's inside of a case node['platform_family'] block.

This is fixed in 6eb91d7. I did it a bit differently to try to be more DRY.

@olivielpeau olivielpeau removed the ready label Jul 15, 2016

@olivielpeau

This comment has been minimized.

Copy link
Member

olivielpeau commented Jul 15, 2016

@iancward Just added a comment actually, let me know what you think, thanks!

@olivielpeau

This comment has been minimized.

Copy link
Member

olivielpeau commented Jul 18, 2016

Thanks @iancward, looks better now, I think this is ready for the next minor release

@irabinovitch

This comment has been minimized.

Copy link
Contributor

irabinovitch commented Aug 3, 2016

@iancward Thanks for your efforts on this PR. We're currently working through a QA/testing cycle on the next release, which we expect to include this PR.

@olivielpeau olivielpeau merged commit 07a524c into DataDog:master Aug 3, 2016

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.