puppetlabs/Razor — pull request #272 by tjmcs (2012-12-11 22:58:58+00:00)
Title: Fixes issues with hostname/version validation in new Broker Plugin API

This pull request changes the hostname and version validations in the new Broker Plugin API as follows:

1. both hostnames and version names have been revised to support correctly formatted hostnames and version names (previously, the patterns used were too strict).
1. blank lines are now passed through the validation process correctly, allowing for blank values for the `@server` and `@broker_version` parameters in the `req_metadata_hash`).
1. the `@server` parameter (which is used to specify the hostname of the Puppet Master) has been changed from a required parameter to an optional parameter

In addition, a small change was made to the base class for these broker plugins (`ProjectRazor::Broker::Base`) to support the use of empty string as a default value (if the default value is set to any non-nil value, including the empty string, then that value is used if the user enters an empty string on the CLI). Previously, an empty string was not allowed as a default value, making it impossible to set a value back to an empty string (which was allowed when adding an optional parameter by simply entering the value "SKIP" at the CLI prompt) once it had been set to a non-empty string (via an update, for example). With the change made to this base class a `nil` value should be used when there is "no default value defined" for a parameter, not an empty string (which is now taken as a valid default value).

There is an assumption made here that the puppet-razor Broker will be able to handle a handoff where these two (optional) parameters are defined as empty strings, but @nanliu assures me that this is the case...

Note; the method that was changed to support the use of blank default values for a parameter in the req_metadata_hash (the `ProjectRazor::Broker::Base.cli_create_metadata method`) is a copy of a method with the same name from the `ProjectRazor::Model::Base` class. As such, we might want to consider making a similar change to that method so that it behaves similarly (keying off of a `nil` value instead of an empty string when detecting parameters that have no defined default value), but that change is not being made in this commit (since it represents a separate issue).