Juju2: websockets API should alert, error or warn about ignored parameters

Bug #1585289 reported by Chad Smith
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical Juju
Expired
Medium
Unassigned

Bug Description

Websockets API silently ignores invalid parameters, which make debugging "failed" websocket API calls harder for consumers (Landscape).

In the following example, I overlooked the parameter name change from ToMachineSpec -> Placement when performing an add unit call, the result is that no placement constraints are made and a new machine is consumed. Given the number of name changes that juju api has undergone, the API should probably return an ErrorCode or warning message if there are parameters provided which are ignored or invalid.

Note that no error message comes from the UI for invalid parameters, no matter how they are supplied.
https://pastebin.canonical.com/157143/
https://pastebin.canonical.com/157145/

Changed in juju-core:
status: New → Triaged
importance: Undecided → Medium
tags: added: kanban-cross-team
tags: removed: kanban-cross-team
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

This bug is expensive.

This week it caused a lot of trouble again:
- CI jobs failing all over the place during the weekend
- time spent to triage. Bug filed: https://bugs.launchpad.net/landscape/+bug/1606220
- autopilot all of a sudden started grabbing *random* nodes from MAAS for cloud deployments, even though it was told to grab specific nodes
- we had to introspect the raw juju api calls on the wire
- there we saw that we were requesting the right nodes, but juju was telling MAAS to grab random ones
- lots of heads were scratched
- finally the developer saw that it was a mistake in landscape where juju2 api syntax was being used against juju1. This is the fix:
- "ToMachineSpec":
- {"Directive": u"maas-name",
- "Scope": "uuid1"}}]}
+ "Placement": {
+ u"Directive": u"maas-name",
+ u"Scope": u"uuid1"}}]}

So this bug here exists in juju1 as well: the api just swallows things and ignores what it doesn't like.

This it the call we were making against juju1:
Jul 25 20:54:20 job-handler-1 INFO ANDREAS JUJU request {'Request': 'AddMachines', 'Params': {'MachineParams': [{'ToMachineSpec': {'Scope': u'863cb02c-035f-4f80-8e65-4cfeb4b7c2b6', 'Directive': u'node-13.vmwarestack'}, 'Jobs': ['JobHostUnits']}]}, 'Type': 'Client', 'RequestId': 2}

MAAS got requests like these (clock about 3min ahead of juju's):
Jul 25 20:57:15 maaslds maas.api: [INFO] Request from user andreas to acquire a node with constraints <QueryDict: {u'agent_name': [u'4219a16c-72d1-4b34-83aa-70c655bd2f97'], u'zone': [u'zone-1']}>

And this is the response we got from juju:
Jul 25 20:54:22 job-handler-1 INFO ANDREAS JUJU response {u'RequestId': 2, u'Response': {u'Machines': [{u'Machine': u'1', u'Error': None}]}}

No error. The outcome? Random nodes being grabbed in MAAS :)

Revision history for this message
Ian Booth (wallyworld) wrote :

We discussed this today with Bjorn and Geoff. Unfortunately, Go does not support strict json unmarshalling, so without bespoke development effort, there's not a lot we can do to easily validate the received parameters.

Revision history for this message
Tim Penhey (thumper) wrote :

We could actually do this without too much difficulty by adding a strict mode to the rpc layer itself. I proposed this in London, and it is several days work.

Changed in juju-core:
milestone: none → 2.0.0
affects: juju-core → juju
Changed in juju:
milestone: 2.0.0 → none
milestone: none → 2.0.0
Curtis Hovey (sinzui)
Changed in juju:
milestone: 2.0.0 → 2.0.1
Curtis Hovey (sinzui)
Changed in juju:
milestone: 2.0.1 → none
Revision history for this message
Canonical Juju QA Bot (juju-qa-bot) wrote :

This bug has not been updated in 5 years, so we're marking it Expired. If you believe this is incorrect, please update the status.

Changed in juju:
status: Triaged → Expired
tags: added: expirebugs-bot
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.