(PDK-887) Add checks for read_only values being set or modified

lib/puppet/resource_api.rb

@@ -142,72 +142,76 @@ def feature_support?(feature)
             if options.key? :default
               defaultto options[:default]
             end
+          end
 
-            type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type])
-            validate do |value|
-              return true if type.instance?(value)
+          type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type])
+          validate do |value|
+            if options[:behaviour] == :read_only
+              raise Puppet::ResourceError, "Attempting to set `#{name}` read_only attribute value to `#{value}`"
+            end
+
+            return true if type.instance?(value)
 
-              if value.is_a? String
-                # when running under `puppet resource`, we need to try to coerce from strings to the real type
-                case value
-                when %r{^-?\d+$}, Puppet::Pops::Patterns::NUMERIC
-                  value = Puppet::Pops::Utils.to_n(value)
-                when %r{\Atrue|false\Z}
-                  value = value == 'true'
-                end
-                return true if type.instance?(value)
-
-                inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
-                error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("#{definition[:name]}.#{name}", type, inferred_type)
-                raise Puppet::ResourceError, error_msg
+            if value.is_a? String
+              # when running under `puppet resource`, we need to try to coerce from strings to the real type
+              case value
+              when %r{^-?\d+$}, Puppet::Pops::Patterns::NUMERIC
+                value = Puppet::Pops::Utils.to_n(value)
+              when %r{\Atrue|false\Z}
+                value = value == 'true'
               end
-            end
+              return true if type.instance?(value)
 
-            if type.instance_of? Puppet::Pops::Types::POptionalType
-              type = type.type
+              inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
+              error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("#{definition[:name]}.#{name}", type, inferred_type)
+              raise Puppet::ResourceError, error_msg
             end
+          end
 
-            # provide better handling of the standard types
-            case type
-            when Puppet::Pops::Types::PStringType
-              # require any string value
-              Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{})
-            # rubocop:disable Lint/BooleanSymbol
-            when Puppet::Pops::Types::PBooleanType
-              Puppet::ResourceApi.def_newvalues(self, param_or_property, 'true', 'false')
-              aliasvalue true, 'true'
-              aliasvalue false, 'false'
-              aliasvalue :true, 'true'
-              aliasvalue :false, 'false'
-
-              munge do |v|
-                case v
-                when 'true', :true
-                  true
-                when 'false', :false
-                  false
-                else
-                  v
-                end
-              end
-            # rubocop:enable Lint/BooleanSymbol
-            when Puppet::Pops::Types::PIntegerType
-              Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{^-?\d+$})
-              munge do |v|
-                Puppet::Pops::Utils.to_n(v)
-              end
-            when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
-              Puppet::ResourceApi.def_newvalues(self, param_or_property, Puppet::Pops::Patterns::NUMERIC)
-              munge do |v|
-                Puppet::Pops::Utils.to_n(v)
+          if type.instance_of? Puppet::Pops::Types::POptionalType
+            type = type.type
+          end
+
+          # provide better handling of the standard types
+          case type
+          when Puppet::Pops::Types::PStringType
+            # require any string value
+            Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{})
+          # rubocop:disable Lint/BooleanSymbol
+          when Puppet::Pops::Types::PBooleanType
+            Puppet::ResourceApi.def_newvalues(self, param_or_property, 'true', 'false')
+            aliasvalue true, 'true'
+            aliasvalue false, 'false'
+            aliasvalue :true, 'true'
+            aliasvalue :false, 'false'
+
+            munge do |v|
+              case v
+              when 'true', :true
+                true
+              when 'false', :false
+                false
+              else
+                v
               end
             end
-
-            case options[:type]
-            when 'Enum[present, absent]'
-              Puppet::ResourceApi.def_newvalues(self, param_or_property, :absent, :present)
+          # rubocop:enable Lint/BooleanSymbol
+          when Puppet::Pops::Types::PIntegerType
+            Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{^-?\d+$})
+            munge do |v|
+              Puppet::Pops::Utils.to_n(v)
+            end
+          when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
+            Puppet::ResourceApi.def_newvalues(self, param_or_property, Puppet::Pops::Patterns::NUMERIC)
+            munge do |v|
+              Puppet::Pops::Utils.to_n(v)
             end
           end
+
+          case options[:type]
+          when 'Enum[present, absent]'
+            Puppet::ResourceApi.def_newvalues(self, param_or_property, :absent, :present)
+          end
         end
       end
 

spec/acceptance/validation_spec.rb

@@ -51,6 +51,23 @@
       expect(output.strip).to match %r{Test_validation\[bye\]: test_validation.param expect.* an Integer value, got String}
       expect(status.exitstatus).to eq 1
     end
