MAAS passes a commissioning script parameter type instead of its name as a command-line argument name

Bug #2056792 reported by Dmitrii Shcherbakov
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
Fix Committed
High
Andrew Lamzed-Short

Bug Description

MAAS currently passes a commissioning script parameter type instead of its name as a command-line argument name when invoking commissioning scripts with argument types other than "storage" or "interface".

https://github.com/maas/maas/blob/1027c766413d7019ed1d3468c3ca18ac929b2475/src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py#L872-L876
        else:
            argument_format = param.get(
                "argument_format", "--%s={input}" % param_type
            )
            ret += argument_format.format(input=param["value"]).split()

For example, using the "url" parameter type (https://discourse.maas.io/t/commissioning-and-hardware-testing-scripts/833#heading--parameters) as follows:

parameters:
  interface:
    type: interface
    foo:
      type: url
      default: https://foo.example.com
    bar:
      type: url
      default: https://bar.example.com

will result in a commissioning script to be called as:

<script-name> --url https://foo.example.com --url https://bar.example.com

Instead of:

<script-name> --foo https://foo.example.com --bar https://bar.example.com

Related branches

description: updated
description: updated
Revision history for this message
Bill Wear (billwear) wrote :

rare that i *don't* need a reproducer, but this seems like one of those times. given i'm triaging this one on logical analysis alone, i'd better share my thinking, so someone can slap me if i'm wrong:

- the code snippet (indeed from the MAAS code) shows that the argument format defaults to `param_type` if no `argument_format` is present.
- this does indeed mean that the code's constructing argument names based on type, rather than parameter names, as intended.
- the given example bears this out.

the expected behavior, based on MAAS documentation (hey, i *wrote* that) and typical command-line conventions, is to use named parameters so that scripts can actually *differentiate* between the passed arguments. that clearly ain't happenin' here, y'all. again quoting myself, the MAAS commissioning and hardware testing scripts documentation says that parameters are meant to be passed to scripts with specific names that the scripts will recognize. again, ain't happening.

that makes this a bug, for sure, imnsho.

this is a bit problematic, since this leads to ambiguity in script execution, since the script would tell the two apart. this can cascade to unexpected arguments, causing runtime errors, incorrect behavior, or just general weirdness.

triaging this and giving it some heat.

Changed in maas:
status: New → Triaged
importance: Undecided → High
milestone: none → 3.6.0
Revision history for this message
Dmitrii Shcherbakov (dmitriis) wrote :

Bill, thanks for triaging so quickly!

I tried this in a lab environment when debugging a commissioning script and later found the code responsible for options.

One of the reasons it hasn't been spotted before could be that there's only one builtin script that currently uses this code path and the parameter name matches the type so it doesn't trigger the issue:
https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/maas/maas%24+%22type:+url%22&patternType=keyword&sm=0
   url:
     type: url
     description: A comma separated list of URLs, IPs, or domains to test if

Changed in maas:
assignee: nobody → Andrew Lamzed-Short (andyls)
Changed in maas:
status: Triaged → Fix Committed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.