Merge pull request #94 from edx/jsa/heartbeat-no-key

don't require api key for heartbeat.

app.rb

@@ -84,11 +84,10 @@ APIPREFIX = CommentService::API_PREFIX
 DEFAULT_PAGE = 1
 DEFAULT_PER_PAGE = 20
 
-if RACK_ENV.to_s != "test" # disable api_key auth in test environment
-  before do
-    api_key = CommentService.config[:api_key]
-    error 401 unless params[:api_key] == api_key or env["HTTP_X_EDX_API_KEY"] == api_key
-  end
+before do
+  pass if request.path_info == '/heartbeat'
+  api_key = CommentService.config[:api_key]
+  error 401 unless params[:api_key] == api_key or env["HTTP_X_EDX_API_KEY"] == api_key
 end
 
 before do
@@ -170,30 +169,53 @@ end
 
 CommentService.blocked_hashes = Content.mongo_session[:blocked_hash].find.select(hash: 1).each.map {|d| d["hash"]}
 
-get '/heartbeat' do
-  t1 = Time.now
-  # any expected config / environment variables are set to sane values
-  # TODO: is there anything else applicable to check wrt config?
+def get_db_is_master
+  Mongoid::Sessions.default.command(isMaster: 1)
+end
+
+def get_es_status
+  res = Tire::Configuration.client.get Tire::Configuration.url
+  JSON.parse res.body
+end
 
+get '/heartbeat' do
   # mongo is reachable and ready to handle requests
-  db = Mongoid::Sessions.default
-  res = db.command(isMaster: 1)
-  if res["ismaster"] != true or Integer(res["ok"]) != 1
-    error 500, res.to_json
+  db_ok = false
+  begin
+    res = get_db_is_master
+    db_ok = ( res["ismaster"] == true and Integer(res["ok"]) == 1 )
+  rescue
   end
+  error 500, JSON.generate({"OK" => false, "check" => "db"}) unless db_ok
 
   # E_S is reachable and ready to handle requests
-  res = Tire::Configuration.client.get Tire::Configuration.url
-  es_status = JSON.parse res.body
-  if es_status["ok"] != true or es_status["status"] != 200
-    error 500, res.to_json
+  es_ok = false
+  begin
+    es_status = get_es_status
+    es_ok = ( es_status["ok"] == true and es_status["status"] == 200 )
+  rescue
   end
+  error 500, JSON.generate({"OK" => false, "check" => "es"}) unless es_ok
 
-  status = {
-    "last_post_created" => db[:contents].find().sort(_id: -1).limit(1).one["created_at"],
-    "total_posts" => db[:contents].find.count,
-    "total_users" => db[:contents].find.count,
-    "elapsed_time" => Time.now - t1
-  }
-  JSON.generate(status)
+  JSON.generate({"OK" => true})
 end
+
+get '/selftest' do
+  begin
+    t1 = Time.now
+    status = {
+      "db" => get_db_is_master,
+      "es" => get_es_status,
+      "last_post_created" => (Content.last.created_at rescue nil),
+      "total_posts" => Content.count,
+      "total_users" => User.count,
+      "elapsed_time" => Time.now - t1
+    }
+    JSON.generate(status)
+  rescue => ex
+    [ 500,
+      {'Content-Type' => 'text/plain'},
+      "#{ex.backtrace.first}: #{ex.message} (#{ex.class})\n\t#{ex.backtrace[1..-1].join("\n\t")}"
+    ]
+  end
+end
\ No newline at end of file

spec/api/abuse_spec.rb

@@ -34,7 +34,12 @@ end
 
 describe "app" do
   describe "abuse" do
-    before(:each) { init_without_subscriptions }
+
+    before(:each) do
+      init_without_subscriptions
+      set_api_key_header
+    end
+
     describe "flag a comment as abusive" do
       it "create or update the abuse_flags on the comment" do
         comment = Comment.first

spec/api/comment_spec.rb

@@ -2,6 +2,9 @@ require 'spec_helper'
 require 'unicode_shared_examples'
 
 describe "app" do
