Merge pull request #2083 from 3scale/truncate-long-url-labels

Uri shortener for long host label endpoints

app/models/proxy.rb

@@ -212,11 +212,13 @@ def config
     def generate(name)
       template = config.fetch(name.try(:to_sym)) { return }
 
-      format template, {
+      uri = format template, {
         system_name: service.parameterized_system_name, account_id: service.account_id,
         tenant_name: provider_subdomain,
         env: proxy.proxy_env, port: proxy.proxy_port
       }
+
+      UriShortener.call(uri).to_s
     end
   end
 

app/services/service_creator.rb

@@ -22,10 +22,6 @@ def call!(params = {})
   def call(params = {})
     call!(params)
   rescue ActiveRecord::RecordInvalid
-    return if !proxy || proxy.errors[:endpoint].none? { |message| message =~ /is too long/ }
-
-    @service.errors.add(:system_name, :invalid_for_proxy_endpoints)
-
     false
   end
 

app/services/uri_shortener.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+class UriShortener
+  def self.call(uri)
+    new(uri).call
+  end
+
+  def initialize(uri)
+    @uri = URI.parse(uri)
+  rescue URI::InvalidURIError
+  end
+
+  attr_reader :uri
+
+  def call
+    return unless uri
+    labels = uri.host.split('.')
+    uri.host = labels.map(&UriLabelShortner.method(:call)).join('.')
+    uri
+  end
+
+  class UriLabelShortner
+    def self.call(label)
+      new(label).call
+    end
+
+    def initialize(label)
+      @label = label
+    end
+
+    attr_reader :label
+    delegate :size, to: :label
+
+    MAX_LABEL_SIZE = UriValidator::UriThreeScaleComplianceChecker::MAX_LABEL_SIZE
+    HASH_SIZE = 7
+    KEEP_SIZE = (MAX_LABEL_SIZE - HASH_SIZE - 1).freeze
+
+    def call
+      return label if size <= MAX_LABEL_SIZE
+      [*label.slice(0, KEEP_SIZE).split('-'), hash].join('-')
+    end
+
+    def hash
+      Digest::SHA1.hexdigest(label).slice(0, HASH_SIZE)
+    end
+  end
+end

config/locales/en.yml

@@ -891,7 +891,6 @@ en:
           attributes:
             system_name:
               invalid: invalid. Only ASCII letters, numbers, dashes and underscores are allowed.
-              invalid_for_proxy_endpoints: must be shorter.
 
         cinstance:
           attributes:

test/integration/api/services_controller_test.rb

@@ -236,16 +236,5 @@ class ServiceCreateTest < Api::ServicesControllerTest
       post admin_services_path, service: { name: 'example-service', system_name: '###' }
       assert_equal 'System name invalid. Only ASCII letters, numbers, dashes and underscores are allowed.', flash[:error]
     end
-
-    test 'chosen system name affects proxy endpoint validation' do
-      @provider.settings.allow_multiple_services!
-
-      post admin_services_path, service: { name: 'My New Product', system_name: 'this-hostname-label-is-longer-than-63-chars-which-is-not-allowed-according-to-rfc-1035' }
-      assert_equal 'System name must be shorter.', flash[:error]
-
-      post admin_services_path, service: { name: 'My New Product', system_name: 'short-labels-are-ok' }
-      refute flash[:error].presence
-      assert_response :redirect
-    end
   end
 end

test/unit/proxy_test.rb

@@ -168,6 +168,19 @@ def test_endpoints_on_create
       assert proxy.production_endpoint
     end
 
+    test 'generates shortened URI label endpoints' do
+      service = FactoryBot.create(:simple_service, deployment_option: 'hosted', system_name: 'long-system-name-that-will-cause-invalid-host', proxy: nil)
+
+      proxy_config = System::Application.config.three_scale.sandbox_proxy
+      proxy_config.expects(:fetch).with(:port).at_least_once.returns('8080')
+      proxy_config.expects(:fetch).with(:apicast_staging_endpoint).at_least_once.returns('http://%{system_name}-needs-to-be-63-chars-or-shorter.staging.apicast.dev:8080')
+      proxy_config.expects(:fetch).with(:apicast_production_endpoint).at_least_once.returns('http://%{system_name}-needs-to-be-63-chars-or-shorter.production.apicast.dev:8080')
+
+      proxy = FactoryBot.create(:proxy, service: service, apicast_configuration_driven: true)
+      assert_equal 'http://long-system-name-that-will-cause-invalid-host-needs-to-4207987.staging.apicast.dev:8080', proxy.sandbox_endpoint
+      assert_equal 'http://long-system-name-that-will-cause-invalid-host-needs-to-4207987.production.apicast.dev:8080', proxy.endpoint
+    end
+
     def test_deployable
       assert_predicate Service.new(deployment_option: 'hosted').build_proxy, :deployable?
       assert_predicate Service.new(deployment_option: 'self_managed').build_proxy, :deployable?

test/unit/services/service_creator_test.rb

@@ -46,11 +46,10 @@ class ServiceCreatorTest < ActiveSupport::TestCase
     assert backend_api_config.persisted?
   end
 
-  test 'service with a too long system_name for its proxy' do
+  test 'service with a long system_name used in its proxy endpoint' do
     system_name = 'this-hostname-label-is-longer-than-63-chars-which-is-not-allowed-according-to-rfc-1035'
     service = FactoryBot.build(:service, system_name: system_name)
-    creator = ServiceCreator.new(service: service)
-    creator.call
-    assert_includes service.errors[:system_name], 'must be shorter.'
+    ServiceCreator.new(service: service).call
+    assert_match /this-hostname-label-is-longer-than-63-chars-which-is-no-/, service.proxy.endpoint
   end
 end

test/unit/services/uri_shortener_test.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class UriShortenerTest < ActiveSupport::TestCase
+  test 'uri with long label' do
+    shortener = UriShortener.new('http://this-hostname-label-is-longer-than-63-chars-which-is-not-allowed-according-to-rfc-1035.example.com')
+    assert_equal 'http://this-hostname-label-is-longer-than-63-chars-which-is-no-a6ca0fb.example.com', shortener.call.to_s
+  end
+
+  test 'uri with multiple long labels' do
+    shortener = UriShortener.new('http://this-hostname-label-is-longer-than-63-chars-which-is-not-allowed-according-to-rfc-1035.this-other-label-is-as-well-longer-than-63-chars-also-violating-the-rfc-1035.example.com')
+    assert_equal 'http://this-hostname-label-is-longer-than-63-chars-which-is-no-a6ca0fb.this-other-label-is-as-well-longer-than-63-chars-also-v-57fa2c4.example.com', shortener.call.to_s
+  end
+
+  test 'does not duplicate leading dash' do
+    shortener = UriShortener.new('http://long-system-name-that-will-cause-invalid-host-needs-to-be-63-chars-or-shorter.example.com')
+    assert_not_equal 'http://long-system-name-that-will-cause-invalid-host-needs-to--4207987.example.com', shortener.call.to_s
+    assert_equal     'http://long-system-name-that-will-cause-invalid-host-needs-to-4207987.example.com', shortener.call.to_s
+  end
+
+  test 'uri with no long labels' do
+    shortener = UriShortener.new('http://this-label-is-ok.example.com')
+    assert_equal 'http://this-label-is-ok.example.com', shortener.call.to_s
+  end
+
+  test 'invalid uri' do
+    shortener = UriShortener.new('not a uri')
+    refute shortener.call
+  end
+end