Merge f610685 into 9e426ab

.codeclimate.yml

@@ -5,6 +5,9 @@ checks:
   identical-code:
     config:
       threshold: 40 # Unfortunately, we have a lot of duplicate code in places like lib/inspec/control_eval_context.rb
+  method-complexity: # 'Cognitive Complexity' in the UI
+    config:
+      threshold: 10 # 5 Default 5 is really tight
 plugins:
   fixme:
     enabled: true

docs/profiles.md

@@ -212,9 +212,7 @@ depends:
 
 ### git
 
-A `git` setting specifies a profile that is located in a git repository, with optional settings for branch, tag, commit, and version. The source location is translated into a URL upon resolution. This type of dependency supports version constraints via semantic versioning as git tags.
-
-For example:
+A `git` setting specifies a profile that is located in a git repository, with optional settings for branch, tag, commit, version, and relative_path. The source location is translated into a URL upon resolution. This type of dependency supports version constraints via semantic versioning as git tags.
 
 ```YAML
 depends:
@@ -224,6 +222,7 @@ depends:
   tag:     desired_version
   commit:  pinned_commit
   version: semver_via_tags
+  relative_path: relative/optional/path/to/profile
 ```
 
 ### supermarket

lib/fetchers/git.rb

@@ -39,36 +39,75 @@ def initialize(remote_url, opts = {})
       @branch = opts[:branch]
       @tag = opts[:tag]
       @ref = opts[:ref]
-      @remote_url = remote_url
+      @remote_url = expand_local_path(remote_url)
       @repo_directory = nil
+      @relative_path = opts[:relative_path] if opts[:relative_path] && !opts[:relative_path].empty?
     end
 
-    def fetch(dir)
-      @repo_directory = dir
-      FileUtils.mkdir_p(dir) unless Dir.exist?(dir)
+    def expand_local_path(url_or_file_path)
+      # This paths to local on-disk repos, not relative paths within repos.
+      # This is especially needed with testing.
+
+      # We could try to do something clever with URI
+      # processing, but then again, if you passed a relative path
+      # to an on-disk repo, you probably expect it to exist.
+      return url_or_file_path unless File.exist?(url_or_file_path)
+      # It's important to expand this path, because it may be specified
+      # locally in the metadata files, and when we clone, we will be
+      # in a temp dir.
+      File.expand_path(url_or_file_path)
+    end
+
+    def fetch(destination_path)
+      @repo_directory = destination_path # Might be the cache, or vendoring, or something else
+      FileUtils.mkdir_p(destination_path) unless Dir.exist?(destination_path)
 
       if cloned?
         checkout
       else
-        Dir.mktmpdir do |tmpdir|
-          checkout(tmpdir)
-          Inspec::Log.debug("Checkout of #{resolved_ref} successful. Moving checkout to #{dir}")
-          FileUtils.cp_r(tmpdir + "/.", @repo_directory)
+        Dir.mktmpdir do |working_dir|
+          checkout(working_dir)
+          if @relative_path
+            perform_relative_path_fetch(destination_path, working_dir)
+          else
+            Inspec::Log.debug("Checkout of #{resolved_ref} successful. " \
+                              "Moving checkout to #{destination_path}")
+            FileUtils.cp_r(working_dir + "/.", destination_path)
+          end
         end
       end
       @repo_directory
     end
 
+    def perform_relative_path_fetch(destination_path, working_dir)
+      Inspec::Log.debug("Checkout of #{resolved_ref} successful. " \
+                        "Moving #{@relative_path} to #{destination_path}")
+      unless File.exist?("#{working_dir}/#{@relative_path}")
+        # Cleanup the destination path - otherwise we'll have an empty dir
+        # in the cache, which is enough to confuse the cache reader
+        # This is a courtesy, assuming we're writing to the cache; if we're
+        # vendoring to something more complex, don't bother.
+        FileUtils.rmdir(destination_path) if Dir.empty?(destination_path)
+
+        raise ArgumentError, "Cannot find relative path '#{@relative_path}' " \
+                             "within profile in git repo specified by '#{@remote_url}'"
+      end
+      FileUtils.cp_r("#{working_dir}/#{@relative_path}", destination_path)
+    end
+
     def cache_key
-      resolved_ref
+      return resolved_ref unless @relative_path
+      OpenSSL::Digest::SHA256.hexdigest(resolved_ref + @relative_path)
     end
 
     def archive_path
       @repo_directory
     end
 
     def resolved_source
-      { git: @remote_url, ref: resolved_ref }
+      source = { git: @remote_url, ref: resolved_ref }
+      source[:relative_path] = @relative_path if @relative_path
+      source
     end
 
     private

test/functional/git_fetcher_test.rb

