Skip to content

schema changes for posit connect cloud #2693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

schema changes for posit connect cloud #2693

wants to merge 40 commits into from

Conversation

mslynch
Copy link
Collaborator

@mslynch mslynch commented Jul 1, 2025

Intent

Resolves #2677.

The changes to the schema in v4 are:

  • publishing schema:
    • Add .server_type field, defaulting to connect
    • Rename draft field .connect.access to .connect.access_control
    • Add .connect_cloud field
      • Contains the following fields:
        • .vanity_name
        • .access_control
          • public_access: boolean
          • organization_access: disabled | viewer | editor
      • Contains a subset of of the fields previously allowed for Connect. The following fields are not settable for PCC:
        • .has_parameters
        • .python.package_file
        • .python.package_manager
        • .python.requires_python
        • .r.package_file
        • .r.package_manager
        • .r.requires_r
        • .jupyter.hide_all_input
        • .jupyter.hide_tagged_input
        • .quarto.version
        • .quarto.engines
        • .connect
  • Publishing record schema:
    • Add connect_cloud enum value to .server_type field
    • Add .account_name field, which represents the Posit Connect Cloud account the content was deployed to

This PR also removes some dead code from initialize.

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

This PR also includes changes for backwards compatibility with old schemas. When loading a TOML file, instead of loading it directly into a struct, we load it into a map, extract $schema, and validate against that schema. Then depending on the schema version, we apply changes to make the file data compatible with the current schema version.

User Impact

New publishes and published records will be created in the V4 format. V3 files are still supported.

Automated Tests

Added tests for conversion from v3 schema to v4.

Directions for Reviewers

  • Publish something new - it should create files in the v4 format.
  • Publish something old with v3 files - it should work, but a new deployment record should be created in the v4 format.

Checklist

@mslynch mslynch changed the base branch from main to pcc-creds July 7, 2025 19:58
@mslynch mslynch requested a review from Copilot July 8, 2025 13:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades the publishing and record schemas from v3 to v4 to support Posit Connect Cloud, adds a required server_type field, and introduces the new connect_cloud configuration with vanity URLs and access control. It also refactors TOML decoding to use strict mapstructure rules, adds schema‐version upgrade logic, and updates tests/examples to v4.

  • Bump $schema URLs to v4 and require server_type on all configs and records
  • Add connect_cloud fields (vanity_name & access_control) and update Go types with mapstructure tags
  • Implement UpgradePublishingSchema/UpgradePublishingRecordSchema and switch validators to support multiple schema versions

Reviewed Changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/schema/schema.go Switched ConfigSchemaURL/DeploymentSchemaURL to v4 and made the validator accept legacy & v4 URLs
internal/schema/conversion.go Introduced schema‐upgrade functions to migrate v3 data to v4
internal/util/toml.go Added DecodeTOMLMap helper using mapstructure for strict decoding
internal/config/types.go Extended Config struct with ServerType & ConnectCloud fields and mapstructure tags
internal/services/api/put_configuration.go Changed NewValidator call to take []string of schema URLs
Comments suppressed due to low confidence (2)

internal/schema/conversion.go:10

  • There are currently no unit tests for UpgradePublishingSchema/UpgradePublishingRecordSchema; consider adding tests to verify that v3 data is correctly migrated to v4.
func UpgradePublishingSchema(data map[string]interface{}) {

internal/services/api/http_service.go:32

  • [nitpick] The tag 'AccessControl Log' differs from the 'AccessControl' tag used in tests; consider unifying the log-request name for consistency.
		handler = middleware.LogRequest("AccessControl Log", log, handler)

Base automatically changed from pcc-creds to main July 8, 2025 19:23
@mslynch mslynch requested a review from rodriin July 8, 2025 20:17
@mslynch mslynch marked this pull request as ready for review July 8, 2025 20:17
@rodriin
Copy link
Collaborator

rodriin commented Jul 15, 2025

@sagerb @dotNomad I have verified the E2E tests are passing successfully. It would be great if you could help review this PR since I think it is ready for review. Thank you!

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw a couple things as I started to review this. Commenting at the end of my day. Apologies for the long review on this. It is quite large, and the schema changes do not make this for an easy review.

"type": "string",
"default": "pip",
"description": "Package manager that will install the dependencies. If package-manager is none, dependencies are assumed to be pre-installed on the server. The default is 'pip'.",
"examples": ["pip", "conda", "pipenv", "poetry", "none"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stood out to me. My understanding is we only support uv and pip. Looks like these were added with the update to v4.

Comment on lines +37 to +41
"id": {
"type": "string",
"description": "Unique ID of this deployment.",
"examples": ["de2e7bdb-b085-401e-a65c-443e40009749"]
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed up the drafts to the CDN to test this out and I'm seeing "id" is a required property which doesn't appear to occur with v3.

Image

Comment on lines +416 to +417
"type": "object",
"description": "Python language and dependencies.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to keep the additionalProperties: false on these sections. I'm not 100% sure though so let me know if I'm missing something there.

"type": "string",
"default": "",
"description": "Python interpreter version, in PEP 440 format, required to run the content. If not specified it will be detected from the one in use.",
"examples": [">3.8", "<3.9", "==3.5"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this got a adjusted. Not sure which is more correct.

Suggested change
"examples": [">3.8", "<3.9", "==3.5"]
"examples": [">3.8", "<3.9", "=3.5"]

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took another look through some of this. When pointing at the draft schema for the configuration file I'm seeing an error "Unknown Title"

Image

Even Better TOML doesn't seem to show the error, but something is having trouble validating that.


It looks like we should update the docs/configuration.md file to include new information about the server type and other Connect Cloud fields.


Conversion on the deployment record to v4 worked great, but I had a similar issue where when I pointed to schemas/draft/posit-publishing-record-schema-v4.json (which are basically the same, but not exactly?) then the Publisher couldn't validate it and I could no longer select it as a deployment. My worry is that if that is happening with the draft it will happen with the non-draft v4 record schema.

It looks like the configuration saved to the record uses v4 which is nice even though the Configuration is v3. What are our thoughts on how to migrate configurations? Should we consider that or are we accepting that v3 configurations work with v4 deployment records?


I've been looking through this for a few hours and I'm having trouble verifying everything here and keeping all of the changes in my head.

Would it be possible to split this up a bit into easier to review PRs?

Some ideas for where to split it:

  • the removal of dead code from initialize
  • potentially the TOML backwards compatibility could be its own PR since that changes how schemas are validated
  • the schema change to use $defs more
    • We could try a draft version with just Connect changes and verify those changes then another PR with the Posit Connect Cloud def with those schema properties

I really want to verify the new changes to confirm things are working as expected, but its difficult with the amount of them. Let me know your thoughts on a potential split up to make it easier to verify.

If we want to keep everything in the same PR we can start verifying functionality with the internal/schema/schemas/draft/...-v4.json files as I was doing above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the TOML file schemas
3 participants