Merge branch '20378-10io-docker-notifications-snowplow-integration' into 'master'

Add tracking events in API::ContainerRegistryEvent

Closes #20378

See merge request gitlab-org/gitlab!27001

app/models/container_registry/event.rb

+# frozen_string_literal: true
+
+module ContainerRegistry
+  class Event
+    ALLOWED_ACTIONS = %w(push delete).freeze
+    PUSH_ACTION = 'push'
+    EVENT_TRACKING_CATEGORY = 'container_registry:notification'
+
+    attr_reader :event
+
+    def initialize(event)
+      @event = event
+    end
+
+    def supported?
+      action.in?(ALLOWED_ACTIONS)
+    end
+
+    def handle!
+      # no op
+    end
+
+    def track!
+      tracked_target = target_tag? ? :tag : :repository
+      tracking_action = "#{action}_#{tracked_target}"
+
+      if target_repository? && action_push? && !container_repository_exists?
+        tracking_action = "create_repository"
+      end
+
+      ::Gitlab::Tracking.event(EVENT_TRACKING_CATEGORY, tracking_action)
+    end
+
+    private
+
+    def target_tag?
+      # There is no clear indication in the event structure when we delete a top-level manifest
+      # except existance of "tag" key
+      event['target'].has_key?('tag')
+    end
+
+    def target_repository?
+      !target_tag? && event['target'].has_key?('repository')
+    end
+
+    def action
+      event['action']
+    end
+
+    def action_push?
+      PUSH_ACTION == action
+    end
+
+    def container_repository_exists?
+      return unless container_registry_path
+
+      ContainerRepository.exists_by_path?(container_registry_path)
+    end
+
+    def container_registry_path
+      path = event.dig('target', 'repository')
+      return unless path
+
+      ContainerRegistry::Path.new(path)
+    end
+  end
+end
+
+::ContainerRegistry::Event.prepend_if_ee('EE::ContainerRegistry::Event')

app/models/container_repository.rb

@@ -16,6 +16,13 @@ class ContainerRepository < ApplicationRecord
     where(project_id: Project.for_group_and_its_subgroups(group).with_container_registry.select(:id))
   end
 
+  def self.exists_by_path?(path)
+    where(
+      project: path.repository_project,
+      name: path.repository_name
+    ).exists?
+  end
+
   # rubocop: disable CodeReuse/ServiceClass
   def registry
     @registry ||= begin

ee/app/models/ee/container_registry/event.rb

+# frozen_string_literal: true
+
+module EE
+  module ContainerRegistry
+    module Event
+      extend ::Gitlab::Utils::Override
+
+      override :handle!
+      def handle!
+        super
+        create_geo_container_repository_updated_event_store!
+      end
+
+      private
+
+      def create_geo_container_repository_updated_event_store!
+        return unless media_type_manifest? || target_tag?
+        return unless container_repository_exists?
+
+        ::Geo::ContainerRepositoryUpdatedEventStore.new(find_container_repository!)
+                                                   .create!
+      end
+
+      def media_type_manifest?
+        event.dig('target', 'mediaType') =~ /manifest/
+      end
+
+      def find_container_repository!
+        ::ContainerRepository.find_by_path!(container_registry_path)
+      end
+    end
+  end
+end

ee/changelogs/unreleased/20378-10io-docker-notifications-snowplow-integration.yml

+---
+title: Add Snowplow tracking for Container Registry events
+merge_request: 27001
+author:
+type: added

ee/lib/container_registry/event_handler.rb

-# frozen_string_literal: true
-
-module ContainerRegistry
-  class EventHandler
-    attr_reader :events
-
-    def initialize(events)
-      @events = events
-    end
-
-    def execute
-      events.each do |event|
-        handle_event(event) if %w(push delete).include?(event['action'])
-      end
-    end
-
-    private
-
-    def handle_event(event)
-      return unless manifest_push?(event) || manifest_delete?(event)
-
-      ::Geo::ContainerRepositoryUpdatedEventStore.new(find_repository!(event)).create!
-    end
-
-    def manifest_push?(event)
-      event['target']['mediaType'] =~ /manifest/
-    end
-
-    def manifest_delete?(event)
-      # There is no clear indication in the event structure when we delete a top-level manifest
-      # except existance of "tag" key
-      event['target'].has_key?('tag')
-    end
-
-    def find_repository!(event)
-      repository_name = event['target']['repository']
-      path = ContainerRegistry::Path.new(repository_name)
-      ContainerRepository.find_by_path!(path)
-    end
-  end
-end

ee/lib/ee/api/api.rb

@@ -21,7 +21,6 @@ module EE
         mount ::API::ElasticsearchIndexedNamespaces
         mount ::API::FeatureFlags
         mount ::API::FeatureFlagScopes
