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

Adds known #1763

Merged
merged 1 commit into from
Dec 7, 2016
Merged

Adds known #1763

merged 1 commit into from
Dec 7, 2016

Conversation

pierreozoux
Copy link
Contributor

@pierreozoux pierreozoux commented May 22, 2016

The documentation is here: docker-library/docs#585

Checklist for Review

  • 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)
  • 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?)
    • FROM php
  • 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)

@pierreozoux
Copy link
Contributor Author

Any news on that?

@pierreozoux
Copy link
Contributor Author

What is missing to get this going? Can I tick the checkbox?

@tianon
Copy link
Member

tianon commented Jul 19, 2016

Sorry for the delay!

A few comments/questions on the Dockerization:

  • curl should include the -f/--fail flag (to ensure failure to download stops the build properly)
  • gpg --verify known.zip.sig should be gpg --batch --verify known.zip.sig known.zip (see Fix suggested "gpg" usage to stop relying on deprecated and insecure behavior #1420 (comment) for more info)
  • grabbing https://github.com/idno/Markdown's source ought to be a specific commit rather than relying on master to stay stable and supported regardless of what the latest version of Known is
  • in docker-entrypoint.sh, DB_CONNECTABLE=$(mysql -uroot ... >/dev/null 2>&1; echo "$?"); if [[ DB_CONNECTABLE -eq 0 ]]; then would be more reliable as just if mysql -uroot ... &> /dev/null; then (using a separate variable to track "failure to connect", or even just spinning up the application regardless?)
  • is the entirety of config.ini represented in that entrypoint? is hard-coding root as the DB user and db as the dbhost in the best interest of the users of the known image? would it be possible to instead supply most of this as a "default" configuration and only modify bits of it as necessary from the entrypoint, thus allowing the user to supply their own? (or simpler yet, only creating that if one does not already exist?)

@pierreozoux pierreozoux force-pushed the known branch 2 times, most recently from bf76501 to 0a6327d Compare August 10, 2016 11:01
@pierreozoux
Copy link
Contributor Author

(sorry for the delay)

  • curl is fixed
  • gpg is fixed
  • removed the plugins
  • I changed the logic for DB_CONNECTABLE is it what you expected?
  • I copied WP logic for the env variables from the DB. (and updated doc about it)

Let me know if you see anything else?

@pierreozoux
Copy link
Contributor Author

Do you need anymore comments on my side to move this forward?

@tianon
Copy link
Member

tianon commented Nov 23, 2016

Ok, Dockerfile looks good now, but I think the ENTRYPOINT script needs a little guarding against unexpected commands (similar to wordpress):

Build test of #1763; 0a6327d (known):

$ bashbrew build known:0.9.2
warning: insecure protocol git:// detected: git://github.com/idno/Known-Docker
Building bashbrew/cache:d2bf897ca47f6b71b5b1051e6d797674967d56b47f6c20631e300418e839c261 (known:0.9.2)
Tagging known:0.9.2
Tagging known:0.9
Tagging known:0
Tagging known:latest

$ test/run.sh known:0.9.2
testing known:0.9.2
	'utc' [1/4]...passed
	'cve-2014--shellshock' [2/4]...passed
	'no-hard-coded-passwords' [3/4]...passed
	'override-cmd' [4/4]...error: missing required KNOWN_DB_PASSWORD environment variable
  Did you forget to -e KNOWN_DB_PASSWORD=... ?

  (Also of interest might be KNOWN_DB_USER and KNOWN_DB_NAME.)
failed

The following images failed at least one test: known:0.9.2

Also, it looks like the script still replaces config.ini completely rather than adding/modifying in-place (and it does appear that upstream supports other configuration options besides those represented in the configuration that's hard-coded there). Would it be possible to instead supply most of this as a "default" configuration in the Dockerfile and only modify bits of it as necessary from the entrypoint, thus allowing the user to supply their own? (or even simpler yet, only creating that if one does not already exist provided by the user via bind-mount or COPY?)

@pierreozoux
Copy link
Contributor Author

Ok thansk for your suggestion, I updated things.

I just run ./bashbrew/bashbrew.sh build known:0.9.2 without error, so I guess this is promising ;)

@tianon
Copy link
Member

tianon commented Nov 30, 2016

Thanks! 👍

This is looking good:

error: failed fetching repo "known"
unable to find a manifest named "known" (in "/tmp/tmp.r5ItHLvzav/oi/library" or as a remote URL)
fatal: pathspec '.' did not match any files
warning: insecure protocol git:// detected: git://github.com/idno/Known-Docker
diff --git a/known_latest/Dockerfile b/known_latest/Dockerfile
new file mode 100644
index 0000000..0ba370b
--- /dev/null
+++ b/known_latest/Dockerfile
@@ -0,0 +1,51 @@
+FROM php:5.6-fpm
+
+MAINTAINER [email protected]
+
+RUN apt-get update && apt-get install -y \
+      bzip2 \
+      libcurl4-openssl-dev \
+      libfreetype6-dev \
+      libicu-dev \
+      libjpeg-dev \
+      libmcrypt-dev \
+      libpng12-dev \
+      libpq-dev \
+      libxml2-dev \
+      mysql-client \
+      unzip \
+ && rm -rf /var/lib/apt/lists/*
+
+#gpg key from [email protected]
+RUN gpg --keyserver ha.pool.sks-keyservers.net --recv-keys "53DE 5B99 2244 9132 8B92  7516 052D B5AC 742E 3B47"
+
+RUN docker-php-ext-configure gd --with-png-dir=/usr --with-jpeg-dir=/usr \
+ && docker-php-ext-install exif gd intl mbstring mcrypt mysql opcache pdo_mysql zip json xmlrpc
+
+# set recommended PHP.ini settings
+# see https://secure.php.net/manual/en/opcache.installation.php
+RUN { \
+  echo 'opcache.memory_consumption=128'; \
+  echo 'opcache.interned_strings_buffer=8'; \
+  echo 'opcache.max_accelerated_files=4000'; \
+  echo 'opcache.revalidate_freq=60'; \
+  echo 'opcache.fast_shutdown=1'; \
+  echo 'opcache.enable_cli=1'; \
+} > /usr/local/etc/php/conf.d/opcache-recommended.ini
+
+# PECL extensions
+RUN pecl install APCu-4.0.10 \
+ && docker-php-ext-enable apcu
+
+ENV KNOWN_VERSION 0.9.2
+VOLUME /var/www/html
+
+RUN curl -o known.zip -fSL http://assets.withknown.com/releases/known-${KNOWN_VERSION}.zip \
+ && curl -o known.zip.sig -fSL http://assets.withknown.com/releases/known-${KNOWN_VERSION}.zip.sig \
+ && gpg --batch --verify known.zip.sig known.zip \
+ && unzip known.zip -d /usr/src/known/ \
+ && rm known.zip*
+
+COPY docker-entrypoint.sh /entrypoint.sh
+ENTRYPOINT ["/entrypoint.sh"]
+CMD ["php-fpm"]
diff --git a/known_latest/docker-entrypoint.sh b/known_latest/docker-entrypoint.sh
new file mode 100755
index 0000000..60617a1
--- /dev/null
+++ b/known_latest/docker-entrypoint.sh
@@ -0,0 +1,84 @@
+#!/bin/bash
+set -e
+
+if [ "$1" == php-fpm ]; then
+  : "${KNOWN_DB_HOST:=db}"
+  # if we're linked to MySQL and thus have credentials already, let's use them
+  : ${KNOWN_DB_USER:=${DB_ENV_MYSQL_USER:-root}}
+  if [ "$KNOWN_DB_USER" = 'root' ]; then
+    : ${KNOWN_DB_PASSWORD:=$DB_ENV_MYSQL_ROOT_PASSWORD}
+  fi
+  : ${KNOWN_DB_PASSWORD:=$DB_ENV_MYSQL_PASSWORD}
+  : ${KNOWN_DB_NAME:=${DB_ENV_MYSQL_DATABASE:-known}}
+
+  if [ -z "$KNOWN_DB_PASSWORD" ]; then
+    echo >&2 'error: missing required KNOWN_DB_PASSWORD environment variable'
+    echo >&2 '  Did you forget to -e KNOWN_DB_PASSWORD=... ?'
+    echo >&2
+    echo >&2 '  (Also of interest might be KNOWN_DB_USER and KNOWN_DB_NAME.)'
+    exit 1
+  fi
+
+  if [ ! -e '/var/www/html/version.php' ]; then
+    tar cf - --one-file-system -C /usr/src/known . | tar xf -
+    chown -R root:www-data /var/www/html
+    chmod -R 650 /var/www/html
+    chmod -R 770 /var/www/html/Uploads
+  fi
+
+  DB_CONNECTABLE=0
+  echo -n 'Connecting to database'
+  for ((i=0;i<10;i++))
+  do
+    if mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e 'status' &> /dev/null; then
+      DB_CONNECTABLE=1
+      echo 'Ok'
+      break
+    fi
+    echo -n "."
+    sleep 5
+  done
+
+  if [[ $DB_CONNECTABLE -eq 1 ]]; then
+    DB_EXISTS=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e "SHOW DATABASES LIKE '"${KNOWN_DB_NAME}"';" 2>&1 | grep ${KNOWN_DB_NAME} > /dev/null ; echo "$?")
+
+    if [[ DB_EXISTS -eq 1 ]]; then
+      echo "=> Creating database ${KNOWN_DB_NAME}"
+      RET=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e "CREATE DATABASE ${KNOWN_DB_NAME}")
+      if [[ RET -ne 0 ]]; then
+        echo >&2 'Cannot create database.'
+        exit $RET
+      fi
+      echo "=> Loading initial database data to ${KNOWN_DB_NAME}"
+      RET=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} ${KNOWN_DB_NAME} < /var/www/html/schemas/mysql/mysql.sql)
+      if [[ RET -ne 0 ]]; then
+        echo >&2 'Cannot load initial database data for known'
+        exit $RET
+      fi
+      echo '=> Done!'
+    else
+      echo '=> Skipped creation of database for known, it already exists.'
+    fi
+  else
+    echo >&2 'Cannot connect to Mysql. Starting anyway...'
+  fi
+
+  if [ ! -e '/var/www/html/config.ini' ]; then
+    # Environment creation
+    # http://docs.withknown.com/en/latest/install/config/
+    echo "filesystem = 'local'"         > /var/www/html/config.ini
+    echo "uploadpath = '/var/www/html/Uploads'" >> /var/www/html/config.ini
+    echo "database = 'MySQL'"          >> /var/www/html/config.ini
+    echo "dbname = '${KNOWN_DB_NAME}'"       >> /var/www/html/config.ini
+    echo "dbhost = '${KNOWN_DB_HOST}'"       >> /var/www/html/config.ini
+    echo "dbuser = '${KNOWN_DB_USER}'"       >> /var/www/html/config.ini
+    echo "dbpass = '${KNOWN_DB_PASSWORD}'"       >> /var/www/html/config.ini
+    echo "smtp_host = ${MAIL_HOST}"    >> /var/www/html/config.ini
+    echo "smtp_port = ${MAIL_PORT}"    >> /var/www/html/config.ini
+    echo "smtp_username = ${MAIL_USER}" >> /var/www/html/config.ini
+    echo "smtp_password = ${MAIL_PASS}" >> /var/www/html/config.ini
+    echo "smtp_secure = ${MAIL_SECURE}" >> /var/www/html/config.ini
+  fi
+fi
+
+exec "$@"

@tianon
Copy link
Member

tianon commented Nov 30, 2016

Going to check out the docs now!

@pierreozoux
Copy link
Contributor Author

I'm not sure I understand what you are saying?
Where is the error, and what can I do to solve?

Thanks to check this out ;)

@tianon
Copy link
Member

tianon commented Nov 30, 2016 via email

@yosifkit
Copy link
Member

yosifkit commented Dec 2, 2016

I just have a question about these lines (inside the if [[ DB_EXISTS -eq 1 ]]; then block):

      echo "=> Loading initial database data to ${KNOWN_DB_NAME}"
      RET=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} ${KNOWN_DB_NAME} < /var/www/html/schemas/mysql/mysql.sql)
      if [[ RET -ne 0 ]]; then
        echo >&2 'Cannot load initial database data for known'
        exit $RET
      fi
      echo '=> Done!'

Is the schemas/mysql/mysql.sql idempotent? As far as I can see, this schema would not be loaded if a user created their database with MYSQL_DATABASE on their mysql container. Would that cause a failure?

We also noticed some of the variables in the if statements are missing their $. 😉

@pierreozoux
Copy link
Contributor Author

You are really good! Thanks @yosifkit it is now fixed (I tested it, it wouldn't fail if there is already a database).

@yosifkit
Copy link
Member

yosifkit commented Dec 7, 2016

New diff!

diff --git a/known_latest/Dockerfile b/known_latest/Dockerfile
new file mode 100644
index 0000000..0ba370b
--- /dev/null
+++ b/known_latest/Dockerfile
@@ -0,0 +1,51 @@
+FROM php:5.6-fpm
+
+MAINTAINER [email protected]
+
+RUN apt-get update && apt-get install -y \
+      bzip2 \
+      libcurl4-openssl-dev \
+      libfreetype6-dev \
+      libicu-dev \
+      libjpeg-dev \
+      libmcrypt-dev \
+      libpng12-dev \
+      libpq-dev \
+      libxml2-dev \
+      mysql-client \
+      unzip \
+ && rm -rf /var/lib/apt/lists/*
+
+#gpg key from [email protected]
+RUN gpg --keyserver ha.pool.sks-keyservers.net --recv-keys "53DE 5B99 2244 9132 8B92  7516 052D B5AC 742E 3B47"
+
+RUN docker-php-ext-configure gd --with-png-dir=/usr --with-jpeg-dir=/usr \
+ && docker-php-ext-install exif gd intl mbstring mcrypt mysql opcache pdo_mysql zip json xmlrpc
+
+# set recommended PHP.ini settings
+# see https://secure.php.net/manual/en/opcache.installation.php
+RUN { \
+  echo 'opcache.memory_consumption=128'; \
+  echo 'opcache.interned_strings_buffer=8'; \
+  echo 'opcache.max_accelerated_files=4000'; \
+  echo 'opcache.revalidate_freq=60'; \
+  echo 'opcache.fast_shutdown=1'; \
+  echo 'opcache.enable_cli=1'; \
+} > /usr/local/etc/php/conf.d/opcache-recommended.ini
+
+# PECL extensions
+RUN pecl install APCu-4.0.10 \
+ && docker-php-ext-enable apcu
+
+ENV KNOWN_VERSION 0.9.2
+VOLUME /var/www/html
+
+RUN curl -o known.zip -fSL http://assets.withknown.com/releases/known-${KNOWN_VERSION}.zip \
+ && curl -o known.zip.sig -fSL http://assets.withknown.com/releases/known-${KNOWN_VERSION}.zip.sig \
+ && gpg --batch --verify known.zip.sig known.zip \
+ && unzip known.zip -d /usr/src/known/ \
+ && rm known.zip*
+
+COPY docker-entrypoint.sh /entrypoint.sh
+ENTRYPOINT ["/entrypoint.sh"]
+CMD ["php-fpm"]
diff --git a/known_latest/docker-entrypoint.sh b/known_latest/docker-entrypoint.sh
new file mode 100755
index 0000000..71733d8
--- /dev/null
+++ b/known_latest/docker-entrypoint.sh
@@ -0,0 +1,88 @@
+#!/bin/bash
+set -e
+
+if [ "$1" == php-fpm ]; then
+  : "${KNOWN_DB_HOST:=db}"
+  # if we're linked to MySQL and thus have credentials already, let's use them
+  : ${KNOWN_DB_USER:=${DB_ENV_MYSQL_USER:-root}}
+  if [ "$KNOWN_DB_USER" = 'root' ]; then
+    : ${KNOWN_DB_PASSWORD:=$DB_ENV_MYSQL_ROOT_PASSWORD}
+  fi
+  : ${KNOWN_DB_PASSWORD:=$DB_ENV_MYSQL_PASSWORD}
+  : ${KNOWN_DB_NAME:=${DB_ENV_MYSQL_DATABASE:-known}}
+
+  if [ -z "$KNOWN_DB_PASSWORD" ]; then
+    echo >&2 'error: missing required KNOWN_DB_PASSWORD environment variable'
+    echo >&2 '  Did you forget to -e KNOWN_DB_PASSWORD=... ?'
+    echo >&2
+    echo >&2 '  (Also of interest might be KNOWN_DB_USER and KNOWN_DB_NAME.)'
+    exit 1
+  fi
+
+  if [ ! -e '/var/www/html/version.php' ]; then
+    tar cf - --one-file-system -C /usr/src/known . | tar xf -
+    chown -R root:www-data /var/www/html
+    chmod -R 650 /var/www/html
+    chmod -R 770 /var/www/html/Uploads
+  fi
+
+  DB_CONNECTABLE=0
+  echo -n 'Connecting to database'
+  for ((i=0;i<10;i++))
+  do
+    if mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e 'status' &> /dev/null; then
+      DB_CONNECTABLE=1
+      echo 'Ok'
+      break
+    fi
+    echo -n "."
+    sleep 5
+  done
+
+  if [[ $DB_CONNECTABLE -eq 1 ]]; then
+    DB_EXISTS=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e "SHOW DATABASES LIKE '"${KNOWN_DB_NAME}"';" 2>&1 | grep ${KNOWN_DB_NAME} > /dev/null ; echo "$?")
+
+    if [[ $DB_EXISTS -eq 1 ]]; then
+      echo "=> Creating database ${KNOWN_DB_NAME}"
+      RET=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e "CREATE DATABASE ${KNOWN_DB_NAME}")
+      if [[ $RET -ne 0 ]]; then
+        echo >&2 'Cannot create database.'
+        exit $RET
+      fi
+    else
+      echo '=> Skipped creation of database for known, it already exists.'
+    fi
+    DB_INITIATED=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} -e "USE '"${KNOWN_DB_NAME}"';SHOW TABLES;" 2>&1 | grep config > /dev/null ; echo "$?")
+
+    if [[ $DB_INITIATED -eq 1 ]]; then
+      echo "=> Loading initial database data to ${KNOWN_DB_NAME}"
+      RET=$(mysql -u${KNOWN_DB_USER} -p${KNOWN_DB_PASSWORD} -h${KNOWN_DB_HOST} ${KNOWN_DB_NAME} < /var/www/html/schemas/mysql/mysql.sql)
+      if [[ $RET -ne 0 ]]; then
+        echo >&2 'Cannot load initial database data for known'
+        exit $RET
+      fi
+    fi
+    echo '=> Done!'
+  else
+    echo >&2 'Cannot connect to Mysql. Starting anyway...'
+  fi
+
+  if [ ! -e '/var/www/html/config.ini' ]; then
+    # Environment creation
+    # http://docs.withknown.com/en/latest/install/config/
+    echo "filesystem = 'local'"         > /var/www/html/config.ini
+    echo "uploadpath = '/var/www/html/Uploads'" >> /var/www/html/config.ini
+    echo "database = 'MySQL'"          >> /var/www/html/config.ini
+    echo "dbname = '${KNOWN_DB_NAME}'"       >> /var/www/html/config.ini
+    echo "dbhost = '${KNOWN_DB_HOST}'"       >> /var/www/html/config.ini
+    echo "dbuser = '${KNOWN_DB_USER}'"       >> /var/www/html/config.ini
+    echo "dbpass = '${KNOWN_DB_PASSWORD}'"       >> /var/www/html/config.ini
+    echo "smtp_host = ${MAIL_HOST}"    >> /var/www/html/config.ini
+    echo "smtp_port = ${MAIL_PORT}"    >> /var/www/html/config.ini
+    echo "smtp_username = ${MAIL_USER}" >> /var/www/html/config.ini
+    echo "smtp_password = ${MAIL_PASS}" >> /var/www/html/config.ini
+    echo "smtp_secure = ${MAIL_SECURE}" >> /var/www/html/config.ini
+  fi
+fi
+
+exec "$@"

0.9.2: git://github.com/idno/Known-Docker@986a2618080f32bbbcb9af3c8e7c15297d9658ea
0.9: git://github.com/idno/Known-Docker@986a2618080f32bbbcb9af3c8e7c15297d9658ea
0: git://github.com/idno/Known-Docker@986a2618080f32bbbcb9af3c8e7c15297d9658ea
latest: git://github.com/idno/Known-Docker@986a2618080f32bbbcb9af3c8e7c15297d9658ea
Copy link
Member

Choose a reason for hiding this comment

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

I'd like this to be updated to the newer RFC 2822 based format but this PR was made before that and it still works. @tianon will probably update it like rocket.chat and piwik anyway.

@yosifkit
Copy link
Member

yosifkit commented Dec 7, 2016

Build test of #1763; 2f46a2b (known):

$ bashbrew build known:0.9.2
Building bashbrew/cache:cdc4128ea22b87c10b9e44eec408d2c8a066caddfd97df79a4c2e49c394c6291 (known:0.9.2)
Tagging known:0.9.2
Tagging known:0.9
Tagging known:0
Tagging known:latest

$ test/run.sh known:0.9.2
testing known:0.9.2
	'utc' [1/4]...passed
	'cve-2014--shellshock' [2/4]...passed
	'no-hard-coded-passwords' [3/4]...passed
	'override-cmd' [4/4]...passed

@yosifkit
Copy link
Member

yosifkit commented Dec 7, 2016

Checking docs in the morning and we'll be good to go!

@yosifkit yosifkit merged commit b3768ac into docker-library:master Dec 7, 2016
@pierreozoux
Copy link
Contributor Author

Amazing :)

Thanks a lot @yosifkit and @tianon :)

Have a wonderful day!

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.

4 participants