feat: re-implement create storage pool facades for Juju 4.0

[tlm]

This commit re-implements storage pool creation from the facade down to the state layer. An attempt was made to sort of do this at the start of the domain but it left a lot to be desired on the implementation front.

The domain it self still has several rough edges but they will get fixed in follow up PR's as other facade calls are wired in. The below is the set of notable changes in this PR and why they were performed.

Storage Pool Related Changes

Provider Type type

Introduced a new type to the storage domain for representing the type of a provider. Previous to this our code was referring to the internal package type offering similar functionality. Long term the internal type will go away but we don't need to or want to have the internal package sprawl out. This is now a domain concern and so it makes sense for the provider type to be owned and controlled by the domain.

Provider Type type Validation

We rely on the client passing us the provider type value in certain operations. For example when creating storage pools, until now we have no exercised the value we have been given. The client could pass us an excessively long string or Unicode code points that we don't support. In order to be more sane, fail early and stamp out denial of service issues a new validator was introduced in this PR to check that the value falls within the certain permissible boundaries that we expect. This now gives confidence to the code that the memory occupied by the client is safe for further processing.

Return Params Error For Unauthorised Out of The Facade

When asserting if a caller to the facade is authorised for a given storage action we were directly returning the error received from the authorizer out of the facade func. This then relied on the facade catch all handler to finally deal with the error and return a correct params based error to the client.

The problem with this is you can't reasonably write good tests for the facade to assert the authorisation logic that is has. Instead of relying on how the facades get wired up with can be fraught with danger on change the facade now returns the correct params error for unauthorized making it clear what has gone wrong to the caller.

This allows for testing of the facade to assert permission access over time and make sure that internal changes to the facade do not break authz.

New Error Type For Incorrect Storage Provider Attribute Values

I have introduced a new error type StoragePoolAttributeInvalid to the storage domain so that when creating storage pools and validating the supplied config with the storage provider each component can safely communicate upwards what about the config is invalid.

This error type isn't in use in this PR except outside of tests. It required one more pass through to update the storage providers themselves. But in affect what this allows is for the validation of storage config to have errors passed up to the facade where they can be written in a user facing way to convey the problem. Without this we always going to be writing user facing errors in environs with no context on how we ended up here.

Removal Of Storage Pool Attribute Type

The initial implementation of the domain had a specialised type to represent the config attributes of a storage pool. The reality was this type wasn't adding any value to the problem and a map[string]any is more then sufficient to represent the raw information.

Storage Pool UUID creation

Storage pool UUID creation now happens in the service layer and not that of the state layer. A new internal type representing the create args for a storage pool was made to help pass this information along. The service func now also returns the UUID of the new pool so we can move away from using the StoragePool name as the identifier.

No Upsert Operation On Storage Pool Creation

This PR removes the potentially dangerous upsert operation when create storage pools. Storage pool creation is now just an insert with a check on UUID and storage pool name for uniqueness.

Storage Pool Attributes Batch Inserted

Storage pool attributes are now batch inserted with the transaction instead of being one by one potentially leading to a costly operation.

Drive By Changes

To do this work I tried to move the storage domain towards a better direction and touch up some of the initial implementations problems it had. These include:

• Removed excessive validation of storage pool data out of the state layer. The state layer was trying to do validation on user input. This MUST be happening at the service level and shouldn't make it this far.
• Domain errors for storage where lacking a consistent pattern. For example referring to storage pools as pools when the rest of the modelling is using storage pool. Removing Error suffix from the end of error consts as this was already implied based on the import package.
• Added tests to the storage facade for asserting permissions of already implemented facade methods. What we had already was not covered. New tests are more exhaustive and reflect the contracts the facade maintains with a client.
• Removed prescriptive setting up of state from storage facade base test. We don't want tests that establish a certain state which all sub tests inherit. This leads to confusing tests relying on behaviour that is easily missed. The test should tell a story about the pre and post.
• Renamed the pool list test suite for the storage facade to poolListSuite as this follows the pattern of having different suites in the facade for testing isolated and related components together instead of an all in test suite that is hard to grok the larger it gets.

Future PR

• At the moment a storage provider can and will add extra attributes to a storage pool to support operations. These extra attributes are dynamic and not tracked by the controller. To avoid changes in behaviour and be repeatable we need to capture these defaults in the storage pool on creation.

Checklist

• Code style: imports ordered, good names, simple structure, etc
• Comments saying why design decisions were made
• Go unit tests, with comments saying what you're testing
• [ ] Integration tests, with comments saying what you're testing
• [ ] doc.go added or updated in changed packages

QA steps

There is a bit to cover for this PR......

QA 1

Demonstrate that Juju 3.6 has some gremlins that needed to be addressed specifically by this PR. That is the

1. Bootstrap a Kubernetes controller. Provider substrate doesn't matter for this test.
2. Add a new model
3. Add a new storage pool to the model with juju create-storage-pool 'foo?00' kubernetes

This demonstrates with juju storage-pools that we ended up with a bug in our validation regex for storage pool names that allow '?' characters. Based on historical digging of PR's this should have never been the case.

We will have to allow this bad pattern during model migration (handled in future PR). But as of this PR we won't be allowing this anymore for new storage pools. Potentially because we now have RI in the storage model Juju 4.0 could support renaming the storage pool as it is just a display component. We will have to investigate what that may mean for resources deployed in the cloud under the storage pool and if changing the name would affect those.

In Juju4.0

1. Bootstrap a new controller (any substrate will do)
2. Add a new model
3. Add a new storage pool to the model with juju create-storage-pool 'foo?00' kubernetes

We expect to see this now fail with something along the lines of ERROR storage pool name is either not set or is not a valid name.

QA 2

Create a new storage pool in the model and confirm the database representation is correct. We also want to make sure that the origin_id is set 0 meaning user defined.

1. Bootstrap a new controller, add a model
2. Create a new storage pool juju create-storage-pool foo tmpfs
3. juju ssh -m controller 0
4. juju_db_repl
5. .switch model-test
6. select * from storage_pool;

QA 3

Bootstrap a new controller with defined storage pools

1. juju bootstrap localhost --storage-pool=name=test --storage-pool=type=tmpfs
2. juju switch controller
3. juju stoage-pools

Confirm the bootstrapped storage pool shows up in the list

Documentation changes

N/A

Links

Jira card: JUJU-8890

[tlm]

/build

[manadart]

This exceeds the size that would anticipate effective review. Any way to break it up?