joyent/node-triton#116 triton package list name=~foo doesn't seem to filter package ~

[joyent-automation]

TritonDataCenter/node-triton#116 triton package list name=~foo doesn't seem to filter package ~

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/1619/.
The raw archive of this CR is here.
See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@marsell commented at 2017-03-07T04:30:50

Patch Set 1:

We need a test covering the ~ functionality.

@marsell commented at 2017-03-07T05:07:58

Patch Set 2: Code-Review+1 Integration-Approval+1

Intergrate whenever you feel appropriate. :)

@YangYong3 commented at 2017-03-07T05:09:34

Patch Set 2:

Intergrate whenever you feel appropriate. :)

thx your review ~

@trentm commented at 2017-03-16T23:41:39

Patch Set 2: Code-Review-1 Integration-Approval-1

@marsell,

PAPI ListPackages already has a mechanism for wildcard searching: '?name=foo'. We don't need another.

That wildcarding is what triton package list is attempting to do:

$ triton -v pkg ls 'name=*8*' 2> >(bunyan)
...
[2017-03-16T22:32:03.812Z] TRACE: triton/13864 on danger0.local (/Users/trentm/joy/node-triton/node_modules/restify-clients/lib/HttpClient.js:314 in rawRequest): request sent
    GET /trentm/packages?name=*8* HTTP/1.1
...

But that is broken because this is currently what PAPI seems for that request to cloudapi:

[2017-03-16T22:32:04.225Z]  INFO: audit/14358 on e8de7577-994b-4308-b430-d0bb5a152ac6: handled: 200 (req_id=dbbce6ac-78c1-4118-bec8-682d7a953319, audit=true, remoteAddress=::ffff:10.99.99.37, remotePort=57590, latency=18, _audit=true, req.version=*)
    GET /packages?name=%7B%5C2a%7D8*&active=true&owner_uuids=7a6850ee-6e07-4705-90c0-bc86f12372ea HTTP/1.1
...
    --
    req.query: {
      "name": "{\\2a}8*",
      "active": "true",
      "owner_uuids": "7a6850ee-6e07-4705-90c0-bc86f12372ea"
    }

And that escaping is due to this node-sdc-clients change:

commit 4a711d06f7fb4f6e71cf0415d6dd9fecf9b7068e
Author: Marsell Kukuljevic <marsell@joyent.com>
Date:   Tue Feb 4 00:23:40 2014 +1100

    PAPI-86 - add escaping to PAPI queries by default.

and this cloudapi change using that:

commit af77905e0606be4162745dfdfe51fb3a4ceef29d
Author: Marsell Kukuljevic <marsell@joyent.com>
Date:   Tue Feb 4 02:40:34 2014 +1100

    cherry-picked fd63186b6: PAPI-27 - bunch of optimizations and some refactoring. Also alter to use newest version o
f node-sdc-clients.

which is before triton package list existed.

So:

• triton package list name=~FOO never worked.
• The escapeParam JS code (in the PAPI index.md docs and used in node-sdc-clients/lib/papi.js) has at least two bugs in it: (a) it only escapes the first occurrence of a special char and (b) it will escape '\' in an escape code:

> esc('*a(s)\\Xdf*')
'{\\2a}a{{\\5c}28}s{\\29}\\Xdf*'

Marsell,

Could you please take on a few things:

1. Fix the escapeParam code in lib/papi.js
2. Fix the suggested JS escape code in PAPI's docs/index.md
3. Start a PUBAPI ticket to support (or at least discuss) passing through the '*' char in cloudapi's ListPackages?name=FOO to PAPI's ListPackages.

Separately I'd be curious whether the feature added in PAPI-78 is used at all. The ticket doesn't include why that feature was added. This is the change that led to adding escaping of special chars.

[trentm]

-1 See CR discussion.