Added support for removing images/tags

Soft delete support has been supported in Docker Distribution since at least
2.1. This was not enough to implement the removal of images/tags in Portus
because there was no support to GC these soft deleted blobs. Since 2.4, it's
possible to not just delete the manifest, but also to GC blobs no longer
referenced by any image manifest. This means that after being able to track
digests, we can now safely provide image/tag removal support. For safety
concerns, tags with an empty digest will not be allowed to be removed (this
is more likely to be the case of Portus versions that have been running for
some time). In previous PRs this has already been addressed, so admins can
update their DB filling in the empty gaps (e.g. see PRs #825 or #830).

The main downside of this change is that there is no way for a client to detect
whether a remote registry supports GC. Because of this, a configuration option
has been provided, which is disabled by default. The rationale is that
administrators that are really sure about the availability of GC on their
private registry will have to enable it explicitly.

Fixes #197

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>

.rubocop.yml

@@ -2,9 +2,9 @@
 inherit_from:
   - ./config/rubocop-suse.yml
 
-# TODO: (mssola) only the LDAP class requires this.
+# TODO: (mssola) only the LDAP class and portusctl require this.
 Metrics/ClassLength:
-  Max: 150
+  Max: 160
 
 # TODO: (mssola) Some methods are offending this cop. In the SUSE's style guide
 # the approach is to use Rubocop's default value. In the near future I will

app/assets/stylesheets/repositories.scss

@@ -1,3 +1,11 @@
 .tags .label.label-success {
   margin: 0px 2px;
 }
+
+#remove-repo button {
+  padding: 0px 10px 0px 0px;
+}
+
+.remove-repo:focus, .remove-repo:hover, .remove-tag:focus, .remove-tag:hover {
+  text-decoration: none;
+}

app/controllers/concerns/delete_enabled.rb

@@ -0,0 +1,14 @@
+# DeleteEnabeld redirects the user back if delete support is not enabled. A
+# `before_action` will be created for the :destroy method.
+module DeleteEnabled
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :delete_enabled?, only: [:destroy]
+  end
+
+  # Returns true if users can delete images/tags.
+  def delete_enabled?
+    redirect_to :back, status: :forbidden unless APP_CONFIG.enabled?("delete")
+  end
+end

app/controllers/repositories_controller.rb

@@ -1,5 +1,7 @@
 class RepositoriesController < ApplicationController
-  before_action :set_repository, only: [:show, :toggle_star]
+  include DeleteEnabled
+
+  before_action :set_repository, only: [:show, :destroy, :toggle_star]
 
   # GET /repositories
   # GET /repositories.json
@@ -23,6 +25,28 @@ def toggle_star
     render template: "repositories/star", locals: { user: current_user }
   end
 
+  # Removes all the tags that belong to this repository, and removes it.
+  def destroy
+    # First of all we mark the repo and the tags, so we don't have concurrency
+    # problems when "delete" events come in.
+    @repository.tags.update_all(marked: true)
+    @repository.update_attributes(marked: true)
+
+    # Remove all tags, effectively removing them from the registry too.
+    @repository.groupped_tags.map { |t| t.first.delete_by_digest!(current_user) }
+
+    # Delete this repository if all tags were successfully deleted.
+    if @repository.reload.tags.any?
+      redirect_to repository_path(@repository), alert: "Could not remove all the tags"
+    else
+      @repository.delete_and_update!(current_user)
+      redirect_to namespace_path(@repository.namespace),
+        notice: "Repository removed with all its tags"
+    end
+  end
+
+  protected
+
   def set_repository
     @repository = Repository.find(params[:id])
   end

app/controllers/tags_controller.rb

@@ -0,0 +1,24 @@
+class TagsController < ApplicationController
+  include DeleteEnabled
+
+  # Removes all tags the match the digest of the tag with the given ID.
+  # Moreover, it will also remove the image if it's left empty after removing
+  # the tags.
+  def destroy
+    tag = Tag.find(params[:id])
+
+    # And now remove the tag by the digest. If the repository containing said
+    # tags becomes empty after that, remove it too.
+    repo = tag.repository
+    if tag.delete_by_digest!(current_user)
+      if repo.tags.empty?
+        repo.delete_and_update!(current_user)
+        redirect_to namespace_path(repo.namespace), notice: "Image removed with all its tags"
+      else
+        redirect_to repository_path(tag.repository), notice: "Tag removed successfully"
+      end
+    else
+      redirect_to repository_path(tag.repository), alert: "Tag could not be removed"
+    end
+  end
+end

app/helpers/repositories_helper.rb

@@ -2,18 +2,28 @@
 # them, dangling repositories that used to contain them. Because of this, this
 # helper renders the proper HTML for push activities, while being safe at it.
 module RepositoriesHelper
-  # Renders a push activity, that is, a repository has been pushed.
+  # Renders a push activity, that is, a repository/tag has been pushed.
   def render_push_activity(activity)