-        mount ::API::ContainerRegistryEvent
         mount ::API::Geo
         mount ::API::GeoReplication
         mount ::API::GeoNodes

ee/spec/lib/container_registry/event_handler_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe ContainerRegistry::EventHandler do
-  include ::EE::GeoHelpers
-
-  let(:container_repository) { create(:container_repository) }
-
-  let(:event_target_for_push) do
-    { 'mediaType' => 'application/vnd.docker.distribution.manifest.v2+json', 'repository' => container_repository.path }
-  end
-
-  let(:event_target_for_delete) do
-    { 'tag' => 'latest', 'repository' => container_repository.path }
-  end
-
-  let_it_be(:primary_node)   { create(:geo_node, :primary) }
-  let_it_be(:secondary_node) { create(:geo_node) }
-
-  before do
-    stub_current_geo_node(primary_node)
-  end
-
-  it 'creates event for push' do
-    push_event = { action: 'push', target: event_target_for_push }.with_indifferent_access
-
-    expect { described_class.new([push_event]).execute }
-        .to change { ::Geo::ContainerRepositoryUpdatedEvent.count }.by(1)
-  end
-
-  it 'creates event for delete' do
-    delete_event = { action: 'delete', target: event_target_for_delete }.with_indifferent_access
-
-    expect { described_class.new([delete_event]).execute }
-        .to change { ::Geo::ContainerRepositoryUpdatedEvent.count }.by(1)
-  end
-
-  it 'ignores pull events' do
-    pull_event = { action: 'pull', target: {} }.with_indifferent_access
-
-    expect { described_class.new([pull_event]).execute }
-        .to change { ::Geo::ContainerRepositoryUpdatedEvent.count }.by(0)
-  end
-end

ee/spec/models/container_registry/event_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ContainerRegistry::Event do
+  using RSpec::Parameterized::TableSyntax
+  include ::EE::GeoHelpers
+
+  let_it_be(:group) { create(:group, name: 'group') }
+  let_it_be(:project) { create(:project, name: 'test', namespace: group) }
+
+  RSpec.shared_examples 'creating a geo event' do
+    it 'creates geo event' do
+      expect { subject }
+        .to change { ::Geo::ContainerRepositoryUpdatedEvent.count }.by(1)
+    end
+  end
+
+  RSpec.shared_examples 'not creating a geo event' do
+    it 'does not create geo event' do
+      expect { subject }
+        .not_to change { ::Geo::ContainerRepositoryUpdatedEvent.count }
+    end
+  end
+
+  describe '#handle!' do
+    context 'geo event' do
+      let_it_be(:container_repository) { create(:container_repository, name: 'container', project: project) }
+      let_it_be(:primary_node)   { create(:geo_node, :primary) }
+      let_it_be(:secondary_node) { create(:geo_node) }
+
+      let(:raw_event) { { 'action' => action, 'target' => target } }
+
+      subject { described_class.new(raw_event).handle! }
+
+      before do
+        stub_current_geo_node(primary_node)
+      end
+
+      context 'with a respository target' do
+        let(:target) do
+          {
+            'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE,
+            'repository' => repository_path
+          }
+        end
+
+        where(:repository_path, :action, :example_name) do
+          'group/test/container' | 'push'   | 'creating a geo event'
+          'group/test/container' | 'delete' | 'creating a geo event'
+          'foo/bar'              | 'push'   | 'not creating a geo event'
+          'foo/bar'              | 'delete' | 'not creating a geo event'
+        end
+
+        with_them do
+          it_behaves_like params[:example_name]
+        end
+      end
+
+      context 'with a tag target' do
+        let(:target) do
+          {
+            'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE,
+            'repository' => repository_path,
+            'tag' => 'latest'
+          }
+        end
+
+        where(:repository_path, :action, :example_name) do
+          'group/test/container' | 'push'   | 'creating a geo event'
+          'group/test/container' | 'delete' | 'creating a geo event'
+          'foo/bar'              | 'push'   | 'not creating a geo event'
+          'foo/bar'              | 'delete' | 'not creating a geo event'
+        end
+
+        with_them do
+          it_behaves_like params[:example_name]
+        end
+      end
+    end
+  end
+end

lib/api/api.rb

@@ -121,6 +121,7 @@ module API
       mount ::API::BroadcastMessages
       mount ::API::Commits
       mount ::API::CommitStatuses
+      mount ::API::ContainerRegistryEvent
       mount ::API::DeployKeys
       mount ::API::DeployTokens
       mount ::API::Deployments

ee/lib/api/container_registry_event.rb → lib/api/container_registry_event.rb

