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

Replace ssl with tls #4

Merged
merged 3 commits into from
May 20, 2020
Merged

Replace ssl with tls #4

merged 3 commits into from
May 20, 2020

Conversation

r2ronoha
Copy link

Pull Request (PR) description

Replace the use of deprecated ssl connection options to tls

This Pull Request (PR) fixes the following issues

  • Use of hostname as bind_ip
Error: Evaluation Error: Error while evaluating a Resource Statement, Class[Mongodb::Server]: parameter 'bind_ip' index 0 expects a Stdlib::Compat::Ip_address = Variant[Stdlib::Compat::Ipv4 = Pattern[/^((([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))[.]){3}([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d)))(\/((([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))[.]){3}([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))|[0-9]+))?$/], Stdlib::Compat::Ipv6 = Pattern[/\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*$/]] value, got String (file: /etc/puppetlabs/code/modules/profile/manifests/mongodb.pp, line: 155, column: 3) on node mdb-501-qa.eu-west-1a.ie.wandera.co.uk
  • Use of tls options instead of ssl
Error: Could not prefetch mongodb_replset provider 'mongo': 765: unexpected token at '-05-07T05:03:56.091+0000 W  CONTROL  [main] Option: ssl is deprecated. Please use tls instead.
2020-05-07T05:03:56.091+0000 W  CONTROL  [main] Option: sslPEMKeyFile is deprecated. Please use tlsCertificateKeyFile instead.
2020-05-07T05:03:56.091+0000 W  CONTROL  [main] Option: sslCAFile is deprecated. Please use tlsCAFile instead.

@r2ronoha r2ronoha requested a review from gerricchaplin May 19, 2020 07:27
@@ -535,8 +535,8 @@ Set to true to disable fqdn SSL cert check
Default: False

##### `ssl_mode`

Choose a reason for hiding this comment

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

Shouldn't we support both to keep backwards compatibility otherwise this is then a breaking change?
Do you think they will accept this upstream?
Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

I thought of and started doing it, but it gets to messy with conditions.
So I thought for lower versions we'd use previous releases and use latest for new versions

Choose a reason for hiding this comment

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

Ok, that means we diverge from upstream and can't submit to upstream either.
If you are fine with that then that's fine by me - hopefully, short term and upstream add this support at some point or we rework this later when we have some time.

Copy link
Author

Choose a reason for hiding this comment

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

I'll raise a ticket to do the work separately

@r2ronoha r2ronoha merged commit af956c0 into master May 20, 2020
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.

2 participants