Including Service Bindings for nginx.conf and mime.types

[voor]

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)

• Can you provide a sample app or relevant configuration (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

• [ ] I have included log output.
• [ ] The log output includes an error message.
• [ ] I have included steps for reproduction.

[arjun024]

@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?

[voor]

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.

[dmikusa]

+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.

[voor]

I'm using kpack syntax for this, but I'd be looking for something like this:

apiVersion: kpack.io/v1alpha1
kind: Image
metadata:
  name: simple-static-app
  namespace: default
spec:
  build:
    bindings:
    - name: nginx
      secretRef:
        name: nginx-config
      metadataRef:
        name: settings-binding-metadata
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: settings-binding-metadata
data:
  kind: nginx
  provider: nginx
---
apiVersion: v1
kind: Secret
metadata:
  name: nginx-config
type: Opaque
stringData:
  mime.types: |
    types {
        ...
    }
  nginx.confg: |
    worker_processes  5;  ## Default: 1
    daemon off;
    worker_rlimit_nofile 8192;
    ...

[arjun024]

We'll take it as a feature request and look into it.

[menehune23]

Note, we'll follow this spec instead, which supersedes the originally posted link: https://github.com/k8s-service-bindings/spec

[voor]

Awesome, we've been using https://github.com/vmware-labs/service-bindings as a possible implementation of this spec.

[menehune23]

Also note, that we'll play https://github.com/paketo-buildpacks/packit/issues/107 first, as this buildpack is packit-based. Marking this story is blocked on that for now.

[menehune23]

@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).

[menehune23]

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.

[voor]

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).

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.

[menehune23]

@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.

[arjun024]

Service bindings should not become embedded in the final app image, by design

Why shouldn't the nginx.conf be part of the final image? I believe the use-case here is for user to provide from outside version-control and wants the conf to end up in the final image

This means that the need here is really a launch-time concern and not a build-time concern

Nginx buildpack currently "detects" based on the presence of an nginx.conf file in the application. If nginx.conf is not going to mounted during detect/build time, then there has to be another robust mechanism to run detection for this buildpack.

I can't remember if there's a way to alter the launch command manually or by some other buildpack

Launch-time alteration of start command shouldn't even be a concern. I think a user should be able to use Procfile buildpack to change the start command and consume an nginx.conf from a mounted directory.

I think this work constitutes a "substantial change" as described here and would like to see an RFC proposed with the design/implementation & achieve consensus before we implement this.

[menehune23]

@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.

[menehune23]

@voor @arjun024 @dmikusa-pivotal RFC here: https://github.com/paketo-buildpacks/rfcs/pull/110

[menehune23]

@voor Based on the conversation in the RFC, could you try volume mounting to /workspace/nginx.conf, etc during a build (and also at launch if that's a concern), and see if that works for your use case (whether pack/kpack/etc)?

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).

[menehune23]

@voor any update? Looking to see if this issue (and the RFC) can be closed.

[voor]

@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.

[fg-j]

@paketo-buildpacks/web-servers-maintainers Is this still a relevant feature request?

[voor]

I am seeing BP_NGINX_CONF_LOCATION now allows a configurable location for the nginx conf, does that mean BP_NGINX_CONF_LOCATION could be enhanced with a service binding location?

[ForestEckhardt]

@voor Have you been able to test using BP_NGINX_CONF_LOCATION as a configuration option? If so does it do everything you would expect?

[voor]

I have not unfortunately 😞