Invoice Ninja

[lalop]

This PR adds Invoice Ninja official image.
Invoice Ninja is an app to manage invoices and timetracking.
The image is currently know as invoiceninja/invoiceninja and donwloaded more then 100000 times on docker hub.

Checklist for Review

NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)

• associated with or contacted upstream?
• does it fit into one of the common categories? ("service", "language stack", "base distribution")
• is it reasonably popular, or does it solve a particular use case well?
• does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • Invoice Ninja docs#1198
• dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
• 2+ dockerization review?
• existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
• if FROM scratch, tarballs only exist in a single commit within the associated history?
• passes current tests? any simple new tests that might be appropriate to add? (https://github.com/docker-library/official-images/tree/master/test)

[lalop]

I added a PR for documentation too docker-library/docs#1198

[docker-library-bot]

Hello! ✨

Thanks for your interest in contributing to the official images program. 💭

As you may have noticed, we've usually got a pretty decently sized queue of new images (not to mention image updates and maintenance of images under @docker-library which are maintained by the core official images team). As such, it may be some time before we get to reviewing this image (image updates get priority both because users expect them and because reviewing new images is a more involved process than reviewing updates), so we apologize in advance! Please be patient with us (and avoid poking us about your image via other communication means -- rest assured, we've seen your PR and it's in the queue). ❤️

We do try to proactively add and update the "new image checklist" on each PR, so if you haven't looked at it yet, that's a good use of time while you wait. ☔

Thanks! 💖 💙 💚 ❤️

[tianon]

Thanks @docker-library-bot ❤️

A few initial notes:

1. you're aware of Archiving the project: suspending the development ariya/phantomjs#15344, right? is that going to be an issue?

2. should /var/www/app/storage also be a volume also? (should the contents of that be persisted between container restarts and/or upgrades?)

3. the code that does "clone public directory" should be above the code that copies just logo/, shouldn't it? is it right for that entire directory's contents to be overwritten if the image version is updated and the volume persisted?

4. ln -s /usr/include/x86_64-linux-gnu/gmp.h /usr/local/include/ on Alpine is probably not right -- gmp-dev installs directly to /usr/include/gmp.h, so I don't think a symlink is necessary at all

5. COPY --from=xxx is a multi-stage-ism that our tooling doesn't support (see Add multi-stage build support #3383)

6. are you sure you only want to support FPM? it's been our experience that folks have a hard time getting a web server configured to point at it appropriately, so a lot of folks prefer the (simpler) Apache-based image experience

7. the Alpine image appears to set significantly more environment variables than the Debian-based image, is that intentional?

[tianon]

8. RUN chmod +x ... following a COPY is unreliable -- I'd recommend doing chmod +x in Git instead so that the file already has +x when it gets COPY

[lalop]

Thanks @docker-library-bot ;)
and Thank you @tianon for your review

Some answers :

1. you're aware of Archiving the project: suspending the development ariya/phantomjs#15344, right? is that going to be an issue?

Yes we are aware, an issue is open on the project repository invoiceninja/invoiceninja#1934 and we are looking for some alternative.
This will be change before that become a real issue.

2. should /var/www/app/storage also be a volume also? (should the contents of that be persisted between container restarts and/or upgrades?)

The only directory in storage that should be keep between version is /var/www/app/storage/documents

3. the code that does "clone public directory" should be above the code that copies just logo/, shouldn't it? is it right for that entire directory's contents to be overwritten if the image version is updated and the volume persisted?

Logo uploaded in the application are stored in pulbic/logo so we recommand to mount this folder from a volume. Since the application comes with it own logo in the same directory I want to init the folder with the application's logo backuped during the build to be sure to found the logo in it.

In a second time, if the public folder is mounted And the application is updated I need to update the public directory to the new version for assets mainly.

I don't see any issue with that actually but I can be wrong...

4. ln -s /usr/include/x86_64-linux-gnu/gmp.h /usr/local/include/ on Alpine is probably not right -- gmp-dev installs directly to /usr/include/gmp.h, so I don't think a symlink is necessary at all

It's appear yes, I removed it

5. COPY --from=xxx is a multi-stage-ism that our tooling doesn't support (see Add multi-stage build support #3383)

I updated it to avoid this command

6. are you sure you only want to support FPM? it's been our experience that folks have a hard time getting a web server configured to point at it appropriately, so a lot of folks prefer the (simpler) Apache-based image experience

An apache version should come in the next days invoiceninja/dockerfiles#79

7. the Alpine image appears to set significantly more environment variables than the Debian-based image, is that intentional?

Due to phantomjs issue with alpine I worked more on the standard version this last month.
I just updated alpine to reflect the last change on the standard version (env, copy --form)

8. RUN chmod +x ... following a COPY is unreliable -- I'd recommend doing chmod +x in Git instead so that the file already has +x when it gets COPY

... good point.
It's done

[yosifkit]

Gif to brighten the day:

Apologies for the long delay. 🙇‍♂️

Is this PR still a desirable addition? I see that the Dockerfiles are still being maintained 👍. It doesn't look like there was a conclusion with regards to phantomjs being archived (invoiceninja/invoiceninja#1934); is that going to be a problem for the images? Phantomjs doesn't work on linux alpine; does that mean the Alpine based images don't work?

If this is still something desired I'm willing to submit a PR to invoiceninja/dockerfiles with some concrete suggestions to improve Dockerization (and hopefully image size and multiple architecture support).

[lalop]

Hello @yosifkit

Glad to see this is not dead.

The internal team of invoiceninja is working on a new version of the app, this will come with a rebranding (new name, version will restart at 1.0).
So I'm not sure this make really sense anymore.

Anyway if think I can manage this rebranding in an official image this pr is still a desirable addition.

[lalop]

And to answer on the phantomjs part, the new version will go with chrome headless the current one will keep phantomjs as dependency

[tianon]

If there's a rebranding in progress, I would urge caution -- the best we can do as far as an "image rename" is concerned is deprecation of the old name, which amounts to a notice on the Docker Hub image description which many users won't notice or take note of (see for example the old java image documentation: https://github.com/docker-library/docs/blob/99fd882c2c9df8b2c3f09991c3d756ea400c3a56/java/README.md), and docker pull certainly won't pick up on or notify the user about in any way. 😞

[lalop]

I'm completely aware of this kind of limitation, so I think this pr can be closed in favour of a new one when the rebranding will be ready.
Thank you anyway