From 4bc0bbf2b5850ebfacd6112dbe699d8e914c35a6 Mon Sep 17 00:00:00 2001
From: Aaron Kuehler <aaron.kuehler@gmail.com>
Date: Mon, 16 Dec 2019 17:42:11 -0500
Subject: [PATCH] Elevate the "Repository" concept

Summary
-------

At PowerHRG we desire to implement the ability to automatically deploy
a branch as a new "review instance" of our application to our
Kubernetes cluster when a Pull Request for that branch is opened - and
subsequently clean up the instance when the PR is closed or merged -
functionality similar to Heroku's Review Apps. However, we maintain a
number of projects and do NOT want to expose this functionality for
every repository.

As we understand it, Shipit offers no mechanism at the repository
level to control which repositories can dynamically provision
stacks. It seems the Stack is the highest level concept which involves
a repository, but this remains insufficient since the Stack also
marries a particular branch and environment to the repository
concept. What we _think_ we'd like is a higher level concept which
represents the repository to which stacks could belong. This
repository could then contain its own configuration for the
project-level features we desire.

This change set extracts the repository data from the stack concept
while retaining the original stack API - all repository related
functionality of the Stack is now delegated to the stack's Repository
object.

References
----------

- https://github.com/Shopify/shipit-engine/issues/960
- https://github.com/Shopify/shipit-engine/pull/963
---
 .../shipit/api/stacks_controller.rb           | 21 ++++-
 .../shipit/merge_status_controller.rb         |  8 +-
 app/controllers/shipit/stacks_controller.rb   | 19 ++++-
 app/controllers/shipit/webhooks_controller.rb |  2 +-
 app/models/shipit/repository.rb               | 38 ++++++++++
 app/models/shipit/stack.rb                    | 53 ++++++-------
 ...191209231045_create_shipit_repositories.rb | 12 +++
 ...1307_add_repository_reference_to_stacks.rb | 15 ++++
 ...20191216162728_backfill_repository_data.rb | 20 +++++
 ...move_repository_information_from_stacks.rb | 20 +++++
 .../controllers/api/ccmenu_controller_test.rb |  2 +-
 .../controllers/api/stacks_controller_test.rb | 18 +++--
 .../merge_status_controller_test.rb           |  6 +-
 test/dummy/db/schema.rb                       | 16 +++-
 test/dummy/db/seeds.rb                        |  6 +-
 test/fixtures/shipit/repositories.yml         | 23 ++++++
 test/fixtures/shipit/stacks.yml               | 27 +++----
 test/models/deploys_test.rb                   |  8 +-
 test/models/shipit/repository_test.rb         | 76 +++++++++++++++++++
 test/models/stacks_test.rb                    | 56 ++------------
 20 files changed, 320 insertions(+), 126 deletions(-)
 create mode 100644 app/models/shipit/repository.rb
 create mode 100644 db/migrate/20191209231045_create_shipit_repositories.rb
 create mode 100644 db/migrate/20191209231307_add_repository_reference_to_stacks.rb
 create mode 100644 db/migrate/20191216162728_backfill_repository_data.rb
 create mode 100644 db/migrate/20191216163010_remove_repository_information_from_stacks.rb
 create mode 100644 test/fixtures/shipit/repositories.yml
 create mode 100644 test/models/shipit/repository_test.rb

diff --git a/app/controllers/shipit/api/stacks_controller.rb b/app/controllers/shipit/api/stacks_controller.rb
index 019433dea..608810ace 100644
--- a/app/controllers/shipit/api/stacks_controller.rb
+++ b/app/controllers/shipit/api/stacks_controller.rb
@@ -18,7 +18,10 @@ def index
         accepts :merge_queue_enabled, Boolean
       end
       def create
-        render_resource Stack.create(params)
+        stack = Stack.new(create_params)
+        stack.repository = repository
+        stack.save
+        render_resource stack
       end
 
       def show
@@ -32,9 +35,25 @@ def destroy
 
       private
 
+      def create_params
+        params.reject { |key, _| %i(repo_owner repo_name).include?(key) }
+      end
+
       def stack
         @stack ||= stacks.from_param!(params[:id])
       end
