Skip to content

Disable swagger-ui in prod.#2800

Merged
fisx merged 2 commits intodevelopfrom
hide-swagger
Oct 30, 2022
Merged

Disable swagger-ui in prod.#2800
fisx merged 2 commits intodevelopfrom
hide-swagger

Conversation

@fisx
Copy link
Contributor

@fisx fisx commented Oct 26, 2022

this hides https://prod-nginz-https.wire.com/api/swagger-ui/ from prod backends to minimize the attack surface.

it should also hide https://prod-nginz-https.wire.com/swagger-ui/ from prod, which delivers js code needed until we can close https://wearezeta.atlassian.net/browse/BE-529. i haven't figured out how to do that yet. but it's somehow related to this and this.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@fisx fisx temporarily deployed to cachix October 26, 2022 14:20 Inactive
@supersven supersven self-assigned this Oct 26, 2022
@flokli
Copy link
Contributor

flokli commented Oct 26, 2022

wire-server helm charts shouldn't know what prod and staging is. We should make the fact if we want to expose/run swagger-ui configurable via helm values, and set them then accordingly in environment-specific config (cailleach)

@jschaul jschaul temporarily deployed to cachix October 26, 2022 17:19 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Oct 26, 2022
@jschaul jschaul marked this pull request as ready for review October 26, 2022 17:20
Copy link
Member

Choose a reason for hiding this comment

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

Now this block is under {{ if not (eq $.Values.nginx_conf.env "prod") }} as well.

@jschaul jschaul temporarily deployed to cachix October 26, 2022 17:21 Inactive
@jschaul
Copy link
Member

jschaul commented Oct 26, 2022

wire-server helm charts shouldn't know what prod and staging is. We should make the fact if we want to expose/run swagger-ui configurable via helm values, and set them then accordingly in environment-specific config (cailleach)

Valid point. That would require a general overhaul of charts/nginz to remove the concept of environment, and synchronize merging of that refactoring with new values on our systems. I propose to do such a refactoring in an independent PR.

As for staying with the current design, this PR achieves its objective as-is without further configuration changes in other environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments