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

Creating edge-proxy helm chart #280

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

BennyG93
Copy link

@BennyG93 BennyG93 commented Oct 3, 2024

Thanks for submitting a PR! Please check the boxes below:

  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have bumped the version number in /charts/flagsmith/Chart.yaml in the section version or I'm merging to a
    release branch

Changes

Creating a helm chart for deploying the Flagsmith Edge Proxy to Kubernetes.

Explanation:
The chart was initially created using the helm create...

Users can write their proxy configuration in edgeProxy.configMap.content or provide the name of an existing configmap that has their configuration to edgeProxy.configMap.name.

The chart will create a Volume using this ConfigMap and mount it to each container at the path /app/config.json.

How did you test this code?

We have deployed this chart to our infra.

  • Clone the repo.
  • Edit/create a values.yaml file and add your configuration to edgeProxy.configMap.content.
  • Optionally add an ingress config to access the service.
  • Install the helm chart, e.g. helm install -f myvalues.yaml edge-proxy ./edge-proxy
  • Test the proxy response.

This is an initial attempt, hopefully it provides a solution for anyone who wants to self host the proxy service on Kubernetes.

@BennyG93 BennyG93 requested a review from a team as a code owner October 3, 2024 09:45
@BennyG93 BennyG93 requested review from zachaysan and removed request for a team October 3, 2024 09:45
Copy link
Contributor

@zachaysan zachaysan left a comment

Choose a reason for hiding this comment

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

Changes look good but please take a look at the linting failures in CI.

@BennyG93
Copy link
Author

BennyG93 commented Oct 9, 2024

Changes look good but please take a look at the linting failures in CI.

Hey @zachaysan - I noticed the failing CI check as well, its failing because it wants me to bump the flagsmith chart version.

 ✔︎ edge-proxy => (version: "0.1.0", path: "charts/edge-proxy")
 ✖︎ flagsmith => (version: "0.59.0", path: "charts/flagsmith") > Chart version not ok. Needs a version bump!

I bumped it to make it work, but I guess you don't want to be bumping the version of a chart which is not being updated?

@matthewelwell
Copy link
Contributor

Thanks @BennyG93, yeah, this is a similar problem to one that we've been putting off for too long (well, it just exacerbates the problem really). I think it's finally time to integrate release-please here @khvn26 ?

Maybe we can merge this PR first and just accept the pointless bump on the flagsmith version though, what do you think?

@BennyG93
Copy link
Author

BennyG93 commented Oct 9, 2024

My 2 cents here, if we merge this PR before that fix/change is made, then I guess whenever you update the main Flagsmith helm chart, it will ask you to bump this proxy chart as well. (and vice versa). So just consider that.

The other thing I can do when I have some time is to try and merge the templates of this chart inside the main Flagsmith chart so that it becomes more of an optional sub-component that can be enabled and disabled. With this approach, there will only be 1 chart and you can continue putting off the problem ;)

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.

3 participants