+
+      def repository
+        @repository ||= Repository.find_or_create_by(owner: repo_owner, name: repo_name)
+      end
+
+      def repo_owner
+        params[:repo_owner]
+      end
+
+      def repo_name
+        params[:repo_name]
+      end
     end
   end
 end
diff --git a/app/controllers/shipit/merge_status_controller.rb b/app/controllers/shipit/merge_status_controller.rb
index 1901d19f8..1ef1facde 100644
--- a/app/controllers/shipit/merge_status_controller.rb
+++ b/app/controllers/shipit/merge_status_controller.rb
@@ -60,9 +60,11 @@ def stack
       @stack ||= if params[:stack_id]
         Stack.from_param!(params[:stack_id])
       else
-        scope = Stack.order(merge_queue_enabled: :desc, id: :asc).where(
-          repo_owner: referrer_parser.repo_owner,
-          repo_name: referrer_parser.repo_name,
+        scope = Stack.order(merge_queue_enabled: :desc, id: :asc).includes(:repository).where(
+          repositories: {
+            owner: referrer_parser.repo_owner,
+            name: referrer_parser.repo_name,
+          },
         )
         scope = if params[:branch]
           scope.where(branch: params[:branch])
diff --git a/app/controllers/shipit/stacks_controller.rb b/app/controllers/shipit/stacks_controller.rb
index ffd5a7c58..ba80100cf 100644
--- a/app/controllers/shipit/stacks_controller.rb
+++ b/app/controllers/shipit/stacks_controller.rb
@@ -48,6 +48,7 @@ def lookup
 
     def create
       @stack = Stack.new(create_params)
+      @stack.repository = repository
       unless @stack.save
         flash[:warning] = @stack.errors.full_messages.to_sentence
       end
@@ -112,7 +113,7 @@ def load_stack
     end
 
     def create_params
-      params.require(:stack).permit(:repo_name, :repo_owner, :environment, :branch, :deploy_url, :ignore_ci)
+      params.require(:stack).permit(:environment, :branch, :deploy_url, :ignore_ci)
     end
 
     def update_params
@@ -124,5 +125,21 @@ def update_params
         :merge_queue_enabled,
       )
     end
+
+    def repository
+      @repository ||= Repository.find_or_create_by(owner: repo_owner, name: repo_name)
+    end
+
+    def repo_owner
+      repository_params[:repo_owner]
+    end
+
+    def repo_name
+      repository_params[:repo_name]
+    end
+
+    def repository_params
+      params.require(:stack).permit(:repo_owner, :repo_name)
+    end
   end
 end
diff --git a/app/controllers/shipit/webhooks_controller.rb b/app/controllers/shipit/webhooks_controller.rb
index 49874874e..1e4efd114 100644
--- a/app/controllers/shipit/webhooks_controller.rb
+++ b/app/controllers/shipit/webhooks_controller.rb
@@ -28,7 +28,7 @@ def process
       private
 
       def stacks
-        @stacks ||= Stack.repo(repository_name)
+        @stacks ||= Repository.from_github_repo_name(repository_name).stacks
       end
 
       def repository_name
diff --git a/app/models/shipit/repository.rb b/app/models/shipit/repository.rb
new file mode 100644
index 000000000..2f9368590
--- /dev/null
+++ b/app/models/shipit/repository.rb
@@ -0,0 +1,38 @@
+module Shipit
+  class Repository < ApplicationRecord
+    OWNER_MAX_SIZE = 39
+    private_constant :OWNER_MAX_SIZE
+
+    NAME_MAX_SIZE = 100
+    private_constant :NAME_MAX_SIZE
+
+    validates :name, uniqueness: {scope: %i(owner), case_sensitive: false,
+                                  message: 'cannot be used more than once'}
+    validates :owner, :name, presence: true, ascii_only: true
+    validates :owner, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: OWNER_MAX_SIZE}
+    validates :name, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: NAME_MAX_SIZE}
+
+    has_many :stacks, dependent: :destroy
+
+    def self.from_github_repo_name(github_repo_name)
+      repo_owner, repo_name = github_repo_name.downcase.split('/')
+      find_by(owner: repo_owner, name: repo_name)
+    end
+
+    def name=(n)
+      super(n&.downcase)
+    end
+
+    def owner=(o)
+      super(o&.downcase)
+    end
+
+    def http_url
+      Shipit.github.url("#{owner}/#{name}")
+    end
+
+    def git_url
+      "https://#{Shipit.github.domain}/#{owner}/#{name}.git"
+    end
+  end
+end
diff --git a/app/models/shipit/stack.rb b/app/models/shipit/stack.rb
index b4c2a54a0..789964b0d 100644
--- a/app/models/shipit/stack.rb
+++ b/app/models/shipit/stack.rb
@@ -22,8 +22,6 @@ def blank?
       end
     end
 
