Add support to specify images name and to enable HTTP for st2web by GGabriele · Pull Request #44 · StackStorm/stackstorm-k8s
Hello, this is an effort to add the possibility to specify the images name to pull from a custom registry. If no image name is specified, the default behavior is used.
I'm also enabling the toggling of HTTP/HTTPS for st2web (another PR to patch the st2web container will follow).
Let me know if any of this doesn't make sense :)
We tried to keep the values.yaml less overwhelmed with the configuration, awaiting for contributions like this to appear when there is a need from our users.
Good to see this happening and thanks for opening a PR! 👍
One question. Do you really need to change Docker image name for each service or specifying a custom registry would be enough? If you could keep the image names the same with original st2 images, then you won't need any custom names, just registry. Would it solve your case? Let me know your thoughts.
| # Many st2web instances, placed behind a load balancer that serve web app and proxify requests to st2auth, st2api, st2stream. | ||
| st2web: | ||
| protocol: | ||
| http: false |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @armab , thanks for reviewing this.
One question. Do you really need to change Docker image name for each service or specifying a custom registry would be enough?
Ideally I don't want to build all the images, but just few of them (in this case, only st2web actually). Maybe we can name the images following the original st2 images, but we'd still need a way to specify if an image should be pulled from the custom registry or not, e.g.:
st2web:
images:
custom: true
That said, I wouldn't even need to build/pull any custom image if we can solve the HTTP/HTTPS problem in the upstream, so let me know if I can help tackling it somehow :)
We talked with @warrenvw and how that can fit the roadmap/plans.
In short, in future we indeed want to expose st2web in K8s via HTTP by default.
On top of that, allow users to configure HTTPS/SSL settings via K8s Ingress controller (not yet done #6) or LoadBalancer annotations (done).
This way SSL termination responsibility will shift from st2web nginx to K8s-controlled optional layers. It looks like a obvious practice across https://github.com/helm/charts. This way we'll also get rid of beta dirt like https://github.com/StackStorm/stackstorm-ha/blob/master/values.yaml#L140-L198 which will improve #16 story.
First part (st2web HTTP) will require adjusting https://github.com/StackStorm/st2-dockerfiles/tree/master/st2web and K8s repo to reflect the changes.
Second part is adding Ingress controller #6 option to stackstorm-ha to compensate the absense of HTTPS by default.
@GGabriele How that fits your story, at what point of your infra you plan to add secure layer on top of HTTP-only st2web?
arm4b
mentioned this pull request
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rebase this PR or merge in master?
Several things have changed, including at least:
- the enterprise stuff is gone, so maybe that would simplify some of these
imageNameblocks. - the st2web pod is using HTTP now instead of HTTPS. I'm not sure when that changed, but if an HTTP/HTTPS flag is still needed, maybe that could be put in a separate PR?
- looks like we can get the custom registry bit from
image.repositorynow, right?
#200 was just merged which adds the ability to define a custom tag for each of the pods. That will also affect making image names configurable in this PR.
@cognifloyd Yeah indeed, good call 👍 From the conversation history:
For https://github.com/StackStorm/st2-dockerfiles/tree/master/st2web I think we'll need a switch so nginx will pick up HTTP or HTTPS configs on startup based on user's specified ENV variable. This will keep some level of compatibility for those who need HTTPS from that Docker image.
In short, in future we indeed want to expose st2web in K8s via HTTP by default.
On top of that, allow users to configure HTTPS/SSL settings via K8s Ingress controller (not yet done #6) or LoadBalancer annotations (done).This way SSL termination responsibility will shift from st2web nginx to K8s-controlled optional layers. It looks like a obvious practice across https://github.com/helm/charts. This way we'll also get rid of beta dirt like https://github.com/StackStorm/stackstorm-ha/blob/master/values.yaml#L140-L198 which will improve #16 story.
First part (st2web HTTP) will require adjusting https://github.com/StackStorm/st2-dockerfiles/tree/master/st2web and K8s repo to reflect the changes.
Second part is adding Ingress controller #6 option to stackstorm-ha to compensate the absense of HTTPS by default.
Both features were implemented a while ago.
The st2web listens on HTTP by default and SSL termination happens and configurable in another layer: Ingress controller or LoadBalancer.
@GGabriele Thanks for the initial PR, the proposed approach and discussion here helped to eventually implement it the right way!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters