Arvados

15795-sys-root-token @ f9b77d02131fc84a59ba72118dd41f34c894277f

https://ci.curoverse.com/view/Developer/job/developer-run-tests/1667/

Tests will probably do better after merging master.

I was surprised to find this inside the "not a v2 token" else block. Unless there's a reason SystemRootToken shouldn't be allowed to look like a v2 token, I think "if SystemRoot ... elsif v2 ... else ..." would be a bit less mysterious.

The create_superuser_token.rb script creates an api_client entry with is_trusted=true. Here we use api_client_id=0 instead, which (I think) will mean is_trusted=false. Does this have any implications for services or other expected root token usage?

Add SystemRootToken instructions to https://doc.arvados.org/master/install/install-api-server.html, and remove the steps on keepstore (and other?) pages that say to run create_superuser_token.rb and copy it to SystemRootToken

Ensure a 1.4-to-1.5 upgrade doesn't result in an API server that won't come up (is there anything in 1.4 that would have told the admin to add a SystemRootToken?)

Seems like we could also remove the superuser token fixture from the test database, and instruct the admin to remove their old database record after upgrading (or even do that automatically -- maybe at server startup?).

Tom Clegg wrote:

The create_superuser_token.rb script creates an api_client entry with is_trusted=true. Here we use api_client_id=0 instead, which (I think) will mean is_trusted=false. Does this have any implications for services or other expected root token usage?

This ends up putting a stake through the heart of "don't do a database lookup" if we can't use a static api_client_id. So I went with a new approach: synchronize the configuration system root token with a special database record "zzzzz-gj3su-000000000000000".

The "without a database lookup" part of the story is no longer true, but otherwise this ensures everything behaves normally by having the system root token be a real record.

Add SystemRootToken instructions to https://doc.arvados.org/master/install/install-api-server.html, and remove the steps on keepstore (and other?) pages that say to run create_superuser_token.rb and copy it to SystemRootToken

I will be doing a big refactor/rewrite of install and configuration docs in #15572, so can I defer this and address it there?

Ensure a 1.4-to-1.5 upgrade doesn't result in an API server that won't come up (is there anything in 1.4 that would have told the admin to add a SystemRootToken?)

Ok, I made SystemRootToken optional for API to start up. However, without setting a value there, the controller callbacks that use it won't work. Tokens issued by create_superuser_token.rb will continue to work, though.

15795-sys-root-token @ 6749376dd2fbd5f6bc6f870ae382bdb5cb8aaf17

https://ci.curoverse.com/view/Developer/job/developer-run-tests/1669/

this ensures everything behaves normally by having the system root token be a real record

Hm. Copying/synchronizing SystemRootToken into the database feels like a step backward. Could we use a new unsaved api_client record, just like the new unsaved api_client_authorization record? Or even update the small number of places where a_c_a.a_c.is_trusted gets checked?

This is blocking #15720 because #15720 behaves badly when SystemRootToken is empty -- but isn't this latest version back to allowing SystemRootToken to be empty?

Seems like the config loader should be helping with the migration: if SystemRootToken is missing, get it from the database & warn; if not in database either, must be a new cluster, so just error out.

Tom Clegg wrote:

this ensures everything behaves normally by having the system root token be a real record

Hm. Copying/synchronizing SystemRootToken into the database feels like a step backward. Could we use a new unsaved api_client record, just like the new unsaved api_client_authorization record? Or even update the small number of places where a_c_a.a_c.is_trusted gets checked?

Ok I backed all that out. Creates a trusted ApiClient record to go with the ApiClientAuthorization. Passes tests. Rebased so the branch history isn't confusing.

a789502c40006646e3929246cf79f50bc89c0e54 -- https://ci.curoverse.com/view/Developer/job/developer-run-tests/1673/

Got a successful build here

https://ci.curoverse.com/job/developer-run-tests-apps-workbench-integration/1775/

Will merge now.

The merge didn't include the fix from note-12. Looks like the missing uuid also causes a crash in containers#update when using SystemRootToken, so containers can't finish.

2019-11-28T14:31:02.716988850Z error in UpdateContainerFinal: arvados API server error: #<TypeError: no implicit conversion of nil into String> (req-14awf5imf8b9s74mztv6) (422: 422 Unprocessable Entity) returned by 4xphq.arvadosapi.com

TypeError: no implicit conversion of nil into String
/data/var-www/arvados-api/current/app/models/api_client_authorization.rb:301:in `+'
/data/var-www/arvados-api/current/app/models/api_client_authorization.rb:301:in `v2token'
/data/var-www/arvados-api/current/app/models/api_client_authorization.rb:293:in `token'
/data/var-www/arvados-api/current/app/models/container.rb:505:in `validate_change'
/data/var-www/arvados-api/current/app/controllers/application_controller.rb:120:in `update'
/data/var-www/arvados-api/current/app/controllers/arvados/v1/containers_controller.rb:33:in `block in update'
/data/var-www/arvados-api/current/app/controllers/arvados/v1/containers_controller.rb:32:in `update'
/data/var-www/arvados-api/current/app/controllers/application_controller.rb:423:in `block in set_current_request_id'
/data/var-www/arvados-api/current/app/controllers/application_controller.rb:422:in `set_current_request_id'
/data/var-www/arvados-api/current/app/middlewares/arvados_api_token.rb:66:in `call'

Merged 3d5e30397 to master @ fb72a4315bfcf64621d023a38d1544319aa3666c