-    REPO_OWNER_MAX_SIZE = 39
-    REPO_NAME_MAX_SIZE = 100
     ENVIRONMENT_MAX_SIZE = 50
     REQUIRED_HOOKS = %i(push status).freeze
 
@@ -40,6 +38,12 @@ def blank?
     has_many :hooks, dependent: :destroy
     has_many :api_clients, dependent: :destroy
     belongs_to :lock_author, class_name: :User, optional: true
+    belongs_to :repository
+    validates_associated :repository
+
+    def repository
+      super || build_repository
+    end
 
     def lock_author(*)
       super || AnonymousUser.new
@@ -51,7 +55,7 @@ def lock_author=(user)
 
     def self.repo(full_name)
       repo_owner, repo_name = full_name.downcase.split('/')
-      where(repo_owner: repo_owner, repo_name: repo_name)
+      Repository.find_by(owner: repo_owner, name: repo_name).stacks
     end
 
     before_validation :update_defaults
@@ -66,11 +70,8 @@ def self.repo(full_name)
     after_commit :sync_github, on: :create
     after_commit :schedule_merges_if_necessary, on: :update
 
-    validates :repo_name, uniqueness: {scope: %i(repo_owner environment), case_sensitive: false,
-                                       message: 'cannot be used more than once with this environment'}
-    validates :repo_owner, :repo_name, :environment, presence: true, ascii_only: true
-    validates :repo_owner, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: REPO_OWNER_MAX_SIZE}
-    validates :repo_name, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: REPO_NAME_MAX_SIZE}
+    validates :repository, uniqueness: {scope: %i(environment), case_sensitive: false,
+                                        message: 'cannot be used more than once with this environment'}
     validates :environment, format: {with: /\A[a-z0-9\-_\:]+\z/}, length: {maximum: ENVIRONMENT_MAX_SIZE}
     validates :deploy_url, format: {with: URI.regexp(%w(http https ssh))}, allow_blank: true
 
@@ -321,21 +322,12 @@ def merge_method
       cached_deploy_spec&.pull_request_merge_method || Shipit.default_merge_method
     end
 
-    def repo_name=(name)
-      super(name&.downcase)
-    end
-
-    def repo_owner=(name)
-      super(name&.downcase)
-    end
-
-    def repo_http_url
-      Shipit.github.url("#{repo_owner}/#{repo_name}")
-    end
-
-    def repo_git_url
-      "https://#{Shipit.github.domain}/#{repo_owner}/#{repo_name}.git"
-    end
+    delegate :name=, to: :repository, prefix: :repo
+    delegate :name, to: :repository, prefix: :repo
+    delegate :owner=, to: :repository, prefix: :repo
+    delegate :owner, to: :repository, prefix: :repo
+    delegate :http_url, to: :repository, prefix: :repo
+    delegate :git_url, to: :repository, prefix: :repo
 
     def base_path
       Rails.root.join('data', 'stacks', repo_owner, repo_name, environment)
@@ -390,7 +382,7 @@ def refresh_repository!
       if resource.try(:message) == 'Moved Permanently'
         resource = Shipit.github.api.get(resource.url)
       end
-      update!(repo_owner: resource.owner.login, repo_name: resource.name)
+      repository.update!(owner: resource.owner.login, name: resource.name)
     end
 
     def active_task?
