diff --git a/lib/aptible/cli/helpers/vhost/option_set_builder.rb b/lib/aptible/cli/helpers/vhost/option_set_builder.rb index e7ed1542..6f08f59f 100644 --- a/lib/aptible/cli/helpers/vhost/option_set_builder.rb +++ b/lib/aptible/cli/helpers/vhost/option_set_builder.rb @@ -25,22 +25,21 @@ class OptionSetBuilder 'TLSv1.3' ].freeze - SSL_PROTOCOL_PFS_VALUES = - SSL_PROTOCOL_VALUES.select { |v| v.include?(' PFS') }.freeze + ALB_PROTOCOL_VALUES = SSL_PROTOCOL_VALUES - SSL_PROTOCOL_NON_PFS_VALUES = + ELB_PROTOCOL_VALUES = SSL_PROTOCOL_VALUES.reject { |v| v.include?(' PFS') }.freeze SSL_PROTOCOL_ALB_DESC = ( 'Specify the allowed SSL protocols. Valid options: ' + - SSL_PROTOCOL_VALUES.map { |v| "\"#{v}\"" }.join(', ') + + ALB_PROTOCOL_VALUES.map { |v| "\"#{v}\"" }.join(', ') + '. PFS options require an HTTPS (ALB) endpoint. ' \ 'Use "default" to reset to the platform default' ).freeze - SSL_PROTOCOL_NON_ALB_DESC = ( + SSL_PROTOCOL_ELB_DESC = ( 'Specify the allowed SSL protocols. Valid options: ' + - SSL_PROTOCOL_NON_PFS_VALUES.map { |v| "\"#{v}\"" }.join(', ') + + ELB_PROTOCOL_VALUES.map { |v| "\"#{v}\"" }.join(', ') + '. Use "default" to reset to the platform default' ).freeze @@ -211,13 +210,13 @@ def declare_options(thor) 'on this Endpoint' ) - option( - :ssl_protocols_override, - type: :string, - desc: SSL_PROTOCOL_NON_ALB_DESC - ) - unless builder.alb? + option( + :ssl_protocols_override, + type: :string, + desc: SSL_PROTOCOL_ELB_DESC + ) + option( :ssl_ciphers_override, type: :string, @@ -309,16 +308,17 @@ def prepare(account, options) if (proto = options[:ssl_protocols_override]) && proto != 'default' - unless SSL_PROTOCOL_VALUES.include?(proto) + unless ALB_PROTOCOL_VALUES.include?(proto) raise Thor::Error, "Invalid --ssl-protocols-override: \"#{proto}\". " \ - "Valid options are: #{SSL_PROTOCOL_VALUES.join(', ')}" + "Valid options are: #{ALB_PROTOCOL_VALUES.join(', ')}" end - if SSL_PROTOCOL_PFS_VALUES.include?(proto) && !alb? + if !ELB_PROTOCOL_VALUES.include?(proto) && !alb? raise Thor::Error, - '--ssl-protocols-override: PFS options are only ' \ - 'available on HTTPS (ALB) endpoints' + "Invalid --ssl-protocols-override: \"#{proto}\". " \ + 'PFS options are only valid for an HTTPS (ALB) endpoint. ' \ + "Valid options are: #{ELB_PROTOCOL_VALUES.join(', ')}" end end diff --git a/spec/aptible/cli/helpers/vhost/option_set_builder_spec.rb b/spec/aptible/cli/helpers/vhost/option_set_builder_spec.rb new file mode 100644 index 00000000..44179de7 --- /dev/null +++ b/spec/aptible/cli/helpers/vhost/option_set_builder_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +describe Aptible::CLI::Helpers::Vhost::OptionSetBuilder do + def register_options(builder) + klass = Class.new(Thor) { include Aptible::CLI::Helpers::App } + builder.declare_options(klass) + klass.instance_variable_get(:@method_options) + end + + describe '--ssl-protocols-override option description' do + context 'HTTPS endpoints (ALB, alb! flag set)' do + let(:builder) do + described_class.new do + app! + tls! + alb! + end + end + + it 'includes PFS values' do + desc = register_options(builder)[:ssl_protocols_override].description + expect(desc).to include('PFS') + end + end + + context 'TLS endpoints (ELB, tls! without alb!)' do + let(:builder) do + described_class.new do + app! + tls! + end + end + + it 'does not include PFS values' do + desc = register_options(builder)[:ssl_protocols_override].description + expect(desc).not_to include('PFS') + end + + it 'is still present' do + expect(register_options(builder)).to have_key(:ssl_protocols_override) + end + end + + context 'gRPC endpoints (ELB, tls! without alb!)' do + let(:builder) do + described_class.new do + app! + port! + tls! + end + end + + it 'does not include PFS values' do + desc = register_options(builder)[:ssl_protocols_override].description + expect(desc).not_to include('PFS') + end + end + + context 'TCP endpoints (no tls! flag)' do + let(:builder) do + described_class.new do + app! + ports! + end + end + + it 'is absent' do + expect(register_options(builder)).not_to have_key(:ssl_protocols_override) + end + end + end + + describe 'SSL_PROTOCOL_ALB_DESC' do + subject { described_class::SSL_PROTOCOL_ALB_DESC } + + it 'lists all PFS protocol values' do + pfs_values = described_class::SSL_PROTOCOL_VALUES.select { |v| v.include?('PFS') } + pfs_values.each { |v| is_expected.to include(v) } + end + end + + describe 'SSL_PROTOCOL_ELB_DESC' do + subject { described_class::SSL_PROTOCOL_ELB_DESC } + + it 'contains no PFS values' do + is_expected.not_to include('PFS') + end + + it 'lists all non-PFS protocol values' do + non_pfs_values = described_class::SSL_PROTOCOL_VALUES.reject { |v| v.include?('PFS') } + non_pfs_values.each { |v| is_expected.to include(v) } + end + end +end