-    owner = content_tag(:strong, "#{fetch_owner(activity)} pushed ")
+    render_repo_activity(activity, "pushed")
+  end
+
+  # Renders a delete activity, that is, a repository/tag has been deleted.
+  def render_delete_activity(activity)
+    render_repo_activity(activity, "deleted")
+  end
+
+  protected
+
+  # General method for rendering an activity regarding repositories.
+  def render_repo_activity(activity, action)
+    owner = content_tag(:strong, "#{fetch_owner(activity)} #{action} ")
 
     namespace = render_namespace(activity)
     namespace += " / " unless namespace.empty?
 
     owner + namespace + render_repository(activity)
   end
 
-  protected
-
   # Fetches the owner of the activity in a safe way.
   def fetch_owner(activity)
     activity.owner.nil? ? "Someone" : activity.owner.username
@@ -27,22 +37,24 @@ def fetch_owner(activity)
   def render_namespace(activity)
     tr = activity.trackable
 
-    if tr.nil?
+    if tr.nil? || tr.is_a?(Namespace)
       if activity.parameters[:namespace_name].nil?
         ""
       else
         namespace = Namespace.find_by(id: activity.parameters[:namespace_id])
-        if namespace.nil?
-          content_tag(:span, activity.parameters[:namespace_name])
-        else
-          link_to activity.parameters[:namespace_name], namespace
-        end
+        tag_or_link(namespace, activity.parameters[:namespace_name])
       end
     else
       link_to tr.namespace.clean_name, tr.namespace
     end
   end
 
+  # Returns a link if the namespace is not nil, otherwise just a tag with the
+  # given name.
+  def tag_or_link(namespace, name)
+    namespace.nil? ? content_tag(:span, name) : link_to(name, namespace)
+  end
+
   # Renders the repository part of the activity in a safe manner.
   def render_repository(activity)
     repo, link, tag = get_repo_link_tag(activity)
@@ -59,16 +71,17 @@ def get_repo_link_tag(activity)
     tr = activity.trackable
 
     if tr.nil?
-      if activity.parameters[:repo_name].nil?
+      if repo_name(activity).nil?
         ["a repository", nil, ""]
       else
-        repo = activity.parameters[:repo_name]
+        repo = repo_name(activity)
         ns   = Namespace.find_by(id: activity.parameters[:namespace_id])
         link = ns.nil? ? nil : namespace_path(ns.id)
         [repo, link, tag_part(activity)]
       end
     else
-      [tr.name, tr, tag_part(activity)]
+      name, l = name_and_link(tr, activity)
+      [name, l, tag_part(activity)]
     end
   end
 
@@ -86,4 +99,15 @@ def tag_part(activity)
       ":#{activity.recipient.name}"
     end
   end
+
+  # Fetch the name of the repo from the given activity.
+  def repo_name(activity)
+    activity.parameters[:repo_name] || activity.parameters[:repository_name]
+  end
+
+  # Returns the name and the link to the given tr depending on whether it's a
+  # Namespace or not.
+  def name_and_link(tr, activity)
+    tr.is_a?(Namespace) ? [repo_name(activity), nil] : [tr.name, tr]
+  end
 end

app/jobs/catalog_job.rb

@@ -38,7 +38,8 @@ def update_registry!(catalog)
 
     # At this point, the remaining items in the "repos" array are repos that
     # exist in the DB but not in the catalog. Remove all of them.
-    Tag.where(repository_id: dangling_repos).find_each(&:delete_and_update!)
-    Repository.where(id: dangling_repos).destroy_all
+    portus = User.find_by(username: "portus")
+    Tag.where(repository_id: dangling_repos).find_each { |t| t.delete_and_update!(portus) }
+    Repository.where(id: dangling_repos).find_each { |r| r.delete_and_update!(portus) }
   end
 end

app/models/namespace.rb

@@ -8,9 +8,8 @@
 #  updated_at  :datetime         not null
 #  team_id     :integer
 #  public      :boolean          default("0")
-#  registry_id :integer          not null
+#  registry_id :integer
 #  global      :boolean          default("0")
-#  description :text(65535)
 #
 # Indexes
 #

app/models/repository.rb

@@ -7,6 +7,7 @@
 #  namespace_id :integer
 #  created_at   :datetime         not null
 #  updated_at   :datetime         not null
+#  marked       :boolean          default("0")
 #
 # Indexes
 #
@@ -59,6 +60,33 @@ def groupped_tags
     end
   end
 
