Optum / dce-cli

Disposable Cloud Environment CLI
Apache License 2.0
37 stars 19 forks source link

`dce leases list` does not support both -p and -s flags #58

Closed eschwartz closed 4 years ago

eschwartz commented 4 years ago

Version information

dce cli v0.3.0

Describe the bug

Providing both a -p and -s flag to dce-cli leases list does not work

To Reproduce

dce-cli leases list -p me@example.com -s Active

...will return all leases where principalId=me@example.com, no matter the status.

Expected behavior

Should onlu return leases where principalId=me@example.com AND leaseStatus=Active

Additional context

The API supports this query, as GET /leases?principalId=me@exampe.com&status=Active

joshmarsh commented 4 years ago

@eschwartz It looks like the current behavior is to replace old leases with new ones in the leases table. E.g. see the example below where I create a new lease for the admin principal. The list of leases still only contains two entries, and the admin's lease has been replaced with a new one. I verified this was consistent with the leases table.

>./dce-cli leases list
[
    {
        "accountId": "258412077092",
        "budgetAmount": 100,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            "jane.doe@email.com"
        ],
        "createdOn": 1587415776,
        "expiresOn": 1588020576,
        "id": "8442ce70-fe4c-43e1-9ee8-f840824ebf3f", <---before
        "lastModifiedOn": 1587415776,
        "leaseStatus": "Inactive",
        "leaseStatusModifiedOn": 1587415776,
        "leaseStatusReason": "Destroyed",
        "principalId": "admin"
    },
    {
        "accountId": "258412077092",
        "budgetAmount": 100,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            "jane.doe@email.com"
        ],
        "createdOn": 1587411359,
        "expiresOn": 1588020828,
        "id": "c092bb05-deb8-44d3-80bf-8923f7096beb",
        "lastModifiedOn": 1587416028,
        "leaseStatus": "Inactive",
        "leaseStatusModifiedOn": 1587416028,
        "leaseStatusReason": "Destroyed",
        "principalId": "user"
    }
]
> ./dce-cli leases create --budget-amount 100 --budget-currency USD --email jane.doe@email.com --principal-id admin
{
    "accountId": "258412077092",
    "budgetAmount": 100,
    "budgetCurrency": "USD",
    "budgetNotificationEmails": [
        "jane.doe@email.com"
    ],
    "createdOn": 1587415776,
    "expiresOn": 1588021195,
    "id": "90746a3c-2b37-4930-b059-559920a11272",
    "lastModifiedOn": 1587416396,
    "leaseStatus": "Active",
    "leaseStatusModifiedOn": 1587416396,
    "leaseStatusReason": "Active",
    "principalId": "admin"
}
> ./dce-cli leases list
[
    {
        "accountId": "258412077092",
        "budgetAmount": 100,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            "jane.doe@email.com"
        ],
        "createdOn": 1587415776,
        "expiresOn": 1588021195,
        "id": "90746a3c-2b37-4930-b059-559920a11272", <--- after
        "lastModifiedOn": 1587416396,
        "leaseStatus": "Active",
        "leaseStatusModifiedOn": 1587416396,
        "leaseStatusReason": "Active",
        "principalId": "admin"
    },
    {
        "accountId": "258412077092",
        "budgetAmount": 100,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            "jane.doe@email.com"
        ],
        "createdOn": 1587411359,
        "expiresOn": 1588020828,
        "id": "c092bb05-deb8-44d3-80bf-8923f7096beb",
        "lastModifiedOn": 1587416028,
        "leaseStatus": "Inactive",
        "leaseStatusModifiedOn": 1587416028,
        "leaseStatusReason": "Destroyed",
        "principalId": "user"
    }
]

When I apply the -s and -p flags, they seem to work as expected:

> ./dce-cli leases list -p user -s Inactive
[
    {
        "accountId": "258412077092",
        "budgetAmount": 100,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            "jane.doe@email.com"
        ],
        "createdOn": 1587411359,
        "expiresOn": 1588020828,
        "id": "c092bb05-deb8-44d3-80bf-8923f7096beb",
        "lastModifiedOn": 1587416028,
        "leaseStatus": "Inactive",
        "leaseStatusModifiedOn": 1587416028,
        "leaseStatusReason": "Destroyed",
        "principalId": "user"
    }
]
> ./dce-cli leases list -p user -s Active
[]
eschwartz commented 4 years ago

It looks like the current behavior is to replace old leases with new ones in the leases table.

@joshmarsh -- that's correct. But you're testing with a single account, so you're missing some of the behavior.

Consider if I create a lease for account=123, end that lease, then create another on account=456. In that case, I want a way to see only my the active account for my user. I don't think the CLI supports that today.

For context, this is causing some pain our DCE pipeline: https://github.com/Optum/dce/blob/master/pipelines/pipeline.yml#L176

eschwartz commented 4 years ago

I just tried dce leases list -p my-user -s Active using DCE CLI v0.29.0, and DCE v0.30.0, and it seems to be applying both query params. I'm going to call this resolved.