@@ -430,11 +422,14 @@ def self.run_deploy_in_foreground(stack:, revision:)
 
     def self.from_param!(param)
       repo_owner, repo_name, environment = param.split('/')
-      where(
-        repo_owner: repo_owner.downcase,
-        repo_name: repo_name.downcase,
-        environment: environment,
-      ).first!
+      includes(:repository)
+        .where(
+          repositories: {
+            owner: repo_owner.downcase,
+            name: repo_name.downcase,
+          },
+          environment: environment,
+        ).first!
     end
 
     delegate :plugins, :task_definitions, :hidden_statuses, :required_statuses, :soft_failing_statuses,
diff --git a/db/migrate/20191209231045_create_shipit_repositories.rb b/db/migrate/20191209231045_create_shipit_repositories.rb
new file mode 100644
index 000000000..8e9023f3c
--- /dev/null
+++ b/db/migrate/20191209231045_create_shipit_repositories.rb
@@ -0,0 +1,12 @@
+class CreateShipitRepositories < ActiveRecord::Migration[6.0]
+  def change
+    create_table :repositories do |t|
+      t.string :owner, limit: 100, null: false
+      t.string :name, limit: 39, null: false
+
+      t.timestamps
+    end
+
+    add_index :repositories, ["owner", "name"], name: "repository_unicity", unique: true
+  end
+end
diff --git a/db/migrate/20191209231307_add_repository_reference_to_stacks.rb b/db/migrate/20191209231307_add_repository_reference_to_stacks.rb
new file mode 100644
index 000000000..9f4504c07
--- /dev/null
+++ b/db/migrate/20191209231307_add_repository_reference_to_stacks.rb
@@ -0,0 +1,15 @@
+class AddRepositoryReferenceToStacks < ActiveRecord::Migration[6.0]
+  def up
+    change_table(:stacks) do |t|
+      t.references :repository
+    end
+  end
+
+  def down
+    change_column :stacks, :repo_name, :string, null: false
+    change_column :stacks, :repo_owner, :string, null: false
+    change_table(:stacks) do |t|
+      t.remove_references :repository
+    end
+  end
+end
diff --git a/db/migrate/20191216162728_backfill_repository_data.rb b/db/migrate/20191216162728_backfill_repository_data.rb
new file mode 100644
index 000000000..8b5e8f9ac
--- /dev/null
+++ b/db/migrate/20191216162728_backfill_repository_data.rb
@@ -0,0 +1,20 @@
+class BackfillRepositoryData < ActiveRecord::Migration[6.0]
+  def up
+    repositories = Shipit::Stack.distinct.select(:repo_owner, :repo_name).pluck(:repo_owner, :repo_name)
+    repositories.each do |repo_owner, repo_name|
+      repository = Shipit::Repository.create_or_find_by(
+        owner: repo_owner,
+        name: repo_name,
+      )
+
+      stacks = Shipit::Stack.where(repo_owner: repository.owner, repo_name: repository.name)
+      stacks.update(repository: repository)
+    end
+  end
+
+  def down
+    Shipit::Repository.find_each do |repository|
+      repository.stacks.update_all(repo_owner: repository.owner, repo_name: repository.name)
+    end
+  end
+end
diff --git a/db/migrate/20191216163010_remove_repository_information_from_stacks.rb b/db/migrate/20191216163010_remove_repository_information_from_stacks.rb
new file mode 100644
index 000000000..2def05c6a
--- /dev/null
+++ b/db/migrate/20191216163010_remove_repository_information_from_stacks.rb
@@ -0,0 +1,20 @@
+class RemoveRepositoryInformationFromStacks < ActiveRecord::Migration[6.0]
+  def up
+    change_column :stacks, :repository_id, :integer, null: false
+    change_table(:stacks) do |t|
+      t.remove_index ["repo_owner", "repo_name", "environment"]
+      t.remove :repo_owner
+      t.remove :repo_name
+      t.index ["repository_id", "environment"], name: "stack_unicity", unique: true
+    end
+  end
+
+  def down
+    change_table(:stacks) do |t|
+      t.column :repo_name, :string, limit: 100
+      t.column :repo_owner, :string, limit: 39
+      t.remove_index ["repository_id", "environment"]
+      t.index ["repo_owner", "repo_name", "environment"], name: "stack_unicity", unique: true
+    end
+  end
+end
diff --git a/test/controllers/api/ccmenu_controller_test.rb b/test/controllers/api/ccmenu_controller_test.rb
index 5584d6d79..8387f0cc9 100644
--- a/test/controllers/api/ccmenu_controller_test.rb
+++ b/test/controllers/api/ccmenu_controller_test.rb
@@ -43,7 +43,7 @@ class CCMenuControllerTest < ActionController::TestCase
       end
 
       test "stacks with no deploys render correctly" do
