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

Upgrade to Puppet 8(.1.0) renders job.conf unusable as fragments are base64 encoded. #189

Open
MvRiegen opened this issue Aug 19, 2023 · 4 comments

Comments

@MvRiegen
Copy link

After upgrading to Puppet 8.1.0 (server and agent), the bacula job description (i.e. /etc/bacula/conf.d/job.conf) collects the exported resources on the director node as before, although all fragments are now base64 encoded in the file and the director can not start due to this.

Not sure if it's related to something similar mentioned here: PuppetDB gets base64 encoded string on exported ressources

Tried several things like up/downgrading the concat module, but so far it simply does not work as expected.

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 8.1.0
  • Distribution: Debian
  • Module version: 7.0.0
  • Concat module: 6.4.0 (also tried 9.0.0, doesn't work)
@MvRiegen
Copy link
Author

MvRiegen commented Aug 20, 2023

Ok, got it working as a workaround by defining a custom function 'bacula::enforce_utf8' that only overrides the encoding:

Puppet::Functions.create_function(:'bacula::enforce_utf8') do
  dispatch :enforce_utf8 do
    param 'String', :value
    return_type 'String'
  end

  def enforce_utf8(value)
    Puppet::Util::CharacterEncoding.override_encoding_to_utf_8(value)
  end
end

and then use the function in manifest/job.pp when collecting the job ressources:

@@bacula::director::job { $name:
    content => bacula::enforce_utf8(epp($template, $epp_job_variables)),
    tag     => $resource_tags,
}

Not sure if this is the valid fix, I need a bit more testing before submitting a pull request.
But at least it works again here with Puppet 8.1.0.

@smortex
Copy link
Member

smortex commented Aug 20, 2023

Interesting, I have not switched my infra with backups to Puppet 8 yet so not faced it. This look like a regression that better be fixed in the PuppetDB / PuppetServer code rather than in each module that consume exported epp resources.

All templates in the module are "ASCII text", do your $epp_job_variables introduce non-ASCII chars? Can you show what file /etc/bacula/conf.d/job.conf report on the system where you workaround the issue?

@smortex
Copy link
Member

smortex commented Aug 21, 2023

I could reproduce the issue, the base64 encoded string once decoded is really "ASCII text" so no surprise here. The exported bacula::director::fileset which do not use epp() for the parameter value is not affected, so it looks like the epp() method provided by Puppet should be fixed.

Adding a .force_encoding('UTF-8') at the end of the epp() function fix this issue, but maybe this is not the proper solution. Will do more test tomorrow.

@MvRiegen
Copy link
Author

All templates in the module are "ASCII text", do your $epp_job_variables introduce non-ASCII chars? Can you show what file /etc/bacula/conf.d/job.conf report on the system where you workaround the issue?

As far as i can see, there are no non-ASCII chars in my config - and I just realized you were already able to reproduce it, cool 👍

Anything I can do more for help? I assume a pull request with my workaround would only fix the symptom but not the root cause in this case.

smortex referenced this issue in puppetlabs/puppet Aug 21, 2023
In preparation for moving to frozen/immutable strings, explicitly create mutable
strings in cases where we create a string buffer and append to it. There are
multiple ways of creating a mutual string:

    ''.dup
    -''
    String.new

Although the latter is more verbose, I think it's better to be explicit.
smortex added a commit to smortex/puppet that referenced this issue Aug 21, 2023
In 2077971 empty string literals where
replaced by a call to `String.new` in preparation for moving to
frozen/immutable strings.

However, as stated in the String#new documentation, when no value is
passed to `#new` the returned string has the 'ASCII-8BIT' encoding
instead of the default one (which is assumed to be 'UTF-8' by Puppet).

This cause regressions in some areas of the code, for example when using
the concat module with exported resource built using epp templates, the
incorrect encoding cause the fragment to be misinterpreted and is base64
encoded.  When collected, the base64 representation of the string is
used instead of the actual value of the string, as reported here:
voxpupuli/puppet-bacula#189

Replace calls to `String.new` with `''.dup` which use the current
encoding.  Do not change the few explicit but redundant occurences of
`String.new.force_encoding('ASCII-8BIT')` (so that the intent is clearly
visible).  Where appropriate, slightly adjust the code for better
readability.
smortex added a commit to smortex/puppet that referenced this issue Aug 21, 2023
In 2077971 empty string literals where
replaced by a call to `String.new` in preparation for moving to
frozen/immutable strings.

However, as stated in the String#new documentation, when no value is
passed to `#new` the returned string has the 'ASCII-8BIT' encoding
instead of the default one (which is assumed to be 'UTF-8' by Puppet).

This cause regressions in some areas of the code, for example when using
the concat module with exported resource built using epp templates, the
incorrect encoding cause the fragment to be misinterpreted and is base64
encoded.  When collected, the base64 representation of the string is
used instead of the actual value of the string, as reported here:
voxpupuli/puppet-bacula#189

Replace calls to `String.new` with `''.dup` which use the current
encoding.  Do not change the few explicit but redundant occurrences of
`String.new.force_encoding('ASCII-8BIT')` (so that the intent is clearly
visible).  Where appropriate, slightly adjust the code for better
readability.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants