Apply suggestions from code review

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>

CHANGELOG.md

@@ -1,15 +1,10 @@
 ## 7.3.0
-  - Standardized SSL settings: [#42](https://github.com/logstash-plugins/logstash-mixin-http_client/pull/42)  
-    - Deprecated `cacert` in favor of `ssl_certificate_authorities`
-    - Deprecated`client_cert` in favor of `ssl_certificate`
-    - Deprecated `client_key` in favor of `ssl_key`
-    - Deprecated `keystore` in favor of `ssl_keystore_path`
-    - Deprecated `keystore_password` in favor of `ssl_keystore_password`
-    - Deprecated `keystore_type` in favor of `ssl_keystore_type`
-    - Deprecated `truststore` in favor of `ssl_truststore_path`
-    - Deprecated `truststore_password` in favor of `ssl_truststore_password`
-    - Deprecated `truststore_type` in favor of `ssl_truststore_type`
-  - Added a module configuration to disable the deprecated SSL configs `:with_deprecated`
+  - Adds standardized SSL settings and deprecates their non-standard counterparts. Deprecated settings will continue to work, and will provide pipeline maintainers with guidance toward using their standardized counterparts [#42](https://github.com/logstash-plugins/logstash-mixin-http_client/pull/42)
+    - Adds new `ssl_truststore_path`, `ssl_truststore_password`, and `ssl_truststore_type` settings for configuring SSL-trust using a PKCS-12 or JKS trust store, deprecating their `truststore`, `truststore_password`, and `truststore_type` counterparts.
+    - Adds new `ssl_certificate_authorities` setting for configuring SSL-trust using a PEM-formated list certificate authorities, deprecating its `cacert` counterpart.
+   - Adds new `ssl_keystore_path`, `ssl_keystore_password`, and `ssl_keystore_type` settings for configuring SSL-identity using a PKCS-12 or JKS key store, deprecating their `keystore`, `keystore_password`, and `keystore_type` counterparts.
+    - Adds new `ssl_certificate` and `ssl_key` settings for configuring SSL-identity using a PEM-formatted certificate/key pair, deprecating their `client_cert` and `client_key` counterparts. 
+  - Added a way for plugin maintainers to include this mixin _without_ supporting the now-deprecated SSL options.
   - Added the `ssl_cipher_suites` option
 
 ## 7.2.0

lib/logstash/plugin_mixins/http_client.rb

@@ -199,15 +199,15 @@ def ssl_options
         options[:truststore_type] = @ssl_truststore_type if @ssl_truststore_type
         options[:truststore_password] = @ssl_truststore_password.value if @ssl_truststore_password
       elsif @ssl_truststore_password
-        fail LogStash::ConfigurationError, "truststore_password requires ssl_truststore_path you fool"
+        fail LogStash::ConfigurationError, "An `ssl_truststore_password` cannot be specified unless `ssl_truststore_path` is also provided."
       end
 
       if @ssl_keystore_path
         options[:keystore] = @ssl_keystore_path
         options[:keystore_type] = @ssl_keystore_type if @ssl_keystore_type
         options[:keystore_password] = @ssl_keystore_password.value if @ssl_keystore_password
       elsif @ssl_keystore_password
-        fail LogStash::ConfigurationError, "ssl_keystore_password requires ssl_keystore_path you fool"
+        fail LogStash::ConfigurationError, "An `ssl_keystore_password` cannot be specified unless `ssl_keystore_path` is also provided."
       end
 
       if @ssl_certificate && @ssl_key

spec/plugin_mixin/http_client_ssl_spec.rb

@@ -4,22 +4,22 @@
 require 'stud/temporary'
 
 shared_examples 'setting ca bundles' do |key, type|
-  subject { plugin_class.new(conf).send(:client_config) }
+  subject(:client_config) { plugin_class.new(conf).send(:client_config) }
 
   it 'should correctly set the path' do
-    expect(subject[:ssl][key]).to eql(path), "Expected to find path for #{key}"
+    expect(client_config[:ssl][key]).to eql(path), "Expected to find path for #{key}"
   end
 
   if type == :jks
     let(:store_password) { conf["#{use_deprecated_config ? '' : 'ssl_'}#{key}_password"] }
     let(:store_type) { conf["#{use_deprecated_config ? '' : 'ssl_'}#{key}_type"]}
 
     it 'should set the bundle password' do
-      expect(subject[:ssl]["#{key}_password".to_sym]).to eql(store_password)
+      expect(client_config[:ssl]["#{key}_password".to_sym]).to eql(store_password)
     end
 
     it 'should set the bundle type' do
-      expect(subject[:ssl]["#{key}_type".to_sym]).to eql(store_type)
+      expect(client_config[:ssl]["#{key}_type".to_sym]).to eql(store_type)
     end
   end
 end
@@ -409,7 +409,7 @@ class PluginWithDeprecatedFalse < LogStash::Inputs::Base
       }
     end
 
-    subject { plugin_class.new(settings) }
+    subject(:plugin_instance) { plugin_class.new(settings) }
 
     after do
       File.unlink(cacert)
@@ -419,26 +419,26 @@ class PluginWithDeprecatedFalse < LogStash::Inputs::Base
       File.unlink(truststore)
     end
 
-    it 'should normalize deprecated settings' do
-      expect(subject.ssl_certificate_authorities).to eq([cacert])
-      expect(subject.ssl_certificate).to eq(client_cert)
-      expect(subject.ssl_key).to eq(client_key)
-      expect(subject.ssl_keystore_path).to eq(keystore)
-      expect(subject.ssl_keystore_password.value).to eq(keystore_password)
-      expect(subject.ssl_keystore_type).to eq(keystore_type)
-      expect(subject.ssl_truststore_path).to eq(truststore)
-      expect(subject.ssl_truststore_password.value).to eq(truststore_password)
-      expect(subject.ssl_truststore_type).to eq(truststore_type)
-
-      expect(subject.params['ssl_certificate_authorities']).to eq([cacert])
-      expect(subject.params['ssl_certificate']).to eq(client_cert)
-      expect(subject.params['ssl_key']).to eq(client_key)
-      expect(subject.params['ssl_keystore_path']).to eq(keystore)
-      expect(subject.params['ssl_keystore_password'].value).to eq(keystore_password)
-      expect(subject.params['ssl_keystore_type']).to eq(keystore_type)
-      expect(subject.params['ssl_truststore_path']).to eq(truststore)
-      expect(subject.params['ssl_truststore_password'].value).to eq(truststore_password)
-      expect(subject.params['ssl_truststore_type']).to eq(truststore_type)
+    it 'normalizes deprecated settings' do
+      expect(plugin_instance.ssl_certificate_authorities).to eq([cacert])
+      expect(plugin_instance.ssl_certificate).to eq(client_cert)
+      expect(plugin_instance.ssl_key).to eq(client_key)
+      expect(plugin_instance.ssl_keystore_path).to eq(keystore)
+      expect(plugin_instance.ssl_keystore_password.value).to eq(keystore_password)
+      expect(plugin_instance.ssl_keystore_type).to eq(keystore_type)
+      expect(plugin_instance.ssl_truststore_path).to eq(truststore)
+      expect(plugin_instance.ssl_truststore_password.value).to eq(truststore_password)
+      expect(plugin_instance.ssl_truststore_type).to eq(truststore_type)
+
+      expect(plugin_instance.params['ssl_certificate_authorities']).to eq([cacert])
+      expect(plugin_instance.params['ssl_certificate']).to eq(client_cert)
+      expect(plugin_instance.params['ssl_key']).to eq(client_key)
+      expect(plugin_instance.params['ssl_keystore_path']).to eq(keystore)
+      expect(plugin_instance.params['ssl_keystore_password'].value).to eq(keystore_password)
+      expect(plugin_instance.params['ssl_keystore_type']).to eq(keystore_type)
+      expect(plugin_instance.params['ssl_truststore_path']).to eq(truststore)
+      expect(plugin_instance.params['ssl_truststore_password'].value).to eq(truststore_password)
+      expect(plugin_instance.params['ssl_truststore_type']).to eq(truststore_type)
     end
   end