Merge branch 'mattkasa/gitlab-195964-functionuri-refactor' into 'master'

Refactor Gitlab::Serverless::FunctionURI class

See merge request gitlab-org/gitlab!23922

app/finders/serverless_domain_finder.rb

+# frozen_string_literal: true
+
+class ServerlessDomainFinder
+  attr_reader :match, :serverless_domain_cluster, :environment
+
+  def initialize(uri)
+    @match = ::Serverless::Domain::REGEXP.match(uri)
+  end
+
+  def execute
+    return unless serverless?
+
+    @serverless_domain_cluster = ::Serverless::DomainCluster.for_uuid(serverless_domain_cluster_uuid)
+    return unless serverless_domain_cluster
+
+    @environment = ::Environment.for_id_and_slug(match[:environment_id].to_i(16), match[:environment_slug])
+    return unless environment
+
+    ::Serverless::Domain.new(
+      function_name: match[:function_name],
+      serverless_domain_cluster: serverless_domain_cluster,
+      environment: environment
+    )
+  end
+
+  def serverless_domain_cluster_uuid
+    return unless serverless?
+
+    match[:cluster_left] + match[:cluster_middle] + match[:cluster_right]
+  end
+
+  def serverless?
+    !!match
+  end
+end

app/models/environment.rb

@@ -95,6 +95,10 @@ class Environment < ApplicationRecord
     end
   end
 
+  def self.for_id_and_slug(id, slug)
+    find_by(id: id, slug: slug)
+  end
+
   def self.max_deployment_id_sql
     Deployment.select(Deployment.arel_table[:id].maximum)
     .where(Deployment.arel_table[:environment_id].eq(arel_table[:id]))

app/models/serverless/domain.rb