+
+  before(:each) { set_api_key_header }
+
   describe "comments" do
     before(:each) { init_without_subscriptions }
     describe "GET /api/v1/comments/:comment_id" do

spec/api/comment_thread_spec.rb

@@ -4,6 +4,7 @@ require 'unicode_shared_examples'
 describe "app" do
   describe "comment threads" do
 
+    before(:each) { set_api_key_header }
 
     describe "GET /api/v1/threads" do
 

spec/api/commentable_spec.rb

@@ -3,7 +3,12 @@ require 'unicode_shared_examples'
 
 describe "app" do
   describe "commentables" do
-    before(:each) { init_without_subscriptions }
+
+    before(:each) do
+      init_without_subscriptions
+      set_api_key_header
+    end
+
     describe "DELETE /api/v1/:commentable_id/threads" do
       it "delete all associated threads and comments of a commentable" do
         delete '/api/v1/question_1/threads'

spec/api/i18n_spec.rb

 require "spec_helper"
 
 describe "i18n" do
+
+  before(:each) { set_api_key_header }
+
   it "should respect the Accept-Language header" do
     put "/api/v1/comments/does_not_exist/votes", {}, {"HTTP_ACCEPT_LANGUAGE" => "x-test"}
     last_response.status.should == 400

spec/api/notifications_spec.rb

@@ -2,7 +2,12 @@ require 'spec_helper'
 
 describe "app" do
   describe "notifications" do
-    before(:each) { init_without_subscriptions }
+
+    before(:each) do
+      init_without_subscriptions
+      set_api_key_header
+    end
+
     describe "POST /api/v1/notifications" do
       it "returns notifications by class and user" do
         start_time = Time.now

spec/api/query_spec.rb

 require 'spec_helper'
 
 describe "app" do
-	let(:author) { create_test_user(1) }
-	describe "thread search" do
-		describe "GET /api/v1/search/threads" do
-			it "returns thread with query match" do
-				commentable = Commentable.new("question_1")
 
-				random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
+  before (:each) { set_api_key_header }
 
-				thread = CommentThread.new(title: "Test title", body: random_string, course_id: "1", commentable_id: commentable.id)
-				thread.author = author
-				thread.save!
+  let(:author) { create_test_user(1) }
+  describe "thread search" do
+    describe "GET /api/v1/search/threads" do
+      it "returns thread with query match" do
+        commentable = Commentable.new("question_1")
 
-				sleep 3
+        random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
-				get "/api/v1/search/threads", text: random_string
-				last_response.should be_ok
-				threads = parse(last_response.body)['collection']
-				check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, true)
-			end
+        thread = CommentThread.new(title: "Test title", body: random_string, course_id: "1", commentable_id: commentable.id)
+        thread.author = author
+        thread.save!
 
-		end
-	end
+        sleep 3
 
-	describe "comment search" do
-		describe "GET /api/v1/search/threads" do
-			it "returns thread with comment query match" do
-				commentable = Commentable.new("question_1")
+        get "/api/v1/search/threads", text: random_string
+        last_response.should be_ok
+        threads = parse(last_response.body)['collection']
+        check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, true)
+      end
 
-				random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
+    end
+  end
 
-				thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id)
-				thread.author = author
-				thread.save!
+  describe "comment search" do
+    describe "GET /api/v1/search/threads" do
+      it "returns thread with comment query match" do
+        commentable = Commentable.new("question_1")
 
-				sleep 3
+        random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
-				comment = Comment.new(body: random_string, course_id: "1", commentable_id: commentable.id)
-				comment.author = author
-				comment.comment_thread = thread
-				comment.save!
+        thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id)
+        thread.author = author
+        thread.save!
 
-				sleep 1
+        sleep 3
 
-				get "/api/v1/search/threads", text: random_string
-				last_response.should be_ok
-				threads = parse(last_response.body)['collection']
-				check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, true)
-			end
-		end
-	end
+        comment = Comment.new(body: random_string, course_id: "1", commentable_id: commentable.id)
+        comment.author = author
+        comment.comment_thread = thread
+        comment.save!
+
+        sleep 1
+
+        get "/api/v1/search/threads", text: random_string
+        last_response.should be_ok
+        threads = parse(last_response.body)['collection']
+        check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, true)
+      end
+    end
+  end
 end

spec/api/search_spec.rb

@@ -3,6 +3,9 @@ require 'unicode_shared_examples'
 
 describe "app" do
   describe "search" do
+
+    before (:each) { set_api_key_header }
+
     let(:author) { create_test_user(42) }
 
     describe "GET /api/v1/search/threads" do

spec/api/user_spec.rb

@@ -7,6 +7,7 @@ describe "app" do
       User.delete_all
       create_test_user 1
       create_test_user 2
+      set_api_key_header
     end
     describe "POST /api/v1/users" do
       it "creates a user" do

spec/api/vote_spec.rb

@@ -2,7 +2,12 @@ require 'spec_helper'
 
 describe "app" do
   describe "votes" do
-    before(:each) { init_without_subscriptions }
+
+    before(:each) do
+      init_without_subscriptions
+      set_api_key_header
+    end
+
     describe "PUT /api/v1/comments/:comment_id/votes" do
       it "create or update the vote on the comment" do
         user = User.first

spec/app_spec.rb

+require 'spec_helper'
+
+describe "app" do
+
+  describe "access control" do
+    let(:user) { create_test_user(42) }
+    # all routes (even nonexistent ones) are covered by the api key
+    # /heartbeat is the only exception, covered in the heartbeat tests below
+    let(:urls) { {
+        "/" => 404,
+        "/api/v1/users/#{user.id}" => 200,
+        "/api/v1/users/doesnotexist" => 404,
+        "/selftest" => 200
+      }
+    }
+
+    it "returns 401 when api key header is unset" do
+      urls.each do |url, _|
+        get url
+        last_response.status.should == 401
+      end
+    end
+
+    it "returns 401 when api key value is incorrect" do
+      urls.each do |url, _|
+        get url, {}, {"HTTP_X_EDX_API_KEY" => "incorrect-#{TEST_API_KEY}"}
+        last_response.status.should == 401
+      end
+    end
+
+    it "allows requests when api key value is correct" do
+      urls.each do |url, status|
+        get url, {}, {"HTTP_X_EDX_API_KEY" => TEST_API_KEY}
+        last_response.status.should == status
+      end
+    end
+  end
+
+  describe "heartbeat monitoring" do
+    it "does not require api key" do
+      get "/heartbeat"
+      last_response.status.should == 200
+    end
+
+    context "db check" do
+      def test_db_check(response, is_success)
+        db = double("db")
+        stub_const("Mongoid::Sessions", Class.new).stub(:default).and_return(db)
+        db.should_receive(:command).with({:isMaster => 1}).and_return(response)
+        get "/heartbeat"
+        if is_success
+          last_response.status.should == 200
+          parse(last_response.body).should == {"OK" => true}
+        else
+          last_response.status.should == 500
+          parse(last_response.body).should == {"OK" => false, "check" => "db"}
+        end
+      end
+
+      it "reports success when mongo is ready" do
+        test_db_check({"ismaster" => true, "ok" => 1}, true)
+      end
+
+      it "reports failure when mongo is not master" do
+        test_db_check({"ismaster" => false, "ok" => 1}, false)
+      end
+
+      it "reports failure when mongo is not OK" do
+        test_db_check({"ismaster" => true, "ok" => 0}, false)
+      end
+
+      it "reports failure when command response is unexpected" do
+        test_db_check({"foo" => "bar"}, false)
+      end
+
+      it "reports failure when db command raises an error" do
+        db = double("db")
+        stub_const("Mongoid::Sessions", Class.new).stub(:default).and_return(db)
+        db.should_receive(:command).with({:isMaster => 1}).and_raise(StandardError)
+        get "/heartbeat"
+        last_response.status.should == 500
+        parse(last_response.body).should == {"OK" => false, "check" => "db"}
+      end
+    end
+
+    context "elasticsearch check" do
+      def test_es_check(response, is_success)
+        # fake HTTP call
+        client = double()
+        tire_config = stub_const("Tire::Configuration", Class.new)
+        tire_config.stub(:url).and_return("foo")
+        tire_config.stub(:client).and_return(client)
+        # fake HTTP response based on our response parameter
+        es_response = double()
+        es_response.stub(:body).and_return(JSON.generate(response))
+        client.should_receive(:get).and_return(es_response)
+
+        get "/heartbeat"
+        if is_success
+          last_response.status.should == 200
+          parse(last_response.body).should == {"OK" => true}
+        else
+          last_response.status.should == 500
+          parse(last_response.body).should == {"OK" => false, "check" => "es"}
+        end
+      end
+
+      it "reports success when es is ready" do
+        test_es_check({"ok" => true, "status" => 200}, true)
+      end
+
+      it "reports failure when es is not ok" do
+        test_es_check({"ok" => false, "status" => 200}, false)
+      end
+
+      it "reports failure when es status is unexpected" do
+        test_es_check({"ok" => true, "status" => 201}, false)
+      end
+
+      it "reports failure when es status is malformed" do
+        test_es_check("", false)
+      end
+
+      it "reports failure when the es command raises an error" do
+        client = double()
+        tire_config = stub_const("Tire::Configuration", Class.new)
+        tire_config.stub(:url).and_return("foo")
+        tire_config.stub(:client).and_raise(StandardError)
+        get "/heartbeat"
+        last_response.status.should == 500
+        parse(last_response.body).should == {"OK" => false, "check" => "es"}
+      end
+    end
+  end
+
+  describe "selftest" do
+
+    it "returns valid JSON on success" do
+      get "/selftest", {}, {"HTTP_X_EDX_API_KEY" => TEST_API_KEY}
+      res = parse(last_response.body)
+      %w(db es total_posts total_users last_post_created elapsed_time).each do |k|
+        res.should have_key k
+      end
+    end
+
+    it "handles when the database is empty" do
+      get "/selftest", {}, {"HTTP_X_EDX_API_KEY" => TEST_API_KEY}
+      res = parse(last_response.body)
+      res["total_users"].should == 0
+      res["total_posts"].should == 0
+      res["last_post_created"].should == nil
+    end
+
+    it "handles when the database is not empty" do
+      user = create_test_user(42)
+      thread = make_thread(user, "foo", "abc", "123")
+      get "/selftest", {}, {"HTTP_X_EDX_API_KEY" => TEST_API_KEY}
+      res = parse(last_response.body)
+      res["total_users"].should == 1
+      res["total_posts"].should == 1
+      Time.parse(res["last_post_created"]).to_i.should == thread.created_at.to_i
+    end
+
+    it "displays tracebacks on failure" do
+      Tire::Configuration.client.should_receive(:get).and_raise(StandardError)
+      get "/selftest", {}, {"HTTP_X_EDX_API_KEY" => TEST_API_KEY}
+      last_response.status.should == 500
+      # lightweight assertion that we're seeing a traceback
+      last_response.headers["Content-Type"].should == 'text/plain'
+      last_response.body.should include "StandardError"
+      last_response.body.should include File.expand_path(__FILE__)
+    end
+
+  end
+end
\ No newline at end of file

spec/presenters/thread_search_result_spec.rb

@@ -5,7 +5,7 @@ describe ThreadSearchResultsPresenter do
   
     before(:each) { setup_10_threads }
 
-    # NOTE: throrough coverage of search result hash structure is presently provided in spec/api/search_spec
+    # NOTE: throrough coverage of search result hash structure is presently provided in spec/presenters/thread_spec
     def check_search_result_hash(search_result, hash)
       hash["highlighted_body"].should == ((search_result.highlight[:body] || []).first || hash["body"])
       hash["highlighted_title"].should == ((search_result.highlight[:title] || []).first || hash["title"])

spec/spec_helper.rb

@@ -21,6 +21,13 @@ def app
   Sinatra::Application
 end
 
+TEST_API_KEY = 'comments-service-test-api-key'
+CommentService.config[:api_key] = TEST_API_KEY
+
+def set_api_key_header
+  current_session.header "X-Edx-Api-Key", TEST_API_KEY
+end
+
 RSpec.configure do |config|
   config.include Rack::Test::Methods
   config.treat_symbols_as_metadata_keys_with_true_values = true