Feature #15000
closed[controller] publish safe config
Description
Controller should publish at a well-known API endpoint a subset of the configuration which is not sensitive, which can be accessed without authorization (similar to the discovery doc). It should have the same shape as the full configuration, but only include whitelisted keys/sections.
Sections that should be public:
- "ExternalURL" for "Services"
- Collections
- DefaultReplication
- DefaultTrashLifetime
- CollectionVersioning
- BlobSigningTTL
- Containers
- SupportedDockerImageFormats
- DefaultKeepCacheRAM
MaxDispatchAttempts- MaxRetryAttempts
- UsePreemptibleInstances
Logging (all)
- RemoteClusters
Workbench
Files
Updated by Peter Amstutz almost 6 years ago
- Status changed from New to In Progress
Updated by Peter Amstutz almost 6 years ago
- Status changed from In Progress to New
Updated by Peter Amstutz almost 6 years ago
- Blocks Idea #14813: [Workbench2] Use cluster config added
Updated by Tom Morris almost 6 years ago
- Target version changed from To Be Groomed to Arvados Future Sprints
Updated by Ward Vandewege over 5 years ago
- Related to Idea #13648: [Epic] Use one cluster configuration file for all components added
Updated by Peter Amstutz over 5 years ago
- Blocks Feature #14812: [Workbench1] Load configuration from cluster config file added
Updated by Tom Clegg over 5 years ago
- Category set to API
- Assigned To set to Tom Clegg
- Target version changed from Arvados Future Sprints to 2019-06-19 Sprint
Updated by Tom Clegg over 5 years ago
- added Workbench section to config.default.yml
- everything in config.default.yml is now represented in the Go config struct, too (and there's a test to prevent us from adding other stuff without updating the struct)
- controller exports the safe parts of the config struct at /arvados/v1/config
- every config has to be explicitly classified as safe/unsafe in export.go (there's a test that spits out some helpful Go code if anything is missing)
We still need to figure out how Workbench will get its secrets in #14812, since they can't be included in this unauthenticated endpoint. I don't think that needs to hold up this branch, though.
The gofmt test fails because I saved this with gofmt 1.12. I could use a useless-comment hack like we've done in other places, but I'm hoping Jenkins gets updated to 1.12 soon and makes that unnecessary. Jenkins is on Go 1.12. Make sure you have Go 1.12 to pass gofmt tests.
Updated by Peter Amstutz over 5 years ago
It appears that a duration without a suffix is now a fatal error?
2019-06-12_20:38:29.42932 {"error":"transcoding config data: duration must be given as a string like \"600s\" or \"1h30m\"","level":"info","msg":"exiting","time":"2019-06-12T20:38:29.429230820Z"}
Unfortunately it didn't tell me what key has the problem.
Updated by Tom Clegg over 5 years ago
Peter Amstutz wrote:
It appears that a duration without a suffix is now a fatal error?
s/now/still/; cluster config has always required units for durations.
Updated by Tom Clegg over 5 years ago
Updated error to include the bad value, since that's easy. Including the key/line/column from the original YAML would be ideal, but beyond scope here.
15000-config-api @ eaf26ebd285952ec6695270f0eda04ea7bf7e6c6 -- https://ci.curoverse.com/view/Developer/job/developer-run-tests/1303/
Updated by Peter Amstutz over 5 years ago
Tom Clegg wrote:
Peter Amstutz wrote:
It appears that a duration without a suffix is now a fatal error?
s/now/still/; cluster config has always required units for durations.
Got it. Then probably this configuration key wasn't previously read by the Go code.
Updated by Tom Clegg over 5 years ago
Peter Amstutz wrote:
Got it. Then probably this configuration key wasn't previously read by the Go code.
Ah, yes.
(Aside -- perhaps add to #14812? -- now that the keys are all there, we should make RailsAPI call config-dump instead of loading the cluster config file directly, so it gets the same checks/munges as everything else.)
Updated by Peter Amstutz over 5 years ago
Noticed this in config.default.yml
ImageID: ami-01234567890abcdef
I don't think it is a good idea to have bogus values in the defaults file.
Also the VM image id feels like an internal implementation detail that maybe shouldn't be exposed without a management token.
I don't think the public config should include Containers.CloudVMs, Containers.JobsAPI or Containers.SLURM.
Git.Repositories and Workbench.RepositoryCache both have filesystem-local paths that don't need to be public.
I see Containers.DispatchPrivateKey ummmmmm
I don't think the "Users" and "Mail" sections should be published.
PostgreSQL includes "ConnectionPool"
TLS.Certificate I'm not sure what is supposed to go there, but if it is a path to a file it shouldn't be published.
Updated by Tom Clegg over 5 years ago
Peter Amstutz wrote:
I don't think it is a good idea to have bogus values in the defaults file.
Agreed. Changed to "".
Now that we've established this won't be used by system components at all, I've moved the threshold from "non-secret configs needed by components" to "configs needed by clients".
15000-config-api @ 3f45203069cda0b906ceeaf64d1ac8146a085895 -- https://ci.curoverse.com/view/Developer/job/developer-run-tests/1306/
Updated by Tom Clegg over 5 years ago
- Blocks deleted (Feature #14812: [Workbench1] Load configuration from cluster config file)
Updated by Peter Amstutz over 5 years ago
I noticed that some services have {ExternalURL: "-"} which I assume means it is an internal service that isn't supposed to have an external URL (as opposed to the services with {ExternalURL: ""} where the external URL is missing from the configuration...), how hard would it be to just filter those out? What if an admin fills in ExternalURL by accident?
Updated by Tom Clegg over 5 years ago
- File 15000-docs.png 15000-docs.png added
15000-config-api @ d768d4e5b5b61949aeb2bb2e473c4ceec93957f1 adds a step to the install docs to check for accidentally-published stuff:
Updated by Peter Amstutz over 5 years ago
Tom Clegg wrote:
15000-config-api @ d768d4e5b5b61949aeb2bb2e473c4ceec93957f1 adds a step to the install docs to check for accidentally-published stuff:
Thanks, LGTM.