+# frozen_string_literal: true
+
+module Serverless
+  class Domain
+    include ActiveModel::Model
+
+    REGEXP = %r{^(?<scheme>https?://)?(?<function_name>[^.]+)-(?<cluster_left>\h{2})a1(?<cluster_middle>\h{10})f2(?<cluster_right>\h{2})(?<environment_id>\h+)-(?<environment_slug>[^.]+)\.(?<pages_domain_name>.+)}.freeze
+    UUID_LENGTH = 14
+
+    attr_accessor :function_name, :serverless_domain_cluster, :environment
+
+    validates :function_name, presence: true, allow_blank: false
+    validates :serverless_domain_cluster, presence: true
+    validates :environment, presence: true
+
+    def self.generate_uuid
+      SecureRandom.hex(UUID_LENGTH / 2)
+    end
+
+    def uri
+      URI("https://#{function_name}-#{serverless_domain_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{serverless_domain_cluster.domain}")
+    end
+
+    def knative_uri
+      URI("http://#{function_name}.#{namespace}.#{serverless_domain_cluster.knative.hostname}")
+    end
+
+    private
+
+    def namespace
+      serverless_domain_cluster.cluster.kubernetes_namespace_for(environment)
+    end
+
+    def serverless_domain_cluster_uuid
+      [
+        serverless_domain_cluster.uuid[0..1],
+        'a1',
+        serverless_domain_cluster.uuid[2..-3],
+        'f2',
+        serverless_domain_cluster.uuid[-2..-1]
+      ].join
+    end
+  end
+end

app/models/serverless/domain_cluster.rb

@@ -16,11 +16,18 @@ module Serverless
       algorithm: 'aes-256-gcm'
 
     validates :pages_domain, :knative, presence: true
-    validates :uuid, presence: true, uniqueness: true, length: { is: Gitlab::Serverless::Domain::UUID_LENGTH },
+    validates :uuid, presence: true, uniqueness: true, length: { is: ::Serverless::Domain::UUID_LENGTH },
               format: { with: HEX_REGEXP, message: 'only allows hex characters' }
 
-    default_value_for(:uuid, allows_nil: false) { Gitlab::Serverless::Domain.generate_uuid }
+    default_value_for(:uuid, allows_nil: false) { ::Serverless::Domain.generate_uuid }
 
     delegate :domain, to: :pages_domain
+    delegate :cluster, to: :knative
+
+    def self.for_uuid(uuid)
+      joins(:pages_domain, :knative)
+        .includes(:pages_domain, :knative)
+        .find_by(uuid: uuid)
+    end
   end
 end

lib/gitlab/serverless/domain.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Serverless
-    class Domain
-      UUID_LENGTH = 14
-
-      def self.generate_uuid
-        SecureRandom.hex(UUID_LENGTH / 2)
-      end
-    end
-  end
-end

lib/gitlab/serverless/function_uri.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Serverless
-    class FunctionURI < URI::HTTPS
-      SERVERLESS_DOMAIN_REGEXP = %r{^(?<scheme>https?://)?(?<function>[^.]+)-(?<cluster_left>\h{2})a1(?<cluster_middle>\h{10})f2(?<cluster_right>\h{2})(?<environment_id>\h+)-(?<environment_slug>[^.]+)\.(?<domain>.+)}.freeze
-
-      attr_reader :function, :cluster, :environment
-
-      def initialize(function: nil, cluster: nil, environment: nil)
-        initialize_required_argument(:function, function)
-        initialize_required_argument(:cluster, cluster)
-        initialize_required_argument(:environment, environment)
-
-        @host = "#{function}-#{cluster.uuid[0..1]}a1#{cluster.uuid[2..-3]}f2#{cluster.uuid[-2..-1]}#{"%x" % environment.id}-#{environment.slug}.#{cluster.domain}"
-
-        super('https', nil, host, nil, nil, nil, nil, nil, nil)
-      end
-
-      def self.parse(uri)
-        match = SERVERLESS_DOMAIN_REGEXP.match(uri)
-        return unless match
-
-        cluster = ::Serverless::DomainCluster.find(match[:cluster_left] + match[:cluster_middle] + match[:cluster_right])
-        return unless cluster
-
-        environment = ::Environment.find(match[:environment_id].to_i(16))
-        return unless environment&.slug == match[:environment_slug]
-
-        new(
-          function: match[:function],
-          cluster: cluster,
-          environment: environment
-        )
-      end
-
-      private
-
-      def initialize_required_argument(name, value)
-        raise ArgumentError.new("missing argument: #{name}") unless value
-
-        instance_variable_set("@#{name}".to_sym, value)
-      end
-    end
-  end
-end

lib/gitlab/serverless/service.rb

@@ -60,7 +60,11 @@ class Gitlab::Serverless::Service
 
   def proxy_url
     if cluster&.serverless_domain
-      Gitlab::Serverless::FunctionURI.new(function: name, cluster: cluster.serverless_domain, environment: environment)
+      ::Serverless::Domain.new(
+        function_name: name,
+        serverless_domain_cluster: cluster.serverless_domain,
+        environment: environment
+      ).uri.to_s
     end
   end
 

spec/controllers/projects/serverless/functions_controller_spec.rb

@@ -135,7 +135,7 @@ describe Projects::Serverless::FunctionsController do
 
         context 'when there is no serverless domain for a cluster' do
           it 'keeps function URL as it was' do
-            expect(Gitlab::Serverless::Domain).not_to receive(:new)
+            expect(::Serverless::Domain).not_to receive(:new)
 
             get :index, params: params({ format: :json })
             expect(response).to have_gitlab_http_status(:ok)

spec/factories/serverless/domain.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :serverless_domain, class: '::Serverless::Domain' do
+    function_name { 'test-function' }
+    serverless_domain_cluster { create(:serverless_domain_cluster) }
+    environment { create(:environment) }
+
+    skip_create
+  end
+end

spec/finders/serverless_domain_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ServerlessDomainFinder do
+  let(:function_name) { 'test-function' }
+  let(:pages_domain_name) { 'serverless.gitlab.io' }
+  let(:pages_domain) { create(:pages_domain, :instance_serverless, domain: pages_domain_name) }
+  let!(:serverless_domain_cluster) { create(:serverless_domain_cluster, uuid: 'abcdef12345678', pages_domain: pages_domain) }
+  let(:valid_cluster_uuid) { 'aba1cdef123456f278' }
+  let(:invalid_cluster_uuid) { 'aba1cdef123456f178' }
+  let!(:environment) { create(:environment, name: 'test') }
+
+  let(:valid_uri) { "https://#{function_name}-#{valid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+  let(:valid_fqdn) { "#{function_name}-#{valid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+  let(:invalid_uri) { "https://#{function_name}-#{invalid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+
+  let(:valid_finder) { described_class.new(valid_uri) }
+  let(:invalid_finder) { described_class.new(invalid_uri) }
+
+  describe '#serverless?' do
+    context 'with a valid URI' do
+      subject { valid_finder.serverless? }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with an invalid URI' do
+      subject { invalid_finder.serverless? }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#serverless_domain_cluster_uuid' do
+    context 'with a valid URI' do
+      subject { valid_finder.serverless_domain_cluster_uuid }
+
+      it { is_expected.to eq serverless_domain_cluster.uuid }
+    end
+
+    context 'with an invalid URI' do
+      subject { invalid_finder.serverless_domain_cluster_uuid }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#execute' do
+    context 'with a valid URI' do
+      let(:serverless_domain) do
+        create(
+          :serverless_domain,
+          function_name: function_name,
+          serverless_domain_cluster: serverless_domain_cluster,
+          environment: environment
+        )
+      end
+
+      subject { valid_finder.execute }
+
+      it 'has the correct function_name' do
+        expect(subject.function_name).to eq function_name
+      end
+
+      it 'has the correct serverless_domain_cluster' do
+        expect(subject.serverless_domain_cluster).to eq serverless_domain_cluster
+      end
+
+      it 'has the correct environment' do
+        expect(subject.environment).to eq environment
+      end
+    end
+
+    context 'with an invalid URI' do
+      subject { invalid_finder.execute }
+
+      it { is_expected.to be_nil }
+    end
+  end
+end

spec/lib/gitlab/serverless/domain_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Serverless::Domain do
-  describe '.generate_uuid' do
-    it 'has 14 characters' do
-      expect(described_class.generate_uuid.length).to eq(described_class::UUID_LENGTH)
-    end
-
-    it 'consists of only hexadecimal characters' do
-      expect(described_class.generate_uuid).to match(/\A\h+\z/)
-    end
-
-    it 'uses random characters' do
-      uuid = 'abcd1234567890'
-
-      expect(SecureRandom).to receive(:hex).with(described_class::UUID_LENGTH / 2).and_return(uuid)
-      expect(described_class.generate_uuid).to eq(uuid)
-    end
-  end
-end

spec/lib/gitlab/serverless/function_uri_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Serverless::FunctionURI do
-  let(:function) { 'test-function' }
-  let(:domain) { 'serverless.gitlab.io' }
-  let(:pages_domain) { create(:pages_domain, :instance_serverless, domain: domain) }
-  let!(:cluster) { create(:serverless_domain_cluster, uuid: 'abcdef12345678', pages_domain: pages_domain) }
-  let(:valid_cluster) { 'aba1cdef123456f278' }
-  let(:invalid_cluster) { 'aba1cdef123456f178' }
-  let!(:environment) { create(:environment, name: 'test') }
-
-  let(:valid_uri) { "https://#{function}-#{valid_cluster}#{"%x" % environment.id}-#{environment.slug}.#{domain}" }
-  let(:valid_fqdn) { "#{function}-#{valid_cluster}#{"%x" % environment.id}-#{environment.slug}.#{domain}" }
-  let(:invalid_uri) { "https://#{function}-#{invalid_cluster}#{"%x" % environment.id}-#{environment.slug}.#{domain}" }
-
-  shared_examples 'a valid FunctionURI class' do
-    describe '#to_s' do
-      it 'matches valid URI' do
-        expect(subject.to_s).to eq valid_uri
-      end
-    end
-
-    describe '#function' do
-      it 'returns function' do
-        expect(subject.function).to eq function
-      end
-    end
-
-    describe '#cluster' do
-      it 'returns cluster' do
-        expect(subject.cluster).to eq cluster
-      end
-    end
-
-    describe '#environment' do
-      it 'returns environment' do
-        expect(subject.environment).to eq environment
-      end
-    end
-  end
-
-  describe '.new' do
-    context 'with valid arguments' do
-      subject { described_class.new(function: function, cluster: cluster, environment: environment) }
-
-      it_behaves_like 'a valid FunctionURI class'
-    end
-
-    context 'with invalid arguments' do
-      subject { described_class.new(function: function, environment: environment) }
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ArgumentError)
-      end
-    end
-  end
-
-  describe '.parse' do
-    context 'with valid URI' do
-      subject { described_class.parse(valid_uri) }
-
-      it_behaves_like 'a valid FunctionURI class'
-    end
-
-    context 'with valid FQDN' do
-      subject { described_class.parse(valid_fqdn) }
-
-      it_behaves_like 'a valid FunctionURI class'
-    end
-
-    context 'with invalid URI' do
-      subject { described_class.parse(invalid_uri) }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-  end
-end

spec/lib/gitlab/serverless/service_spec.rb

@@ -94,17 +94,19 @@ describe Gitlab::Serverless::Service do
   end
 
   describe '#url' do
+    let(:serverless_domain) { instance_double(::Serverless::Domain, uri: URI('https://proxy.example.com')) }
+
     it 'returns proxy URL if cluster has serverless domain' do
       # cluster = create(:cluster)
       knative = create(:clusters_applications_knative, :installed, cluster: cluster)
       create(:serverless_domain_cluster, clusters_applications_knative_id: knative.id)
       service = Gitlab::Serverless::Service.new(attributes.merge('cluster' => cluster))
 
-      expect(Gitlab::Serverless::FunctionURI).to receive(:new).with(
-        function: service.name,
-        cluster: service.cluster.serverless_domain,
+      expect(::Serverless::Domain).to receive(:new).with(
+        function_name: service.name,
+        serverless_domain_cluster: service.cluster.serverless_domain,
         environment: service.environment
-      ).and_return('https://proxy.example.com')
+      ).and_return(serverless_domain)
 
       expect(service.url).to eq('https://proxy.example.com')
     end

spec/models/environment_spec.rb

@@ -1264,6 +1264,14 @@ describe Environment, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '.for_id_and_slug' do
+    subject { described_class.for_id_and_slug(environment.id, environment.slug) }
+
+    let(:environment) { create(:environment) }
+
+    it { is_expected.not_to be_nil }
+  end
+
   describe '.find_or_create_by_name' do
     it 'finds an existing environment if it exists' do
       env = create(:environment)

spec/models/serverless/domain_cluster_spec.rb

@@ -10,7 +10,7 @@ describe ::Serverless::DomainCluster do
     it { is_expected.to validate_presence_of(:knative) }
 
     it { is_expected.to validate_presence_of(:uuid) }
-    it { is_expected.to validate_length_of(:uuid).is_equal_to(Gitlab::Serverless::Domain::UUID_LENGTH) }
+    it { is_expected.to validate_length_of(:uuid).is_equal_to(::Serverless::Domain::UUID_LENGTH) }
     it { is_expected.to validate_uniqueness_of(:uuid) }
 
     it 'validates that uuid has only hex characters' do
@@ -31,7 +31,7 @@ describe ::Serverless::DomainCluster do
     context 'when nil' do
       it 'generates a value by default' do
         attributes = build(:serverless_domain_cluster).attributes.merge(uuid: nil)
-        expect(Gitlab::Serverless::Domain).to receive(:generate_uuid).and_call_original
+        expect(::Serverless::Domain).to receive(:generate_uuid).and_call_original
 
         subject = Serverless::DomainCluster.new(attributes)
 
@@ -47,6 +47,10 @@ describe ::Serverless::DomainCluster do
     end
   end
 
+  describe 'cluster' do
+    it { is_expected.to respond_to(:cluster) }
+  end
+
   describe 'domain' do
     it { is_expected.to respond_to(:domain) }
   end

spec/models/serverless/domain_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Serverless::Domain do
+  let(:function_name) { 'test-function' }
+  let(:pages_domain_name) { 'serverless.gitlab.io' }
+  let(:pages_domain) { create(:pages_domain, :instance_serverless, domain: pages_domain_name) }
+  let!(:serverless_domain_cluster) { create(:serverless_domain_cluster, uuid: 'abcdef12345678', pages_domain: pages_domain) }
+  let(:valid_cluster_uuid) { 'aba1cdef123456f278' }
+  let(:invalid_cluster_uuid) { 'aba1cdef123456f178' }
+  let!(:environment) { create(:environment, name: 'test') }
+
+  let(:valid_uri) { "https://#{function_name}-#{valid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+  let(:valid_fqdn) { "#{function_name}-#{valid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+  let(:invalid_uri) { "https://#{function_name}-#{invalid_cluster_uuid}#{"%x" % environment.id}-#{environment.slug}.#{pages_domain_name}" }
+
+  shared_examples 'a valid Domain' do
+    describe '#uri' do
+      it 'matches valid URI' do
+        expect(subject.uri.to_s).to eq valid_uri
+      end
+    end
+
+    describe '#function_name' do
+      it 'returns function_name' do
+        expect(subject.function_name).to eq function_name
+      end
+    end
+
+    describe '#serverless_domain_cluster' do
+      it 'returns serverless_domain_cluster' do
+        expect(subject.serverless_domain_cluster).to eq serverless_domain_cluster
+      end
+    end
+
+    describe '#environment' do
+      it 'returns environment' do
+        expect(subject.environment).to eq environment
+      end
+    end
+  end
+
+  describe '.new' do
+    context 'with valid arguments' do
+      subject do
+        described_class.new(
+          function_name: function_name,
+          serverless_domain_cluster: serverless_domain_cluster,
+          environment: environment
+        )
+      end
+
+      it_behaves_like 'a valid Domain'
+    end
+
+    context 'with invalid arguments' do
+      subject do
+        described_class.new(
+          function_name: function_name,
+          environment: environment
+        )
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'with nil cluster argument' do
+      subject do
+        described_class.new(
+          function_name: function_name,
+          serverless_domain_cluster: nil,
+          environment: environment
+        )
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+  end
+
+  describe '.generate_uuid' do
+    it 'has 14 characters' do
+      expect(described_class.generate_uuid.length).to eq(described_class::UUID_LENGTH)
+    end
+
+    it 'consists of only hexadecimal characters' do
+      expect(described_class.generate_uuid).to match(/\A\h+\z/)
+    end
+
+    it 'uses random characters' do
+      uuid = 'abcd1234567890'
+
+      expect(SecureRandom).to receive(:hex).with(described_class::UUID_LENGTH / 2).and_return(uuid)
+      expect(described_class.generate_uuid).to eq(uuid)
+    end
+  end
+end