Skip to content

Conversation

@leifjones
Copy link

@leifjones leifjones commented Dec 12, 2020

Description

  • Add BASE_URL environment variable
  • Replicate pertinent portions of swagger-ui's nginx.conf and run.sh
  • Dockerfile: Replace deprecated MAINTAINER with LABEL
  • Dockerfile: Replace deprecated ADD with COPY

@fehguy Because you're listed as maintainer for the Dockerfile, I want to let you know about this PR. (To others, though, please note that this pattern used is identical to what is in the analogous Dockerfile for which fehguy is also listed as maintainer.)

@tim-lai thanks for categorizing the recent similar PR (#2257). If that's needed again, please do so here,

Motivation and Context

This adds the option to specify a BASE_URL at which to serve the application. As is done with Swagger UI (see it's Installation documentation), the above PR now allows specification of a BASE_URL. This allows the editor to be served at a different location (e.g. /swagger-editor), which can enable this to be served at a single URL routed via a Kubernetes Ingress object.

Fix: #1956

How Has This Been Tested?

New functionality:

  1. docker build .
  2. docker run -e BASE_URL='/swagger-editor' -p 80:8080 [hash]
  3. Navigate to http://localhost/swagger-editor and see that the page loads

Verify prior functionality without BASE_URL:

  1. docker run -p 80:8080 [same-hash]
  2. Navigate to http://localhost/ and see that the page loads

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

- Add BASE_URL environment variable
- Replicate pertinent portions of swagger-ui's nginx.conf and run.sh
- Dockerfile: Replace deprecated MAINTAINER with LABEL
- Dockerfile: Replace deprecated ADD with COPY
- Revert `index` to prior location
- Add `/` to `alias` as required
- Prevent redirect to 8080
- Restore change to what was in repo previously to reduce unneeded changes
@tim-lai
Copy link
Contributor

tim-lai commented Dec 18, 2020

@mrsegen @PhilippHeuer combining my thoughts about this ticket as well as #2259 here... I don't know the history why a privileged nginx change was added. With that caveat, I'm in favor of having swagger-ui and swagger-editor sharing the same docker config file (this PR), as well as shoring up any potential security issues when docker is used with/without nginx, kubernetes, etc.

Atm, I am inclined to accept this PR and reject #2259, as I don't know who/what might break with #2259. Any additional thoughts?

@tim-lai tim-lai merged commit 2356bb8 into swagger-api:master Jan 4, 2021
@tim-lai
Copy link
Contributor

tim-lai commented Jan 4, 2021

@mrsegen This PR is now merged! Thanks for the contribution!

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.

Add support for dynamic context path

2 participants