Config does not have a unique index on name

Bug #1378835 reported by Gavin Panella
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
Fix Released
High
Raphaël Badin

Bug Description

Though m.models.config.Config is intended to have only one value per configuration item name, there is no unique key to enforce this. This means that races can happen in set_config() because it uses get_or_create(). No error is raised by this race until it comes time to retrieve the configuration item:

Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 783, in __bootstrap
    self.__bootstrap_inner()
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
--- <exception caught here> ---
  File "/usr/lib/python2.7/dist-packages/twisted/python/threadpool.py", line 191, in _worker
    result = context.call(ctx, function, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/twisted/python/context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/usr/lib/python2.7/dist-packages/twisted/python/context.py", line 81, in callWithContext
    return func(*args,**kw)
  File ".../src/provisioningserver/utils/twisted.py", line 143, in wrapper
    return func(*args, **kwargs)
  File ".../src/maasserver/utils/async.py", line 153, in call_within_transaction
    return func(*args, **kwargs)
  File ".../src/maasserver/security.py", line 90, in get_shared_secret
    secret_in_db_hex = Config.objects.get_config("rpc_shared_secret")
  File ".../src/maasserver/models/config.py", line 95, in get_config
    return self.get(name=name).value
  File "/usr/lib/python2.7/dist-packages/django/db/models/manager.py", line 151, in get
    return self.get_queryset().get(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/db/models/query.py", line 310, in get
    (self.model._meta.object_name, num))
maasserver.models.config.MultipleObjectsReturned: get() returned more than one Config -- it returned 2!

Tags: tech-debt

Related branches

Revision history for this message
Gavin Panella (allenap) wrote :

There is a method Config.get_config_list() which does rely on having multiple configuration items for a given name. However, it's not used anywhere, and I think we would be better off to remove it. Config settings can be any JSON-serialisable data structure so it's trivial to store a list if necessary.

Christian Reis (kiko)
Changed in maas:
milestone: none → next
Revision history for this message
Christian Reis (kiko) wrote :

Is there any risk this happens in normal operation of MAAS via the web UI?

What about via the API?

What about in corner cases of normal operation in the nodes -- I assume there's no config involved there?

Revision history for this message
Gavin Panella (allenap) wrote :

The first time I saw this was when implementing get_shared_secret, which is often called in multiple threads or processes at similar times. I had put a lock around the critical section using DatabaseLock, but I eventually realised that wasn't enough, so I implemented and used DatabaseXactLock instead.

Anyway, get_shared_secret is now properly locking, so it won't be causing duplicate config items. In theory it is possible to trigger this bug via the web UI or the API, but no one has yet reported it, and I've not seen it.

Christian Reis (kiko)
Changed in maas:
milestone: next → 1.7.1
Raphaël Badin (rvb)
Changed in maas:
assignee: nobody → Raphaël Badin (rvb)
Revision history for this message
Raphaël Badin (rvb) wrote :

I agree with Gavin's analysis here. Duplicated entries will only have been created by pathological cases (races).

Raphaël Badin (rvb)
Changed in maas:
status: Triaged → Fix Committed
Changed in maas:
status: Fix Committed → Fix Released
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.