Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich Data setting not respected when converting catalogs to data #9470

Closed
justinstoller opened this issue Sep 3, 2024 · 1 comment · Fixed by #9471
Closed

Rich Data setting not respected when converting catalogs to data #9470

justinstoller opened this issue Sep 3, 2024 · 1 comment · Fixed by #9471
Labels
bug Something isn't working triaged Jira issue has been created for this

Comments

@justinstoller
Copy link
Member

Describe the Bug

When calling to_data_hash on a catalog (and consequently on resources within that catalog) we do not respect the rich_data setting, instead of we lookup the value in the Context here, which defaults to false here.

Typically, catalogs go through our networking code, which will correctly chooses json vs application/vnd.puppet.rich+json mime types based on the request and local configuration (see here & here). So this is not a problem during most catalog compilations.

However, for CD4PE we go through an alternate compilation process which does not go through the networking serialization code, and expects the setting value rich_data to be respected. This has become an issue as Puppet 8 sets strict mode to error, and rich data types (like Deferreds) are becoming more popular.

Expected Behavior

The environment.conf / puppet.conf setting respected when calling Catalog.to_data_hash()

Steps to Reproduce

Do a CD4PE Impact Analysis on an environment with Deferred resources.

With a Puppet Server installed (I used the latest PE release):
Update the file /etc/puppetlabs/code/environments/production/manifests/site.pp to have a default node group of

node default {
  $message = Deferred('inline_epp', ['This value is <%= $val %>', {'val' => 'deferred'}])
  notify { "deferred test":
    message => $message,
  }
}

And then request a CD4PE catalog with:

curl -k \
  --cacert /etc/puppetlabs/puppet/ssl/certs/ca.pem \
  --cert /etc/puppetlabs/puppet/ssl/certs/$(hostname -f).pem \
  --key /etc/puppetlabs/puppet/ssl/private_keys/$(hostname -f).pem \
  -H 'content-type: application/json' \
  -d "{\"certname\":\"$(hostname -f)\", \"environment\":\"production\", \"persistence\": { \"facts\": true, \"catalog\": true }}" \
  https://$(hostname -f):8140/puppet/v4/catalog

I also have a unit test reproducer in Puppet Server here: puppetlabs/puppetserver#2875

Environment

  • Version 6+

Additional Context

I would expect something like this to fix the issue:

diff --git a/lib/puppet.rb b/lib/puppet.rb
index 484fda4c4e..e80248b86c 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -237,7 +237,7 @@ module Puppet
       :ssl_context => proc { Puppet.runtime[:http].default_ssl_context },
       :http_session => proc { Puppet.runtime[:http].create_session },
       :plugins => proc { Puppet::Plugins::Configuration.load_plugins },
-      :rich_data => false
+      :rich_data => proc { Puppet.lookup(:current_environment).rich_data? }
     }
   end
 

But I have no idea if that would create more subtle problems than it solves.

@justinstoller justinstoller added the bug Something isn't working label Sep 3, 2024
justinstoller added a commit to justinstoller/puppet that referenced this issue Sep 3, 2024
justinstoller added a commit to justinstoller/puppet that referenced this issue Sep 5, 2024
justinstoller added a commit to justinstoller/puppet that referenced this issue Sep 5, 2024
@joshcooper joshcooper added the triaged Jira issue has been created for this label Sep 6, 2024
Copy link

github-actions bot commented Sep 6, 2024

Migrated issue to PUP-12077

justinstoller added a commit to justinstoller/puppet that referenced this issue Sep 6, 2024
Since Puppet 6.0 "datafication" has inspected the context for the value
of rich_data. However, in only some code paths does the value in the
context get overridden with a value taken from the settings. This means
in some cases the rich_data value will always be true or always be
false, regardless of how the user has configured the rich_data setting.
And, in the case the simply calling `to_data_hash` on a resource the
rich_data value will always be false.

This updates the base_context to set rich_data to the settings value,
ensuring that the default value for rich_data in the context is the
value users have set.

Additional changes are primarily where tests still assumed the default
value of rich_data was false, with one exception - YAML serialization in
the resource application will break if the internal rich_data `__pcore`
values are output. This forces rich_data to be false for that code path
in the resource application.

Fixes GH puppetlabs#9470
@mhashizume mhashizume linked a pull request Sep 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Jira issue has been created for this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants