-
Notifications
You must be signed in to change notification settings - Fork 14
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
Including Service Bindings for nginx.conf and mime.types #281
Comments
@voor It's possible to add such a feature but I'm wondering about the value of such a feature. Bindings are typically used for secrets that users do not/cannot check-in with the their app. Is there a compelling reason to avoid checking in configuration files? |
Yes, different teams are responsible for the hand-off, and we do not want the consumers of the buildpack to change them, if the files are present in the users repository they might accidentally modify or delete it, causing significant issues in the build process. Additionally, if we need to change the files we would need to go to all of the consumers and make sure the file is updated accordingly. |
+1 for this. I haven't tried it, but I think you can use a ConfigMap instead of a secret with a binding. The binding just has to be mapped into the file system in the expected location. I'm pretty sure this is common practice with Nginx config, to pass it in through volume mounts. |
I'm using kpack syntax for this, but I'd be looking for something like this:
|
We'll take it as a feature request and look into it. |
Note, we'll follow this spec instead, which supersedes the originally posted link: https://github.com/k8s-service-bindings/spec |
Awesome, we've been using https://github.com/vmware-labs/service-bindings as a possible implementation of this spec. |
Also note, that we'll play paketo-buildpacks/packit#107 first, as this buildpack is packit-based. Marking this story is blocked on that for now. |
@voor @arjun024 @dmikusa-pivotal Now that paketo-buildpacks/packit#107 is done, I'm coming back around to this. However, there's some things to consider with implementation. Service bindings should not become embedded in the final app image, by design. This means that the need here is really a launch-time concern and not a build-time concern (note that this eliminates the need for the kpack-based workflow @voor posted). If what you want is to avoid having to check in these files, then it seems that the launch command just needs to be altered to load the config from a location given by the binding spec (like this). I can't remember if there's a way to alter the launch command manually or by some other buildpack, or if we'll need to supply a custom means for specifying the config location via this buildpack, like via an env var (perhaps @arjun024 you can enlighten me). |
Of course, I need to look further to see how this buildpack implements the templating for the config, and understand how that would be affected. |
While that does make a lot of sense, my concern in this particular instance is that the image becomes significantly less portable as a result, you need to now have knowledge that this particular nginx backed container doesn't actually have any nginx config inside of it, and you need to account for mounting that in at runtime. |
@voor I see what you mean. I'm wondering about whether or not this is a buildpack concern, since what you're referring to is to inject a file into the app dir prior to build. My gut says that this should be taken care of by something else before the buildpack kicks in, but I also understand that it may not be easy to do. Let me chat with the maintainers of this buildpack and see what they think. |
Why shouldn't the nginx.conf be part of the final image? I believe the use-case here is
Nginx buildpack currently "detects" based on the presence of an nginx.conf file
Launch-time alteration of start command shouldn't even be a concern. I think a user should be able to use I think this work constitutes a "substantial change" as described here and would like to |
@voor I spoke with @arjun024 and it turns out that this nginx case is really a special case of a more generic need -- the need to copy volume-mounted files into the app dir. If we had a utility buildpack that could do this, it could solve for your use case without the need to make this buildpack more complex. It would simply need to be placed before this nginx buildpack. I'll write up an RFC for the buildpack and will provide a link here when done. |
@voor Based on the conversation in the RFC, could you try volume mounting to Give it a try with a templated nginx config, as that apparently works for both build-time and at launch (though I suspect that launch-time is really what you want). |
@voor any update? Looking to see if this issue (and the RFC) can be closed. |
No, sorry, haven't had a chance to revisit this, which is unfortunate because this is something that we really need. |
@paketo-buildpacks/web-servers-maintainers Is this still a relevant feature request? |
I am seeing |
@voor Have you been able to test using |
I have not unfortunately 😞 |
What happened?
I would like to inject a nginx.conf file and mime.types from a common location using https://github.com/k8s-service-bindings/spec so that I do not need developers to commit this into source code.
Build Configuration
gcr.io/paketo-buildpacks/nginx:0.3.1
pack and Tanzu Build Service (kpack)
buildpack.yml
,nginx.conf
, etc.)?nginx.conf: https://github.com/voor/static-build-service-template/blob/5cfaac6b3b4dec9e5238ea290a866bed52b5c369/nginx.conf
mime.types: https://github.com/voor/static-build-service-template/blob/5cfaac6b3b4dec9e5238ea290a866bed52b5c369/mime.types
Checklist
The text was updated successfully, but these errors were encountered: