User and database name inputs validated in MySQL model

[Matthew Van Dijk]

When inputting user and database names the values are validated against the MySQL models, regardless of the datastore, in the api layer. This can allow bad names and block valid names. Bad names which get by only get caught and throw exceptions at a later stage. An example is for MongoDB instance create with an invalid user name. The API layer allows it, but then during prepare the guest will hit an exception and go into ERROR, with the only information available to the user being in the taskmanager logs.

[Matthew Van Dijk (mvandijk)]

This is not a blocking issue, just UX.

[Petr Malik (pmalik)]

The 'collate' and 'character_set' properties are datastore specific and should be moved from the base schema model to derived classes that require them (MySQL, Postgres).

They are currently in the base because other datastores still use the MySQL extensions which try to access them.

[Petr Malik (pmalik)]

We should also get rid of 'models.RootUser' which is just a MySQLUser. Datastores should be using their own root users instead.

The usage in the common extension code should be replaced with an abstract getter method overriden in derived datastore-specific extensions which would be returning the appropriate object.

[Petr Malik (pmalik)]

I poked around the API extensions a bit trying how it could be refactored to allow datastores easily implement their own with minimal amount of code.

The extensions are currently broken down into models, views and service modules. The service module contains the Controller classes (e.g. UserController). The Controllers are where the heavy lifting is done (it is the class guest should be overriding - see the datastore-specific 'root_controller' config property - for handling root-related API calls).

The controllers use model and view classes internally (all static calls). Unfortunately, rather than doing all (datastore specific) validations in the Controller classes the current implementation does *some* (but not all) in the models as well (it calls into guestagent.db.models to build datastore-specific model objects that also perform validations when constructed).

Implementing a new user/database extension for a non-mysql datastore would hence require overriding both the controller and mode/view classes. That's many classes for just building the proper objects...

I would suggest removing the datastore-specific code from models and views (all datastores use the same fields anyways). The validation should be all moved into the Controllers, before the static model methods get called (building the proper model object could be implemented via a single overridable method in the Controller class). The models and views would then simply perform serialization (which does not really require any datastore-specific functionality [it can use the base model's 'deserialize' method to do that, no need to construct a datastore-specific class, alternatively some models deserialize 'manually' - see https://github.com/openstack/trove/blob/master/trove/extensions/mysql/models.py#L105 )

Each datastore would then need to provide the 'user_controller' and 'database_controller' configuration property (like it already does with the 'root_controller'). The base controller implementation could then use the same model/view implementation (current MySQL classes moved into the common package).
Datastore-specific controller would then just implement 'builder' methods for guestagent models called by the base implementation which could then stay pretty much intact.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/278074
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=7a99a124b4cc756cac5e124490a3f57f4582fa83
Submitter: Jenkins
Branch: master

commit 7a99a124b4cc756cac5e124490a3f57f4582fa83
Author: Petr Malik <email address hidden>
Date: Wed Sep 30 14:19:25 2015 -0400

    Cleanup guestagent models

    Cleaned up the guestagent database/user __init__()
    methods by moving the deserialization and
    error-checking code to the base.

    * Also had to update CouchDB service to use
    the new model's serialization/deserialization
    calls (tested with int-tests).

    Make use of the models in the Postgres manager

    The guestagent should make use of these new classes internally.
    Currently it creates serialized versions of those objects 'manually'
    using Python dicts.

    Change-Id: I8b1ae9207948f211de8fcc2a0d3097bd37e06d78
    Related-Bug: 1498573
    Closes-Bug: 1543739

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/236082
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=1c819c3e37e6019464ed59d90bf3d8c83581202d
Submitter: Jenkins
Branch: master

commit 1c819c3e37e6019464ed59d90bf3d8c83581202d
Author: Matt Van Dijk <email address hidden>
Date: Wed Oct 14 14:47:11 2015 -0400

    Improve guestagent datastore models

    All validation of requests is currently done against the MySQL datastore
    models. In order to switch to using datastore extensions for validation
    the models need to be refactored.

    The current approach to guestagent/db/models.py is getting unruly and
    is difficult to apply to other datastores.

    This change moves the models from the guestagent package to the common
    package, so that the extensions package can use it in the future.
    That is: guestagent/db/models.py -> common/db/models.py

    The generic models are separated from the datastore-specific models. For
    datastores with custom models the classes have been moved to their own
    sub-package.
    Example: common/db/mysql/models.py

    Using this new approach any datastores that want custom models just need
    to create packages similar to the common/db/mysql/ package.

    The code references to these models have all been updated. Non-MySQL
    based datastores using these references have been switched to using the
    generic models.

    The tests for these models have been improved.

    Change-Id: If321202a3ec4ab0f57ee8e516b885a8307d464b7
    Partial-Bug: 1498573