+  # Updates the activities related to this repository and adds a new activity
+  # regarding the removal of this.
+  def delete_and_update!(actor)
+    logger.tagged("catalog") { logger.info "Removed the image '#{name}'." }
+
+    # Take care of current activities.
+    PublicActivity::Activity.where(trackable: self).update_all(
+      trackable_type: Namespace,
+      trackable_id:   namespace.id,
+      recipient_type: nil
+    )
+
+    # Add a "delete" activity"
+    namespace.create_activity(
+      :delete,
+      owner:      actor,
+      recipient:  self,
+      parameters: {
+        repository_name: name,
+        namespace_id:    namespace.id,
+        namespace_name:  namespace.clean_name
+      }
+    )
+
+    destroy
+  end
+
   # Handle a push event from the registry.
   def self.handle_push_event(event)
     registry = Registry.find_from_event(event)
@@ -82,11 +110,15 @@ def self.handle_delete_event(event)
     # Fetch the repo.
     ns, repo_name, = registry.get_namespace_from_event(event, false)
     repo = ns.repositories.find_by(name: repo_name)
-    return if repo.nil?
+    return if repo.nil? || repo.marked?
 
     # Destroy tags and the repository if it's empty now.
-    repo.tags.where(digest: event["target"]["digest"]).map(&:delete_and_update!)
-    repo.destroy if !repo.nil? && repo.tags.empty?
+    user = User.find_from_event(event)
+    repo.tags.where(digest: event["target"]["digest"], marked: false).map do |t|
+      t.delete_and_update!(user)
+    end
+    repo = repo.reload
+    repo.delete_and_update!(user) if !repo.nil? && repo.tags.empty?
   end
 
   # Add the repository with the given `repo` name and the given `tag`. The
@@ -169,7 +201,7 @@ def self.create_or_update!(repo)
     end
 
     # Finally remove the tags that are left and return the repo.
-    repository.tags.where(name: to_be_deleted_tags).find_each(&:delete_and_update!)
+    repository.tags.where(name: to_be_deleted_tags).find_each { |t| t.delete_and_update!(portus) }
     repository.reload
   end
 end

app/models/tag.rb

@@ -10,6 +10,7 @@
 #  user_id       :integer
 #  digest        :string(255)
 #  image_id      :string(255)      default("")
+#  marked        :boolean          default("0")
 #
 # Indexes
 #
@@ -29,9 +30,74 @@ class Tag < ActiveRecord::Base
   # and that's guaranteed to have a good format.
   validates :name, uniqueness: { scope: "repository_id" }
 
+  # Delete all the tags that match the given digest. Call this method if you
+  # want to:
+  #
+  # - Safely remove tags (with its re-tags) on the DB.
+  # - Remove the manifest digest on the registry.
+  # - Preserve the activities related to the tags that are to be removed.
+  #
+  # Returns true on success, false otherwise.
+  def delete_by_digest!(actor)
+    dig = fetch_digest
+    return false if dig.blank?
+
+    Tag.where(digest: dig).update_all(marked: true)
+
+    begin
+      Registry.get.client.delete(repository.name, dig, "manifests")
+    rescue StandardError => e
+      Rails.logger.error "Could not delete tag on the registry: #{e.message}"
+      return false
+    end
+
+    Tag.where(digest: dig).map { |t| t.delete_and_update!(actor) }
+  end
+
   # Delete this tag and update its activity.
-  def delete_and_update!
+  def delete_and_update!(actor)
     logger.tagged("catalog") { logger.info "Removed the tag '#{name}'." }
+
+    # If the tag is no longer there, ignore this call and return early.
+    unless Tag.find_by(id: id)
+      logger.tagged("catalog") { logger.info "Ignoring..." }
+      return
+    end
+
+    # Delete tag and create the corresponding activities.
+    destroy
+    create_delete_activities!(actor)
+  end
+
+  protected
+
+  # Fetch the digest for this tag. Usually the digest should already be
+  # initialized since it's provided by the event notification that created this
+  # tag. However, it might happen that the digest column is left blank (e.g.
+  # legacy Portus, unknown error, etc). In these cases, this method will fetch
+  # the manifest from the registry.
+  #
+  # Returns a string containing the digest on success. Otherwise it returns
+  # nil.
+  def fetch_digest
+    if digest.blank?
+      client = Registry.get.client
+
+      begin
+        _, dig, = client.manifest(repository.name, name)
+        update_attributes(digest: dig)
+        dig
+      rescue StandardError => e
+        Rails.logger.error "Could not fetch manifest digest: #{e.message}"
+        nil
+      end
+    else
+      digest
+    end
+  end
+
+  # Create/update the activities for a delete operation.
+  def create_delete_activities!(actor)
     PublicActivity::Activity.where(recipient: self).update_all(
       parameters: {
         namespace_id:   repository.namespace.id,
@@ -40,6 +106,18 @@ def delete_and_update!
         tag_name:       name
       }
     )
-    destroy
+
+    # Create the delete activity.
+    repository.create_activity(
+      :delete,
+      owner:      actor,
+      recipient:  self,
+      parameters: {
+        repository_name: repository.name,
+        namespace_id:    repository.namespace.id,
+        namespace_name:  repository.namespace.clean_name,
+        tag_name:        name
+      }
+    )
   end
 end