Merge branch '390874-add-tracking-to-deploy-token' into 'master'

Add user tracking for deploy tokens from registry events

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966



Merged-by: Drew Blessing <drew@gitlab.com>
Approved-by: Radamanthus Batnag <rbatnag@gitlab.com>
Approved-by: David Fernandez <dfernandez@gitlab.com>
Approved-by: Drew Blessing <drew@gitlab.com>
Reviewed-by: Michał Wielich <mwielich@gitlab.com>
Reviewed-by: David Fernandez <dfernandez@gitlab.com>
Reviewed-by: Radamanthus Batnag <rbatnag@gitlab.com>
Co-authored-by: Adie Po <avpfestin@gitlab.com>

app/models/container_registry/event.rb

@@ -14,6 +14,7 @@ class Event
       personal_access_token
       build
       gitlab_or_ldap
+      deploy_token
     ].freeze
 
     TRACKABLE_ACTOR_EVENTS = %w[
@@ -39,7 +40,6 @@ def handle!
     end
 
     def track!
-      tracked_target = target_tag? ? :tag : :repository
       tracking_action = "#{action}_#{tracked_target}"
 
       if target_repository? && action_push? && !container_repository_exists?
@@ -58,6 +58,10 @@ def track!
 
     private
 
+    def tracked_target
+      target_tag? ? :tag : :repository
+    end
+
     def target_tag?
       # There is no clear indication in the event structure when we delete a top-level manifest
       # except existence of "tag" key
@@ -108,22 +112,51 @@ def usage_data_event_for(tracking_action)
       return unless originator
       return unless TRACKABLE_ACTOR_EVENTS.include?(tracking_action)
 
-      "#{EVENT_PREFIX}_#{tracking_action}_user"
+      "#{EVENT_PREFIX}_#{tracking_action}_#{originator_suffix}"
+    end
+
+    def originator
+      return unless ALLOWED_ACTOR_TYPES.include?(originator_type)
+
+      origin_id = get_origin_id(originator_type)
+      return unless origin_id
+
+      origin_class.find(origin_id)
+    end
+    strong_memoize_attr :originator
+
+    def originator_suffix
+      originator.is_a?(DeployToken) ? 'deploy_token' : 'user'
     end
 
     def originator_type
       event.dig('actor', 'user_type')
     end
 
-    def originator
-      return unless ALLOWED_ACTOR_TYPES.include?(originator_type)
+    def origin_class
+      deploy_token?(originator_type) ? DeployToken : User
+    end
 
-      username = event.dig('actor', 'name')
-      return unless username
+    def get_origin_id(originator_type)
+      encoded_user_jwt = event.dig('actor', 'user')
+      return unless encoded_user_jwt
 
-      strong_memoize(:originator) do
-        User.find_by_username(username)
-      end
+      key = deploy_token?(originator_type) ? 'deploy_token_id' : 'user_id'
+      user_info = decode_user_info(encoded_user_jwt)
+      user_info&.dig('user_info', key)
+    end
+
+    def decode_user_info(encoded_user_jwt)
+      registry_key_file = File.read(Gitlab.config.registry.key)
+      registry_key = OpenSSL::PKey::RSA.new(registry_key_file)
+
+      JSONWebToken::RSAToken.decode(encoded_user_jwt, registry_key).first
+    rescue Errno::ENOENT, JWT::VerificationError, JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature
+      nil
+    end
+
+    def deploy_token?(originator_type)
+      originator_type == 'deploy_token'
     end
 
     def manifest_delete_event?

config/metrics/counts_28d/20231227173838_i_container_registry_push_tag_deploy_token_monthly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_push_tag_deploy_token_monthly
+description: A monthly count of deploy tokens that have pushed a tag to the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_push_tag_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20231227174251_i_container_registry_delete_tag_deploy_token_monthly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_delete_tag_deploy_token_monthly
+description: A monthly count of deploy tokens that have deleted a tag from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_delete_tag_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20231227175541_i_container_registry_push_repository_deploy_token_monthly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_push_repository_deploy_token_monthly
+description: A monthly count of deploy tokens that have pushed a repository to the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_push_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20231227175706_i_container_registry_delete_repository_deploy_token_monthly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_delete_repository_deploy_token_monthly
+description: A monthly count of deploy tokens that have deleted a repository from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_delete_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20231227180108_i_container_registry_create_repository_deploy_token_monthly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_create_repository_deploy_token_monthly
+description: A monthly count of deploy tokens that have created a repository from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_create_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20231223170002_i_container_registry_push_repository_deploy_token_weekly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_push_repository_deploy_token_weekly
+description: A weekly count of deploy tokens that have pushed a repository to the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_push_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20231227165432_i_container_registry_push_tag_deploy_token_weekly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_push_tag_deploy_token_weekly
+description: A weekly count of deploy tokens that have pushed a tag to the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_push_tag_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20231227165706_i_container_registry_delete_tag_deploy_token_weekly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_delete_tag_deploy_token_weekly
+description: A weekly count of deploy tokens that have deleted a tag from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_delete_tag_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20231227170213_i_container_registry_delete_repository_deploy_token_weekly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_delete_repository_deploy_token_weekly
+description: A weekly count of deploy tokens that have deleted a repository from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_delete_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20231227170547_i_container_registry_create_repository_deploy_token_weekly.yml

+---
+key_path: redis_hll_counters.user_container_registry.i_container_registry_create_repository_deploy_token_weekly
+description: A weekly count of deploy tokens that have created a repository from the registry
+product_section: ops
+product_stage: package
+product_group: container_registry
+value_type: number
+status: active
+milestone: "16.9"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131966
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+  - i_container_registry_create_repository_deploy_token
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

lib/json_web_token/rsa_token.rb

@@ -2,6 +2,8 @@
 
 module JSONWebToken
   class RSAToken < Token
+    ALGORITHM = 'RS256'
+
     attr_reader :key_file
 
     def initialize(key_file)
@@ -14,7 +16,11 @@ def encoded
         kid: kid,
         typ: 'JWT'
       }
