repeated flags are truncated

[squillace]

@mikkelhegn was trying to us a repeating flag for an az-mixin command and ran in to the problem of the corresponding az command not accepting multiple parameters of the same name (basically only taking the last into account).

Here’s his setup:

Porter.yaml snippet:

  - az:
      description: 'Deploy Web API configurations'
      arguments:
        - webapp
        - config
        - appsettings
        - set
      flags:
        ids: '{{ bundle.outputs.WEBAPI_ID }}'
        settings:
        - 'PGHOST={{ bundle.outputs.POSTGRES_HOST }}'
        - 'PGUSER={{ bundle.outputs.POSTGRES_USER }}'
        - 'PGPASSWORD={{ bundle.outputs.POSTGRES_PASSWORD }}'
        - 'PGDB={{ bundle.outputs.POSTGRES_DB }}'

The generated ‘az’ command – according to debug output:

az webapp config appsettings set --ids /subscriptions//resourceGroups/MyRG/providers/Microsoft.Web/sites/webapi-b2g2ndukr2eqy --output json --settings PGHOST= --settings PGUSER= --settings PGPASSWORD= --settings PGDB=***

This results in only the last --settings argument is being stored by the RP. Now is it a “wrong” mixin or az implementation?

(For a second let’s disregard that the command takes a json-file as an input for a collection of settings as an alternative)

Reading through this, I tend to be of the opinion, that there could be a better az command implementation: https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md#collection-management-with-commands

e.g.

webapp config appsettings setting set --ids xxxxx --setting-name PSUSER --setting-value *****

webapp config appsettings setting set --ids xxxxx --file settings.json

The implementation which the mixin assumes, is not the recommended implementation, however it’s acceptable ☺: https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md#collection-management-with-actions

Two questions:

Do you agree it’s right to raise the issue towards the command owner to get the command fixed? Should the az mixin support repeatable flags, if it’s an anti-pattern in az?

[vdice]

I do think it may be worth inquiring upstream (w/ the az cli maintainers) if adjustment of the --settings flag on that command, to support repeated invocations, might be possible. I believe there are other az commands where this repeated form is used, so there is precedent.

For unblocking this particular use case, we can always fall back on exec mixin use. It isn't as nice to read/compose, but it should get the job done:

  - exec:
      description: 'Deploy Web API configurations'
      command: az
      arguments:
        - webapp
        - config
        - appsettings
        - set
        - --ids '{{ bundle.outputs.WEBAPI_ID }}'
        - --settings 'PGHOST={{ bundle.outputs.POSTGRES_HOST }} PGUSER={{ bundle.outputs.POSTGRES_USER }} PGPASSWORD={{ bundle.outputs.POSTGRES_PASSWORD }} PGDB={{ bundle.outputs.POSTGRES_DB }}'

Lastly, I do think it might be worth considering adding configurability on how a repeated flag is parsed and turned into a cli invocation. I could see this being applicable to any of our mixins. For instance, maybe a toggle like repeat: true/false and a new repeatedFlags section, such that:

  repeatedFlags:
  repeat: false
    settings:
        - 'PGHOST={{ bundle.outputs.POSTGRES_HOST }}'
        - 'PGUSER={{ bundle.outputs.POSTGRES_USER }}'

results in ... --settings PGHOST={{ bundle.outputs.POSTGRES_HOST }} PGUSER={{ bundle.outputs.POSTGRES_USER }}

and the default can be repeat: true (behavior remains the same as what it is today):

  repeatedFlags:
    settings:
        - 'PGHOST={{ bundle.outputs.POSTGRES_HOST }}'
        - 'PGUSER={{ bundle.outputs.POSTGRES_USER }}'

results in ... --settings PGHOST={{ bundle.outputs.POSTGRES_HOST }} --settings PGUSER={{ bundle.outputs.POSTGRES_USER }}

Just spitballing there... design/approach could definitely be changed; but the gist of the functionality that we might want is there.