diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 46a3e871..ea91258b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Fixed a bug where `rsconnect deploy manifest` and `rsconnect deploy bundle`, + when redeploying to an existing `--app-id` without passing `--title`, always + issued a `PATCH /v1/content/{guid}` to rename the content to its + manifest/bundle-derived default title. Under trusted publishing this request + is forbidden outright, failing the whole deploy with a 403; under an API key + it silently renamed the content. These commands now only update the title + when the user explicitly passes `--title`, matching the other deploy + subcommands; new content still defaults its title from the manifest/bundle as + before. - Added support for Python 3.14. The test suite now runs on Python 3.14 in CI. - `rsconnect deploy` subcommands now accept `--quiet`, which suppresses the step-by-step progress lines and the streamed server build log, printing only diff --git a/rsconnect/api.py b/rsconnect/api.py index 29b0f238..6a5157a1 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -1267,6 +1267,7 @@ def __init__( new: Optional[bool] = None, app_id: Optional[str] = None, title: Optional[str] = None, + title_is_default: Optional[bool] = None, visibility: Optional[str] = None, disable_env_management: Optional[bool] = None, env_vars: Optional[dict[str, str]] = None, @@ -1293,7 +1294,12 @@ def __init__( self.app_store: AppStore = AppStore(fake_module_file_from_directory(self.path)) self.app_store_version: int | None = None self.api_key_is_required: bool | None = None - self.title_is_default: bool = not title + # Callers that pre-resolve a default title (e.g. `deploy manifest` / `deploy + # bundle`, which need the manifest/bundle-derived default rather than the + # generic `_default_title(self.path)` fallback above) can pass + # `title_is_default` explicitly so this still reflects whether the user + # actually supplied `--title`, rather than always being False. + self.title_is_default: bool = not title if title_is_default is None else title_is_default self.deployment_name: str | None = None # Git deployment parameters diff --git a/rsconnect/main.py b/rsconnect/main.py index 2202872f..e09f4d11 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1898,6 +1898,7 @@ def deploy_manifest( file_name = validate_manifest_file(file) app_mode = read_manifest_app_mode(file_name) + title_is_default = not title title = title or default_title_from_manifest(file) ce = RSConnectExecutor( @@ -1915,6 +1916,7 @@ def deploy_manifest( new=new, app_id=app_id, title=title, + title_is_default=title_is_default, visibility=visibility, env_vars=env_vars, ) @@ -1994,6 +1996,7 @@ def deploy_bundle( output_params(ctx, locals().items()) app_mode = read_bundle_app_mode(file) + title_is_default = not title title = title or default_title_from_bundle(file) ce = RSConnectExecutor( @@ -2011,6 +2014,7 @@ def deploy_bundle( new=new, app_id=app_id, title=title, + title_is_default=title_is_default, visibility=visibility, env_vars=env_vars, ) diff --git a/tests/test_main.py b/tests/test_main.py index 2ad2e666..eb16b74a 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -972,6 +972,289 @@ def post_application_deploy_callback(request, uri, response_headers): if original_server_value: os.environ["CONNECT_SERVER"] = original_server_value + def _register_redeploy_endpoints(self, guid, existing_title, patch_calls, app_mode_ordinal=15): + # Common Connect endpoints for redeploying to an existing --app-id: no + # POST /v1/content (creation) or GET /v1/content?name=... (uniqueness + # check) is needed, since those are only exercised for brand new content. + # app_mode_ordinal defaults to AppModes.PYTHON_SHINY (15), matching + # pyshiny_with_manifest; the bundle test overrides it to + # AppModes.PYTHON_API (8) to match bundle.tar.gz's manifest. + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/server_settings", + body=json.dumps({"version": "9999.99.99"}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/v1/user", + body=open("tests/testdata/connect-responses/me.json", "r").read(), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + content_body = json.dumps( + { + "id": "1234", + "guid": guid, + "title": existing_title, + "app_mode": app_mode_ordinal, + "content_url": f"http://fake_server/content/{guid}", + "dashboard_url": f"http://fake_server/connect/#/apps/{guid}", + } + ) + httpretty.register_uri( + httpretty.GET, + f"http://fake_server/__api__/v1/content/{guid}", + body=content_body, + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + + def patch_callback(request, uri, response_headers): + patch_calls.append(_load_json(request.body)) + return [200, {"Content-Type": "application/json"}, content_body] + + httpretty.register_uri( + httpretty.PATCH, + f"http://fake_server/__api__/v1/content/{guid}", + body=patch_callback, + ) + httpretty.register_uri( + httpretty.POST, + f"http://fake_server/__api__/v1/content/{guid}/bundles", + body=json.dumps({"id": "FAKE_BUNDLE_ID"}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + + deploy_api_invoked = [] + + def post_application_deploy_callback(request, uri, response_headers): + deploy_api_invoked.append(True) + return [201, {"Content-Type": "application/json"}, json.dumps({"task_id": "FAKE_TASK_ID"})] + + httpretty.register_uri( + httpretty.POST, + f"http://fake_server/__api__/v1/content/{guid}/deploy", + body=post_application_deploy_callback, + ) + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/v1/tasks/FAKE_TASK_ID?wait=1", + body=json.dumps({"output": ["FAKE_OUTPUT"], "last": "FAKE_LAST", "finished": True, "code": 0}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + return deploy_api_invoked + + @httpretty.activate(verbose=True, allow_net_connect=False) + def test_deploy_manifest_redeploy_preserves_title(self, caplog): + # Regression test for #835: redeploying existing content via + # `deploy manifest --app-id` without `--title` must not PATCH the + # content's title, even though `title` is pre-resolved to the + # manifest-derived default ("app5") before RSConnectExecutor is + # constructed. Under trusted publishing, PATCH /v1/content/{guid} is + # forbidden outright, so a spurious PATCH here would 403 the whole + # deploy; under an API key it would silently rename the content. + original_api_key_value = os.environ.pop("CONNECT_API_KEY", None) + original_server_value = os.environ.pop("CONNECT_SERVER", None) + guid = "1234-5678-9012-3456" + patch_calls = [] + + try: + deploy_api_invoked = self._register_redeploy_endpoints(guid, "My Curated Title", patch_calls) + + runner = CliRunner() + args = apply_common_args( + ["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")], + server="http://fake_server", + key="FAKE_API_KEY", + ) + args += ["--app-id", guid, "--no-verify"] + with caplog.at_level("INFO"): + result = runner.invoke(cli, args) + assert result.exit_code == 0, result.output + assert deploy_api_invoked == [True] + assert patch_calls == [] + finally: + if original_api_key_value: + os.environ["CONNECT_API_KEY"] = original_api_key_value + if original_server_value: + os.environ["CONNECT_SERVER"] = original_server_value + + @httpretty.activate(verbose=True, allow_net_connect=False) + def test_deploy_manifest_redeploy_with_explicit_title_still_updates(self, caplog): + # When the user does pass --title, an existing app with a different + # title should still be updated (the pre-#835-fix behavior, preserved). + original_api_key_value = os.environ.pop("CONNECT_API_KEY", None) + original_server_value = os.environ.pop("CONNECT_SERVER", None) + guid = "1234-5678-9012-3456" + patch_calls = [] + + try: + deploy_api_invoked = self._register_redeploy_endpoints(guid, "My Curated Title", patch_calls) + + runner = CliRunner() + args = apply_common_args( + ["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")], + server="http://fake_server", + key="FAKE_API_KEY", + ) + args += ["--app-id", guid, "--title", "Explicit New Title", "--no-verify"] + with caplog.at_level("INFO"): + result = runner.invoke(cli, args) + assert result.exit_code == 0, result.output + assert deploy_api_invoked == [True] + assert patch_calls == [{"title": "Explicit New Title"}] + finally: + if original_api_key_value: + os.environ["CONNECT_API_KEY"] = original_api_key_value + if original_server_value: + os.environ["CONNECT_SERVER"] = original_server_value + + @httpretty.activate(verbose=True, allow_net_connect=False) + def test_deploy_bundle_redeploy_preserves_title(self, caplog): + # Same regression as test_deploy_manifest_redeploy_preserves_title, but + # for `deploy bundle --app-id`. + original_api_key_value = os.environ.pop("CONNECT_API_KEY", None) + original_server_value = os.environ.pop("CONNECT_SERVER", None) + guid = "1234-5678-9012-3456" + patch_calls = [] + bundle_path = join("tests", "testdata", "bundle.tar.gz") + + try: + deploy_api_invoked = self._register_redeploy_endpoints( + guid, "My Curated Title", patch_calls, app_mode_ordinal=8 + ) + + runner = CliRunner() + args = apply_common_args(["deploy", "bundle", bundle_path], server="http://fake_server", key="FAKE_API_KEY") + args += ["--app-id", guid, "--no-verify"] + with caplog.at_level("INFO"): + result = runner.invoke(cli, args) + assert result.exit_code == 0, result.output + assert deploy_api_invoked == [True] + assert patch_calls == [] + finally: + if original_api_key_value: + os.environ["CONNECT_API_KEY"] = original_api_key_value + if original_server_value: + os.environ["CONNECT_SERVER"] = original_server_value + + @httpretty.activate(verbose=True, allow_net_connect=False) + def test_deploy_manifest_new_content_uses_manifest_derived_title(self, caplog): + # New content (no --app-id) must still default its title from the + # manifest ("app5", derived from the entrypoint), not the executor's + # generic path-based fallback ("manifest"). + original_api_key_value = os.environ.pop("CONNECT_API_KEY", None) + original_server_value = os.environ.pop("CONNECT_SERVER", None) + guid = "1234-5678-9012-3456" + + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/server_settings", + body=json.dumps({"version": "9999.99.99"}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/v1/user", + body=open("tests/testdata/connect-responses/me.json", "r").read(), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/v1/content?name=app5", + body=json.dumps([]), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + # content_create only ever sends {"name": ...}, so Connect's response + # here reflects a server-assigned placeholder title, distinct from the + # manifest-derived default. The deploy flow must PATCH it to "app5". + create_body = json.dumps( + { + "id": "1234", + "guid": guid, + "title": "Untitled", + "content_url": f"http://fake_server/content/{guid}", + "dashboard_url": f"http://fake_server/connect/#/apps/{guid}", + } + ) + httpretty.register_uri( + httpretty.POST, + "http://fake_server/__api__/v1/content", + body=create_body, + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + + patch_calls = [] + + def patch_callback(request, uri, response_headers): + patch_calls.append(_load_json(request.body)) + return [200, {"Content-Type": "application/json"}, create_body] + + httpretty.register_uri( + httpretty.PATCH, + f"http://fake_server/__api__/v1/content/{guid}", + body=patch_callback, + ) + httpretty.register_uri( + httpretty.GET, + f"http://fake_server/__api__/v1/content/{guid}", + body=create_body, + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + httpretty.register_uri( + httpretty.POST, + f"http://fake_server/__api__/v1/content/{guid}/bundles", + body=json.dumps({"id": "FAKE_BUNDLE_ID"}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + deploy_api_invoked = [] + + def post_application_deploy_callback(request, uri, response_headers): + deploy_api_invoked.append(True) + return [201, {"Content-Type": "application/json"}, json.dumps({"task_id": "FAKE_TASK_ID"})] + + httpretty.register_uri( + httpretty.POST, + f"http://fake_server/__api__/v1/content/{guid}/deploy", + body=post_application_deploy_callback, + ) + httpretty.register_uri( + httpretty.GET, + "http://fake_server/__api__/v1/tasks/FAKE_TASK_ID?wait=1", + body=json.dumps({"output": ["FAKE_OUTPUT"], "last": "FAKE_LAST", "finished": True, "code": 0}), + adding_headers={"Content-Type": "application/json"}, + status=200, + ) + + try: + runner = CliRunner() + args = apply_common_args( + ["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")], + server="http://fake_server", + key="FAKE_API_KEY", + ) + args.append("--no-verify") + with caplog.at_level("INFO"): + result = runner.invoke(cli, args) + assert result.exit_code == 0, result.output + assert deploy_api_invoked == [True] + assert patch_calls == [{"title": "app5"}] + finally: + if original_api_key_value: + os.environ["CONNECT_API_KEY"] = original_api_key_value + if original_server_value: + os.environ["CONNECT_SERVER"] = original_server_value + # noinspection SpellCheckingInspection @pytest.mark.skip(reason="Skipping R manifest test (requires R 3.5, docker containers have moved on).") def test_deploy_manifest(self):