docs: strict and dynamic mode #477
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
CLI.md
Outdated
| @@ -48,7 +48,7 @@ This document contains the help content for the `unleash-edge` command-line prog | |||
| Default value: `3043` | |||
| * `--instance-id <INSTANCE_ID>` — Instance id. Used for metrics reporting | |||
|
|
|||
| Default value: `01HX9QXY3MNKKAMP7XRS6RG2Q4` | |||
| Default value: `01J20TW2327N1JHE5XYHBE55KM` | |||
There was a problem hiding this comment.
Hmm won't this be a new ulid every time?
| Default value: `01J20TW2327N1JHE5XYHBE55KM` | |
| Default value: Automatically generated ulid. |
I guess the reason for this, and especially this change in the value, is that we just copy paste the CLI output 🙂
There was a problem hiding this comment.
Yup, good point. I'll update the default.
There was a problem hiding this comment.
Hmm, no I won't since this is fully auto generated. For now we'll just accept that this will update each time we generate CLI.md
There was a problem hiding this comment.
Hmm, I changed it to <GENERATED_ULID> now. We should see if we can have our generator replace that value automatically
There was a problem hiding this comment.
For now we'll have to remember to replace the value with the placeholder again if we regenerate CLI.md
There was a problem hiding this comment.
Yep, makes sense 👍 Either way just something I noticed that is out of scope for the PR, so no worries.
Co-authored-by: Nuno Góis <[email protected]>
Co-authored-by: Nuno Góis <[email protected]>
Co-authored-by: Nuno Góis <[email protected]>
README.md
Outdated
| be | ||
| given | ||
| tokens at startup. | ||
| Edge will lock down access for new tokens that have a wider or different access scope than the tokens it was started |
There was a problem hiding this comment.
| Edge will lock down access for new tokens that have a wider or different access scope than the tokens it was started | |
| Edge will refuse access for new tokens that have a wider or different access scope than the tokens it was started |
README.md
Outdated
| this behavior is still the default, but Edge will log that you should make a choice between dynamic or strict behavior. | ||
| If | ||
| you | ||
| explicitly choose dynamic behavior, Edge will warn about the planned deprecation. |
There was a problem hiding this comment.
Even if you don't explicitly choose it and we default to it, we still show the warning. It behaves as shown in the PR description of #476
There was a problem hiding this comment.
Agreed. Changed text to When dynamic behavior is selected (by default or by choice), Edge will print a warning about the planned deprecation
| @@ -32,7 +32,8 @@ This document contains the help content for the `unleash-edge` command-line prog | |||
| * `-b`, `--base-path <BASE_PATH>` — Which base path should this server listen for HTTP traffic on | |||
|
|
|||
| Default value: `` | |||
| * `-w`, `--workers <WORKERS>` — How many workers should be started to handle requests. Defaults to number of physical cpus | |||
| * `-w`, `--workers <WORKERS>` — How many workers should be started to handle requests. Defaults to number of physical | |||
| cpus | |||
There was a problem hiding this comment.
Doesn't really affect the markdown but what's going on here? IntelliJ enforcing a 120 character line?
Edit: Okay I changed my mind, it's happening everywhere and it makes the commit quite noisy
README.md
Outdated
| be | ||
| given | ||
| tokens at startup. | ||
| Edge will lock down access for new tokens that have a wider or different access scope than the tokens it was started |
There was a problem hiding this comment.
I think this is cool but I'd like to get a bit more specific in terms of what access means here. We do a nice job providing examples but I think we can do even better just by explaining what this is.
Let's lift this:
Strict behavior still allows using tokens (both frontend and client) with narrower access than what the tokens it was started with has.
And combine it with this section and then build on that. Something like...?
Edge will refuses requests from SDKs that have a wider or different access scope than the initial tokens. Specifically, this means incoming requests must have a token that exactly matches the environment and is bound to a project or projects specified in that token.
There was a problem hiding this comment.
I think we can also tell a story on why this is valuable. Struggling to not go "full marketer" here but something like "this gives you more predictable control over which SDKs have access to Edge and prevents you from expanding the scope at runtime accidentally"
README.md
Outdated
| token accessing a different environment, say production `*:production.<somedifferentrandomstring>` Edge will return 403, | ||
| because that would change the scope of access. | ||
|
|
||
| Strict behavior still allows using tokens (both frontend and client) with narrower access than what the tokens it was |
There was a problem hiding this comment.
I think we could take this and the paragraph below out
README.md
Outdated
| to be valid, a refresh job will be configured with the minimum set of tokens that are able to fetch all projects and | ||
| environments Edge has seen. Set with `--dynamic` or the `DYNAMIC` environment variable. (19.2.0 / July 4th 2024): We're | ||
| looking to deprecate this | ||
| behavior. If your company needs this behavior, reach out to us on Slack, the final decision has not been made yet. In |
There was a problem hiding this comment.
Just feels a bit more friendly
| behavior. If your company needs this behavior, reach out to us on Slack, the final decision has not been made yet. In | |
| behavior. If you need this behavior, reach out to us on Slack, the final decision has not been made yet. In |
No description provided.