Ability to read root credentials from file when bootstrapping

[adutra]

This was initially done in support of #878, which has been closed.

But I think that the feature is still useful, so I went ahead and opened a PR.

PolarisCredentialsBootstrap has been refactored into two components in order to facilitate JSON and YAML parsing: RootCredentialsSet and RootCredentials.

The expected YAML format is as follows:

realm1:
  client-id: client1
  client-secret: secret1
realm2:
  client-id: client2
  client-secret: secret2

[flyrain]

cc @eric-maynard

[dimas-b]

LGTM overall 👍 I think it improves usability of the bootstrap command and environment-based runs with auto-bootstrapping (in-memory).

Just a few minor comments and suggestions.

[dimas-b]

Still TODO?

[adutra]

Yes, waiting for #889

[dimas-b]

Do we still need this as a separate option? Why not extract after parsing the -c value?

[adutra]

The idea is that users may want to bootstrap a realm without providing credentials (they would be randomly-generated). Should I remove this option? It is true that the file-based approach does not permit randomly-generated credentials.

[dimas-b]

Ah, yes, I missed that use case 🤦

I was just trying to simplify the user experience for the manual case. WDYT about this:

--generate-secrets realm1 --generate-secrets realm2 --credentials realm2,id2,sec2 --credentials realm3,id3,sec3

[adutra]

I don't know, honestly... the more it goes, the more I'm in favor of requiring the user to provide all the credentials.

But your suggestion is at least coherent... I'm not opposed to it.

[dimas-b]

In the end, I think it's a minor point compared to the addition of reading secrets from files. We can always refactor that later.

[eric-maynard]

Conceptually I think this PR is great, but we need to standardize on what a "serialized" principal looks like across the APIs, the CLI, and this file format

[dimas-b]

[...] we need to standardize on what a "serialized" principal looks like across the APIs, the CLI, and this file format

I do not think establishing a standard for principal serialization should be a blocker for this PR. This change, IMHO, improves user experience already. If the future standard is different, we will fix the tool at the time the standard becomes effective.

Polaris is a young project without a release yet. Users should expect changes in file formats and CLI arguments as the project matures.

[dimas-b]

In the end, I think it's a minor point compared to the addition of reading secrets from files. We can always refactor that later.

[eric-maynard]

@dimas-b

I do not think establishing a standard for principal serialization should be a blocker for this PR. This change, IMHO, improves user experience already. If the future standard is different, we will fix the tool at the time the standard becomes effective.

It's not necessarily about establishing a standard across the application, but at least within the CLI. #461 was already open proposing a JSON format for the CLI, so it seems odd that we should have merged a change to the CLI that accepts YAML without a consensus about the format to use.

[dimas-b]

[...] it seems odd that we should have merged a change to the CLI that accepts YAML without a consensus about the format to use.

I believe this PR did not have any impact on existing functionalities or use cases. So, I do not think this change is an "API" kind of change. Also, as far as I can tell the code in this PR is able to parse the JSON structures proposed in #461.

But point taken on reaching consensus. We can change the format of CLI inputs in #461 or another PR if you prefer.