summaryrefslogtreecommitdiffstats
path: root/spec
Commit message (Collapse)AuthorAgeFilesLines
...
* | | Remove :fails_on_windows from file type tests that no longer fail on WindowsJacob Helwig2011-08-021-7/+5
| | | | | | | | | | | | | | | | | | | | | | | | With the previous changes to the file bucket and the handling of what is considered an absolute path, enough pieces have slid into place that the file type is now mostly working on Windows. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Disable file bucket diffing tests on WindowsJacob Helwig2011-08-021-1/+1
| | | | | | | | | | | | | | | | | | | | | The diffing ability of the file bucket is purely master functionality, and not supported on Windows. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Always put a slash between the checksum and path in filebucket URLsJacob Helwig2011-08-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since absolute paths on Windows do not always start with /, we need to make sure that there is always a slash between the checksum and the path, or the drive letter will end up being considered as part of the checksum. On systems where absolute paths always start with /, the extra slash is removed by the parsing done to the constructed URL. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Treat Windows absolute paths as absolute pathsJacob Helwig2011-08-021-216/+164
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we only considered files that matched the *nix concept of 'absolute' as being absolute paths. Since absolute paths on Windows look more like URLs with this world-view, we need to specifically look for the Windows absolute paths, and treat them as such. We will still treat *nix absolute paths as absolute on Windows, even though they are actually relative to the "current" drive. We do not currently limit which "style" of absolute path is allowed based on what the agent is. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Consolidate test logic determining if a registered file is in the temp directoryJacob Helwig2011-08-021-10/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we were always using string comparisons, and hard-coded paths to temp locations on non-Windows platforms. This was problematic for a few reasons. We had to maintain a list of temp locations for the various platforms, and the string comparisons were unreliable on Windows, since paths have two string representations (the "short" name containing a ~ followed by a number, and the "full" name). By getting the current temp location using Dir.tempdir (the same mechanism our temp creation code uses), we no longer need to maintain the list of temp locations. Also, rather than doing string comparisons on file paths, we can use a combination of Pathname#ascend, and File.identical? to determine if the path registered as a temp file for deletion was actually created in the temp location. With this refactoring, the same code now works for both Windows, and non-Windows platforms. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Clarify logic and error messages when initializing Puppet::FileBucket::FileJacob Helwig2011-08-021-3/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rather than stating the logic as 'if !thing', the two checks done when initializing a new Puppet::FileBucket::File are now phrased as 'unless thing', which should lessen the likelihood of overlooking the '!'. We also now provide a reason for the ArgumentError being raised, which should help users of Puppet::FileBucket::File quickly figure out what is the problem when these exceptions are raised. In addition to updating the tests to look for these new error messages, we update the existing tests to specify which type of exception, and what message it should have, when something is raised. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | Disable symlink related file tests on WindowsJacob Helwig2011-08-021-2/+2
| | | | | | | | | | | | | | | | | | | | | While Windows filesystems typically have support for symlinks (NTFS), there are no immediate plans to support symlinks on Windows. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | (#8644) Host provider on WindowsJosh Cooper2011-07-293-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The host provider did not work on Windows because it didn't know where to find its hosts file. The provider now uses Win32::Resolv, which is part of the standard ruby library, to find it. Several host type/provider spec tests were marked as fails_on_windows, but now that the provider is working, I removed the tag from those tests, and verified that the tests now pass. There are two tests in resources_spec that fail because the user and exec providers are not supported on Windows yet, so those tests are marked as fails_on_windows. Reviewed-by: Pieter van de Bruggen <pieter@puppetlabs.com>
* | | (#8660) Default config dir to %PROGRAMDATA% on WindowsJosh Cooper2011-07-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The puppet install.rb script now defaults the config directory to %PROGRAMDATA%\PuppetLabs\puppet\etc on Windows. This is more inline with Windows best-practices, as this directory is used to store application data across all users. The PROGRAMDATA environment variable also takes into account alternate system drives, by using the SYSTEMDRIVE environment variable. Note that the Dir::COMMON_APPDATA constant is so named because it corresponds to the CSIDL_COMMON_APPDATA constant, which on 2000, XP, and 2003 is %ALLUSERSPROFILE%\Application Data, and on Vista, Win7 and 2008 is %SYSTEMDRIVE%\ProgramData. This commit also updates puppet's default run_mode var and conf directories when running as "root" to match the install script, and fixes the spec test, which was looking in the Dir::WINDOWS directory. Reviewed-by: Cameron Thomas <cameron@puppetlabs.com>
* | | (#8663) Disable spec tests for unsupported functionality on WindowsJosh Cooper2011-07-286-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The mount, shell, and ssh_authorized_key types are not supported on Windows, so these spec tests have been disabled when running on Windows. One of the compiler spec tests fails on Windows because Puppet::Util.execute attempts to execute a program named "git rev-parse HEAD". This has different semantics than Unix, where the command is splatted, Kernel.exec(*command). Since this truly is a Windows bug, I removed the fails_on_windows tag and updated ticket #8410. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
* | | Merge branch 'ticket/master/8663-windows-spec-tests'Josh Cooper2011-07-276-10/+13
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ticket/master/8663-windows-spec-tests: (#8663) Drive letters are not valid absolute paths on Windows (#8663) Update the run_mode spec test on Windows to match the code (#8663) The ssh_authorized_key type is not supported on Windows (#8663) Reenable spec tests on Windows that now pass
| * | | (#8663) Drive letters are not valid absolute paths on WindowsJosh Cooper2011-07-271-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One of the file spec tests was attempting to use X: as a fully qualified file path, and the file type was correctly rejecting it, since it is a relative path. This commit changes the spec test to expect the file type to raise an exception. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
| * | | (#8663) Update the run_mode spec test on Windows to match the codeJosh Cooper2011-07-271-3/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When running as root, the default conf and var directories on Windows are currently puppet/etc and puppet/var within the windows directory. Updated the spec tests to match what the code does on Windows. Whether or not this is the correct behavior is something that will be addressed in the future. See #8660. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
| * | | (#8663) The ssh_authorized_key type is not supported on WindowsJosh Cooper2011-07-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are not supporting the ssh_authorized_key type on Windows at this time, since Windows does not ship with an ssh server (though there are third-party versions available). As a result these spec tests have been disabled. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
| * | | (#8663) Reenable spec tests on Windows that now passJosh Cooper2011-07-273-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The cacher was causing spec tests to fail due to Time.now not having millisecond resolution on Windows. Now that usage of the cacher has been removed from many places, these tests now pass. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
* | | | Merge branch 'ticket/master/8392-disable-master-tests-on-windows'Jacob Helwig2011-07-272-2/+2
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | * ticket/master/8392-disable-master-tests-on-windows: (#8392) Disable master related tests on Windows
| * | | | (#8392) Disable master related tests on WindowsJacob Helwig2011-07-272-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The master, and queue applications are not used for agent functionality and since only agent functionality is supported on Windows, these tests do not need to run. Reviewed-by: Josh Cooper <josh@puppetlabs.com>
* | | | | Merge branch 'ticket/master/8272'Nick Lewis2011-07-261-58/+59
|\ \ \ \ \ | |/ / / / |/| | | |
| * | | | (#8272) Allow disabled Windows services to be startedNick Lewis2011-07-261-0/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because Windows allows a service to be both running and disabled, we now support that capability. If a service is explicitly requested to be running and disabled, we will set it to manual start if necessary, start it, and then disable it. If the service is requested to be running and enabled, we will now enable it before attempting to start it (previously, this would simply try to start it and fail). The exception to this rule is a service which is disabled, and for which we are not managing the enable property. In that case, we assume that some other authority has disabled the service, and respect that, failing to start. Thus, if the user actually wants a service to be running and disabled, they must explicitly declare that intent. Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
| * | | | (#8272) Refactor specs for Windows service providerNick Lewis2011-07-261-58/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These were using fake objects when that's not really necessary, so replace them with real ones. Additionally, many of these specs were doing the same thing (like creating a resource), so that has been extracted to the before block. Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
| * | | | (#8272) Use symbols instead of booleans for enabled property on WindowsNick Lewis2011-07-261-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because the enable property of the service type uses :true and :false as its valid values, rather than true and false, we need to return :true and :false from our enabled? method. Otherwise, the property was being synced every time it was enabled or disabled, regardless of whether it was actually in sync or not. Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
* | | | | Merge branch '2.7.x'Jacob Helwig2011-07-264-1/+177
|\ \ \ \ \ | |/ / / / |/| | / / | | |/ / | |/| | | | | | | | | | | | | | * 2.7.x: Deprecate RestAuthConfig#allowed? in favor of #check_authorization Fix #6026 - security file should support inline comments Fix #5010 - Allow leading whitespace in auth.conf Fix #5777 - rule interpolation broke auth.conf CIDR rules
| * | | Deprecate RestAuthConfig#allowed? in favor of #check_authorizationBrice Figureau2011-07-262-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #allowed? was a poorly named method since it isn't actually a predicate method. Instead of returning a boolean, this methods throws an exception when the access is denied (in order to keep the full context of what ACE triggered the deny). Given that #allowed? was overriding the behavior from AuthConfig, we leave a version of #allowed? in place that will issue a deprecation warning before delegating to #check_authorization. Once support for XML-RPC agents is removed from the master, we will be able to remove this delegation, since there should no longer be a reason for a distinction between AuthConfig and RestAuthConfig. Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com> Signed-off-by: Jacob Helwig <jacob@puppetlabs.com>
| * | | Fix #6026 - security file should support inline commentsBrice Figureau2011-07-263-1/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Auth.conf, namespaceauth.conf and fileserver.conf were not supporting trailing inlined comments. Also this commit fixes some indentation and error management. Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
| * | | Fix #5010 - Allow leading whitespace in auth.confBrice Figureau2011-07-262-1/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The regex used to detect ACE is too lax and would allow trailing spaces to sneak in, which in turn would confuse the ACE parser. Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
| * | | Fix #5777 - rule interpolation broke auth.conf CIDR rulesBrice Figureau2011-07-261-0/+147
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Due to the interpolation performed to support regex back-references, various type of rules (mainly IP CIDR notations and wildcard hosts) were not working anymore. This patch adds an auth.conf integration test that covers most of the various rules types. Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
* | | | Merge branch 'master' of github.com:puppetlabs/puppetJosh Cooper2011-07-262-36/+57
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | * 'master' of github.com:puppetlabs/puppet: maint: Suggest where to start troubleshooting SSL error message maint: Fix cert app to print help and exit if no subcommand
| * | | Merge branch '2.7.x'Matt Robinson2011-07-262-36/+57
| |\| | | | | | | | | | | | | | | | | | | | | | * 2.7.x: maint: Suggest where to start troubleshooting SSL error message maint: Fix cert app to print help and exit if no subcommand
| | * | Merge branch 'ticket/2.7.x/maint-show_help_for_cert_without_subcommand' into ↵Matt Robinson2011-07-261-0/+10
| | |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 2.7.x * ticket/2.7.x/maint-show_help_for_cert_without_subcommand: maint: Fix cert app to print help and exit if no subcommand
| | | * | maint: Fix cert app to print help and exit if no subcommandMatt Robinson2011-07-251-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 2.6.x this was the behavior, but the changes to the way options parsing worked in 2.7.x to support faces changed the behavior of how help was called. This resulted in the follow unhelpful error message when you just called `puppet cert`: /Users/matthewrobinson/work/puppet/lib/puppet/ssl/certificate_authority/interface.rb:85:in `method=' Invalid method to apply Reviewed-by: Pieter van de Bruggen <pieter@puppetlabs.com>
| | * | | maint: Suggest where to start troubleshooting SSL error messageMatt Robinson2011-07-261-36/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Much like the infamous "hostname was not match" error message, there's another SSL error that people run into that isn't clear how to troubleshoot. err: Could not send report: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed. As far as I can tell this only ever happens when the clock is off on the master or client. People seem to think it will happen other times, but I haven't been able to reproduce it other ways - missing private key, revoked cert, offline CA all have their own errors. I googled around and the only thing I've seen for this error in relation to puppet is the time sync problem. So the error message text just has some additional info to suggest you check your clocks. Reviewed-by: Nick Lewis <nick@puppetlabs.com>
* | | | | Fix issue with forward and backslashes in Windows pathsJosh Cooper2011-07-263-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The environment validates its modulepath and manifestdir settings, but it uses Dir.getwd to convert a relative path into an absolute path. The problem is that on Windows, Dir.getwd returns a path with backslashes. (Interestingly this only happens when puppet is loaded, not in irb for example.) And since we do not yet support backslashes in Windows paths or UNC paths, the directory is not included in the environment. For the time being, I am using File.expand_path to normalize the path. It has the side-effect of converting backslashes to forward slashes. This is sufficient to work around backslashes in Dir.getwd. In the near future, I will be refactoring how paths are split, validated, tested, etc, and I have a REMIND in place to fix the environment. But as a result of this change it exposed a bug in our rdoc parser dealing with the finding the root of a path. The parser assumed that the root was '/', but caused an infinite loop when passed a Windows path. I added a test for this case, which is only run on Windows, because on Unix File.dirname("C:/") == '.'. After all of that, I had to disable one of the rdoc spec tests, because it attempted to reproduce a specific bug, which caused rdoc to try to create a directory of the form: C:/.../files/C:/.... Of course, this fails because ':' is not a valid filename character on Windows. Paired-with: Nick Lewis <nick@puppetlabs.com> Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
* | | | | Disable spec tests for unsupported functionality on WindowsJosh Cooper2011-07-2610-27/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Ruby does not support creating symlinks on Windows (though Windows does support them), and since the tidy spec test is designed to reproduce a specific bug (as opposed to testing symlink functionality on Windows) it has been disabled. Ruby on Windows also does not support File.chmod, so the inspect spec test has been disabled. The general issue of File.chmod on Windows is something I know needs to be investigated. Also disabled the cron spec test as this functionality will not be supported on Windows (instead there will be task manager support). Re-enable the autoload spec tests as those now pass on Windows (this is probably due to the cacher changes recently made). The inventory ssl spec is not supported on Windows and so is disabled. However, while researching the failure, it was due to Time.now.strftime("%Z") returning "Pacific Daylight Time" on Windows, instead of "PDT" like it does on other platforms. As a result, the split method was sometimes splitting in the wrong place. As far as I can tell, the inventory code is only called from the CA (to keep track of serial numbers for certs that it has issued). But it's something to watch out for on Windows when calling strftime. Webrick, fileserver and CA functionality are not supported on Windows so these spec tests are disabled. Also fix path issue with catalog spec. Also several spec tests were creating tempfiles manually and calling system("rm -rf ..."), these have been replaced with PuppetSpec::Files.tmpdir. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
* | | | | Update certificate spec tests for WindowsJosh Cooper2011-07-267-44/+42
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Disable CA related spec tests on Windows, since that functionality is not supported. Some cert spec tests are still marked as fails_on_windows because settings attempts to create and apply a catalog corresponding to ssl/cert related directories. This fails because on Windows Puppet.features.root? always returns true (which is a separate bug), and as a result attempts to set the owner and group, which fails because the provider is not implemented yet on Windows. Also many of these tests were using Tempfile.new and trying to system("rm -rf ...") later. I changed these to use PuppetSpec::Files.tmpdir instead, which automatically cleans up temporary directories after the tests have run. Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>
* | | | Merge branch '2.7.x'Nick Lewis2011-07-2512-38/+182
|\| | | | | | | | | | | | | | | | | | | | | | | Conflicts: lib/puppet/type/file/source.rb spec/unit/resource/catalog_spec.rb
| * | | Merge branch '2.6.x' into 2.7.xNick Lewis2011-07-253-12/+66
| |\ \ \ | | |/ / | |/| / | | |/
| | * (#8596) Detect resource alias conflicts when titles do not matchNick Lewis2011-07-251-7/+54
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The introduction of composite namevars caused the resource title used in resource aliases to be set as an array, even when the resource only had one namevar. This would fail to conflict with non-alias entries in the resource table, which used a string for the title, even though the single element array contained the same string. Now, we flatten the key used in the resource table, so that single element arrays are represented as strings, and will properly conflict with resource titles. Paired-With: Jacob Helwig <jacob@puppetlabs.com>
| | * Merge branch 'ticket/2.6.x/8418-inspect_respect_run_mode' into 2.6.xMatt Robinson2011-07-252-5/+12
| | |\ | | | | | | | | | | | | | | | | | | | | | | | | * ticket/2.6.x/8418-inspect_respect_run_mode: (#8418) Fix inspect app to have the correct run_mode maint: Adding logging to include environment when source fails maint: Add debug logging when the master receives a report
| | | * (#8418) Fix inspect app to have the correct run_modeMatt Robinson2011-07-251-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Requiring puppet before the run_mode has been set by the application causes the default run_mode of 'user' to be set instead of what the application wants. This leads to the incorrect default settings to be used, which lead to inspect not being able to properly retrieve file metadata from a fileserver. Reviewed-by: Max Martin <max@puppetlabs.com>
| | | * maint: Add debug logging when the master receives a reportMatt Robinson2011-07-251-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's always bothered me that when running puppet inspect (or any application that produces a report really) the master gives no indication that anything happened when it processes the report. Reviewed-by: Max Martin <max@puppetlabs.com>
| * | | Merge branch '2.7rc' into 2.7.xMichael Stahnke2011-07-251-0/+237
| |\ \ \
| * \ \ \ Merge branch 'tickets/2.7.x/7266' into 2.7.xPieter van de Bruggen2011-07-251-4/+16
| |\ \ \ \
| | * | | | (#7266) Move Certificate option validation into face.Pieter van de Bruggen2011-07-251-4/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The validation for the ca_location option on the certificate application continued to hang around on the application long after the face realized its potential to take responsibility for itself. This change moves (and adds) validation code as appropriate into the Face. Reviewed-By: Matt Robinson
| * | | | | maint: Fix Face testing bug 1.9.2 revealed.Daniel Pittman2011-07-252-4/+2
| |/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Ruby 1.8.7 is fairly lax about various bits of introspection, including that we can't tell much about what arguments a block takes. Ruby 1.9.2 makes it possible to do this, though. Meanwhile, the Faces system uses this to make sure that scripts and actions take the right set of arguments, to avoid surprises: failing early and explicitly is better than failing at runtime. Which, in final turn, exposes that I forgot to accept the right arguments in a couple of my testing actions for Faces, but didn't notice because 1.8.7 doesn't check that, and I didn't test on 1.9.2.
| * | | | (#7290) Update indirected Faces to avoid unknown options.Daniel Pittman2011-07-221-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that we enforce that options must be declared, the model we exposed in the Ruby API (but not the CLI facade) was that you could pass additional arguments to the indirection method by passing them as unknown options doesn't work. Instead, explicitly declare an option, `extra`, that accepts the final argument to be passed direct to the indirection. This makes things work smoothly, as well as making it possible (once you can input a hash on the command line) to invoke extra arguments from the facade too... Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
| * | | | (#8561, #7290) Implement the option contract fully.Daniel Pittman2011-07-221-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rewrite the process of validating and updating the options to fully reflect the contract - we fail if there are unknown options passed, report nicely errors of duplicate names, pass only the canonical names to the action code, and generally enforce nicely. Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
| * | | | (#8561) Unify validation and modification of action arguments.Daniel Pittman2011-07-221-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rather than having multiple, separate operations that modify and validate the arguments to an action, a single pass makes sense. This also means less walks across the set of data, and a few less expensive method calls in Ruby. Additionally, we work on a duplicate of the arguments hash rather than directly modifying the original. Because everything we do is at the top level key/value mapping, this is sufficient to isolate the original. While mostly theoretical, we now don't mutilate the hash passed in, so the user won't get nastily surprised by the fact that we could have done so. Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
| * | | | (#7290) Fail on unknown options.Daniel Pittman2011-07-221-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Part of the "social contract" of Faces, Actions and Options is that the metadata we collect is authoritative: it covers everything that is possible. In the initial release we didn't enforce that around options. If you passed an unknown option in the hash, we just silently ignored it in validation and made it available down in the action. Now, instead, we enforce that rule. If you pass an unknown option we raise an error and complain; anything that gets to the action will be listed in the set of inspectable options. Cases that depended on this behaviour to pass arbitrary content in the hash should be rewritten to move that content down a level: take a hash value for one option, and use that for your free content. Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
| * | | | (#8561) Use canonical names for options to actions.Daniel Pittman2011-07-221-6/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we invoke an action, we parse a set of options. These have a canonical name, and optionally a set of aliases. For example, :bar might have :b as an alias to allow a short name to be given. Previously we would just pass this on as received; if you passed :bar you got :bar, and if you passed :b you got :b. This works, but means that every action has to write the same code to extract the appropriate version of an option from whatever set of aliases might be passed. Now, instead, we centralize that and always pass options as their canonical name to the action code. This makes it simpler to work with. (This happens before any validation, or other user-supplied, code to simplify everything that handles options.) Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
| * | | | (#7184) Load the core of obsolete versions of Faces.Daniel Pittman2011-07-224-8/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we define an action on an older version of a Face, we must be sure to directly load the core of that version, not just define it with the external Action(s) that it had. Otherwise we break our contract, which is that any core Actions for a specific version will be available to your external Action for as long as we support that core version. Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>