Skip to content
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

Add a HideDocument flag akin to HideSwaggerUI #297

Merged
merged 5 commits into from
Feb 10, 2022

Conversation

gsteenpa
Copy link
Contributor

@gsteenpa gsteenpa commented Oct 22, 2021

Added a flag to hide the document bindings so that the feature can be "turned off" in a particular configuration of a deployed API.

Setting HideDocument will also hide the UI.

@ghost
Copy link

ghost commented Oct 22, 2021

CLA assistant check
All CLA requirements met.

@justinyoo
Copy link
Contributor

@gsteenpa Thanks for the PR!

But, would you please be able to elaborate this PR? If you hide the OpenAPI document, what's the purpose of using this extension?

@gsteenpa
Copy link
Contributor Author

gsteenpa commented Nov 1, 2021

@gsteenpa Thanks for the PR!

But, would you please be able to elaborate this PR? If you hide the OpenAPI document, what's the purpose of using this extension?

Hi @justinyoo - If you want to use same code build in every environment, but not have the swagger documentation available in every environment, this flag lets you do that via configuration. For example, we use the json to generate api clients in the build pipeline and have the ui /json available in Dev, but off/blocked everywhere else.

@justinyoo
Copy link
Contributor

Hi @justinyoo - If you want to use same code build in every environment, but not have the swagger documentation available in every environment, this flag lets you do that via configuration. For example, we use the json to generate api clients in the build pipeline and have the ui /json available in Dev, but off/blocked everywhere else.

Then, I would recommend modifying your .csproj file to conditionally add this package. You can achieve this like:

<Project ...>
  <PropertyGroup>
    ...
    <OpenApiExtensionRequired>True</OpenApiExtensionRequired>
  </PropertyGroup>

  <ItemGroup Condition="'$(OpenApiExtensionRequired)'=='True'">
    <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.OpenApi" Version="0.9.0-preview" />
  </ItemGroup>
  ...
</Project>

Then, within your release pipeline for environments other than DEV, publish your app like:

dotnet publish FunctionApp.csproj ... /p:OpenApiExtensionRequired=False

That could be much easier, without having to touch codes.

@gsteenpa
Copy link
Contributor Author

gsteenpa commented Nov 2, 2021

Hi @justinyoo - If you want to use same code build in every environment, but not have the swagger documentation available in every environment, this flag lets you do that via configuration. For example, we use the json to generate api clients in the build pipeline and have the ui /json available in Dev, but off/blocked everywhere else.

Then, I would recommend modifying your .csproj file to conditionally add this package. You can achieve this like:

<Project ...>
  <PropertyGroup>
    ...
    <OpenApiExtensionRequired>True</OpenApiExtensionRequired>
  </PropertyGroup>

  <ItemGroup Condition="'$(OpenApiExtensionRequired)'=='True'">
    <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.OpenApi" Version="0.9.0-preview" />
  </ItemGroup>
  ...
</Project>

Then, within your release pipeline for environments other than DEV, publish your app like:

dotnet publish FunctionApp.csproj ... /p:OpenApiExtensionRequired=False

That could be much easier, without having to touch codes.

Thank you for the suggestion, but changing a configuration flag would be much easier than changing the referenced DLLs for us. We deploy via zip file and want to use the same zip file in every environment. We leverage configuration flags to alter the behavior of that deployed code via keyvault secrets. Using the publish parameter would require us to deploy different zip files to different environments which we would like to avoid.

@justinyoo
Copy link
Contributor

Thanks for the clarification. I'm still not 100% convinced whether this is the proper approach or not. Let me take a further look.

@justinyoo justinyoo added the hacktoberfest Hacktoberfest label Nov 4, 2021
Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

I took a look and have one ask for your to consider.

Because Swagger UI without OpenAPI document can't render the UI page properly, the HideDocument flag is set to true, it should also hide the UI page, regardless of the HideSwaggerUI value being true or false. Therefore, the logic between these two flags should be like below:

HideSwaggerUI HideDocument Swagger UI OpenAPI Document
false false SHOW SHOW
false true ERROR 👈 (current) NO SHOW
false true NO SHOW 👈 (to be) NO SHOW
true false NO SHOW SHOW
true true NO SHOW NO SHOW

@justinyoo justinyoo added enhancement New feature or request v1.1.0 labels Nov 8, 2021
@gsteenpa
Copy link
Contributor Author

gsteenpa commented Nov 8, 2021

I took a look and have one ask for your to consider.

Because Swagger UI without OpenAPI document can't render the UI page properly, the HideDocument flag is set to true, it should also hide the UI page, regardless of the HideSwaggerUI value being true or false. Therefore, the logic between these two flags should be like below:

HideSwaggerUI HideDocument Swagger UI OpenAPI Document
false false SHOW SHOW
false true ERROR 👈 (current) NO SHOW
false true NO SHOW 👈 (to be) NO SHOW
true false NO SHOW SHOW
true true NO SHOW NO SHOW

All set.

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

Thanks for the long wait! I've left a couple of comments for your to take a look - both are to avoid additonal indentation depth.

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the wait!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request hacktoberfest Hacktoberfest v1.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants