add support for deploying to Posit Connect Cloud - #840
Conversation
Adds Posit Connect Cloud as a deployment target alongside Posit Connect and shinyapps.io, mirroring the R rsconnect package's support: - Select the target with --connect-cloud or -s connect.posit.cloud. - Authenticate with an interactive OAuth device-code login or a service account client ID/secret (client credentials grant), with automatic token refresh and write-back to the credential store. - Register credentials with `rsconnect add`, verifying the account exists and grants the content:create permission before storing. - Deploy through the Connect Cloud revision model: create or update content, upload the bundle to a presigned URL, publish, poll the revision, and print the publish log from the logs service on failure. - Record deployments locally before publishing, since Connect Cloud cannot look content up by name. - Support the production, staging, and development environments via CONNECT_CLOUD_ENVIRONMENT, pinned to the saved server's URL. Fixes #817
# Conflicts: # docs/CHANGELOG.md
`cast(dict[str, Any], ...)` evaluates its first argument at runtime, so `from __future__ import annotations` does not cover it and `dict[...]` raises TypeError on Python 3.8. Describe the response with a TypedDict instead, matching the other ConnectCloudClient methods.
ConnectCloudClient._attempt_token_refresh logged every failure at debug and returned False, so an expired session or a revoked service account secret surfaced only as the original opaque 401. oauth.py now raises a typed InvalidGrantError when the token endpoint returns error=invalid_grant, carrying the server's error_description. The Cloud client acts on the two credential rejections it can explain: - invalid_grant on the refresh-token path clears connect_cloud_access_token and connect_cloud_refresh_token on the saved servers.json entry (account name/id and nickname are kept) and raises "Your Posit Connect Cloud session has expired and could not be renewed. Authenticate again with `rsconnect add --connect-cloud -n <name> -A <account>`." - invalid_client on the client-credentials path raises a message saying the service account credential was revoked or rotated, pointing at <auth host>/identity/credentials and the `rsconnect add` command with --client-id/--client-secret. The stored entry is left alone. Everything else — network failures, a rejected CLI OAuth client, unexpected responses — still returns False so the original 401 surfaces, but logs at warning instead of debug, matching the Connect refresh path. The servers.json write-back moved into _persist_tokens so the clearing path reuses the field-preserving update. Connect's refresh is unchanged: its generic `except Exception` already covers the new error type.
A saved Connect Cloud entry is a credential, not an account binding: the login behind it can publish to every account its user has rights on. So -A/--account no longer picks which saved credential to use, only where to publish. With one credential saved it is used whatever account is named; with several, -n/--name is now required and the error lists the saved nicknames with the account each publishes to by default. This drops the account-filter branch and its two error paths from ServerStore._get_connect_cloud_server, along with the now-dead account_name argument to get_by_url and resolve. Behavior change: With several saved credentials, -n selects the credential; -A no longer matches against entries and instead always selects the account to publish to, so `-n cred -A other-account` publishes there with that credential.
Connect Cloud tokens and service account client secrets now go to the system keyring, keyed "<url>#<nickname>" because every Connect Cloud entry records the same API URL. `rsconnect add` and token refresh write there when a keyring is available and leave the matching servers.json fields out, which moves the secrets of an entry saved before this change out of the file on its next add or refresh. Reads prefer the keyring and fall back to those fields, so a machine without a usable keyring (a CI runner) keeps working as before, and `rsconnect server remove` deletes the entries for the removed nickname. `rsconnect list` reports which of the two holds the credentials. The keyring helpers in oauth.py now take the entry key explicitly. Posit Connect keeps passing the bare server URL, so its "<url>:access_token" and "<url>:refresh_token" usernames are unchanged and existing logins are untouched. Tests get a conftest fixture that makes the keyring unavailable by default, since the module is installed in the test environment and would otherwise reach the machine's real keychain.
RSConnectClient and ConnectCloudClient each had their own copy of "send the request, on 401 mint a new token, send it once more". Both now inherit it from BearerTokenHTTPServer, which calls the subclass's _attempt_token_refresh to mint and apply the token and asks _can_refresh_token whether there is anything to mint from -- false for an API key, a bootstrap JWT, or a Snowflake token exchange. Connect Cloud gains the seekable-body rewind that only the Connect copy had, so a streamed body is not sent empty on the retry. The minting stays per-target, unchanged: Connect keeps discovery against its registered client and the InvalidClientError re-registration recovery, and Connect Cloud keeps the client-credentials-versus-refresh choice and its typed error handling. The keyring-with-servers.json-fallback load and write-back is already the same code on both sides, differing only in the key it is given; what remains target-specific is Connect's token expiry tracking and Connect Cloud's field-preserving write-back, which it skips for a run with no saved entry. Connect's three copies of "find the entry this server came from" become ServerStore.saved_entry. No behavior change other than the added rewind; every existing test passes unmodified.
The stream-body retry tests annotate returns as list[Any], which 3.8 evaluates at class-definition time and rejects. Deferring annotation evaluation with the __future__ import fixes collection for the file.
The autouse no_system_keyring fixture requested monkeypatch, hoisting the shared per-test instance ahead of every test-level fixture. Its undo then ran after those fixtures' cleanup, so a test using monkeypatch.chdir into a TemporaryDirectory had the directory deleted while it was still the working directory, which Windows rejects (WinError 32 in test_git_metadata teardown). The fixture now saves and restores sys.modules itself.
The teardown guard treated a missing sys.modules key the same as the fixture's own None marker, so a test that deleted the entry would raise KeyError during restore. The sentinel default now separates the cases.
Extracts the fixture body into an importable generator and adds tests driving each teardown branch: previous module restored, marker removed when nothing was stored, and a deleted key left deleted. The marker is reinstated through a fixture finalizer so a failing assertion cannot leak state into later tests.
Nothing asserted this raise; the Connect integration suite exercised it only by accident, and the -n/-A test rework there removed even that.
|
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
| # None (no -E given) means "leave the server's secrets alone"; it reaches | ||
| # update_content as None and is omitted from the PATCH. A non-empty -E set | ||
| # replaces the whole collection, matching the R client. | ||
| secrets = [{"name": name, "value": value} for name, value in env_vars.items()] if env_vars else None |
There was a problem hiding this comment.
If I'm reading this right, a redeploy with a secret (-E NEW_SECRET=xyz) will wipe any existing secrets deployed before the redeploy. Might be worth documenting that in the -E help text
There was a problem hiding this comment.
It already is? too subtle?
A non-empty -E set replaces the whole collection
| if not isinstance(self.client, ConnectCloudClient): | ||
| raise RSConnectException("client must be a ConnectCloudClient.") | ||
| if self.visibility is not None: | ||
| # Connect Cloud has no equivalent setting. |
There was a problem hiding this comment.
Connect Cloud has visibility concept (content.access). Are we intentionally not covering resource access here?
There was a problem hiding this comment.
oops! good catch. i will add that in
karawoo
left a comment
There was a problem hiding this comment.
I tested this locally and was able to deploy to connect cloud successfully. Added a few comments below but they're pretty minor. For next time, if we could get independent changes in separate PRs that would help a lot with reviewing. I think the credential redaction and some of the refactoring could have been their own PRs and made the main feature easier to review.
| - Posit Connect Cloud is now a supported deployment target, alongside Posit | ||
| Connect and shinyapps.io, mirroring the R rsconnect package's support. |
There was a problem hiding this comment.
| - Posit Connect Cloud is now a supported deployment target, alongside Posit | |
| Connect and shinyapps.io, mirroring the R rsconnect package's support. | |
| - Posit Connect Cloud is now a supported deployment target, alongside Posit | |
| Connect and shinyapps.io. |
| it. Authentication is an interactive browser login by default; for CI and | ||
| other non-interactive use, pass a service account credential with | ||
| `--client-id`/`--client-secret` (or the `CONNECT_CLOUD_CLIENT_ID` and | ||
| `CONNECT_CLOUD_CLIENT_SECRET` environment variables). Tokens are refreshed | ||
| automatically. Deploy with any `rsconnect deploy` subcommand by passing | ||
| `--connect-cloud` (or `-s connect.posit.cloud`) with `-A <account>` (or the | ||
| `CONNECT_CLOUD_ACCOUNT` environment variable; a `SHINYAPPS_ACCOUNT` variable | ||
| exported for shinyapps.io is ignored here), or `-n <nickname>` for a saved | ||
| credential — which publishes to the account it was saved with, or to another | ||
| account of the same login when `-A` is given as well. Supported content types: | ||
| Shiny (Python and R), Streamlit, Dash, Bokeh, Jupyter notebooks, Quarto, | ||
| R Markdown, and static content. |
There was a problem hiding this comment.
I think some of this detail is better left to the documentation rather than the changelog
| response.json_data["error"], | ||
| ) | ||
| raise RSConnectException(error, status=response.status) | ||
| if response.status < 200 or response.status > 299: |
There was a problem hiding this comment.
I think this should include response.status is None like we do at 405
| if response.status is None or response.status < 200 or response.status > 299: |
| store.set( | ||
| server.server_name, | ||
| entry_url, | ||
| connect_cloud_account_name=entry.get("connect_cloud_account_name") or server.account_name, | ||
| connect_cloud_account_id=entry.get("connect_cloud_account_id"), | ||
| connect_cloud_client_id=entry.get("connect_cloud_client_id"), | ||
| connect_cloud_client_secret=None if secret_in_keyring else file_client_secret, | ||
| connect_cloud_access_token=None if in_keyring else access_token, | ||
| connect_cloud_refresh_token=None if in_keyring else refresh_token, | ||
| ) |
There was a problem hiding this comment.
This rewrites servers.json on every refresh even if the contents haven't changed (i.e. if the keyring is in use). What do you think about rewriting only if the file has actually changed?
| `content_type` and `primary_file` are always sent alongside `app_mode`: | ||
| the API only recomputes `app_mode` when one of them is present in the | ||
| override set, and the stored content type would otherwise survive a | ||
| redeploy that changes what kind of content this is (--app-id pointing at | ||
| content of another type). |
There was a problem hiding this comment.
I'm not sure I follow this
| # Readability is judged before the suffix so a bad path reports as the real | ||
| # problem: the file type of a file that does not exist is beside the point. | ||
| # Both are operational errors: the CLI no longer checks existence at parse | ||
| # time (the certificate only applies once the target is known), so this is | ||
| # where a bad path surfaces. is_file() sits inside the handler because it | ||
| # raises OSError itself when the path's metadata cannot be read, e.g. | ||
| # through a permission-denied directory. |
There was a problem hiding this comment.
I had a pretty hard time following this comment, I think the code itself is clearer
| if self.visibility is not None: | ||
| # Connect Cloud has no equivalent setting. | ||
| raise RSConnectException( | ||
| "-V/--visibility is not supported by Posit Connect Cloud. " |
There was a problem hiding this comment.
Are there other flags that are supported for connect but should be disabled for connect cloud? thinking about --image, --disable-env-management/--disable-env-management-py/--disable-env-management-r/--disable-env-management-node, --node, --hide-all-input / --hide-tagged-input -- I'm not sure which ones connect cloud supports.
| if draft: | ||
| if not self.supports_verify_before_activate: | ||
| # We can't honor --draft without the activate field: silently activating | ||
| # would be the opposite of what the user asked for, so fail loudly. | ||
| raise RSConnectException("Deploying as a draft requires Posit Connect 2025.06.0 or later.") |
There was a problem hiding this comment.
I think we need to adjust this, "Deploying as a draft requires Posit Connect 2025.06.0 or later" doesn't make sense if you're trying to deploy to Connect Cloud
Intent
Add support for deploying to Posit Connect Cloud: OAuth device-code and service-account (client-credentials) authentication, credential management, content create/update/publish with log streaming, and deployment records — for the content types Connect Cloud supports.
Resolves #817. Supersedes #837 and #839, merging both into one reviewable unit; see #837 for the earlier review history and manual test-plan results (staging and production).
Type of Change
Approach
Beyond the base feature, this incorporates the target-resolution and OAuth feedback from #837 (comment), one commit per item:
invalid_grant/invalid_clienterrors; an expired session clears its dead stored tokens and the error names the exactrsconnect addcommand to re-authenticate (preserving a non-production-sURL and the saved entry's account); transient failures still surface the original 401.-A, several require-n, and-Aalways selects the account to publish to (a Cloud login can publish to every account its user has rights on). Deployment records stay keyed by URL + account id. For non-Cloud targets,-nwith-Astill fails, but after nickname resolution and with a new message.<url>#<nickname>, withservers.jsonas the documented fallback when no usable keyring exists (CI, headless). The existing Connect keyring key format is frozen, with a test guarding it.BearerTokenHTTPServerowns the retry-once skeleton (including request-body rewind) for both clients; token minting stays per-target (Connect keeps OAuth discovery + client re-registration, Cloud keeps client-credentials vs refresh-token and the typed errors above).Two follow-ups from the same review land separately after this merges: redeploy target inference from a directory's deployment record (changes target-resolution precedence for all server types, so it warrants its own review) and the shinyapps.io deployment migration command (#838).
Companion: posit-dev/connect#42351 updates two nightly bats assertions that pinned the old
-n/-Avalidation message. Each repo's CI installs the other'smain, so the original changes were red in isolation; the assertions are now version-agnostic and that PR merges independently, in either order.Automated Tests
Full suite: 1081 passed / 12 skipped; CI green across Python 3.8–3.14 on ubuntu/macos/windows plus the Connect integration suites. Each behavior above has dedicated unit tests (refresh error paths, credential selection rules, keyring fallback/migration incl. the
NoKeyringErrorCI case, retry-once with body rewind), and every commit was individually machine-reviewed.Directions for Reviewers
To validate manually:
Worth poking at:
-n cc -A <other-account>(publish-target selection), a secondaddunder another nickname then a bare deploy (must demand-n),rsconnect list(reports keyring vs file storage), andrsconnect remove(cleans up keyring entries). SetPYTHON_KEYRING_BACKEND=keyring.backends.fail.Keyringto exercise theservers.jsonfallback.Checklist
rsconnect-python-tests-at-nightworkflow in Connect against this feature branch. (Two bats message assertions failed as expected; addressed by posit-dev/connect#42351.)