@@ -27,7 +27,14 @@ module API
       # This endpoint is used by Docker Registry to push a set of event
       # that took place recently.
       post 'events' do
-        ::ContainerRegistry::EventHandler.new(params['events']).execute
+        params['events'].each do |raw_event|
+          event = ::ContainerRegistry::Event.new(raw_event)
+
+          if event.supported?
+            event.handle!
+            event.track!
+          end
+        end
 
         status :ok
       end

spec/models/container_registry/event_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ContainerRegistry::Event do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:group) { create(:group, name: 'group') }
+  let_it_be(:project) { create(:project, name: 'test', namespace: group) }
+
+  describe '#supported?' do
+    let(:raw_event) { { 'action' => action } }
+
+    subject { described_class.new(raw_event).supported? }
+
+    where(:action, :supported) do
+      'delete' | true
+      'push'   | true
+      'mount'  | false
+      'pull'   | false
+    end
+
+    with_them do
+      it { is_expected.to eq supported }
+    end
+  end
+
+  describe '#handle!' do
+    let(:raw_event) { { 'action' => 'push', 'target' => { 'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE } } }
+
+    subject { described_class.new(raw_event).handle! }
+
+    it { is_expected.to eq nil }
+  end
+
+  describe '#track!' do
+    let_it_be(:container_repository) { create(:container_repository, name: 'container', project: project) }
+
+    let(:raw_event) { { 'action' => action, 'target' => target } }
+
+    subject { described_class.new(raw_event).track! }
+
+    context 'with a respository target' do
+      let(:target) do
+        {
+          'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE,
+          'repository' => repository_path
+        }
+      end
+
+      where(:repository_path, :action, :tracking_action) do
+        'group/test/container' | 'push'   | 'push_repository'
+        'group/test/container' | 'delete' | 'delete_repository'
+        'foo/bar'              | 'push'   | 'create_repository'
+        'foo/bar'              | 'delete' | 'delete_repository'
+      end
+
+      with_them do
+        it 'creates a tracking event' do
+          expect(::Gitlab::Tracking).to receive(:event).with('container_registry:notification', tracking_action)
+
+          subject
+        end
+      end
+    end
+
+    context 'with a tag target' do
+      let(:target) do
+        {
+          'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE,
+          'repository' =>  repository_path,
+          'tag' => 'latest'
+        }
+      end
+
+      where(:repository_path, :action, :tracking_action) do
+        'group/test/container' | 'push'   | 'push_tag'
+        'group/test/container' | 'delete' | 'delete_tag'
+        'foo/bar'              | 'push'   | 'push_tag'
+        'foo/bar'              | 'delete' | 'delete_tag'
+      end
+
+      with_them do
+        it 'creates a tracking event' do
+          expect(::Gitlab::Tracking).to receive(:event).with('container_registry:notification', tracking_action)
+
+          subject
+        end
+      end
+    end
+  end
+end

spec/models/container_repository_spec.rb

@@ -29,6 +29,18 @@ describe ContainerRepository do
     end
   end
 
+  describe '.exists_by_path?' do
+    it 'returns true for known container repository paths' do
+      path = ContainerRegistry::Path.new("#{project.full_path}/#{repository.name}")
+      expect(described_class.exists_by_path?(path)).to be_truthy
+    end
+
+    it 'returns false for unknown container repository paths' do
+      path = ContainerRegistry::Path.new('you/dont/know/me')
+      expect(described_class.exists_by_path?(path)).to be_falsey
+    end
+  end
+
   describe '#tag' do
     it 'has a test tag' do
       expect(repository.tag('test')).not_to be_nil

ee/spec/requests/api/container_registry_event_spec.rb → spec/requests/api/container_registry_event_spec.rb

@@ -12,15 +12,21 @@ describe API::ContainerRegistryEvent do
       allow(Gitlab.config.registry).to receive(:notification_secret) { secret_token }
     end
 
-    it 'returns 200 status and events are passed to event handler' do
-      handler = spy(:handle)
-      allow(::ContainerRegistry::EventHandler).to receive(:new).with(events).and_return(handler)
-
+    subject do
       post api('/container_registry_event/events'),
            params: { events: events }.to_json,
            headers: registry_headers.merge('Authorization' => secret_token)
+    end
+
+    it 'returns 200 status and events are passed to event handler' do
+      event = spy(:event)
+      allow(::ContainerRegistry::Event).to receive(:new).and_return(event)
+      expect(event).to receive(:supported?).and_return(true)
+
+      subject
 
-      expect(handler).to have_received(:execute).once
+      expect(event).to have_received(:handle!).once
+      expect(event).to have_received(:track!).once
       expect(response.status).to eq 200
     end