Skip to content

Commit af00807

Browse files
committed
(PUP-10772) Only accept msgpack if gem is installed
Previously the agent always advertised that it accepted the rich data msgpack format (application/vnd.puppet.rich+msgpack) even if the agent didn't have the msgpack gem installed locally. If the msgpack gem was installed on the server, and the server selected the rich data msgpack format, then the agent would fail to deserialize the catalog. Now we call the `suitable?` method in the superclass so the msgpack feature constrain is taken into account. Calling the super `supported?` method triggers a bunch of other logic around whether or not the format implements required methods, so avoid that. Note the "raw" msgpack format didn't have this issue because it didn't override the "supported?" method.
1 parent bfd2220 commit af00807

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

lib/puppet/network/formats.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ def render(instance)
255255
end
256256

257257
def supported?(klass)
258-
klass == Puppet::Resource::Catalog &&
258+
suitable? &&
259+
klass == Puppet::Resource::Catalog &&
259260
Puppet.lookup(:current_environment).rich_data?
260261
end
261262
end

spec/unit/http/service/compiler_spec.rb

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,26 @@
131131
subject.post_catalog(certname, environment: 'production', facts: facts, checksum_type: %w[sha256 sha384])
132132
end
133133

134+
it 'does not accept msgpack by default' do
135+
stub_request(:post, uri)
136+
.with(headers: {'Accept' => 'application/vnd.puppet.rich+json, application/json, text/pson'})
137+
.to_return(**catalog_response)
138+
139+
allow(Puppet.features).to receive(:msgpack?).and_return(false)
140+
141+
subject.post_catalog(certname, environment: environment, facts: facts)
142+
end
143+
144+
it 'accepts msgpack & rich_json_msgpack if the gem is present' do
145+
stub_request(:post, uri)
146+
.with(headers: {'Accept' => 'application/vnd.puppet.rich+json, application/json, application/vnd.puppet.rich+msgpack, application/x-msgpack, text/pson'})
147+
.to_return(**catalog_response)
148+
149+
allow(Puppet.features).to receive(:msgpack?).and_return(true)
150+
151+
subject.post_catalog(certname, environment: environment, facts: facts)
152+
end
153+
134154
it 'returns a deserialized catalog' do
135155
stub_request(:post, uri)
136156
.to_return(**catalog_response)
@@ -140,6 +160,35 @@
140160
expect(cat.name).to eq(certname)
141161
end
142162

163+
it 'deserializes the catalog from msgpack', if: Puppet.features.msgpack? do
164+
body = catalog.to_msgpack
165+
formatter = Puppet::Network::FormatHandler.format(:msgpack)
166+
catalog_response = { body: body, headers: {'Content-Type' => formatter.mime }}
167+
168+
stub_request(:post, uri)
169+
.to_return(**catalog_response)
170+
171+
_, cat = subject.post_catalog(certname, environment: 'production', facts: facts)
172+
expect(cat).to be_a(Puppet::Resource::Catalog)
173+
expect(cat.name).to eq(certname)
174+
end
175+
176+
it 'deserializes the catalog from rich msgpack', if: Puppet.features.msgpack? do
177+
body = Puppet.override(rich_data: true) do
178+
catalog.to_msgpack
179+
end
180+
181+
formatter = Puppet::Network::FormatHandler.format(:rich_data_msgpack)
182+
catalog_response = { body: body, headers: {'Content-Type' => formatter.mime }}
183+
184+
stub_request(:post, uri)
185+
.to_return(**catalog_response)
186+
187+
_, cat = subject.post_catalog(certname, environment: 'production', facts: facts)
188+
expect(cat).to be_a(Puppet::Resource::Catalog)
189+
expect(cat.name).to eq(certname)
190+
end
191+
143192
it 'returns the request response' do
144193
stub_request(:post, uri)
145194
.to_return(**catalog_response)

spec/unit/http/service_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def mime_types(model)
137137
catalog_mimes = if Puppet.features.msgpack?
138138
%w[application/vnd.puppet.rich+json application/json application/vnd.puppet.rich+msgpack application/x-msgpack text/pson]
139139
else
140-
%w[application/vnd.puppet.rich+json application/json application/vnd.puppet.rich+msgpack text/pson]
140+
%w[application/vnd.puppet.rich+json application/json text/pson]
141141
end
142142
expect(service.mime_types(Puppet::Resource::Catalog)).to eq(catalog_mimes)
143143
end

0 commit comments

Comments
 (0)