Skip to content

Set OpenApiServers object in document #56470

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

Merged
merged 6 commits into from
Jul 10, 2024
Merged

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Jun 26, 2024

Closes #56188 by setting servers property in local development based on metadata in IServerAddressFeature.

@captainsafia captainsafia requested a review from a team as a code owner June 26, 2024 19:28
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 26, 2024
@captainsafia captainsafia force-pushed the safia/openapi-follow-ups branch from ee6f2dd to 04799a4 Compare June 28, 2024 19:19
@captainsafia captainsafia requested a review from mikekistler July 1, 2024 16:46
@captainsafia
Copy link
Member Author

I've temporarily reverted readOnly support from this PR in reaction to some questions about how we might evolve readOnly in OpenAPI v3.1.

In OpenAPI v3.0, when readOnly is applied on a property that is in the required list, it's requiredness will only be validated for response types.

If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only.

In OpenAPI v3.1, we rely on JSON schema rules which don't enable similar validation on readOnly. We'll need to revisit how we want to bake in this behavior into the implementation at a later time.

@captainsafia
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

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

What's restricting this to development mode? Is it safe to include the server address(es) in a document that users can query?

Update the PR description so the readonly issue isn't closed 😄

@captainsafia captainsafia changed the title Set readOnly status for properties and support OpenApiServers Set OpenApiServers object in document Jul 3, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jul 10, 2024
@captainsafia captainsafia force-pushed the safia/openapi-follow-ups branch from 99c5aa0 to 4bb0021 Compare July 10, 2024 18:44
@captainsafia captainsafia removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jul 10, 2024
@captainsafia
Copy link
Member Author

What's restricting this to development mode? Is it safe to include the server address(es) in a document that users can query?

Added an explicit check for the host environment here. This should achieve the desired goal of only settings the servers attribute for local scenarios where you might want to use something like Swagger UI or Postman to do E2E testing.

@captainsafia captainsafia added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc labels Jul 10, 2024
Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

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

nit: I didn't notice a test with multiple addresses?

@captainsafia
Copy link
Member Author

nit: I didn't notice a test with multiple addresses?

Just added one.

@captainsafia captainsafia enabled auto-merge (squash) July 10, 2024 19:17
@captainsafia captainsafia mentioned this pull request Jul 10, 2024
1 task
@captainsafia captainsafia merged commit d498542 into main Jul 10, 2024
26 checks passed
@captainsafia captainsafia deleted the safia/openapi-follow-ups branch July 10, 2024 21:37
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview7 milestone Jul 10, 2024
@heedfull
Copy link

Is there a way to get your servers to show when the environment is Production?
I need the servers so that the workaround below for running behind a reverse proxy works.

scalar/scalar#4381 (comment)

@martincostello
Copy link
Member

One way you can do it is using my OpenAPI extensions library, which includes a document transformer that adds the servers to the document for Production via an opt-in setting.

If you don't want to use that, then you'll need to implement your own document transformer to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Servers not emitted in OpenAPI document
4 participants