Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anoncreds - provide an "upgrade()" endpoint to upgrade a wallet from askar to askar-anoncreds #2808

Closed
ianco opened this issue Feb 21, 2024 · 6 comments
Assignees
Labels
AnonCreds Ledger Agnostic AnonCreds

Comments

@ianco
Copy link
Contributor

ianco commented Feb 21, 2024

Add an Admin API endpoint to aca-py to upgrade a wallet from askar to askar-anoncreds. This needs to support a single wallet, which will be the aca-py wallet for a single-instance scenario; or the "base" wallet for a multi-tenant scenario; or the individual tenant wallet which is in scope when in multi-tenancy mode.

The specific wallet records to be updated are identified in issues #2384, #2385, #2386, #2389.

When upgrading a wallet:

  • at the beginning of the upgrade process, update a status in the database to flag that the upgrade is in progress
  • during the upgrade, all admin endpoints should return a "busy" error code
  • agent-to-agent messages received during the upgrade process should be ignored
  • once the upgrade process is complete, update the status to indicate the wallet was upgraded to "askar-anoncreds"
  • the only "source" database that is supported is "askar" (if aca-py is still running on an Indy wallet, it must be first upgraded to askar using the external upgrade script)

The upgrade process itself will just iterate through the impacted record types (identified in the above tickets), read each record, convert to "anoncreds" format, and then write back to the database.

If the upgrade is aborted in process, then it can just be restarted and it should pick up where it left off (i.e. update the "old" format records and ignore anything already converted to "anoncreds" format.) If the upgrade endpoint is called after the upgrade has already been completed, it should return a success code (no error).

For multi-tenancy tenants, the upgrade should be invoked with a Admin API endpoint, when the wallet-type is changed from askar to askar-anoncreds.

For a single-instance aca-py, and for the multi-tenancy "base" wallet, the upgrade endpoint should be called to upgrade the database. To change the overall --wallet-type parameter, the deployer will need to:

  • upgrade the controller code to use the new anoncreds endpoints, and add a call to the upgrade endpoint
  • deploy the updated aca-py code (which supports anoncreds)
  • deploy the new controller, with the call to the upgrade endpoint
  • once the upgrade is completed, shutdown the controller
  • update the aca-py --wallet-type configuration to specify askar-anoncreds
  • restart aca-py
  • at this point the controller can optionally be updated to remove the call to the upgrade endpoint, however repeated calls to this endpoint shouldn't cause any issues
@ianco ianco added the AnonCreds Ledger Agnostic AnonCreds label Feb 21, 2024
@WadeBarnes
Copy link
Contributor

WadeBarnes commented Feb 22, 2024

  • TBD what to do with agent-to-agent messages received during the upgrade process?

Would it be appropriate to simply not respond, basically an indication the agent is not available? Correct me if I'm wrong, but wouldn't any other response require access to the wallet, which we are trying to avoid?

