search

drone / charts

Helm charts for the Drone platform on Kubernetes
Other
166 stars 116 forks source link

Allow custom webserver port #21

Open jtackaberry opened 4 years ago

jtackaberry commented 4 years ago

This PR adds a port value to the Helm chart. The primary use case is to enable binding to high ports to avoid running Drone server as root.

jtackaberry commented 4 years ago

Note that the CI failure does not appear to be a consequence of this PR.

ashwilliams1 commented 4 years ago

can you provide more details why you would change the port inside the container? usually one would change the host port mapping. changing the DRONE_SERVER_PORT is generally not required or recommended. Also, this probably won't work as intended because DRONE_SERVER_PORT must be prefixed by a colon (i.e. :8080).

jtackaberry commented 4 years ago

can you provide more details why you would change the port running inside the container?

So the Drone server pod can be run as non-root. At the moment, it does look like the server and the kube runner work fine non-root, although the build container spawned by the runner currently assumes root.

Also, this probably won't work as intended because DRONE_SERVER_PORT must be prefixed by a colon.

That's no problem, the PR takes this into account.

bakito commented 3 years ago

Hi @jtackaberry I run into the same problem, when running the drone server in OpenShift. Looking for a work around I found a more generic way to override the port. https://github.com/bakito/drone-charts/commit/6822525f1752835f3010e96ed8f314553f2c1adf

Your solution also may lead into having the DRONE_SERVER_PORT variable twice in the configmap if it is also defined as env variable.

james-mcgoodwin commented 3 years ago

Putting a plus-one here as well. Getting drone to run with out root is a goal for us, and it requires the chart to switch to a different port.

At the moment we have to use a very permissive PSP to get drone to run, and we'd prefer not to.

jimsheldon commented 1 year ago

Apologies for the extreme delay on this.

Since this PR, the server chart has changed a bit. containerPort is now hard-coded to port 80 https://github.com/drone/charts/blob/master/charts/drone/templates/deployment.yaml#L41

I think this is a mistake, we should support changing the port.

I am a little hesitant to automatically set DRONE_SERVER_PORT in the configmap, we don't set any other variables this way. Could we instead add a comment in the valyes.yaml file where the port is documented, mentioning that DRONE_SERVER_PORT must be set to the same value in the env section?