@@ -0,0 +1,129 @@
+require "functional/helper"
+require "fileutils"
+require "tmpdir"
+
+describe "running profiles with git-based dependencies" do
+  include FunctionalHelper
+  let(:git_profiles) { "#{profile_path}/git-fetcher" }
+
+  #======================================================================#
+  #                         Git Repo Setup
+  #======================================================================#
+  fixture_repos = ["basic-local", "git-repo-01"]
+
+  before(:all) do
+    skip_windows! # Right now, this is due to symlinking
+
+    # We need a git repo for some of the profile test fixtures,
+    # but we can't store those directly in git.
+    # Here, one approach is to store the .git/ directory under a
+    # different name and then symlink to its proper name.
+    fixture_repos.each do |profile_name|
+      link_src = "#{git_profiles}/#{profile_name}/git-fixture"
+      link_dst = "#{git_profiles}/#{profile_name}/.git"
+      FileUtils.ln_sf(link_src, link_dst) # -f to tolerate existing links created during manual testing
+    end
+  end
+
+  after(:all) do
+    fixture_repos.each do |profile_name|
+      link = "#{git_profiles}/#{profile_name}/.git"
+      FileUtils.rm(link)
+    end
+  end
+
+  #======================================================================#
+  #                        Custom Local Assertions
+  #======================================================================#
+  def assert_relative_fetch_works(profile_name, expected_profiles, expected_controls)
+    run_result = run_inspec_process("exec #{git_profiles}/#{profile_name}", json: true)
+    assert_empty run_result.stderr
+    run_result.must_have_all_controls_passing
+
+    # Should know about the top-level profile and the child profile
+    assert_equal expected_profiles, (run_result.payload.json["profiles"].map { |p| p["name"] })
+
+    controls = run_result.payload.json["profiles"].map { |p| p["controls"] }.flatten.map { |c| c["id"] }.uniq
+    # Should have controls from the top-level and included child profile
+    expected_controls.each { |control| assert_includes controls, control }
+
+    # should not have controls from the profile defined at the top of the repo of the child profile
+    refute_includes controls, "red-dye"
+  end
+
+  #======================================================================#
+  #                        Basic Git Fetching
+  #======================================================================#
+  describe "running a profile with a basic local dependency" do
+    it "should work on a local checkout" do
+      run_result = run_inspec_process("exec #{git_profiles}/basic-local", json: true)
+      assert_empty run_result.stderr
+      run_result.must_have_all_controls_passing
+    end
+  end
+  # describe "running a profile with a basic remote dependency"
+
+  # TODO: move private SSH+git test from inspec_exec_test to here
+
+  #======================================================================#
+  #                        Revision Selection
+  #======================================================================#
+  # NOTE: test branch, rev, and tag capabilities are (lighty) tested in unit tests
+
+  #======================================================================#
+  #                     Relative Path Support
+  #======================================================================#
+
+  #------------ Happy Cases for Relative Path Support -------------------#
+  describe "running a profile with a shallow relative path dependency" do
+    it "should find the relative path profile and execute exactly those controls" do
+      assert_relative_fetch_works("relative-shallow", ["relative-shallow", "child-01"], ["top-level-01", "child-01"])
+    end
+  end
+
+  describe "running a profile with a deep relative path dependency" do
+    it "should find the relative path profile and execute exactly those controls" do
+      assert_relative_fetch_works("relative-deep", ["relative-deep", "child-02"], ["relative-deep-01", "child-02"])
+    end
+  end
+
+  describe "running a profile with a combination of relative path dependencies" do
+    it "should find the relative path profiles and execute exactly those controls" do
+      assert_relative_fetch_works(
+        "relative-combo",
+        ["relative-combo", "child-01", "child-02"],
+        ["relative-combo-01", "child-01", "child-02"]
+      )
+    end
+  end
+
+  #------------ Edge Cases for Relative Path Support -------------------#
+
+  describe "running a profile with an '' relative path dependency" do
+    it "should find the top-level profile in the git-referenced child profile and execute that" do
+      assert_relative_fetch_works("relative-empty", ["relative-empty", "basic-local"], ["relative-empty-01", "basic-local-01"])
+    end
+  end
+
+  describe "running a profile with an ./ relative path dependency" do
+    it "should find the top-level profile in the git-referenced child profile and execute that" do
+      assert_relative_fetch_works("relative-dot-slash", ["relative-dot-slash", "basic-local"], ["relative-dot-slash-01", "basic-local-01"])
+    end
+  end
+
+  describe "running a profile with a relative path dependency that does not exist" do
+    it "should fail gracefully" do
+      run_result = run_inspec_process("exec #{git_profiles}/relative-nonesuch")
+      assert_empty run_result.stdout
+      refute_includes run_result.stderr, "Errno::ENOENT" # No ugly file missing error
+      assert_equal 1, run_result.stderr.lines.count # Not a giant stacktrace
+      # Spot check important parts of the message
+      assert_includes run_result.stderr, "Cannot find relative path"
+      assert_includes run_result.stderr, "no/such/path" # the actual missing path
+      assert_includes run_result.stderr, "profile in git repo"
+      # The containing git repo (the only identifier the user will have)
+      assert_includes run_result.stderr, "test/unit/mock/profiles/git-fetcher/git-repo-01"
+      assert_exit_code(1, run_result) # General user error
+    end
+  end
+end

test/functional/inspec_vendor_test.rb

@@ -62,7 +62,7 @@
   end
 
   it "can vendor profile dependencies from git" do
-    git_depends_path = File.join(profile_path, "git-depends")
+    git_depends_path = File.join(profile_path, "git-fetcher", "basic")
 
     Dir.mktmpdir do |tmpdir|
       FileUtils.cp_r(git_depends_path + "/.", tmpdir)

test/helper.rb

@@ -139,7 +139,7 @@ def expect_deprecation(group, &block)
 end
 
 class Minitest::Test
-  raise "You must remove skip_now" if Time.now > Time.local(2019, 6, 21)
+  raise "You must remove skip_now" if Time.now > Time.local(2019, 7, 21)
 
   def skip_until(y, m, d, msg)
     raise msg if Time.now > Time.local(y, m, d)

test/unit/mock/profiles/git-fetcher/basic-local/README.md

@@ -0,0 +1 @@
+This is a git repo used as a test fixture. Because we cannot directly store the .git/ directory, it is stored as git-fixture/, and a symlink is created at test runtime.

test/unit/mock/profiles/git-fetcher/basic-local/controls/basic-local.rb

@@ -0,0 +1,5 @@
+control 'basic-local-01' do
+  describe 'always-pass' do
+    it { should cmp 'always-pass'}
+  end
+end

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/COMMIT_EDITMSG

@@ -0,0 +1 @@
+Add change only on one branch

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/master

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/config

@@ -0,0 +1,10 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+[user]
+	name = test user
+	email = test@test.org

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/info/exclude

@@ -0,0 +1,6 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/logs/HEAD

@@ -0,0 +1,4 @@
+0000000000000000000000000000000000000000 0e7d2b9c2c5a1372341e36febceab86558439149 test user <test@test.org> 1560485536 -0400	commit (initial): Initial commit
+0e7d2b9c2c5a1372341e36febceab86558439149 0e7d2b9c2c5a1372341e36febceab86558439149 test user <test@test.org> 1560485563 -0400	checkout: moving from master to test-branch
+0e7d2b9c2c5a1372341e36febceab86558439149 54d0671d3e2c4a28865a0ecc98863859bd4d7475 test user <test@test.org> 1560485674 -0400	commit: Add change only on one branch
+54d0671d3e2c4a28865a0ecc98863859bd4d7475 0e7d2b9c2c5a1372341e36febceab86558439149 test user <test@test.org> 1560485682 -0400	checkout: moving from test-branch to master

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/logs/refs/heads/master

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 0e7d2b9c2c5a1372341e36febceab86558439149 test user <test@test.org> 1560485536 -0400	commit (initial): Initial commit

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/logs/refs/heads/test-branch

@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 0e7d2b9c2c5a1372341e36febceab86558439149 test user <test@test.org> 1560485563 -0400	branch: Created from HEAD
+0e7d2b9c2c5a1372341e36febceab86558439149 54d0671d3e2c4a28865a0ecc98863859bd4d7475 test user <test@test.org> 1560485674 -0400	commit: Add change only on one branch

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/objects/26/dec4a490f980527c06528510ed85512fd55e10

@@ -0,0 +1,4 @@
+x
���
+�0D;�+n�$h��{$��j+���o
+]
+;�-��'�y����6ܐ��)�+����	(5t4��lwރV��l@��@,~�������^�;�e&�E2���W�kI

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/objects/38/b63f30119439953105a30b6b54201ccdf161ad

@@ -0,0 +1,2 @@
+x
U��
+�0��o�T�����HS��.7�]M�c�X��T�Y� �$!�Y��q�����tY���s@��a���������-��{�#

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/objects/54/d0671d3e2c4a28865a0ecc98863859bd4d7475

@@ -0,0 +1,4 @@
+x
��Q
+�0D��)�J��&)��Q�Ͷl+i���m��0<�cfx��g�xjUrȎ�3��I,Qo��X��\<��A�S����P0w�L�؀����̒r�D��θN���K�&k�m�
+���\�:����.���i��7���?J�Q
+��A`�_���_5�<�/{fHJ

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/refs/heads/master

@@ -0,0 +1 @@
+0e7d2b9c2c5a1372341e36febceab86558439149

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/refs/heads/test-branch

@@ -0,0 +1 @@
+54d0671d3e2c4a28865a0ecc98863859bd4d7475

test/unit/mock/profiles/git-fetcher/basic-local/git-fixture/refs/tags/v1.0

@@ -0,0 +1 @@
+0e7d2b9c2c5a1372341e36febceab86558439149

test/unit/mock/profiles/git-fetcher/basic-local/inspec.yml

@@ -0,0 +1,8 @@
+name: basic-local
+title: basic-local
+license: Apache-2.0
+summary: A profile to be executed as a local git checkout
+version: 0.1.0
+supports:
+  platform: os
+

test/unit/mock/profiles/git-fetcher/git-repo-01/README.md

@@ -0,0 +1 @@
+This is a git repo used as a test fixture. Because we cannot directly store the .git/ directory, it is stored as git-fixture/, and a symlink is created at test runtime.

test/unit/mock/profiles/git-fetcher/git-repo-01/child-01/controls/child-01.rb

@@ -0,0 +1,5 @@
+control 'child-01' do
+  describe 'always-pass' do
+    it { should cmp 'always-pass'}
+  end
+end