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
@@ -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.

Copy link
@pjdufour-truss

pjdufour-truss Mar 7, 2019

Contributor

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

$(AWS_VAULT) ./bin/gin --build ./cmd/webserver \
--bin /bin/webserver \
--port 8080 --appPort 8081 \
@@ -236,6 +237,7 @@ server_run_default: server_deps server_generate db_dev_run
.PHONY: server_run_debug
server_run_debug:
INTERFACE=localhost DEBUG_LOGGING=true \
SERVE_SWAGGER_UI=true \
$(AWS_VAULT) dlv debug cmd/webserver/main.go

.PHONY: build_tools
@@ -0,0 +1,24 @@
package main

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

What is this script for?

This comment has been minimized.

Copy link
@jim

jim Mar 6, 2019

Author Contributor

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


import (
"flag"
"fmt"
"io/ioutil"
"log"
"net/http"
)

func main() {
img := flag.String("image", "", "path to image to check the type of")
flag.Parse()

if len(*img) == 0 {
log.Fatal("Please provide an image to check using the -image flag")
}
imgBytes, err := ioutil.ReadFile(*img)
if err != nil {
log.Fatal(err)
}
contentType := http.DetectContentType(imgBytes)
fmt.Println(contentType)
}
@@ -103,6 +103,8 @@ type errInvalidPKCS7 struct {
Path string
}

const serveSwaggerUIFlag string = "serve-swagger-ui"

func (e *errInvalidPKCS7) Error() string {
return fmt.Sprintf("invalid DER encoded PKCS7 package: %s", e.Path)
}
@@ -194,6 +196,7 @@ func initFlags(flag *pflag.FlagSet) {
flag.String("internal-swagger", "swagger/internal.yaml", "The location of the internal API swagger definition")
flag.String("orders-swagger", "swagger/orders.yaml", "The location of the Orders API swagger definition")
flag.String("dps-swagger", "swagger/dps.yaml", "The location of the DPS API swagger definition")
flag.Bool(serveSwaggerUIFlag, false, "Whether to serve swagger UI for the APIs")

flag.Bool("debug-logging", false, "log messages at the debug level.")
flag.String("client-auth-secret-key", "", "Client auth secret JWT key.")
@@ -921,6 +924,12 @@ func main() {
},
)

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

This comment has been minimized.

Copy link
@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.

}

// Base routes
site := goji.NewMux()
// Add middleware: they are evaluated in the reverse order in which they
@@ -988,7 +997,9 @@ func main() {

// Allow public content through without any auth or app checks
site.Handle(pat.Get("/static/*"), clientHandler)
site.Handle(pat.Get("/swagger-ui/*"), clientHandler)
if v.GetBool(serveSwaggerUIFlag) {
site.Handle(pat.Get("/swagger-ui/*"), clientHandler)
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@pjdufour-truss

pjdufour-truss Mar 7, 2019

Contributor

I think we should add an info line here too.

}
site.Handle(pat.Get("/downloads/*"), clientHandler)
site.Handle(pat.Get("/favicon.ico"), clientHandler)

@@ -997,8 +1008,10 @@ func main() {
ordersMux.Use(ordersDetectionMiddleware)
ordersMux.Use(noCacheMiddleware)
ordersMux.Use(clientCertMiddleware)
ordersMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("orders-swagger")))
ordersMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "orders.html")))
if v.GetBool(serveSwaggerUIFlag) {
ordersMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("orders-swagger")))
ordersMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "orders.html")))
}
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

We should check with @jamesatheyDDS about whether or not we need to expose this.

This comment has been minimized.

Copy link
@jim

jim Mar 6, 2019

Author Contributor

I have confirmed that it is OK to disable the orders swagger UI.

ordersMux.Handle(pat.New("/*"), ordersapi.NewOrdersAPIHandler(handlerContext))
site.Handle(pat.Get("/orders/v0/*"), ordersMux)

@@ -1007,8 +1020,10 @@ func main() {
dpsMux.Use(dpsDetectionMiddleware)
dpsMux.Use(noCacheMiddleware)
dpsMux.Use(clientCertMiddleware)
dpsMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("dps-swagger")))
dpsMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "dps.html")))
if v.GetBool(serveSwaggerUIFlag) {
dpsMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("dps-swagger")))
dpsMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "dps.html")))
}
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

