Conversation
There was a problem hiding this comment.
Pull request overview
Adds a developer guide describing how to set up Proxygen (key generation, ServiceNow request, proxygen-cli install/config, and basic deploy/get commands).
Changes:
- Added a new developer guide covering Proxygen setup steps and required configuration files.
- Documented proxygen-cli installation and example commands for deploying/retrieving specs.
- Included guidance on Key ID (KID) structure and how to request a Proxygen API/client.
Comments suppressed due to low confidence (1)
docs/developer-guides/Rotating_proxygen_keys.md:85
- The section "Deploying the spec via a pipeline" is currently just a heading with no content. If this is intentionally out of scope for this PR, consider removing the heading; otherwise add the missing steps (or mark it explicitly as TODO with a link) so readers aren’t left at a dead end.
## Deploying the spec via a pipeline
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| This enforces a 12-month expiry for all public keys (The date should be expiry date of the Public Key) | ||
| ``` | ||
|
|
||
| [These docs](https://digital.nhs.uk/developer/guides-and-documentation/security-and-authorisation/application-restricted-restful-apis-signed-jwt-authentication#step-2-generate-a-key-pair) walkthrough how to generate the key pair. Once you've created your JWKS file called YOUR_KID.json you can move onto creating the proxygen deployment |
There was a problem hiding this comment.
Grammar: "walkthrough" is used as a verb here; consider "walks through" or "walk through". Also "move onto" should be "move on to". These small fixes make the instructions read more cleanly.
| [These docs](https://digital.nhs.uk/developer/guides-and-documentation/security-and-authorisation/application-restricted-restful-apis-signed-jwt-authentication#step-2-generate-a-key-pair) walkthrough how to generate the key pair. Once you've created your JWKS file called YOUR_KID.json you can move onto creating the proxygen deployment | |
| [These docs](https://digital.nhs.uk/developer/guides-and-documentation/security-and-authorisation/application-restricted-restful-apis-signed-jwt-authentication#step-2-generate-a-key-pair) walk through how to generate the key pair. Once you've created your JWKS file called YOUR_KID.json you can move on to creating the proxygen deployment |
| base_url: https://identity.{ptl | prod}.api.platform.nhs.uk/realms/api-producers | ||
| client_id: {Client ID} |
There was a problem hiding this comment.
The base_url placeholder includes spaces and a | inside braces ({ptl | prod}), which makes it easy to accidentally copy/paste an invalid URL into credentials.yaml. Consider providing two explicit example URLs (one for PTL and one for PROD), or use a placeholder format without spaces/symbols that would break the URL.
| endpoint_url: https://proxygen.{ptl | prod}.api.platform.nhs.uk | ||
| spec_output_format: yaml |
There was a problem hiding this comment.
Same issue as base_url: endpoint_url uses a {ptl | prod} placeholder that will produce an invalid URL if copied verbatim. Provide explicit PTL/PROD examples or a safer placeholder format.
|
|
||
| Once you have this key pair, you can raise a [service now request](https://nhsdigitallive.service-now.com/csm?id=sc_cat_item&sys_id=b5e2c7ee9777a610dd80f2df9153af87&sysparm_category=63f4716697731e90dd80f2df9153affe) with the information you used to create the key pair to "Create an API on Proxygen". | ||
|
|
||
| Once APIM have done this, they will provide you with a client ID in the service now request, e.g. im1-pfs-auth-client which you will need to setup machine access to the API. |
There was a problem hiding this comment.
"setup" is used as a verb here; in prose it should typically be "set up" (verb) vs "setup" (noun/adjective). Aligning the phrasing helps readability.
| Once APIM have done this, they will provide you with a client ID in the service now request, e.g. im1-pfs-auth-client which you will need to setup machine access to the API. | |
| Once APIM have done this, they will provide you with a client ID in the service now request, e.g. im1-pfs-auth-client which you will need to set up machine access to the API. |
|
|
||
| ### .proxygen config files | ||
|
|
||
| To fully setup machine access, you will need a credentials.yaml and settings.yaml file inside of ~/.proxygen |
There was a problem hiding this comment.
Same "set up" vs "setup" issue: "To fully setup machine access" reads as a verb phrase, so "set up" would be clearer.
| To fully setup machine access, you will need a credentials.yaml and settings.yaml file inside of ~/.proxygen | |
| To fully set up machine access, you will need a credentials.yaml and settings.yaml file inside of ~/.proxygen |
| From the [APIM slack message](https://nhsuk.slack.com/archives/C016JRWN6AY/p1773828313442569): | ||
| ```Follow the new Key ID (KID) structure: | ||
|
|
||
| YYYY-mm-dd-PTL-<<api-name>> | ||
| YYYY-mm-dd-Prod-<<api-name>> | ||
| This enforces a 12-month expiry for all public keys (The date should be expiry date of the Public Key) | ||
| ``` |
There was a problem hiding this comment.
The fenced code block for the Slack quote is malformed (the opening fence includes text: "Follow..."). This will render incorrectly in Markdown. Put the opening on its own line (optionally with a language like text) and move the quote content onto the next line.
|
|
||
| ## Generating a key pair | ||
|
|
||
| You'll need to generate a public and private key pair before raising a service now request to create the proxygen API |
There was a problem hiding this comment.
"service now" should be the product name "ServiceNow" (one word, proper capitalization) and this sentence is missing terminal punctuation. Adjusting this improves clarity and searchability.
| You'll need to generate a public and private key pair before raising a service now request to create the proxygen API | |
| You'll need to generate a public and private key pair before raising a ServiceNow request to create the proxygen API. |
| @@ -0,0 +1,84 @@ | |||
| # Setting up Proxygen | |||
There was a problem hiding this comment.
The document title is "Setting up Proxygen" but the file is named Rotating_proxygen_keys.md. This mismatch makes the doc harder to discover and link correctly. Consider renaming the file (or adjusting the title) so the filename reflects the actual content.
| # Setting up Proxygen | |
| # Rotating Proxygen keys |
| You're looking for one that looks like `/Users/elliot/Library/Python/3.13/bin/proxygen` | ||
|
|
||
| Once you have this path, you can add it to your .zshrc with `export PATH="$PATH:/Users/elliot/Library/Python/3.13/bin/"` |
There was a problem hiding this comment.
This example uses a specific developer machine path (/Users/elliot/...) and a specific Python version. That’s likely to confuse others and will go stale. Prefer using $HOME/~ and/or a more general way to locate the user-base bin directory so the instructions work across machines.
| You're looking for one that looks like `/Users/elliot/Library/Python/3.13/bin/proxygen` | |
| Once you have this path, you can add it to your .zshrc with `export PATH="$PATH:/Users/elliot/Library/Python/3.13/bin/"` | |
| You're looking for one that looks like `~/Library/Python/.../bin/proxygen` | |
| Once you have this path, add the directory containing it to your `.zshrc`, for example with `export PATH="$PATH:$(dirname "$(find ~ -name "proxygen" 2>/dev/null | head -n 1)")"` |
|
|
||
| YYYY-mm-dd-PTL-<<api-name>> | ||
| YYYY-mm-dd-Prod-<<api-name>> | ||
| This enforces a 12-month expiry for all public keys (The date should be expiry date of the Public Key) |
There was a problem hiding this comment.
Worth saying YYYY-mm-dd should be the expiry date, not today's date. This confused me, from what's written
Pull Request
🧾 Ticket Link
https://nhsd-jira.digital.nhs.uk/browse/NPA-6550
📄 Description/Summary of Changes
🧪 Developer Testing Carried Out
📋 PR Principles
🏷️ Naming Conventions Reminder
Please ensure the following naming conventions are followed:
NPA-XXXX: <short-description><type>/NPA-XXXX/<short-description>NPA-XXXX: <short-description>