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 environment variable that controls if swagger UI is served. #1819

Merged
merged 10 commits into from Mar 7, 2019

Conversation

3 participants
@jim
Copy link
Contributor

jim commented Mar 5, 2019

Description

This PR disables serving the Swagger UI in production. It remains enabled in all other envs.

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • Request review from a member of a different team.

@jim jim requested review from pjdufour-truss and chrisgilmerproj Mar 5, 2019

@@ -0,0 +1,24 @@
package main

This comment has been minimized.

@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

What is this script for?

This comment has been minimized.

@jim

jim Mar 6, 2019

Author Contributor

Whoops. It was not supposed to be here. Will remove.

if v.GetBool(serveSwaggerUIFlag) {
logger.Info("Swagger UI serving is enabled")
} else {
logger.Info("Swagger UI serving is disabled")

This comment has been minimized.

@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

I feel like we could move these log lines into the if statements below and be more specific, like Swagger UI service is enabled for DPS or something.

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

Let's check in with the owners of the API about what they promised their customers about visability of swagger docs.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #1819 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1819   +/-   ##
=======================================
  Coverage   49.41%   49.41%           
=======================================
  Files         427      427           
  Lines       18352    18352           
  Branches     1629     1629           
=======================================
  Hits         9068     9068           
  Misses       8486     8486           
  Partials      798      798
@pjdufour-truss
Copy link
Contributor

pjdufour-truss left a comment

You'll need to add SERVE_SWAGGER_UI=false to config/env/prod.env. The way our templating currently works, it's required to template config/app.container-definition.json.

@jim jim marked this pull request as ready for review Mar 7, 2019

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

I took this over from @jim and updated it.

Makefile Outdated
@@ -227,6 +227,7 @@ server_run:
# This command runs the server behind gin, a hot-reload server
server_run_default: server_deps server_generate db_dev_run
INTERFACE=localhost DEBUG_LOGGING=true \
SERVE_SWAGGER_UI=true \

This comment has been minimized.

@pjdufour-truss

pjdufour-truss Mar 7, 2019

Contributor

Can we remove these lines and allow it to be managed using .envrc?

@pjdufour-truss pjdufour-truss self-requested a review Mar 7, 2019

@chrisgilmerproj chrisgilmerproj merged commit 6ef0351 into master Mar 7, 2019

19 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: client_test_coverage Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 1496f09...8b9d773
Details
codecov/project 49.41% remains the same compared to 1496f09
Details

@chrisgilmerproj chrisgilmerproj deleted the jb-swagger-ui-redux branch Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.