Merge pull request #854 from mssola/delete-ui

Added support for removing images/tags

.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/deletable.rb

@@ -0,0 +1,14 @@
+# Deletable redirects the user back if delete support is not enabled. A
+# `before_action` will be created for the :destroy method.
+module Deletable
+  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 Deletable
+
+  before_action :set_repository, only: [:show, :destroy, :toggle_star]
 
   # GET /repositories
   # GET /repositories.json
@@ -23,6 +25,30 @@ 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?
+      ts = @repository.tags.pluck(:name).join(", ")
+      logger.error "The following tags could not be removed: #{ts}."
+      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 Deletable
+
+  # Removes all tags that 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/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