We should talk with @aileendds about whether or not ewe need to expose this.

This comment has been minimized.

Copy link
@jim

jim Mar 6, 2019

Author Contributor

I have confirmed that it is OK to disable the DPS swagger UI.

dpsMux.Handle(pat.New("/*"), dpsapi.NewDPSAPIHandler(handlerContext))
site.Handle(pat.New("/dps/v0/*"), dpsMux)

@@ -1053,9 +1068,10 @@ func main() {

apiMux := goji.SubMux()
root.Handle(pat.New("/api/v1/*"), apiMux)
apiMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("swagger")))
apiMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "api.html")))

if v.GetBool(serveSwaggerUIFlag) {
apiMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("swagger")))
apiMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "api.html")))
}
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

If this is supposed to be the "Public" API should it be available? Or did we make a product decision to hide this for now since we're not really going to deliver on it?

This comment has been minimized.

Copy link
@jim

jim Mar 6, 2019

Author Contributor

I will check with @mtorres253, but that is my current understanding.

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 6, 2019

Contributor

My one concern about hiding it is that if we start treating the public api like an internal-only api then there's really no reason to have both (there already isn't). But having it public was "supposed" to force us to treat it with a bit more respect so we would learn good practices now (like versioning, backwards compatibility, good docs, etc) vs when the TSPs actually need it. But I'm also on the fence about supporting anything that isn't really a product focus.

externalAPIMux := goji.SubMux()
apiMux.Handle(pat.New("/*"), externalAPIMux)
externalAPIMux.Use(noCacheMiddleware)
@@ -1064,9 +1080,10 @@ func main() {

internalMux := goji.SubMux()
root.Handle(pat.New("/internal/*"), internalMux)
internalMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("internal-swagger")))
internalMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "internal.html")))

if v.GetBool(serveSwaggerUIFlag) {
internalMux.Handle(pat.Get("/swagger.yaml"), fileHandler(v.GetString("internal-swagger")))
internalMux.Handle(pat.Get("/docs"), fileHandler(path.Join(build, "swagger-ui", "internal.html")))
}
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj
// Mux for internal API that enforces auth
internalAPIMux := goji.SubMux()
internalMux.Handle(pat.New("/*"), internalAPIMux)
@@ -141,6 +141,10 @@
{
"name": "SEND_PROD_INVOICE",
"value": "{{ .SEND_PROD_INVOICE }}"
},
{
"name": "SERVE_SWAGGER_UI",
"value": "{{ .SERVE_SWAGGER_UI }}"
}
],
"logConfiguration": {
@@ -161,6 +161,10 @@
{
"name": "SEND_PROD_INVOICE",
"value": "{{ .SEND_PROD_INVOICE }}"
},
{
"name": "SERVE_SWAGGER_UI",
"value": "{{ .SERVE_SWAGGER_UI }}"
}
],
"logConfiguration": {
@@ -6,4 +6,5 @@ HTTP_SDDC_SERVER_NAME=mymove-experimental.sddc.army.mil
DPS_REDIRECT_URL=https://dpstest.sddc.army.mil/cust
DPS_COOKIE_NAME=DPSIVV
GEX_URL=https://gexweba.daas.dla.mil/msg_data/submit/
SEND_PROD_INVOICE=false
SEND_PROD_INVOICE=false
SERVE_SWAGGER_UI=true
@@ -6,4 +6,5 @@ HTTP_SDDC_SERVER_NAME=mymove-staging.sddc.army.mil
DPS_REDIRECT_URL=https://dpstest.sddc.army.mil/cust
DPS_COOKIE_NAME=DPSIVV
GEX_URL=https://gexweba.daas.dla.mil/msg_data/submit/
SEND_PROD_INVOICE=false
SEND_PROD_INVOICE=false
SERVE_SWAGGER_UI=true
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 5, 2019

Contributor

Should we also put this in our .envrc so that its on for devs by default?

This comment has been minimized.

Copy link
@pjdufour-truss
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.