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

Replace st2web https with http #72

Merged
merged 11 commits into from
May 31, 2019
Merged

Replace st2web https with http #72

merged 11 commits into from
May 31, 2019

Conversation

warrenvw
Copy link
Contributor

@warrenvw warrenvw commented May 29, 2019

See StackStorm/st2-dockerfiles#20 for more context.

Since st2web backend now defaults to using HTTP instead of HTTPS, remove all the HTTPS related stuff, including the certs. Update ports.

Remaining tasks:

  • Review code to see if any additional changes need to be made.
  • Ensure st2web can be reached when ST2WEB_HTTPS=0.

@warrenvw warrenvw requested a review from arm4b May 29, 2019 21:14
@warrenvw warrenvw self-assigned this May 29, 2019
@warrenvw warrenvw marked this pull request as ready for review May 30, 2019 05:09
@warrenvw warrenvw added the RFR label May 30, 2019
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Also needs an update to Ingress servicePort defaults, README.md update for st2web with note that's it's HTTP-by-default, CHANGELOG and updating the Chart version.

CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

## In Development

## v0.16.0
* st2web now uses HTTP by default (#72)
Copy link
Member

Choose a reason for hiding this comment

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

Ideally to add a few more words that it's now recommended to rely on "LoadBalancer" or "Ingress" to add HTTPs layer on top of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea. Thanks.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Please take a look also at NOTES.txt
There are some https mentions there which are not defaults anymore.

README.md Outdated
@@ -83,6 +83,8 @@ kubectl exec -it ${ST2CLIENT} /bin/bash
### [st2web](https://docs.stackstorm.com/latest/reference/ha.html#nginx-and-load-balancing)
st2web is a StackStorm Web UI admin dashboard. By default, st2web K8s config includes a Pod Deployment and a Service.
`2` replicas (configurable) of st2web serve the web app and proxy requests to st2auth, st2api, st2stream.
By default, st2web uses HTTP instead of HTTPS. Pass in ST2WEB_HTTPS environment variable with a non-zero value
(e.g. "1") and ensure SSL certificates are configured.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to mention ST2WEB_HTTPS or certificates here as it was designed for possible docker-compose use cases outside of K8s where users don't have concept of Ingress layer.

@@ -83,6 +83,8 @@ kubectl exec -it ${ST2CLIENT} /bin/bash
### [st2web](https://docs.stackstorm.com/latest/reference/ha.html#nginx-and-load-balancing)
st2web is a StackStorm Web UI admin dashboard. By default, st2web K8s config includes a Pod Deployment and a Service.
`2` replicas (configurable) of st2web serve the web app and proxy requests to st2auth, st2api, st2stream.
By default, st2web uses HTTP instead of HTTPS. Pass in ST2WEB_HTTPS environment variable with a non-zero value
(e.g. "1") and ensure SSL certificates are configured.
> **Note!** By default, st2web is a NodePort Service and is not exposed to the public net.
If your Kubernetes cluster setup supports the LoadBalancer service type, you can edit the corresponding helm values to configure st2web as a LoadBalancer service in order to expose it and the services it proxies to the public net.
Copy link
Member

@arm4b arm4b May 30, 2019

Choose a reason for hiding this comment

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

Can mention Ingress here as well as that's one of the possible solutions apart of LoadBalancer.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

👍

@warrenvw warrenvw merged commit 7a05b9b into master May 31, 2019
@warrenvw warrenvw deleted the feature/http branch May 31, 2019 21:23
@cognifloyd cognifloyd removed the RFR label Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants