Skip to content

Conversation

@rhiaro
Copy link

@rhiaro rhiaro commented May 1, 2016

First pass Dockerfile, plus instructions in readme.

Works, but would appreciate review by @KitB for best practices/glaring holes.

@nicola, I used the acl we adapted from gold the other day, but let me know if this needs shortening or something.

@nicola
Copy link
Contributor

nicola commented May 1, 2016

Great!

To avoid having the flag admin_create we should fix #326

@@ -0,0 +1,32 @@
FROM node:5.11-wheezy
Copy link
Contributor

Choose a reason for hiding this comment

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

By removing 5.11-wheezy, I think we are guaranteed to get the latest, should we change it to that? is it good practice?

Copy link

Choose a reason for hiding this comment

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

Pinning is probably the better idea, always updating from latest is handy when developing and cuts down on "update base image to ..." commits, but pinned versions mean repeatable builds and always knowing where you're building from, in particular it means that the version of the base image is tracked in git which I consider to be important.

@nicola
Copy link
Contributor

nicola commented May 2, 2016

@rhiaro don't worry about the variable, there will be no need for that soon, we will be using --owner for that.

@rhiaro
Copy link
Author

rhiaro commented May 2, 2016

@nicola Okay so we can include the acl file with an empty space for the WebID and have the person running it fill that in by hand before building (for now)?

@rhiaro
Copy link
Author

rhiaro commented May 2, 2016

Actually if we're going to include a template acl as its own file we might as well automate the input of the WebID in the dockerfile since @KitB found a solution. Until the --owner thing is implemented at least.

Dockerfile Outdated

EXPOSE 8443

CMD ["node", "/src/bin/ldnode.js", "--port=8443", "--ssl-key=/opt/ldnode/certs/ssl-key.pem", "--ssl-cert=/opt/ldnode/certs/ssl-cert.pem"]
Copy link

Choose a reason for hiding this comment

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

consider

ENTRYPOINT ["node"]
CMD ["/src/bin/ldnode.js" ...]

@nicola nicola mentioned this pull request May 2, 2016
Dockerfile Outdated
<#owner>\n\
a n0:Authorization;\n\
n0:accessTo <./>;\n\
n0:agent ' + $admin_user + '\n\
Copy link
Contributor

@nicola nicola May 2, 2016

Choose a reason for hiding this comment

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

should this be ' ' or < > ?
is this missing ; ?

Copy link
Author

@rhiaro rhiaro May 2, 2016

Choose a reason for hiding this comment

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

Yeah it should, fixing that with my next updates (it somehow worked anyway though.. is your turtle parser lax?)

&& npm install

ENTRYPOINT ["node", "/src/bin/ldnode.js"]
CMD ["--port=8443", "--ssl-key=/opt/ldnode/certs/ssl-key.pem", "--ssl-cert=/opt/ldnode/certs/ssl-cert.pem", "--root=/src/data"] No newline at end of file

Choose a reason for hiding this comment

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

please add an EXPOSE 8443 line to let users (and docker) know that you're expecting to expose that port in the container :)

@melvincarvalho
Copy link
Contributor

Merged #607

Can revisit this again in January when letsencrypt release wildcard certificates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants