Age | Commit message (Collapse) | Author | Files | Lines |
|
Also this patch splits out the long line in upstart_spec.rb to be
one line per service, so more diff-friendly.
|
|
The persistent http connection work introduced a regression,
preventing the agent from displaying useful error messages when SSL
verification fails, e.g. the server's SSL certificate doesn't match
the hostname the agent tried to connect to. The connection_spec test
didn't catch the issue, because those tests execute with the
non-caching pool, which always uses non-persistent connections.
The root cause is because the Connection class assumed http
connections are started by ruby in the `Net::HTTP#request`
method, so the OpenSSL rescue block wrapped that call.
However, in order to use persistent http connection, the caller needs
to explicitly start the connection prior to calling Net::HTTP#request,
which happens in the outer `Connection#with_connection` method.
This commit expands the scope of the rescue block. This way we receive
meaningful error messages if the connection is started explicitly for
persistent connections, or on-demand for non-persistent connections. It
also executes the ssl verification tests using persistent connections.
Also note that `with_connection` is private, so the fact that
`Pool#with_connection` or `Net::HTTP#request` can start the connection
is not visible to users of the Connection class.
|
|
This commit refactors the spec tests that ensure puppet displays
meaningful error messages when SSL verification fails, e.g. when the
server certificate doesn't match the hostname we connected to.
|
|
create config from exported resources taken from hiera
Some changes introduced in hiera around 3.4 resulted in type coersion
from strings to FixNums when creating exported resources that were
partially built from hiera data.
This patch ensures that any non-array parameter passed through the
Nagios::Base.to_s is actually a string before trying to run the .sub
method on it, and so fixes the above error with exported resources
whilst also introducing support for fixnum params for any of the nagios
type parameters
|
|
Previously, the test was trying to verify that the downloader objects
were created with the correct arguments, and testing that the
downloader was evaluated twice (since the same downloader was used for
both pluginsync and facts).
However, the downloader_factory_spec now tests that downloader objects
are created correctly, e.g. based on Puppet[:plugindest], so it is not
necessary to repeat that here.
This commit modifies the test to ensure the Downloader#evaluate
methods are called for both downloader objects, and adds a test to
ensure we skip downloading facts if the external facts feature is
false.
|
|
Previously, the plugin_handler module was responsible for creating
downloaders and evaluating them. Since all of this happens within a
single method it is hard to test that the downloaders are created with
the correct set of properties. For example, we `ignore` source
permissions when downloading plugins, but we `use` source permissions
when downloading external facts, since those often contain scripts,
which must be executable.
This commit moves the logic for creating the downloaders to a factory
and updates the tests accordingly. As part of this change, the
PluginHandler module is changed to a class and is no longer mixed into
the Configurer.
|
|
(PUP-2971) Fix yum package provider missing install_options for list.
|
|
(PUP-744) Support persistent HTTP connections
|
|
Previously PMT was checking for integer status codes when downloading
modules. This results in all module downloads failing. After this PMT
will check for string status codes when downloading modules.
|
|
Previously, validator_spec.rb was requiring
'puppet/ssl/configuration', because puppet/ssl.rb did not require
it. Commit 658e4fd34 fixed puppet/ssl.rb, so it is no longer necessary
or desired for validator_spec.rb to require puppet/ssl/configuration.
Also, the default and no_validators were not consistent in expressing
their dependencies on openssl and puppet/ssl.
|
|
Before this PMT would not check if the module download request
succeeded before trying install the module. This causes informative
errors from forge to be hidden by an error about the checksum of the
download not matching. After this PMT will only move forward with module
installation if a 200 is received from forge otherwise it will display
the error returned by forge.
|
|
There was a regression in the future parser in d25e462 which caused
resources with multiple attributeless bodies constructed like this:
file {
'/foo':;
'/bar':;
}
to fail because the future parser grammar was not allowing the terminal
semicolon. The acceptance suite picked this up running with parser
future. It slipped past specs because, even though we have a spec
testing multiple attributeless resource bodies, it lacked a terminal
semicolon.
Henrik Lindberg figured out the minor egrammar fix here to correct the
expression, and a test case is added to catch it in specs.
|
|
When executing the list command to filter virtual packages (when
allow_virtual is false or nil), the yum package provider was not passing
down the install options to the list command. For users passing options
like --enablerepo, this prevents the packages from being discovered and
the list command fails.
The fix is to properly pass down the install options when executing the
list command.
|
|
PUP-2745 Forge malformed dependency fix
|
|
This adds checks for the specification's assertions that virutal and
exported resources are not in the produced catalog, but are available
for later operations (realization) and reference (reading a parameter
value). This cannot check that the exported resources are made available
to catalog processors, since that is implementation dependent.
In order to check this the compile_to_catalog method needed to be
changed to filter out expoerted resources, just like the catalog
indirection does. This required a change to the create_resources test.
|
|
There were a few formatting problems with the dump action. This fixes
those up and adds tests around the dump functionality.
|
|
This changes the following:
* Assignment has lower precedence than resource expressions
* Local defaults are supported in resource instantiation expressions
* A Resource body may have a new expression AttributesOperation that
must evaluate to a hash.
* Stricter validation of resource titles
* Changes to TreeModelDumper wrt how BlockExpression is rendered (to
improve debugging output). As a consequence many tests needed
adjustments.
* Tests that previously passed have been modified because of stricter
validation.
This also includes maintenance. Removing dead code/fixing stale comments
and documentation.
|
|
The implementation no longer needs to transform resource expressions
to 3x AST. This changes the transformer to raise an error if attempted.
The spec test for resource transformation is also dropped. No new
test added that transformer raises exceptions for resource
transformation since all transformation eventually will be dropped.
|
|
(PUP-2912) Puppet FS symlinks in build/unpack
|
|
Prior to this commit build/unpack were trying to use the Ruby stdlib symlink to
deal with symlinks. This fails on platforms (Windows) that Puppet says supports
symlinks because it does not duck punch Ruby stdlib.
This commit switches the specs and code for build/unpack to use Puppet FS lib's
symlink functions. This should allow it to function correctly on Windows hosts that Puppet
supports symlinks.
|
|
(PUP-2912) Build/unpack spec tests w/o symlinks
|
|
Prior to this commit we would try to use symlinks in the tests on platforms that
do not support symlinking.
This commit uses the Puppet feature checker to conditionally try the tests if
Puppet thinks the platform supports symlinks (Unixes, Linuxes, Win 2k8+).
|
|
(PUP-2860) optimizations and maintenance
|
|
(PUP-2912) PMT Build symlink exception
|
|
Prior to this commit a malformed name in the dependencies for a downloaded forge
module dependency would cause the PMT to fail unexpectedly.
This commit causes the PMT to ignore malformed dependencies.
|
|
fiddyspence/feature/2945/warning_on_windows_package_source
(PUP-398) Windows package provider source munging and test
|
|
Adding spec tests to check for symlinks in build/install.
|
|
Adding munge and slashreplace method to windows package provider
to ensure successful installation of packages. Validation in the
type validate() not possible because we haven't chosen a provider
at that point. Outputs debug as a means of notifying the user that
the source has been changed.
|
|
adrienthebo/fixup/master/pup-2871-install-uninstall-options-pacman
Fixup/master/pup 2871 install uninstall options pacman
|
|
Prior to this commit there wasn't much verification for module dependencies.
This commit adds verification the module dependencies have sane names and version
ranges. If verification fails, it will raise an argument exception.
|
|
These methods should not be called by any outside class; this commit
makes them private and removes the tests that invoke them directly.
|
|
In the same manner as 9137faf uninstall_options were being appended as a
nested array value; this commit changes the uninstall method to
concatentate options to the array.
|
|
Before this commit, install_options were passed as a nested array to
pacman/yaourt. This commit changes that to concatentate the values
with the argument vector.
|
|
|
|
|
|
Since all types now are PAnyType, there is no need to have
an extra abstratin called PAbstractType, it just adds level in
the ancestor hierarchy that needs to be consulted for methods, one
extra step to go when checking is_a? against PAbstractType instead of
PAnyType, etc.
|
|
The old (intermediate) lexer was still hanging around even if it was
not being used. Cycles were also wasted on unit testing it.
The new lexer "Lexer2" has been in use for quite some time and the
old lexer is not useful even as a reference at this point.
This speeds up both loading and unit testing.
|
|
(maint) Add test to ensure upstart is default init daemon in Ubuntu
|
|
Re-word the description of a test in `webrick_spec.rb` as it is testing the
behavior of `masterhttplog` and not `masterlog`.
|
|
(maint) Remove pending face specs
|
|
Previously, it was possible for puppet to create an HTTPS connection
with VERIFY_NONE, and cache the connection. As a result, the pool code
could hand out an insecure connection, when the caller expected a secure
one (VERIFY_PEER).
In practice this isn't an issue, because puppet only uses insecure HTTPS
connections when bootstrapping its SSL key and certs, and that happens
before `Configurer#run` overrides the default non-caching pool
implementation. However, it could be an issue if a provider were to
request a insecure connection *during* catalog application, as that
connection would be cached, and possibly reused by puppet, e.g. when
sending the report.
This commit modifies the connection pool to only cache HTTP or secure
HTTPS connections. All other connections are closed after one HTTP
request/response.
|
|
Add yard doc for http pooling. Also adds a test to verify multiple
persistent connections can be borrowed from the pool.
|
|
Remove expectations that were secondary to the actual test, also
clarify behavior that was being tested.
|
|
Previously, I was concerned that a caller could borrow a connection
from the pool, and perform a long operation before returning it to the
pool. That could make it trivial for the caller to add expired
connections back to the pool, and cause issues for the next HTTP
request.
However, that is not likely, because connections are only borrowed for
the duration of the `Connection#get`, `#post`, etc calls, and those
methods don't take a block. The connection methods that do take a
block, e.g. `#request_get` are deprecated, and are not used within
puppet.
|
|
Previously, the `redirect_limit` specified the maximum number of HTTP
requests that the connection would make, not the maximum number of
redirects that it would follow. As a result, a limit of 0 would prevent
any HTTP requests from being made. This was introduced in 60f22eb1 when
the redirect logic was added.
This commit changes the limit so that 0 means don't follow redirects,
1 means follow one redirect, etc.
|
|
Previously, the connection would mutate its host, port, etc when
following redirects. This wasn't really an issue, because connections
were always single use.
Now that connections can be cached, we do not want to mutate the
original site information even when following permanent redirects.
|
|
Previously, the `Connection#connection` method was used in
`connection_spec.rb` to validate the state of the Net::HTTP object
created by the puppet connection.
Since the puppet connection no longer owns an Net::HTTP object (it may
use newly created or cached instances), the `#connection` method
doesn't make sense, and since it was private, can be removed.
The tests that relied on the private method have been updated, and in
some cases moved to the factory_spec, e.g. verifying proxy settings.
|
|
When starting a persistent HTTP connection, we do not expliclty specify
`Connection: Keep-Alive`, because that is the default in HTTP/1.1[1].
This commit adds a test to ensure we are using HTTP 1.1 or later.
Amazingly ruby has defaulted to 1.1 as far back as ruby 1.8.7[2].
[1] http://tools.ietf.org/html/rfc7230#appendix-A.1.2
[2] https://github.com/ruby/ruby/blob/v1_8_7/lib/net/http.rb#L282
|
|
The puppet connection class doesn't have an ssl_host instance variable.
|
|
Previously, the puppet connection class owned the Net::HTTP factory and
passed it to the pool when requesting a connection. However, the caching
pool only needs the factory when creating a new connection.
This commit makes the factory stateless and pushes it into the
respective pools. The SSL verifier is stateful (contains peer certs)
and remains in puppet's connection class.
When a pool needs to create a connection, it calls back into the puppet
connection object to initialize SSL on the Net::HTTP object. In the case
of the caching poool, it needs to do this before starting the
connection, which will perform the TCP connect and SSL handshake
operations.
|