Only parse variables scoped to facts./trusted. #17

betadots · Jun 2, 2022 · 76d2dca · 76d2dca
1 parent 83c7148
commit 76d2dca
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 46 deletions.
diff --git a/app/models/hiera_data/interpolation.rb b/app/models/hiera_data/interpolation.rb
@@ -1,5 +1,6 @@
 class HieraData
   module Interpolation
+    VARIABLE_REGEXP = /%{(::)?(facts|trusted)\.([^}]+)}/.freeze
 
     module_function
 
@@ -8,19 +9,12 @@ def interpolate_globs(path:, datadir:)
     end
 
     def interpolate_facts(path:, facts:)
-      groups = path.scan(/%{([^}]+)}/)
-      groups.flatten!
-      groups.each { |x| x.gsub!(/^(::|facts\.)/, '')}
-
-      resolved_path = path.dup
-
-      groups.each do |fact|
-        facts_value = facts.dig(*fact.split("."))
-        next unless facts_value
-        resolved_path.gsub!(/%{(::|facts\.)?#{fact}}/, facts_value)
+      path.gsub(VARIABLE_REGEXP) do |variable_string|
+        _, _, scope, name = variable_string.match(VARIABLE_REGEXP).to_a
+        name = "trusted.#{name}" if scope == "trusted"
+        value = facts.dig(*name.split("."))
+        value || variable_string
       end
-
-      resolved_path
     end
   end
 end
diff --git a/test/fixtures/files/puppet/environments/development/hiera.yaml b/test/fixtures/files/puppet/environments/development/hiera.yaml
@@ -6,8 +6,8 @@ defaults:
 hierarchy:
   - name: "Eyaml hierarchy"
     paths:
-      - "nodes/%{::fqdn}.yaml"
-      - "role/%{::role}-%{::env}.yaml"
-      - "role/%{::role}.yaml"
-      - "zone/%{::zone}.yaml"
+      - "nodes/%{::facts.fqdn}.yaml"
+      - "role/%{::facts.role}-%{::facts.env}.yaml"
+      - "role/%{::facts.role}.yaml"
+      - "zone/%{::facts.zone}.yaml"
       - "common.yaml"
diff --git a/test/fixtures/files/puppet/environments/eyaml/hiera.yaml b/test/fixtures/files/puppet/environments/eyaml/hiera.yaml
@@ -8,13 +8,13 @@ defaults:
 
 hierarchy:
   - name: "Host specific"
-    path: "nodes/%{::fqdn}.yaml"
+    path: "nodes/%{::facts.fqdn}.yaml"
 
   - name: "Per-datacenter business group data" # Uses custom facts.
     paths:
-      - "role/%{::role}-%{::env}.yaml"
-      - "role/%{::role}.yaml"
-      - "zone/%{::zone}.yaml"
+      - "role/%{::facts.role}-%{::facts.env}.yaml"
+      - "role/%{::facts.role}.yaml"
+      - "zone/%{::facts.zone}.yaml"
 
   - name: "Global data"
     path: "common.yaml"

diff --git a/test/fixtures/files/puppet/environments/globs/hiera.yaml b/test/fixtures/files/puppet/environments/globs/hiera.yaml
@@ -6,10 +6,10 @@ defaults:
 hierarchy:
   - name: "Eyaml hierarchy"
     paths:
-      - "nodes/%{::fqdn}.yaml"
-      - "role/%{::role}-%{::env}.yaml"
-      - "role/%{::role}.yaml"
-      - "zone/%{::zone}.yaml"
+      - "nodes/%{::facts.fqdn}.yaml"
+      - "role/%{::facts.role}-%{::facts.env}.yaml"
+      - "role/%{::facts.role}.yaml"
+      - "zone/%{::facts.zone}.yaml"
   - name: "Common"
     globs:
       - "common/*.yaml"
diff --git a/test/fixtures/files/puppet/environments/multiple_hierarchies/hiera.yaml b/test/fixtures/files/puppet/environments/multiple_hierarchies/hiera.yaml
@@ -5,13 +5,13 @@ defaults:
 
 hierarchy:
   - name: "Host specific"
-    path: "nodes/%{::fqdn}.yaml"
+    path: "nodes/%{::facts.fqdn}.yaml"
 
   - name: "Per-datacenter business group data" # Uses custom facts.
     paths:
-      - "role/%{::role}-%{::env}.yaml"
-      - "role/%{::role}.yaml"
-      - "zone/%{::zone}.yaml"
+      - "role/%{::facts.role}-%{::facts.env}.yaml"
+      - "role/%{::facts.role}.yaml"
+      - "zone/%{::facts.zone}.yaml"
 
   - name: "Global data"
     path: "common.yaml"

diff --git a/test/fixtures/files/puppet/environments/test/hiera.yaml b/test/fixtures/files/puppet/environments/test/hiera.yaml
@@ -6,8 +6,8 @@ defaults:
 hierarchy:
   - name: "Eyaml hierarchy"
     paths:
-      - "nodes/%{::fqdn}.yaml"
-      - "role/%{::role}-%{::env}.yaml"
-      - "role/%{::role}.yaml"
-      - "zone/%{::zone}.yaml"
+      - "nodes/%{::facts.fqdn}.yaml"
+      - "role/%{::facts.role}-%{::facts.env}.yaml"
+      - "role/%{::facts.role}.yaml"
+      - "zone/%{::facts.zone}.yaml"
       - "common.yaml"
diff --git a/test/models/hiera_data/hierarchy_test.rb b/test/models/hiera_data/hierarchy_test.rb
@@ -25,10 +25,10 @@ class HieraData::HierarchyTest < ActiveSupport::TestCase
   test "#paths returns all non-interpolated path names" do
     hierarchy = HieraData::Hierarchy.new(raw_hash: raw_hash, base_path: ".")
     expected_paths = [
-      "nodes/%{::fqdn}.yaml",
-      "role/%{::role}-%{::env}.yaml",
-      "role/%{::role}.yaml",
-      "zone/%{::zone}.yaml",
+      "nodes/%{::facts.fqdn}.yaml",
+      "role/%{::facts.role}-%{::facts.env}.yaml",
+      "role/%{::facts.role}.yaml",
+      "zone/%{::facts.zone}.yaml",
       "common.yaml"
     ]
     assert_equal expected_paths, hierarchy.paths
@@ -93,10 +93,10 @@ def raw_hash
     {
       "name" => "Yaml hierarchy",
       "paths" => [
-        "nodes/%{::fqdn}.yaml",
-        "role/%{::role}-%{::env}.yaml",
-        "role/%{::role}.yaml",
-        "zone/%{::zone}.yaml",
+        "nodes/%{::facts.fqdn}.yaml",
+        "role/%{::facts.role}-%{::facts.env}.yaml",
+        "role/%{::facts.role}.yaml",
+        "zone/%{::facts.zone}.yaml",
         "common.yaml"
       ]
     }

diff --git a/test/models/hiera_data/interpolation_test.rb b/test/models/hiera_data/interpolation_test.rb
@@ -13,34 +13,56 @@ class HieraData::InterpolationTest < ActiveSupport::TestCase
   end
 
   test "::interpolate_facts replaces facts with leading colons" do
-    path = "/test/%{::test_fact}/file"
+    path = "/test/%{::facts.test_fact}/file"
     facts = {"test_fact" => "replaced"}
     expected_result = "/test/replaced/file"
 
     assert_equal expected_result, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
   end
 
   test "::interpolate_facts replaces facts without leading colons" do
-    path = "/test/%{test_fact}/file"
+    path = "/test/%{facts.test_fact}/file"
     facts = {"test_fact" => "replaced"}
     expected_result = "/test/replaced/file"
 
     assert_equal expected_result, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
   end
 
-  test "::interpolate_facts replaces facts with `facts.` scope" do
-    path = "/test/%{facts.test_fact}/file"
-    facts = {"test_fact" => "replaced"}
+  test "::interpolate_facts replaces facts with `trusted.` scope with leading colons" do
+    path = "/test/%{::trusted.test_fact}/file"
+    facts = {"trusted" => {"test_fact" => "replaced"}}
+    expected_result = "/test/replaced/file"
+
+    assert_equal expected_result, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
+  end
+
+  test "::interpolate_facts replaces facts with `trusted.` scope without leading colons" do
+    path = "/test/%{trusted.test_fact}/file"
+    facts = {"trusted" => {"test_fact" => "replaced"}}
     expected_result = "/test/replaced/file"
 
     assert_equal expected_result, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
   end
 
   test "::interpolate_facts replaces nested facts" do
-    path = "/test/%{nested.fact}/file"
+    path = "/test/%{facts.nested.fact}/file"
     facts = {"nested" => {"fact" => "deep"}}
     expected_result = "/test/deep/file"
 
     assert_equal expected_result, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
   end
+
+  test "::interpolate_facts will not replace arbitrary variables with leading colons" do
+    path = "/test/%{::test_fact}/file"
+    facts = {"test_fact" => "replaced"}
+
+    assert_equal path, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
+  end
+
+  test "::interpolate_facts will not replace arbitrary variables without leading colons" do
+    path = "/test/%{test_fact}/file"
+    facts = {"test_fact" => "replaced"}
+
+    assert_equal path, HieraData::Interpolation.interpolate_facts(path: path, facts: facts)
+  end
 end