Add policyfile attrs to node create/save

2 files changed, 138 insertions, 2 deletions

diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 34a1edf835..3c3286c548 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -610,8 +610,16 @@ class Chef
           chef_server_rest.put_rest("nodes/#{name}", data_for_save)
         end
       rescue Net::HTTPServerException => e
-        raise e unless e.response.code == "404"
-        chef_server_rest.post_rest("nodes", data_for_save)
+        if e.response.code == "404"
+          chef_server_rest.post_rest("nodes", data_for_save)
+        # Chef Server before 12.3 rejects node JSON with 'policy_name' or
+        # 'policy_group' keys, but 'policy_name' will be detected first.
+        # Backcompat can be removed in 13.0
+        elsif e.response.code == "400" && e.response.body.include?("Invalid key policy_name")
+          save_without_policyfile_attrs
+        else
+          raise
+        end
       end
       self
     end
@@ -620,6 +628,15 @@ class Chef
     def create
       chef_server_rest.post_rest("nodes", data_for_save)
       self
+    rescue Net::HTTPServerException => e
+      # Chef Server before 12.3 rejects node JSON with 'policy_name' or
+      # 'policy_group' keys, but 'policy_name' will be detected first.
+      # Backcompat can be removed in 13.0
+      if e.response.code == "400" && e.response.body.include?("Invalid key policy_name")
+        chef_server_rest.post_rest("nodes", data_for_save_without_policyfile_attrs)
+      else
+        raise
+      end
     end
 
     def to_s
@@ -632,6 +649,22 @@ class Chef
 
     private
 
+    def save_without_policyfile_attrs
+      trimmed_data = data_for_save_without_policyfile_attrs
+
+      chef_server_rest.put_rest("nodes/#{name}", trimmed_data)
+    rescue Net::HTTPServerException => e
+      raise e unless e.response.code == "404"
+      chef_server_rest.post_rest("nodes", trimmed_data)
+    end
+
+    def data_for_save_without_policyfile_attrs
+      data_for_save.tap do |trimmed_data|
+        trimmed_data.delete("policy_name")
+        trimmed_data.delete("policy_group")
+      end
+    end
+
     def data_for_save
       data = for_json
       ["automatic", "default", "normal", "override"].each do |level|
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index fcf5f56655..91177233a8 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -1410,6 +1410,109 @@ describe Chef::Node do
         end
       end
 
+      context "when policyfile attributes are present" do
+
+        before do
+          node.name("example-node")
+          node.policy_name = "my-application"
+          node.policy_group = "staging"
+        end
+
+        context "and the server supports policyfile attributes in node JSON" do
+
+          it "creates the object normally" do
+            expect(@rest).to receive(:post_rest).with("nodes", node.for_json)
+            node.create
+          end
+
+          it "saves the node object normally" do
+            expect(@rest).to receive(:put_rest).with("nodes/example-node", node.for_json)
+            node.save
+          end
+        end
+
+        # Chef Server before 12.3
+        context "and the Chef Server does not support policyfile attributes in node JSON" do
+
+          let(:response_body) { %q[{"error":["Invalid key policy_name in request body"]}] }
+
+          let(:response) do
+            Net::HTTPResponse.send(:response_class, "400").new("1.0", "400", "Bad Request").tap do |r|
+              allow(r).to receive(:body).and_return(response_body)
+            end
+          end
+
+          let(:http_exception) do
+            begin
+              response.error!
+            rescue => e
+              e
+            end
+          end
+
+          let(:trimmed_node) do
+            node.for_json.tap do |j|
+              j.delete("policy_name")
+              j.delete("policy_group")
+            end
+
+          end
+
+          context "on Chef Client 13 and later" do
+
+            # Though we normally attempt to provide compatibility with chef
+            # server one major version back, policyfiles were beta when we
+            # added the policyfile attributes to the node JSON, therefore
+            # policyfile users need to be on 12.3 minimum when upgrading Chef
+            # Client to 13+
+            it "lets the 400 pass through", :chef_gte_13_only do
+              expect { node.save }.to raise_error(http_exception)
+            end
+
+          end
+
+          context "when the node exists" do
+
+            it "falls back to saving without policyfile attributes" do
+              expect(@rest).to receive(:put_rest).with("nodes/example-node", node.for_json).and_raise(http_exception)
+              expect(@rest).to receive(:put_rest).with("nodes/example-node", trimmed_node).and_return(@node)
+              expect { node.save }.to_not raise_error
+            end
+
+          end
+
+          context "when the node doesn't exist" do
+
+            let(:response_404) do
+              Net::HTTPResponse.send(:response_class, "404").new("1.0", "404", "Not Found")
+            end
+
+            let(:http_exception_404) do
+              begin
+                response_404.error!
+              rescue => e
+                e
+              end
+            end
+
+            it "falls back to saving without policyfile attributes" do
+              expect(@rest).to receive(:put_rest).with("nodes/example-node", node.for_json).and_raise(http_exception)
+              expect(@rest).to receive(:put_rest).with("nodes/example-node", trimmed_node).and_raise(http_exception_404)
+              expect(@rest).to receive(:post_rest).with("nodes", trimmed_node).and_return(@node)
+              node.save
+            end
+
+            it "creates the node without policyfile attributes" do
+              expect(@rest).to receive(:post_rest).with("nodes", node.for_json).and_raise(http_exception)
+              expect(@rest).to receive(:post_rest).with("nodes", trimmed_node).and_return(@node)
+              node.create
+            end
+          end
+
+        end
+
+      end
+
     end
   end