From 2edff7575c3ae98d040354804665a4f1322ec40b Mon Sep 17 00:00:00 2001 From: Alex Kubacki Date: Thu, 30 Apr 2026 17:01:41 -0600 Subject: [PATCH 1/3] Fix and simplify ALB/ELB protocol option logic --- .../cli/helpers/vhost/option_set_builder.rb | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/aptible/cli/helpers/vhost/option_set_builder.rb b/lib/aptible/cli/helpers/vhost/option_set_builder.rb index e7ed1542..040bdb55 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(', ')}" 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 From e2c0232c471aeffaf126899434ed5eb45b9da296 Mon Sep 17 00:00:00 2001 From: Alex Kubacki Date: Fri, 1 May 2026 09:10:19 -0600 Subject: [PATCH 2/3] Use the declared ALB variable --- lib/aptible/cli/helpers/vhost/option_set_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/aptible/cli/helpers/vhost/option_set_builder.rb b/lib/aptible/cli/helpers/vhost/option_set_builder.rb index 040bdb55..6f08f59f 100644 --- a/lib/aptible/cli/helpers/vhost/option_set_builder.rb +++ b/lib/aptible/cli/helpers/vhost/option_set_builder.rb @@ -311,7 +311,7 @@ def prepare(account, options) 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 !ELB_PROTOCOL_VALUES.include?(proto) && !alb? From afb98d37bb419e1a9d90f1b039afd1f6defffced Mon Sep 17 00:00:00 2001 From: Alex Kubacki Date: Fri, 1 May 2026 09:10:46 -0600 Subject: [PATCH 3/3] Add option description tests --- .../helpers/vhost/option_set_builder_spec.rb | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 spec/aptible/cli/helpers/vhost/option_set_builder_spec.rb 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