-        stack = Stack.create!(repo_owner: 'foo', repo_name: 'bar')
+        stack = Stack.create!(repository: Repository.new(owner: "foo", name: "bar"))
         get :show, params: {stack_id: stack.to_param}
         assert_payload 'lastBuildStatus', 'Success'
       end
diff --git a/test/controllers/api/stacks_controller_test.rb b/test/controllers/api/stacks_controller_test.rb
index 606784c58..4abf0c637 100644
--- a/test/controllers/api/stacks_controller_test.rb
+++ b/test/controllers/api/stacks_controller_test.rb
@@ -25,7 +25,7 @@ class StacksControllerTest < ActionController::TestCase
           post :create, params: {repo_owner: 'some', repo_name: 'owner/path'}
         end
         assert_response :unprocessable_entity
-        assert_json 'errors', 'repo_name' => ['is invalid']
+        assert_json 'errors', 'repository' => ['is invalid']
       end
 
       test "#create creates a stack and renders it back" do
@@ -38,19 +38,25 @@ class StacksControllerTest < ActionController::TestCase
       end
 
       test "#create fails to create stack if it already exists" do
-        Stack.create!(
-          repo_name: 'rails',
-          repo_owner: 'rails',
+        repository = shipit_repositories(:rails)
+        existing_stack = Stack.create!(
+          repository: repository,
           environment: 'staging',
           branch: 'staging',
         )
 
         assert_no_difference -> { Stack.count } do
-          post :create, params: {repo_name: 'rails', repo_owner: 'rails', environment: 'staging', branch: 'staging'}
+          post :create,
+               params: {
+                 repo_name: existing_stack.repo_name,
+                 repo_owner: existing_stack.repo_owner,
+                 environment: existing_stack.environment,
+                 branch: existing_stack.branch,
+               }
         end
 
         assert_response :unprocessable_entity
-        assert_json 'errors', 'repo_name' => ['cannot be used more than once with this environment']
+        assert_json 'errors', 'repository' => ['cannot be used more than once with this environment']
       end
 
       test "#index returns a list of stacks" do
diff --git a/test/controllers/merge_status_controller_test.rb b/test/controllers/merge_status_controller_test.rb
index 3658a570e..b6ecc9246 100644
--- a/test/controllers/merge_status_controller_test.rb
+++ b/test/controllers/merge_status_controller_test.rb
@@ -37,13 +37,11 @@ class MergeStatusControllerTest < ActionController::TestCase
     test "GET show prefers stacks with merge_queue_enabled" do
       existing = shipit_stacks(:shipit)
       Shipit::Stack.where(
-        repo_owner: existing.repo_owner,
-        repo_name: existing.repo_name,
+        repository: existing.repository,
       ).update_all(merge_queue_enabled: false)
 
       Shipit::Stack.create(
-        repo_owner: existing.repo_owner,
-        repo_name: existing.repo_name,
+        repository: existing.repository,
         environment: 'foo',
         branch: existing.branch,
         merge_queue_enabled: true,
diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb
index 47b258fa3..f0c4bf040 100644
--- a/test/dummy/db/schema.rb
+++ b/test/dummy/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_05_02_020249) do
+ActiveRecord::Schema.define(version: 2019_12_16_163010) do
 
   create_table "api_clients", force: :cascade do |t|
     t.text "permissions", limit: 65535
@@ -190,9 +190,15 @@
     t.index ["user_id"], name: "index_deploy_statuses_on_user_id"
   end
 
+  create_table "repositories", force: :cascade do |t|
+    t.string "owner", limit: 100, null: false
+    t.string "name", limit: 39, null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["owner", "name"], name: "repository_unicity", unique: true
+  end
+
   create_table "stacks", force: :cascade do |t|
-    t.string "repo_name", limit: 100, null: false
-    t.string "repo_owner", limit: 39, null: false
     t.string "environment", limit: 50, default: "production", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
@@ -211,7 +217,9 @@
     t.datetime "locked_since"
     t.boolean "merge_queue_enabled", default: false, null: false
     t.datetime "last_deployed_at"
-    t.index ["repo_owner", "repo_name", "environment"], name: "stack_unicity", unique: true
+    t.integer "repository_id", null: false
+    t.index ["repository_id", "environment"], name: "stack_unicity", unique: true
+    t.index ["repository_id"], name: "index_stacks_on_repository_id"
   end
 
   create_table "statuses", force: :cascade do |t|
diff --git a/test/dummy/db/seeds.rb b/test/dummy/db/seeds.rb
index 49571d595..c057160c1 100644
--- a/test/dummy/db/seeds.rb
+++ b/test/dummy/db/seeds.rb
@@ -25,8 +25,10 @@ module Shipit
 
   stacks = 3.times.map do
     Stack.create!(
-      repo_name:  Faker::Internet.domain_name.parameterize,
-      repo_owner: Faker::Company.name.parameterize,
+      repository: Repository.find_or_create_by(
+        name:  Faker::Internet.domain_name.parameterize,
+        owner: Faker::Company.name.parameterize
+      ),
       deploy_url:  "https://#{Faker::Internet.domain_name.parameterize}.#{Faker::Internet.domain_suffix}/",
       cached_deploy_spec: DeploySpec.load(%(
         {
diff --git a/test/fixtures/shipit/repositories.yml b/test/fixtures/shipit/repositories.yml
new file mode 100644
index 000000000..d0abf68d6
--- /dev/null
+++ b/test/fixtures/shipit/repositories.yml
@@ -0,0 +1,23 @@
+shipit:
+  owner: shopify
+  name: shipit-engine
+
+cyclimse:
+  owner: george
+  name: cyclimse
+
+foo-bar:
+  owner: shopify
+  name: foo-bar
+
+soc:
+  owner: "shopify"
+  name: "soc"
+
+check_runs:
+  owner: shopify
+  name: check_runs
+
+rails:
+  owner: rails
+  name: rails
diff --git a/test/fixtures/shipit/stacks.yml b/test/fixtures/shipit/stacks.yml
index b1f69f641..1a69c28b3 100644
--- a/test/fixtures/shipit/stacks.yml
+++ b/test/fixtures/shipit/stacks.yml
@@ -1,6 +1,5 @@
 shipit:
-  repo_owner: "shopify"
-  repo_name: "shipit-engine"
+  repository: shipit
   environment: "production"
   branch: master
   ignore_ci: false
@@ -54,8 +53,7 @@ shipit:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 shipit_canaries:
-  repo_owner: "shopify"
-  repo_name: "shipit-engine"
+  repository: shipit
   environment: "canaries"
   branch: master
   ignore_ci: false
@@ -114,8 +112,7 @@ shipit_canaries:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 cyclimse:
-  repo_owner: george
-  repo_name: cyclimse
+  repository: cyclimse
   environment: production
   branch: master
   ignore_ci: false
@@ -148,8 +145,7 @@ cyclimse:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 undeployed_stack:
-  repo_owner: "shopify"
-  repo_name: "foo-bar"
+  repository: foo-bar
   environment: "production"
   branch: master
   ignore_ci: true
@@ -185,8 +181,7 @@ undeployed_stack:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 soc:
-  repo_owner: "shopify"
-  repo_name: "soc"
+  repository: soc
   environment: "production"
   branch: master
   tasks_count: 0
@@ -220,16 +215,14 @@ soc:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 check_runs:
-  repo_owner: shopify
-  repo_name: check_runs
+  repository: check_runs
   environment: production
   branch: master
   tasks_count: 0
   undeployed_commits_count: 1
 
 shipit_undeployed:
-  repo_owner: "shopify"
-  repo_name: "shipit-engine"
+  repository: shipit
   environment: "undeployed"
   branch: master
   ignore_ci: true
@@ -284,8 +277,7 @@ shipit_undeployed:
   updated_at: <%= 8.days.ago.to_s(:db) %>
 
 shipit_single:
-  repo_owner: "shopify"
-  repo_name: "shipit-engine"
+  repository: shipit
   environment: "single"
   branch: master
   ignore_ci: false
@@ -322,8 +314,7 @@ shipit_single:
   last_deployed_at: <%= 8.days.ago.to_s(:db) %>
 
 shipit_stats:
-  repo_owner: "shopify"
-  repo_name: "shipit-engine"
+  repository: shipit
   environment: "stats"
   branch: master
   ignore_ci: false
diff --git a/test/models/deploys_test.rb b/test/models/deploys_test.rb
index dd81e620f..2bf0280ff 100644
--- a/test/models/deploys_test.rb
+++ b/test/models/deploys_test.rb
@@ -340,10 +340,9 @@ def setup
       assert_equal shipit_commits(:second), @stack.last_deployed_commit
     end
 
-    def create_test_stack
+    def create_test_stack(repository: Shipit::Repository.find_or_create_by!(owner: "shopify-test", name: "shipit-engine-test"))
       Shipit::Stack.create(
-        repo_owner: "shopify-test",
-        repo_name: "shipit-engine-test",
+        repository: repository,
         environment: 'production',
         branch: "master",
         merge_queue_enabled: true,
@@ -554,8 +553,7 @@ def generate_commits(amount:, stack_id:, user_id:, validate:)
       user_id = @user.id
       test_stack = create_test_stack
       test_stack.save
-      other_stack = create_test_stack
-      other_stack.repo_name += "_other"
+      other_stack = create_test_stack(repository: Shipit::Repository.create!(owner: "_", name: "_some-other-repository"))
       other_stack.save
       other_stack.reload
       stack_id = test_stack.id
diff --git a/test/models/shipit/repository_test.rb b/test/models/shipit/repository_test.rb
new file mode 100644
index 000000000..5aca92bcc
--- /dev/null
+++ b/test/models/shipit/repository_test.rb
@@ -0,0 +1,76 @@
+require 'test_helper'
+
+module Shipit
+  class RepositoryTest < ActiveSupport::TestCase
+    setup do
+      @repository = shipit_repositories(:shipit)
+    end
+
+    test "owner, and name uniqueness is enforced" do
+      clone = Repository.new(@repository.attributes.except('id'))
+      refute clone.save
+      assert_equal ["cannot be used more than once"], clone.errors[:name]
+    end
+
+    test "owner, name, and environment can only be ASCII" do
+      @repository.update(owner: 'héllò', name: 'wørld')
+      refute_predicate @repository, :valid?
+    end
+
+    test "owner and name are case insensitive" do
+      assert_no_difference -> { Repository.count } do
+        error = assert_raises ActiveRecord::RecordInvalid do
+          Repository.create!(
+            owner: @repository.owner.upcase,
+            name: @repository.name.upcase,
+          )
+        end
+        assert_equal 'Validation failed: Name cannot be used more than once', error.message
+      end
+
+      new_repository = Repository.create!(owner: 'FOO', name: 'BAR')
+      assert_equal new_repository, Repository.find_by(owner: 'foo', name: 'bar')
+    end
+
+    test "owner is automatically downcased" do
+      @repository.owner = 'George'
+      assert_equal 'george', @repository.owner
+    end
+
+    test "name is automatically downcased" do
+      @repository.name = 'Cyclim.se'
+      assert_equal 'cyclim.se', @repository.name
+    end
+
+    test "owner cannot contain a `/`" do
+      assert @repository.valid?
+      @repository.owner = 'foo/bar'
+      refute @repository.valid?
+    end
+
+    test "name cannot contain a `/`" do
+      assert @repository.valid?
+      @repository.name = 'foo/bar'
+      refute @repository.valid?
+    end
+
+    test "http_url" do
+      assert_equal "https://github.com/#{@repository.owner}/#{@repository.name}", @repository.http_url
+    end
+
+    test "git_url" do
+      assert_equal "https://github.com/#{@repository.owner}/#{@repository.name}.git", @repository.git_url
+    end
+
+    test "from_github_repo_name" do
+      owner = "repository-owner"
+      name = "repository-name"
+      github_repo_name = [owner, name].join("/")
+      expected_repository = Repository.create(owner: owner, name: name)
+
+      found_repository = Repository.from_github_repo_name(github_repo_name)
+
+      assert_equal(expected_repository, found_repository)
+    end
+  end
+end
diff --git a/test/models/stacks_test.rb b/test/models/stacks_test.rb
index 56ef169a5..419484df5 100644
--- a/test/models/stacks_test.rb
+++ b/test/models/stacks_test.rb
@@ -9,43 +9,6 @@ def setup
       GithubHook.any_instance.stubs(:teardown!)
     end
 
-    test "repo_owner, repo_name and environment uniqueness is enforced" do
-      clone = Stack.new(@stack.attributes.except('id'))
-      refute clone.save
-      assert_equal ["cannot be used more than once with this environment"], clone.errors[:repo_name]
-    end
-
-    test "repo_owner, repo_name, and environment can only be ASCII" do
-      @stack.update(repo_owner: 'héllò', repo_name: 'wørld', environment: 'pródüctïòn')
-      refute_predicate @stack, :valid?
-    end
-
-    test "repo_owner and repo_name are case insensitive" do
-      assert_no_difference -> { Stack.count } do
-        error = assert_raises ActiveRecord::RecordInvalid do
-          Stack.create!(
-            repo_owner: @stack.repo_owner.upcase,
-            repo_name: @stack.repo_name.upcase,
-            environment: @stack.environment,
-          )
-        end
-        assert_equal 'Validation failed: Repo name cannot be used more than once with this environment', error.message
-      end
-
-      new_stack = Stack.create!(repo_owner: 'FOO', repo_name: 'BAR')
-      assert_equal new_stack, Stack.find_by(repo_owner: 'foo', repo_name: 'bar')
-    end
-
-    test "repo_owner is automatically downcased" do
-      @stack.repo_owner = 'George'
-      assert_equal 'george', @stack.repo_owner
-    end
-
-    test "repo_name is automatically downcased" do
-      @stack.repo_name = 'Cyclim.se'
-      assert_equal 'cyclim.se', @stack.repo_name
-    end
-
     test "branch defaults to master" do
       @stack.branch = ""
       assert @stack.save
@@ -64,18 +27,6 @@ def setup
       assert_equal 'foo:bar', @stack.environment
     end
 
-    test "repo_owner cannot contain a `/`" do
-      assert @stack.valid?
-      @stack.repo_owner = 'foo/bar'
-      refute @stack.valid?
-    end
-
-    test "repo_name cannot contain a `/`" do
-      assert @stack.valid?
-      @stack.repo_name = 'foo/bar'
-      refute @stack.valid?
-    end
-
     test "repo_http_url" do
       assert_equal "https://github.com/#{@stack.repo_owner}/#{@stack.repo_name}", @stack.repo_http_url
     end
@@ -225,7 +176,7 @@ def setup
 
     test "#create queues a GithubSyncJob" do
       assert_enqueued_with(job: GithubSyncJob) do
-        Stack.create!(repo_name: 'rails', repo_owner: 'rails')
+        Stack.create!(repository: shipit_repositories(:rails))
       end
     end
 
@@ -273,7 +224,10 @@ def setup
     end
 
     test ".run_deploy_in_foreground triggers a deploy" do
-      stack = Stack.create!(repo_owner: 'foo', repo_name: 'bar', environment: 'production')
+      stack = Stack.create!(
+        repository: Repository.new(owner: "foo", name: "bar"),
+        environment: 'production',
+      )
       commit = shipit_commits(:first)
       stack.commits << commit