-      JWT.encode(payload, key, 'RS256', headers)
+      JWT.encode(payload, key, ALGORITHM, headers)
+    end
+
+    def self.decode(token, key)
+      JWT.decode(token, key, true, { algorithm: ALGORITHM })
     end
 
     private

spec/lib/json_web_token/rsa_token_spec.rb

 # frozen_string_literal: true
 
 RSpec.describe JSONWebToken::RSAToken do
-  let(:rsa_key) do
+  let_it_be(:rsa_key) do
     OpenSSL::PKey::RSA.new <<-eos.strip_heredoc
       -----BEGIN RSA PRIVATE KEY-----
       MIIBOgIBAAJBAMA5sXIBE0HwgIB40iNidN4PGWzOyLQK0bsdOBNgpEXkDlZBvnak
@@ -49,4 +49,52 @@
       it { expect { subject }.to raise_error(JWT::DecodeError) }
     end
   end
+
+  describe '.decode' do
+    let(:decoded_token) { described_class.decode(rsa_encoded, rsa_key) }
+
+    context 'with an invalid token' do
+      context 'that is junk' do
+        let(:rsa_encoded) { 'junk' }
+
+        it "raises exception saying 'Not enough or too many segments'" do
+          expect { decoded_token }.to raise_error(JWT::DecodeError, 'Not enough or too many segments')
+        end
+      end
+
+      context 'that has been fiddled with' do
+        let(:rsa_encoded) { rsa_token.encoded.tap { |token| token[0] = 'E' } }
+
+        it "raises exception saying 'Invalid segment encoding'" do
+          expect { decoded_token }.to raise_error(JWT::DecodeError, 'Invalid segment encoding')
+        end
+      end
+
+      context 'that was generated using a different key' do
+        let_it_be(:rsa_key_2) { OpenSSL::PKey::RSA.new 2048 }
+
+        before do
+          # rsa_key is returned for encoding, and rsa_key_2 for decoding
+          allow_any_instance_of(described_class).to receive(:key).and_return(rsa_key, rsa_key_2)
+        end
+
+        it "raises exception saying 'Signature verification failed" do
+          expect { decoded_token }.to raise_error(JWT::VerificationError, 'Signature verification failed')
+        end
+      end
+
+      context 'that is expired' do
+        # Needs the ! so freeze_time() is effective
+        let!(:rsa_encoded) { rsa_token.encoded }
+
+        it "raises exception saying 'Signature has expired'" do
+          # Needs to be 120 seconds, because the default expiry is 60 seconds
+          # with an additional 60 second leeway.
+          travel_to(Time.current + 120) do
+            expect { decoded_token }.to raise_error(JWT::ExpiredSignature, 'Signature has expired')
+          end
+        end
+      end
+    end
+  end
 end

spec/models/container_registry/event_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe ContainerRegistry::Event do
+RSpec.describe ContainerRegistry::Event, feature_category: :container_registry do
   using RSpec::Parameterized::TableSyntax
 
   let_it_be(:group) { create(:group, name: 'group') }
@@ -112,19 +112,22 @@
   describe '#track!' do
     let_it_be(:container_repository) { create(:container_repository, name: 'container', project: project) }
 
+    let_it_be(:rsa_key) { OpenSSL::PKey::RSA.generate(3072) }
     let(:raw_event) { { 'action' => action, 'target' => target } }
