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

Draft: allow creating an OpenShift route #239

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

Conversation

kastl-ars
Copy link

This is not meant as a PR ready to merge. I just wanted to start a discussion, if the functionality to create an OpenShift route via the helm chart would be something worth adding.

I know the chart can create an ingress, but if you want to put OpenCost behind a oauth-proxy, you need to put the route name into an annotation on the serviceAccount. And if the route is being created from an ingress automatically, its name contains a random hash...

@kastl-ars kastl-ars force-pushed the 20241206_add_openshift_route branch 2 times, most recently from 54bb447 to a043849 Compare December 6, 2024 13:07
@kastl-ars kastl-ars force-pushed the 20241206_add_openshift_route branch from a043849 to 7147e1e Compare December 6, 2024 13:12
@AjayTripathy
Copy link
Collaborator

@kastl-ars I'm supportive of this idea! @mittal-ishaan you've been working in this area, do you have any thoughts?

@mittal-ishaan
Copy link
Contributor

I completely agree with @kastl-ars
In addition, I’d like to propose a discussion around introducing an openshift block in the Helm chart to have all OpenShift-specific configurations, such as Routes and in-cluster Prometheus integrations. This change would:

  • Ensure OpenShift-specific features are enabled only in OpenShift environments.
  • Address specific requirements like volume mounts for OpenShift, avoiding workarounds such as those mentioned in opencost-ui issue #4. This could either be solved or a workaround be included as a default in helm chart itself.

Thoughts?

@kastl-ars
Copy link
Author

I think the addition of an openshift block, that globally triggers e.g. the proper setting of the securityContext, would be nice. But I would propose to keep things separate, i.e. let this PR only be about adding a route.

Otherwise I fear this becomes to big to properly test and review...

@mittal-ishaan
Copy link
Contributor

mittal-ishaan commented Dec 9, 2024

yes, agree we can do it in a separate PR. Addition of openshift Route in the helm chart looks good to me.

@kastl-ars
Copy link
Author

Does anyone have better documentation on OpenShift routes than the incomplete and "going into obscure corner cases too fast" documentation here. Especially the exact details on the tls section are hard to find...

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