From bbf0a025c075124ffe2fc2838c27ace22d1ddb89 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 7 Jun 2011 11:32:40 -0700 Subject: maint: fix misnamed acceptance test for #7139 Just s/\.rm/\.rb/ on the filename. --- .../tests/ticket_7139_puppet_resource_file_qualified_paths.rb | 11 +++++++++++ .../tests/ticket_7139_puppet_resource_file_qualified_paths.rm | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) create mode 100644 acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rb delete mode 100644 acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rm diff --git a/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rb b/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rb new file mode 100644 index 000000000..f773ba17c --- /dev/null +++ b/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rb @@ -0,0 +1,11 @@ +test_name "#7139: Puppet resource file failes on path with leading '/'" + +step "Agents: create valid, invalid formatted manifests" +create_remote_file(agents, '/tmp/ticket-7139', %w{foo bar contents} ) + +step "Run puppet file resource on /tmp/ticket-7139" +agents.each do |host| + on(host, "puppet resource file /tmp/ticket-7139") do + assert_match(/file \{ \'\/tmp\/ticket-7139\':/, stdout, "puppet resource file failed on #{host}") + end +end diff --git a/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rm b/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rm deleted file mode 100644 index f773ba17c..000000000 --- a/acceptance/tests/ticket_7139_puppet_resource_file_qualified_paths.rm +++ /dev/null @@ -1,11 +0,0 @@ -test_name "#7139: Puppet resource file failes on path with leading '/'" - -step "Agents: create valid, invalid formatted manifests" -create_remote_file(agents, '/tmp/ticket-7139', %w{foo bar contents} ) - -step "Run puppet file resource on /tmp/ticket-7139" -agents.each do |host| - on(host, "puppet resource file /tmp/ticket-7139") do - assert_match(/file \{ \'\/tmp\/ticket-7139\':/, stdout, "puppet resource file failed on #{host}") - end -end -- cgit From 5587b94ce1bcb4e86df1160e5238b535e10028da Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 7 Jun 2011 11:33:17 -0700 Subject: maint: remove an unhelpful pending test. This has been pending approximately forever, and adds little value; removing the warning reduces noise without value. --- spec/unit/transaction_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb index d7788c06c..4bf615803 100755 --- a/spec/unit/transaction_spec.rb +++ b/spec/unit/transaction_spec.rb @@ -94,8 +94,6 @@ describe Puppet::Transaction do @transaction.report.should == report end - it "should consider a resource to have failed dependencies if any of its dependencies are failed" - describe "when initializing" do it "should create an event manager" do @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new) -- cgit From 5a9998e41bc442ca66ca2ff151750355d2ef6d6f Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Wed, 1 Jun 2011 18:07:40 -0700 Subject: (#7773, 7776, 7764) Several help template tweaks Per UX review of help output, this commit makes several changes to templates and shared help text: * Change "unknown" to "undocumented" * Remove copyright from short help * Point readers to the man pages (issue 7773) * Remove examples from short help (issue 7776) * Remove summary from short help and make it a fallback for description * Edit common option summaries to fit on a single 80-col line --- lib/puppet/face/help/action.erb | 16 ++++------------ lib/puppet/face/help/face.erb | 17 +++++------------ lib/puppet/face/help/man.erb | 14 +++++++++----- lib/puppet/indirector/face.rb | 2 +- 4 files changed, 19 insertions(+), 30 deletions(-) diff --git a/lib/puppet/face/help/action.erb b/lib/puppet/face/help/action.erb index c788f34fd..94b935fed 100644 --- a/lib/puppet/face/help/action.erb +++ b/lib/puppet/face/help/action.erb @@ -1,21 +1,16 @@ -<%= action.summary || face.summary || "unknown face..." %> - <% if action.synopsis -%> USAGE: <%= action.synopsis %> <% end -%> -<% if action.short_description -%> -<%= action.short_description.strip %> +<%= action.short_description || action.summary || face.summary || "undocumented face" %> -<% end -%> <% if action.returns -%> RETURNS: <%= action.returns.strip %> <% end -%> OPTIONS: <%# Remove these options once we can introspect them normally. -%> - --mode MODE - The run mode to use (`user`, `agent`, or - `master`). + --mode MODE - The run mode to use (user, agent, or master). --render-as FORMAT - The rendering format to use. --verbose - Whether to log verbosely. --debug - Whether to log debug information. @@ -26,7 +21,7 @@ OPTIONS: option = action.get_option name -%> <%= " " + option.optparse.join(" | ")[0,(optionroom - 1)].ljust(optionroom) + ' - ' -%> <% if !(option.summary) -%> -unknown option... +undocumented option <% elsif option.summary.length <= summaryroom -%> <%= option.summary %> <% @@ -51,7 +46,4 @@ unknown option... end -%> <% end -%> -<% if action.examples -%> -EXAMPLES: -<%= action.examples %> -<% end -%> +See 'puppet man <%= face.name %>' or 'man puppet-<%= face.name %>' for full help. diff --git a/lib/puppet/face/help/face.erb b/lib/puppet/face/help/face.erb index 2a0f0181c..870cc547f 100644 --- a/lib/puppet/face/help/face.erb +++ b/lib/puppet/face/help/face.erb @@ -1,17 +1,12 @@ -<%= face.summary || "unknown face..." %> - <% if face.synopsis -%> USAGE: <%= face.synopsis %> <% end -%> -<% if face.short_description -%> -<%= face.short_description %> +<%= (face.short_description || face.summary || "undocumented subcommand").strip %> -<% end -%> OPTIONS: <%# Remove these options once we can introspect them normally. -%> - --mode MODE - The run mode to use (`user`, `agent`, or - `master`). + --mode MODE - The run mode to use (user, agent, or master). --render-as FORMAT - The rendering format to use. --verbose - Whether to log verbosely. --debug - Whether to log debug information. @@ -22,7 +17,7 @@ OPTIONS: option = face.get_option name -%> <%= " " + option.optparse.join(" | ")[0,(optionroom - 1)].ljust(optionroom) + ' - ' -%> <% if !(option.summary) -%> -unknown option... +undocumented option <% elsif option.summary.length <= summaryroom -%> <%= option.summary %> <% @@ -54,7 +49,7 @@ ACTIONS: action = face.get_action(actionname) -%> <%= action.name.to_s.ljust(padding) + ' ' -%> <% if !(action.summary) -%> -unknown action... +undocumented action <% elsif action.summary.length <= summaryroom -%> <%= action.summary %> <% else @@ -86,6 +81,4 @@ AUTHOR <%= face.authors.join("\n").gsub(/^/, ' * ') %> <% end -%> -COPYRIGHT AND LICENSE: -<%= face.copyright.gsub(/^/, ' ') %> -<%= face.license.gsub(/^/, ' ') %> +See 'puppet man <%= face.name %>' or 'man puppet-<%= face.name %>' for full help. diff --git a/lib/puppet/face/help/man.erb b/lib/puppet/face/help/man.erb index 6f21fe413..0122d974c 100644 --- a/lib/puppet/face/help/man.erb +++ b/lib/puppet/face/help/man.erb @@ -1,4 +1,4 @@ -puppet-<%= face.name %>(8) -- <%= face.summary || "Unknown face." %> +puppet-<%= face.name %>(8) -- <%= face.summary || "Undocumented face." %> <%= '=' * (_erbout.length - 1) %> <% if face.synopsis -%> @@ -32,7 +32,7 @@ configuration options can also be generated by running puppet with and `master`. * --render-as FORMAT: The format in which to render output. The most common formats are `json`, - `s` (string), and `yaml`, but other options such as `dot` are + `s` (string), `yaml`, and `console`, but other options such as `dot` are sometimes available. * --verbose: Whether to log verbosely. @@ -57,12 +57,15 @@ ACTIONS <%= action.synopsis %> <% end -%> -<% if action.description -%> `DESCRIPTION` +<% if action.description -%> <%= action.description.gsub(/^/, ' ') %> -<% end - unique_options = action.options - face.options +<% else -%> + <%= action.summary || "Undocumented action." %> +<% end -%> + +<% unique_options = action.options - face.options unless unique_options.empty? -%> `OPTIONS` @@ -101,6 +104,7 @@ EXAMPLES `<%= action.name.to_s %>` <%= action.examples.strip %> + <% end end -%> diff --git a/lib/puppet/indirector/face.rb b/lib/puppet/indirector/face.rb index 756306a2f..bb8ce2ceb 100644 --- a/lib/puppet/indirector/face.rb +++ b/lib/puppet/indirector/face.rb @@ -2,7 +2,7 @@ require 'puppet/face' class Puppet::Indirector::Face < Puppet::Face option "--terminus TERMINUS" do - summary "The indirector terminus to use for this action." + summary "The indirector terminus to use." description <<-EOT Indirector faces expose indirected subsystems of Puppet. These subsystems are each able to retrieve and alter a specific type of data -- cgit From 17723634875e308b2752efed8ebdfd3db3239215 Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Fri, 3 Jun 2011 10:28:52 -0700 Subject: (#7764, 7775, 7778) Revisions to Faces help text Per UX review of help text, this commit makes several changes over the breadth of the Faces help: * Preface API-only action summaries/descriptions with "API only." (issue #7775) * Provide both CLI and API info in "returns," with the CLI info first. (issue #7778) * Summaries should be sentences. (Add punctuation.) * First sentences of descriptions should reiterate summaries. (Summaries and descriptions should be displayed far enough apart that this isn't a problem.) * Standardize on "subcommand" instead of "face" when talking about the entity you invoke at the command line. (Use "face" when describing API use.) * Fix outdated or clunky text in several faces. --- lib/puppet/face/catalog.rb | 52 ++++++++++++++++---------- lib/puppet/face/catalog/select.rb | 10 ++--- lib/puppet/face/certificate.rb | 31 ++++++++------- lib/puppet/face/certificate_request.rb | 23 ++++++------ lib/puppet/face/certificate_revocation_list.rb | 24 ++++++------ lib/puppet/face/config.rb | 10 ++--- lib/puppet/face/facts.rb | 34 ++++++++++------- lib/puppet/face/file.rb | 8 ++-- lib/puppet/face/file/download.rb | 10 ++--- lib/puppet/face/help.rb | 12 +++--- lib/puppet/face/help/action.erb | 2 +- lib/puppet/face/help/man.erb | 8 ++-- lib/puppet/face/key.rb | 8 ++-- lib/puppet/face/man.rb | 36 +++++++++--------- lib/puppet/face/node.rb | 25 ++++++++----- lib/puppet/face/parser.rb | 9 +++++ lib/puppet/face/plugin.rb | 17 +++++---- lib/puppet/face/report.rb | 16 ++++---- lib/puppet/face/resource.rb | 24 ++++++------ lib/puppet/face/resource_type.rb | 48 ++++++++++++++---------- lib/puppet/face/secret_agent.rb | 30 +++++++-------- lib/puppet/face/status.rb | 21 ++++++----- lib/puppet/indirector/face.rb | 12 +++--- 23 files changed, 261 insertions(+), 209 deletions(-) diff --git a/lib/puppet/face/catalog.rb b/lib/puppet/face/catalog.rb index 13351540a..9bcaa19c6 100644 --- a/lib/puppet/face/catalog.rb +++ b/lib/puppet/face/catalog.rb @@ -6,25 +6,40 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do summary "Compile, save, view, and convert catalogs." description <<-'EOT' - This face primarily interacts with the compiling subsystem. By default, - it compiles a catalog using the default manifest and the hostname from - `certname`, but you can choose to retrieve a catalog from the server by - specifying '--terminus rest'. You can also choose to print any catalog - in 'dot' format (for easy graph viewing with OmniGraffle or Graphviz) - with '--render-as dot'. + This subcommand deals with catalogs, which are compiled per-node artifacts + generated from a set of Puppet manifests. By default, it interacts with the + compiling subsystem and compiles a catalog using the default manifest and + `certname`, but you can change the source of the catalog with the + `--terminus` option. You can also choose to print any catalog in 'dot' + format (for easy graph viewing with OmniGraffle or Graphviz) with + '--render-as dot'. + EOT + short_description <<-'EOT' + This subcommand deals with catalogs, which are compiled per-node artifacts + generated from a set of Puppet manifests. By default, it interacts with the + compiling subsystem and compiles a catalog using the default manifest and + `certname`; use the `--terminus` option to change the source of the catalog. EOT - get_action(:destroy).summary "Invalid for this face." - get_action(:search).summary "Query format unknown; potentially invalid for this face." + get_action(:destroy).summary "Invalid for this subcommand." + get_action(:search).summary "Invalid for this subcommand." + find = get_action(:find) + find.summary "Retrieve the catalog for a node." + find.arguments "" + find.returns <<-'EOT' + A serialized catalog. When used from the Ruby API, returns a + Puppet::Resource::Catalog object. + EOT action(:apply) do - summary "Apply a Puppet::Resource::Catalog object." + summary "Find and apply a catalog." description <<-'EOT' Finds and applies a catalog. This action takes no arguments, but - the source of the catalog can be managed with the --terminus option. + the source of the catalog can be managed with the `--terminus` option. EOT returns <<-'EOT' - A Puppet::Transaction::Report object. + Nothing. When used from the Ruby API, returns a Puppet::Transaction::Report + object. EOT examples <<-'EOT' Apply the locally cached catalog: @@ -71,18 +86,17 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do action(:download) do summary "Download this node's catalog from the puppet master server." description <<-'EOT' - Retrieves a catalog from the puppet master and saves it to the - local yaml cache. The saved catalog can be used in subsequent - catalog actions by specifying '--terminus rest'. - - This action always contacts the puppet master and will ignore + Retrieves a catalog from the puppet master and saves it to the local yaml + cache. This action always contacts the puppet master and will ignore alternate termini. + + The saved catalog can be used in any subsequent catalog action by specifying + '--terminus yaml' for that action. EOT returns "Nothing." notes <<-'EOT' - As termini are singletons, this action has a side effect of - exporting Puppet::Resource::Catalog.indirection.terminus_class = - yaml to the calling context when used with the Ruby Faces API. The + When used from the Ruby API, this action has a side effect of leaving + Puppet::Resource::Catalog.indirection.terminus_class set to yaml. The terminus must be explicitly re-set for subsequent catalog actions. EOT examples <<-'EOT' diff --git a/lib/puppet/face/catalog/select.rb b/lib/puppet/face/catalog/select.rb index d86963e75..de2ca803b 100644 --- a/lib/puppet/face/catalog/select.rb +++ b/lib/puppet/face/catalog/select.rb @@ -1,15 +1,15 @@ # Select and show a list of resources of a given type. Puppet::Face.define(:catalog, '0.0.1') do action :select do - summary "Select and show a list of resources of a given type" + summary "Retrieve a catalog and filter it for resources of a given type." arguments " " returns <<-'EOT' - An array of resource objects excised from a catalog. When used at - the command line, returns a list of resource references (Type[title]). + A list of resource references ("Type[title]"). When used from the API, + returns an array of Puppet::Resource objects excised from a catalog. EOT description <<-'EOT' - Retrieves a catalog for the specified host and returns an array of - resources of the given type. + Retrieves a catalog for the specified host, then searches it for all + resources of the requested type. EOT notes <<-'NOTES' By default, this action will retrieve a catalog from Puppet's compiler diff --git a/lib/puppet/face/certificate.rb b/lib/puppet/face/certificate.rb index cab8817e3..cb0f61e3b 100644 --- a/lib/puppet/face/certificate.rb +++ b/lib/puppet/face/certificate.rb @@ -5,9 +5,9 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do copyright "Puppet Labs", 2011 license "Apache 2 license; see COPYING" - summary "Provide access to the CA for certificate management" + summary "Provide access to the CA for certificate management." description <<-'EOT' - This face interacts with a local or remote Puppet certificate + This subcommand interacts with a local or remote Puppet certificate authority. Currently, its behavior is not a full superset of `puppet cert`; specifically, it is unable to mimic puppet cert's "clean" option, and its "generate" action submits a CSR rather than creating a @@ -15,10 +15,12 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do EOT option "--ca-location LOCATION" do - summary "The certificate authority to query" + summary "The certificate authority to query (local or remote)." description <<-'EOT' Whether to act on the local certificate authority or one provided by a remote puppet master. Allowed values are 'local' and 'remote.' + + This option is required. EOT before_action do |action, args, options| @@ -27,7 +29,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do end action :generate do - summary "Generate a new certificate signing request for HOST." + summary "Generate a new certificate signing request." arguments "" returns "Nothing." description <<-'EOT' @@ -55,8 +57,10 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do action :list do summary "List all certificate signing requests." returns <<-'EOT' - An array of CSR object #inspect strings. This output is currently messy, - but does contain the names of nodes requesting certificates. + An array of #inspect output from CSR objects. This output is + currently messy, but does contain the names of nodes requesting + certificates. This action returns #inspect strings even when used + from the Ruby API. EOT when_invoked do |options| @@ -70,8 +74,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do summary "Sign a certificate signing request for HOST." arguments "" returns <<-'EOT' - A string that appears to be an x509 certificate, but is actually - not. Retrieve certificates using the `find` action. + A string that appears to be (but isn't) an x509 certificate. EOT examples <<-'EOT' Sign somenode.puppetlabs.lan's certificate: @@ -88,24 +91,24 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do # Indirector action doc overrides find = get_action(:find) - find.summary "Retrieve a certificate" + find.summary "Retrieve a certificate." find.arguments "" find.returns <<-'EOT' An x509 SSL certificate. You will usually want to render this as a - string ('--render-as s'). + string (--render-as s). Note that this action has a side effect of caching a copy of the certificate in Puppet's `ssldir`. EOT destroy = get_action(:destroy) - destroy.summary "Delete a local certificate." + destroy.summary "Delete a certificate." destroy.arguments "" destroy.returns "Nothing." destroy.description <<-'EOT' - Deletes a certificate. This action currently only works with a local CA. + Deletes a certificate. This action currently only works on the local CA. EOT - get_action(:search).summary "Invalid for this face." - get_action(:save).summary "Invalid for this face." + get_action(:search).summary "Invalid for this subcommand." + get_action(:save).summary "Invalid for this subcommand." end diff --git a/lib/puppet/face/certificate_request.rb b/lib/puppet/face/certificate_request.rb index 29cf7dc78..12694ba21 100644 --- a/lib/puppet/face/certificate_request.rb +++ b/lib/puppet/face/certificate_request.rb @@ -6,20 +6,20 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do summary "Manage certificate requests." description <<-'EOT' - Retrieves and submits certificate signing requests (CSRs). Invoke - `search` with a dummy key to retrieve all outstanding CSRs, invoke - `find` with a node certificate name to retrieve a specific request, and - invoke `save` to submit a CSR. + This subcommand retrieves and submits certificate signing requests (CSRs). EOT # Per-action doc overrides - get_action(:destroy).summary "Invalid for this face." + get_action(:destroy).summary "Invalid for this subcommand." get_action(:find).summary "Retrieve a single CSR." get_action(:find).arguments "" get_action(:find).returns <<-'EOT' - A single certificate request. In most cases, you will want to render - this as a string ('--render-as s'). + A single certificate request. When used from the Ruby API, returns a + Puppet::SSL::CertificateRequest object. + + RENDERING ISSUES: In most cases, you will want to render this as a string + ('--render-as s'). EOT get_action(:find).examples <<-'EOT' Retrieve a single CSR from the puppet master's CA: @@ -30,16 +30,17 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do get_action(:search).summary "Retrieve all outstanding CSRs." get_action(:search).arguments "" get_action(:search).returns <<-'EOT' - An array of certificate request objects. In most cases, you will - want to render this as a string ('--render-as s'). + A list of certificate requests; be sure to to render this as a string + ('--render-as s'). When used from the Ruby API, returns an array of + Puppet::SSL::CertificateRequest objects. EOT get_action(:search).notes "This action always returns all CSRs, but requires a dummy search key." get_action(:search).examples <<-'EOT' - Retrieve all CSRs from the local CA: + Retrieve all CSRs from the local CA (similar to 'puppet cert list'): $ puppet certificate_request search x --terminus ca EOT - get_action(:save).summary "Submit a certificate signing request." + get_action(:save).summary "API only: submit a certificate signing request." get_action(:save).arguments "" end diff --git a/lib/puppet/face/certificate_revocation_list.rb b/lib/puppet/face/certificate_revocation_list.rb index 0525a601f..12ba1f73f 100644 --- a/lib/puppet/face/certificate_revocation_list.rb +++ b/lib/puppet/face/certificate_revocation_list.rb @@ -6,16 +6,18 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do summary "Manage the list of revoked certificates." description <<-'EOT' - This face is primarily for retrieving the certificate revocation - list from the CA. Although it exposes search/save/destroy methods, - they shouldn't be used under normal circumstances. + This subcommand is primarily for retrieving the certificate revocation + list from the CA. EOT get_action(:find).summary "Retrieve the certificate revocation list." get_action(:find).arguments "" get_action(:find).returns <<-'EOT' - A certificate revocation list. You will usually want to render this - as a string ('--render-as s'). + A certificate revocation list. When used from the Ruby API: returns an + OpenSSL::X509::CRL object. + + RENDERING ISSUES: this should usually be rendered as a string + ('--render-as s'). EOT get_action(:find).examples <<-'EXAMPLES' Retrieve a copy of the puppet master's CRL: @@ -27,13 +29,11 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do get_action(:destroy).arguments "" get_action(:destroy).returns "Nothing." get_action(:destroy).description <<-'EOT' - Deletes the certificate revocation list. This cannot be done over - REST, but it is possible to both delete the locally cached copy of - the CA's CRL and delete the CA's own copy (if running on the CA - machine and invoked with '--terminus ca'). Needless to say, don't do - this unless you know what you're up to. + Deletes the certificate revocation list. This cannot be done over REST, but + it is possible to delete the locally cached copy or (if run from the CA) the + CA's own copy of the CRL. EOT - get_action(:search).summary "Invalid for this face." - get_action(:save).summary "Invalid for this face." + get_action(:search).summary "Invalid for this subcommand." + get_action(:save).summary "Invalid for this subcommand." end diff --git a/lib/puppet/face/config.rb b/lib/puppet/face/config.rb index 6e5bff071..3fce0ed70 100644 --- a/lib/puppet/face/config.rb +++ b/lib/puppet/face/config.rb @@ -7,13 +7,11 @@ Puppet::Face.define(:config, '0.0.1') do summary "Interact with Puppet's configuration options." action(:print) do - summary "Examine Puppet's current configuration options." - arguments "(all |