For multi-tenancy tenants, the upgrade should be invoked with a Admin API endpoint (either a new "upgrade" endpoint, or as a function executed automatically when the wallet-type is set. NOTE this is to be confirmed.

My vote is the upgrade process is triggered when the tenant explicitly updates the wallet type in their per-tenant configuration from askar to askar-anoncreds. This provides a clear indication the tenant is consciously and intentionally migrating their configuration; a tenant would have no other reason to update their wallet configuration.

@swcurran
Copy link
Contributor

Some comments:

Add a function to aca-py to upgrade a wallet from askar to askar-anoncreds. This needs to support a single wallet, which will be the aca-py wallet for a single-instance scenario; or the "base" wallet for a multi-tenant scenario; or the individual tenant wallet which is in scope when in multi-tenancy mode.

I think it would be clearer to say an “Admin API endpoint” vs. Function, to differentiate between the ACA-Py “upgrade()” function we have today. In the meeting yesterday, the term “set wallet type” was used, and I think that is a good one.

during the upgrade, all admin endpoints should return a "busy" error code

To reiterate what @WadeBarnes already said, but in different (probably worse) words… 😄 :

I think it would be sufficient (but @WadeBarnes would know better) would be to guide Controller DevOps folks to run the “set wallet type” only after dialing down old controller version instances to 0, and then starting a new controller version instances that all call the “set wallet type” endpoint such that only one gets to perform the process, and all wait until it is complete before proceeding (semaphore). That eliminates any other activity during the upgrade. People that call the endpoint in other ways, e.g., by running a new instance before all the old instances terminate, get what they get.

For a single-instance aca-py, and for the multi-tenancy "base" wallet, the upgrade should happen automatically on startup.

I think with the approach we are saying, the single instance / multi-tenant instances are the same — they are endpoints executed by the Controller. I don’t think the upgrade can happen automatically — the Contoller has to invoke it — but the “best practice” (only practice) is that the Controller SHOULD do it on starting the controller, during initialization, and before any processing. There are other initialization things needed — such as setting “per tenant settings”

Upgrading the “base wallet” should be conceptually the same, but is a little more interesting since (AFAIK) there is no “base Controller” to invoke an endpoint. As such, I think that should be done by ACA-Py and on start up — with the guidance to be done the same way — bring down ACA-Py old version, bring up ACA-Py new version with new setting, and run any required upgrade process during initialization.

@ianco
Copy link
Contributor Author

ianco commented Feb 22, 2024

I think with the approach we are saying, the single instance / multi-tenant instances are the same — they are endpoints executed by the Controller.

The difference is that for a tenant, the settings are update by an API call, whereas for a single instance everything is set by aca-py startup params. So if we want to do a wallet upgrade by explicit API call only, then for the single aca-py instance scenario the --wallet-type startup parameter still has to be changed, but will need to be coordinated with the wallet upgrade. Either (a) change the parameter, then startup aca-py and call the wallet upgrade api, or (b) upgrade the wallet, and then restart aca-py with the updated --wallet-type parameter. In either case we have the potential of running aca-py with a mis-matched wallet configuration.

@ianco
Copy link
Contributor Author

ianco commented Feb 22, 2024

when the tenant explicitly updates the wallet type in their per-tenant configuration from askar to askar-anoncreds

I suggested in the other ticket that the tenant shouldn't be allowed to explicitly set their wallet type (askar vs askar-anoncreds). When we create a tenant we use the overall aca-py setting, and after we upgrade the multi-tenant aca-py from askar to askar-anoncreds then the only option for existing askar wallets is to upgrade. I don't see any use case for allowing tenants to set their wallet type explicitly.

@swcurran
Copy link
Contributor

The difference is that for a tenant, the settings are update by an API call, whereas for a single instance everything is set by aca-py startup params.

The logic/function to do the upgrade is still the same for tenants and the ACA-Py instance overall. When a single instance ACA-Py starts up, it could run a function to check its current “wallet-type” and carry out the same logic as a tenant explicitly setting — “is the setting wallet-type the same as we are running”, “if not, is the change allowed”, “if so, semaphore (upgrade)”. This is the same logic we are doing with a “version” update, but triggering it on the wallet-type.

Presumably, this could be used for the base wallet in a multi-tenant.

The thing we don’t (generally) want is for the ACA-Py settings to impact the wallet type of the tenant wallets, since that would require a coordinated upgrade of all tenants at the same time, which we agree is impossible to manage.

after we upgrade the multi-tenant aca-py from askar to askar-anoncreds then the only option for existing askar wallets is to upgrade. I don't see any use case for allowing tenants to set their wallet type explicitly.

Not sure I’m understanding that comment. We have to allow the tenants to execute their upgrades (in this case to askar-anoncreds) independently, since Controller code changes are required. So even if the overall ACA-Py instance supports askar or askar-anoncreds, the tenant transitions from one to the other MUST be at the time the controller wants. Hence the need for the tenant specific set wallet type (or something else?). Further, I’m assuming that we will need to do similar conversions in the future, and a similar technique will be needed.

@ianco
Copy link
Contributor Author

ianco commented Feb 23, 2024

I've updated the main description per the comments, @swcurran @WadeBarnes @jamshale

@jamshale jamshale self-assigned this Mar 6, 2024
@jamshale jamshale moved this to Assigned in CDT Enterprise Apps Mar 6, 2024
@jamshale jamshale moved this from Assigned to In Progress in CDT Enterprise Apps Mar 7, 2024
@jamshale jamshale moved this from In Progress to In Review in CDT Enterprise Apps Mar 21, 2024
@jamshale jamshale closed this as completed May 8, 2024
@github-project-automation github-project-automation bot moved this from In Review to Complete in CDT Enterprise Apps May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AnonCreds Ledger Agnostic AnonCreds
Projects
None yet
Development

No branches or pull requests

4 participants