+
+    context 'when passing a value to a read_only property' do
+      context 'with an existing resource' do
+        it 'throws' do
+          output, status = Open3.capture2e("puppet resource #{common_args} test_validation foo ensure=present prop_ro=3")
+          expect(output.strip).to match %r{Test_validation\[foo\]: Attempting to set `prop_ro` read_only attribute value to `3`}
+          expect(status.exitstatus).to eq 1
+        end
+      end
+      context 'with a resource which should be absent' do
+        it 'throws' do
+          output, status = Open3.capture2e("puppet resource #{common_args} test_validation foo ensure=absent prop_ro=3")
+          expect(output.strip).to match %r{Test_validation\[foo\]: Attempting to set `prop_ro` read_only attribute value to `3`}
+          expect(status.exitstatus).to eq 1
+        end
+      end
+    end
   end
 
   describe 'using `puppet apply`' do
@@ -91,5 +108,22 @@
       expect(output.strip).to match %r{Test_validation\[gone\]: test_validation.param expect.* an Integer value, got String}i
       expect(status.exitstatus).to eq 1
     end
+
+    context 'when passing a value to a read_only property' do
+      context 'with an existing resource' do
+        it 'throws' do
+          output, status = Open3.capture2e("puppet apply #{common_args} -e \"test_validation{ foo: ensure => present, param => 3, prop_ro =>4 }\"")
+          expect(output.strip).to match %r{Test_validation\[foo\]: Attempting to set `prop_ro` read_only attribute value to `4`}
+          expect(status.exitstatus).to eq 1
+        end
+      end
+      context 'with a resource which should be absent' do
+        it 'throws' do
+          output, status = Open3.capture2e("puppet apply #{common_args} -e \"test_validation{ foo: ensure => absent, param => 3, prop_ro =>4 }\"")
+          expect(output.strip).to match %r{Test_validation\[foo\]: Attempting to set `prop_ro` read_only attribute value to `4`}
+          expect(status.exitstatus).to eq 1
+        end
+      end
+    end
   end
 end

spec/fixtures/test_module/lib/puppet/provider/test_validation/test_validation.rb

@@ -9,11 +9,13 @@ def get(_context)
         name: 'foo',
         ensure: :present,
         prop: 2,
+        prop_ro: 8,
       },
       {
         name: 'bar',
         ensure: :present,
         prop: 3,
+        prop_ro: 9,
       },
     ]
   end

spec/fixtures/test_module/lib/puppet/type/test_validation.rb

@@ -26,5 +26,10 @@
       desc:      'A mandatory parameter, that MUST be validated on deleting.',
       behaviour: :parameter,
     },
+    prop_ro:        {
+      type:      'Integer',
+      desc:      'A property that cannot be set by a catalog',
+      behaviour:  :read_only
+    },
   },
 )

spec/puppet/resource_api_spec.rb

@@ -509,6 +509,65 @@ def set(_context, _changes); end
     end
   end
 
+  context 'when registering a type with an `read_only` attribute', agent_test: true do
+    let(:definition) do
+      {
+        name: 'read_only_behaviour',
+        attributes: {
+          ensure: {
+            type: 'Enum[present, absent]',
+          },
+          name: {
+            type: 'String',
+            behavior: :namevar,
+          },
+          immutable: {
+            type: 'String',
+            behaviour: :read_only,
+          },
+        },
+      }
+    end
+
+    it { expect { described_class.register_type(definition) }.not_to raise_error }
+
+    describe 'the registered type' do
+      subject(:type) { Puppet::Type.type(:read_only_behaviour) }
+
+      it { is_expected.not_to be_nil }
+      it { expect(type.parameters[0]).to eq :name }
+    end
+
+    describe 'an instance of the type' do
+      let(:provider_class) do
+        Class.new do
+          def get(_context)
+            [{ name: 'foo_ro', ensure: :present, immutable: 'physics' }]
+          end
+
+          def set(_context, _changes); end
+        end
+      end
+
+      before(:each) do
+        stub_const('Puppet::Provider::ReadOnlyBehaviour', Module.new)
+        stub_const('Puppet::Provider::ReadOnlyBehaviour::ReadOnlyBehaviour', provider_class)
+      end
+
+      context 'when a manifest wants to set the value of a read_only attribute' do
+        let(:instance) { Puppet::Type.type(:read_only_behaviour).new(name: 'new_ro', ensure: :present, immutable: 'new') }
+
+        it { expect { instance.flush }.to raise_error Puppet::ResourceError, %r{Attempting to set `immutable` read_only attribute value to} }
+      end
+
+      context 'when a manifest wants to change the value of a read_only attribute' do
+        let(:instance) { Puppet::Type.type(:read_only_behaviour).new(name: 'foo_ro', ensure: :present, immutable: 'change') }
+
+        it { expect { instance.flush }.to raise_error Puppet::ResourceError, %r{Attempting to set `immutable` read_only attribute value to} }
+      end
+    end
+  end
+
   context 'when registering a type with a malformed attributes' do
     context 'without a type' do
       let(:definition) do