+    let(:key_file) { Tempfile.new('keypath') }
 
-    subject { described_class.new(raw_event).track! }
-
-    shared_examples 'tracking event is sent to HLLRedisCounter with event and originator ID' do |originator_type|
-      it 'fetches the event originator based on username' do
-        count.times do
-          expect(User).to receive(:find_by_username).with(originator.username)
-        end
+    before do
+      allow(Gitlab.config.registry).to receive_messages(enabled: true, issuer: 'rspec', key: key_file.path)
+      allow(OpenSSL::PKey::RSA).to receive(:new).and_return(rsa_key)
 
-        subject
+      allow_next_instance_of(JSONWebToken::RSAToken) do |instance|
+        allow(instance).to receive(:key).and_return(rsa_key)
       end
+    end
+
+    subject { described_class.new(raw_event).track! }
 
+    shared_examples 'tracking event is sent to HLLRedisCounter with event and originator ID' do |originator_type|
       it 'sends a tracking event to HLLRedisCounter' do
         expect(::Gitlab::UsageDataCounters::HLLRedisCounter)
         .to receive(:track_event).with("i_container_registry_#{event}_#{originator_type}", values: originator.id)
@@ -134,6 +137,16 @@
       end
     end
 
+    shared_examples 'event originator is fetched based on ID' do |originator_class|
+      it 'fetches the event originator based on id' do
+        count.times do
+          expect(originator_class).to receive(:find).with(originator.id)
+        end
+
+        subject
+      end
+    end
+
     context 'with a respository target' do
       let(:target) do
         {
@@ -185,43 +198,165 @@
 
     context 'with a deploy token as the actor' do
       let!(:originator) { create(:deploy_token, username: 'username', id: 3) }
-      let(:raw_event) do
-        {
-          'action' => 'push',
-          'target' => { 'tag' => 'latest' },
-          'actor' => { 'user_type' => 'deploy_token', 'name' => originator.username }
-        }
+
+      shared_examples 'no tracking of a deploy token is sent to HLLRedisCounter' do
+        it 'does not send a tracking event to HLLRedisCounter' do
+          expect(DeployToken).not_to receive(:find)
+          expect(DeployToken).not_to receive(:find_by_username)
+          expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
+
+          expect { subject }.not_to raise_error
+        end
       end
 
-      it 'does not send a tracking event to HLLRedisCounter' do
-        expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
+      context 'when only username is provided and no deploy_token_id is given' do
+        let(:raw_event) do
+          {
+            'action' => 'push',
+            'target' => { 'tag' => 'latest' },
+            'actor' => { 'user_type' => 'deploy_token', 'name' => originator.username }
+          }
+        end
 
-        subject
+        it_behaves_like 'no tracking of a deploy token is sent to HLLRedisCounter'
+      end
+
+      context 'when no username or deploy_token_id is given' do
+        let(:raw_event) { { 'action' => 'push', 'target' => {}, 'actor' => { 'user_type' => 'deploy_token' } } }
+
+        it_behaves_like 'no tracking of a deploy token is sent to HLLRedisCounter'
+      end
+
+      context 'when deploy_token_id is given' do
+        let(:deploy_token_info) do
+          {
+            token_type: 'deploy_token',
+            username: originator.username,
+            deploy_token_id: originator.id
+          }
+        end
+
+        let(:token) do
+          JSONWebToken::RSAToken.new(rsa_key).tap do |token|
+            token[:user_info] = deploy_token_info
+          end
+        end
+
+        let(:raw_event) do
+          {
+            'action' => action,
+            'target' => target,
+            'actor' => { 'user_type' => 'deploy_token', 'name' => originator.username, 'user' => token.encoded }
+          }
+        end
+
+        where(:target, :action, :event, :count) do
+          { 'tag' => 'latest' }          | 'push'     | 'push_tag'           |  1
+          { 'tag' => 'latest' }          | 'delete'   | 'delete_tag'         |  1
+          { 'repository' => 'foo/bar' }  | 'push'     | 'create_repository'  |  1
+          { 'repository' => 'foo/bar' }  | 'delete'   | 'delete_repository'  |  1
+          { 'tag' => 'latest' }          | 'copy'     | ''                   |  0
+        end
+
+        with_them do
+          it_behaves_like 'event originator is fetched based on ID', DeployToken
+          it_behaves_like 'tracking event is sent to HLLRedisCounter with event and originator ID', :deploy_token
+        end
+
+        context "when there are errors" do
+          let(:action) { 'push' }
+          let(:target) { { 'tag' => 'latest' } }
+
+          context 'when the registry key file does not exist' do
+            before do
+              allow(File).to receive(:read).and_call_original
+              allow(File).to receive(:read).with(key_file.path).and_raise(Errno::ENOENT)
+            end
+
+            it_behaves_like 'no tracking of a deploy token is sent to HLLRedisCounter'
+          end
+
+          [JWT::VerificationError, JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature].each do |error|
+            context "when JWT decoding encounters #{error}" do
+              before do
+                allow(JWT).to receive(:decode)
+                .with(token.encoded, rsa_key, true, { algorithm: "RS256" })
+                .and_raise(error)
+              end
+
+              it_behaves_like 'no tracking of a deploy token is sent to HLLRedisCounter'
+            end
+          end
+        end
       end
     end
 
     context 'with a user as the actor' do
       let_it_be(:originator) { create(:user, username: 'username') }
-      let(:raw_event) do
-        {
-          'action' => action,
-          'target' => target,
-          'actor' => { 'user_type' => user_type, 'name' => originator.username }
-        }
+
+      context 'when user_id is available' do
+        let(:user_info) do
+          {
+            token_type: user_type,
+            username: originator.username,
+            user_id: originator.id
+          }
+        end
+
+        let(:token) do
+          JSONWebToken::RSAToken.new(rsa_key).tap do |token|
+            token[:user_info] = user_info
+          end
+        end
+
+        let(:raw_event) do
+          {
+            'action' => action,
+            'target' => target,
+            'actor' => { 'user_type' => user_type, 'name' => originator.username, 'user' => token.encoded }
+          }
+        end
+
+        where(:target, :action, :event, :user_type, :count) do
+          { 'tag' => 'latest' }          | 'push'     | 'push_tag'           |  'personal_access_token'   |  1
+          { 'tag' => 'latest' }          | 'delete'   | 'delete_tag'         |  'personal_access_token'   |  1
+          { 'repository' => 'foo/bar' }  | 'push'     | 'create_repository'  |  'build'                   |  1
+          { 'repository' => 'foo/bar' }  | 'delete'   | 'delete_repository'  |  'gitlab_or_ldap'          |  1
+          { 'repository' => 'foo/bar' }  | 'delete'   | 'delete_repository'  |  'not_a_user'              |  0
+          { 'tag' => 'latest' }          | 'copy'     | ''                   |  nil                       |  0
+          { 'repository' => 'foo/bar' }  | 'copy'     | ''                   |  ''                        |  0
+        end
+
+        with_them do
+          it_behaves_like 'event originator is fetched based on ID', User
+          it_behaves_like 'tracking event is sent to HLLRedisCounter with event and originator ID', :user
+        end
       end
 
-      where(:target, :action, :event, :user_type, :count) do
-        { 'tag' => 'latest' }          | 'push'     | 'push_tag'           |  'personal_access_token'   |  1
-        { 'tag' => 'latest' }          | 'delete'   | 'delete_tag'         |  'personal_access_token'   |  1
-        { 'repository' => 'foo/bar' }  | 'push'     | 'create_repository'  |  'build'                   |  1
-        { 'repository' => 'foo/bar' }  | 'delete'   | 'delete_repository'  |  'gitlab_or_ldap'          |  1
-        { 'repository' => 'foo/bar' }  | 'delete'   | 'delete_repository'  |  'not_a_user'              |  0
-        { 'tag' => 'latest' }          | 'copy'     | ''                   |  nil                       |  0
-        { 'repository' => 'foo/bar' }  | 'copy'     | ''                   |  ''                        |  0
+      context 'when only username is available and user_id is not' do
+        let(:raw_event) do
+          {
+            'action' => 'push',
+            'target' => { 'tag' => 'latest' },
+            'actor' => { 'user_type' => 'personal_access_token', 'name' => originator.username }
+          }
+        end
+
+        it 'does not send a tracking event to HLLRedisCounter' do
+          expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
+
+          subject
+        end
       end
 
-      with_them do
-        it_behaves_like 'tracking event is sent to HLLRedisCounter with event and originator ID', :user
+      context 'when no username or id is given' do
+        let(:raw_event) { { 'action' => 'push', 'target' => {}, 'actor' => { 'user_type' => 'build' } } }
+
+        it 'does not send a tracking event to HLLRedisCounter' do
+          expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
+
+          subject
+        end
       end
     end
 
@@ -246,16 +381,5 @@
         subject
       end
     end
-
-    context 'without an actor name' do
-      let(:raw_event) { { 'action' => 'push', 'target' => {}, 'actor' => { 'user_type' => 'personal_access_token' } } }
-
-      it 'does not send a tracking event to HLLRedisCounter' do
-        expect(User).not_to receive(:find_by_username)
-        expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
-
-        subject
-      end
-    end
   end
 end