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

Use HTTPS for yumrepo when applicable #330

Closed
wants to merge 1 commit into from

Conversation

aknarts
Copy link
Contributor

@aknarts aknarts commented Jul 19, 2016

No description provided.

@aknarts
Copy link
Contributor Author

aknarts commented Jul 19, 2016

Would be even easier change if done in #326

@olivielpeau olivielpeau modified the milestones: 2.5.0, Next Major Aug 3, 2016
@olivielpeau
Copy link
Member

olivielpeau commented Aug 4, 2016

Thanks for the PR @aknarts!

This PR changes the value assigned to an attribute by the cookbook, and even though the change is minor I think it still counts as something that should be released as part of a major version (see the Cookbook versioning policy)

I'm going to slate this for the next major version, let me know what you think.

As an aside, even without this change I don't think there should be any security concern around downloading the packages through plain http since the packages are signed and the public key is pulled through https (when the platform allows it).

@aknarts
Copy link
Contributor Author

aknarts commented Aug 29, 2016

Hi, there should be no reason to not use HTTPS if at all possible. And it is now even easier since this got merged https://github.com/DataDog/chef-datadog/pull/326/files#diff-25e5d4a4446ae12a0d6f1162b6160375R66
Line 76 would just need to change the same way as the line 77 did. i.e.
default['datadog']['yumrepo'] = ""#{yum_protocol}://yum.datadoghq.com/rpm/#{architecture_map[node['kernel']['machine']]}/"

As a result this merge is redundant and I am closing it(will create a new one with the proposed change.

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.

3 participants