fix(scripts): surface the NetBox plugin credential in quickstart output - #590
Conversation
Both quickstart scripts create a netbox-to-diode OAuth2 client but only ever printed the diode-ingest one, labelled for orb-agent. That is the only secret a user is handed, so it is the one they paste into the plugin's configuration.py, where it cannot work: the plugin authenticates as netbox-to-diode. The result is Failed to obtain access token: ... Connection refused or an auth failure, with nothing pointing at the credential being for a different client. GET_STARTED.md already documents the correct jq incantation, but users follow the script's output rather than re-reading the guide. In #414 one reporter rediscovered that exact command and posted it as a workaround; it resolved the issue for another reporter in the same thread. Print both credentials, each labelled with what it configures, and name the client the plugin secret belongs to. netbox_to_diode_client_id is not printed as a setting to add: it already defaults to "netbox-to-diode" in the plugin. Also document that diode_target_override must be reachable from inside NetBox. Three reporters in #414 independently worked out that localhost resolves to the NetBox container rather than the Diode server when NetBox is containerised. Closes #414 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
documentation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This PR changes charts/diode/scripts/quickstart.sh, so ct lint requires a chart version bump. 1.15.3 is already released as helm-chart-diode-1.15.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e6cbac4e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The helm quickstart regenerates client-credentials.json whenever the local file is absent (line 110), but skips creating the oauth2 Secret when it already exists in the namespace (line 185). Re-running against an existing namespace from a fresh working directory therefore produces local credentials that were never applied to the cluster. Printing the netbox-to-diode secret from that local file would hand out a credential that cannot authenticate, which is the exact failure this PR exists to prevent. Read it from the deployed Secret instead, falling back to the local file when the Secret is absent, unreadable, or does not contain the client. On a first run the Secret does not exist yet and the local file is what will be applied, so the fallback is correct there. Caught in review by Codex on #590. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38b3345c83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two problems with the plugin block printed at the end of both quickstarts. It was emitted as `key: value`, which is not valid Python. Users are told to put it in configuration.py, where the setting names and string values must be quoted, and generated secrets end in `=` so an unquoted value is a syntax error. Now emits quoted dictionary entries that paste directly into PLUGINS_CONFIG["netbox_diode_plugin"]. The docker quickstart also advertised grpc://localhost:PORT/diode as the target to configure. That address is reused from the orb-agent hint, where it is usually right, but for the plugin it only works when NetBox runs directly on this host outside a container. The getting-started guide explicitly supports NetBox on a separate host, and the previous warning mentioned only the containerised case. Now prints a <diode-host> placeholder with a note covering both. Verified by parsing the emitted lines: they exec as valid Python inside a PLUGINS_CONFIG dict, the secret round-trips intact including its trailing '=', and the target contains no localhost. Caught in review by Codex on #590. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Closes #414.
Problem
Both quickstart scripts create a
netbox-to-diodeOAuth2 client, but only ever print thediode-ingestone:That's correctly labelled for orb-agent, but it's the only secret the user is handed — so it's the one they paste into the plugin's
configuration.py, where it cannot work. The plugin authenticates asnetbox-to-diode.GET_STARTED.md:53already documents the correctjqcommand. But people follow the script's output rather than re-reading the guide: in #414 a reporter rediscovered that exact command and posted it as a workaround, and it resolved the problem for another reporter in the same thread.#414 has six independent reporters over three months, mostly helping each other. It's the most common onboarding failure in the tracker.
Change
Both quickstarts now print both credentials, each labelled with what it configures, and name the client the plugin secret belongs to:
The helm quickstart had the same gap — it creates the client at line 87 and never surfaced it.
netbox_to_diode_client_idis deliberately not printed as a setting to add: it already defaults to"netbox-to-diode"in the plugin (netbox_diode_plugin/__init__.py:33), so listing it would invite people to set something they don't need. The client name appears only as context for which secret they're holding.Docs: added a note that
diode_target_overridemust be reachable from inside NetBox. Three reporters in #414 independently worked out thatlocalhostresolves to the NetBox container rather than the Diode server when NetBox is containerised. Applied to bothGET_STARTED.mdanddocs/getting-started.md.Verification
Ran the credential-selection logic against a representative
client-credentials.jsonwith all three clients; each label picks the right secret.bash -nclean on both scripts.shellcheck -S warningreports 3 findings, all pre-existing in the secret-generation block — identical count ondevelop.Not addressed here
#414 also collected two deployment-specific causes: an nginx container not binding 8080, and (in the last comment) the helm case where the target must go through the ingress. The latter is the same problem as #456 and was documented in #584.
🤖 Generated with Claude Code