(#14822) Re-evaluate features if they previously were false

Allow features to be re-evaluated each time they are checked, if they
previously evaluated to false.  This allows for providers by features to have
dependencies installed during a run and then for the feature state to change
from false to true.

Caching of feature results that were previously true has been kept.  Caching of
blocks used to evaluate features remains unaffected and are still run only once.
(#14822) Use feature confine for feature tests during run

Fixes two providers to use the "confine :feature" statement, allowing for the
features to be re-evaluated during a run instead of only once.
(#16581) Refactor code for converting string and binary sids

This commit moves the methods that were previously part of the security
module into a sid module, and adds tests that only execute on windows due
to its reliance on the `win32/security` gem.
(#16581) Use win32-security gem to resolve SIDs

This commit adds methods for resolving Windows account names into binary
encoded SIDs and back. Account names may be specified with or without the
domain component, or as a SID string. So all of the following are
equivalent names for the LocalSystem account:

    NT AUTHORITY\SYSTEM
    SYSTEM
    S-1-5-18

There is a bug[1] in the `SID.string_to_sid` method in versions of
win32-security prior to 0.1.4, so this commit updates the Gemfile
accordingly.

[1] https://github.com/djberg96/win32-security/pull/1

Conflicts:

	spec/unit/util/windows/sid_spec.rb
(#16581) Documentation changes

Clarify how `name_to_sid` and `string_to_sid_ptr` are different with
respect to accounts that exist versus those that don't. The former returns
nil if the account doesn't exist, while the later doesn't.
(#16581) Use native Win32 APIs to resolve SIDs in providers

Previously, the Windows user, group, and scheduled_task providers used WMI
to resolve SIDs, but it is very slow in a domain environment.

This commit changes the providers to use the `Puppet::Util::Windows::SID`
module instead, and has been observed to be 6000 times faster.
(#16581) Use native Win32 APIs to resolve SIDs in file provider

Previously, the file provider used the ADSI module to resolve SIDs, which
is very slow in domain environments.

This commit changes it to use the Puppet::Util::Windows::Security module
instead. Ideally, the `name2id` and `id2name` methods could be eliminated
and instead have the uid & gid methods delegate directly to the Security
module. But they can't, because the Security module is not require'd on
non-Windows platforms.
(#16581) Deprecate sid_for_account

This commit eliminates the code for using WMI to resolve SIDs, since it is
very slow in domain environments. It also adds a deprecation warning if
any provider calls this method, as they should be using
`Puppet::Util::Security::SID.name_to_sid` instead.
(#16581) Refactor code for sid validation

Previously, the windows file provider knew too much about how to validate
a sid, e.g. catching a specific windows-specific exception.

This commit moves that logic into a `valid_sid?` method and updates the
file provider to call it. In doing so, the logic for `string_to_sid_ptr`
is simplier, in that it always expects a block.

It also removes tests that were refactored into sid_spec.rb in an earlier
commit.
Merge pull request #1225 from joshcooper/ticket/2.7.x/16581-slow-sid-resolution

(#16581) Don't use WMI to resolve SIDs
(#16581) Fix mis-stubbed test

Previously, the spec test failed because it was stubbing the
`sid_for_account` method that the scheduled_task provider no longer
called. This wasn't noticed on my system, because I had a local 'bob'
user, so that test would still "pass" incorrectly, but fail in CI.

This commit removes the `stubs` and adds `expects` for each test as
appropriate.
(#16791) Revert "Merge branch 'perf/2.7.x/backport-activesupport-hook-disabling' of https://github.com/daniel-pittman/puppet into 2.7.x"

This reverts commit 3d747177c3e3f690effe620d2f38c7a7d0f7d2d9, reversing
changes made to e45961f2a4160875ed1069b18734d3a6456af831.

This change to how puppet interacts with ActiveRecord causes problems
for systems that load puppet inside Ruby on Rails. Foreman encountered
this. Rather than introduce this change that can break existing installs
of puppet, this performance is not going to be part of 2.7.
(#16791) Revert "Maint: Fix intermittently failing spec test"

This reverts commit e45961f2a4160875ed1069b18734d3a6456af831.

This commit is being reverted in order to completely and cleanly revert
ba96917.
(#16791) Revert "Merge branch 'feature/2.7.x/json-terminus-backport' of https://github.com/daniel-pittman/puppet into 2.7.x"

This reverts commit ba969173ffad096401bc5d0df44b17324318ca03, reversing
changes made to 5e3fbcae67cfe4e3eb0564735742354bf85d79e7.

This change had introduced a new terminus for the catalog indirection
that wrote json files instead of yaml. Unfortunately the changes didn't
include actually being able to configure puppet to use this. This left
the decision about whether to backport another functionality change, or
to undo this change.

Since 2.7 is supposed to be a stable branch and should only be receiving
bug fixes, this change does not really belong in the 2.7 series. All of
the other backported performance fixes are small tweaks to existing
behavior, whereas this one provides a new set of functionality.
Therefore this change will not be made in the 2.7 series.
Eliminate `symbolize` everywhere, and remove it.

The whole `symbolize` API was confusing and complex: because it didn't do any
type enforcement, just passed through arbitrary data, we had very unclear API
in a whole pile of places.

This replaces that method entirely with a more rigid definition: we intern in
a bunch of places, add the occasional clear failure mode, and then delete the
original definition.

This eliminates one point of complexity from the codebase.

(These other uses have no notable performance penalty, just code complexity
 and clarity issues.)

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/parser/functions.rb
	lib/puppet/provider.rb
	lib/puppet/provider/nameservice/directoryservice.rb
(#16791) Re-instate deprecated symbolize and symbolizehash!

The cherry-pick of commit a09df46f58187ad9ff0f599de47cd8c1e0a417eb to
remove the use of symbolize throughout the code base also removed the
symbolize method, as well as the symbolizehash! method. The cherry-pick
was needed so that the methods could be deprecated without puppet
issuing warnings from using them.

This commit puts the two methods back and also places a deprecation
warning in them.
Merge branch 'ticket/2.7.x/16791-cleanup-performance-backports' into 2.7.x

* ticket/2.7.x/16791-cleanup-performance-backports:
  (#16791) Re-instate deprecated symbolize and symbolizehash!
  Eliminate `symbolize` everywhere, and remove it.
  (#16791) Revert "Merge branch 'feature/2.7.x/json-terminus-backport' of https://github.com/daniel-pittman/puppet into 2.7.x"
  (#16791) Revert "Maint: Fix intermittently failing spec test"
  (#16791) Revert "Merge branch 'perf/2.7.x/backport-activesupport-hook-disabling' of https://github.com/daniel-pittman/puppet into 2.7.x"
(#16791) Remove tests for unused behavior

The symbolize change caused two tests for the behavior around invalid
inputs to start failing. Looking at the tests, they are testing code
that would never reasonably be called with these kinds of values since
there are only a few places that call the methods being tested and those
use hard coded values. The fix here is to remove the tests that were
failing.
Merge pull request #833 from domcleal/tickets/2.7.x/14822-feature-runtime

(#14822) Ensure providers can run if features are delivered during a run
(#10146) `-` in variable names should be deprecated!

In commit b26699a2 I fixed an accidentally introduced change to the lexer,
allowing `-` to be part of a variable name.  That had lasted for a while and
was surprisingly popular.  It was also hugely ambiguous around `-` as minus,
and led to all sorts of failures - unexpected interpolations to nothing -
because of that.

A much better strategy would have been to deprecate the feature, issue proper
warnings, and include an option to allow users to toggle the behaviour.

Initially defaulting that to "permit", and eventually toggling over to "deny",
would have made the whole experience much smoother - even if this was totally
documented as not working, and was a clear bug that it changed.

So, thanks to prompting from Benjamin Irizarry, we do just that: introduce the
configuration option, default it to "deny" to match current behaviour, but
allow users the choice to change this back.

Please be aware that allowing variables with `-` might break all sorts of
things around, for example, Forge modules though.  Most people write code to
the documented standard, and their code might well totally fail to work with
this changed away from the default!

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>