wunderio/charts

A repository for Wunder public helm charts.

charts's Issues

Nginx conf: Duplicate fastcgi_param HTTPS

In Nginx configuration https://github.com/wunderio/charts/blob/master/drupal/templates/drupal-configmap.yaml#L167
the fastcgi_param HTTPS is specified twice.
Lines 167 and 180:

167: fastcgi_param  HTTPS              $https if_not_empty;
180: fastcgi_param  HTTPS              $fastcgi_https;

https://wunder.slack.com/archives/C8UN6AG9W/p1597149736367600

Nginx: duplicate SCRIPT_NAME and SCRIPT_NAME

https://wunder.slack.com/archives/CAC0EPL1G/p1598365008002800

In fastcgi.conf:

fastcgi_param SCRIPT_NAME /index.php;

In many places I see:

include fastcgi.conf;
fastcgi_param SCRIPT_NAME /index.php;

is this useless override, or is there some hidden catch?

Ditto for SCRIPT_FILENAME.

Nginx: Invalid upstream reference

https://wunder.slack.com/archives/C8UN6AG9W/p1597845597476600

Visiting /core/modules/statistics/statistics.php yields 502 Bad Gateway.

Because:

    upstream php {
        server localhost:9000;
    }

There is nothing listening in Nginx container at port 9000.
localhost is wrong and should be replaced by app.

Solution

1. Instead use this:

    upstream php {
        server app:9000;
    }

2. Replace all occurrences of fastcgi_pass app:9000; by fastcgi_pass php;.

Add silta vhost/releasename as environment variable

Could we have the host (.Release.Name ?) as environment variable to Silta, similar to how SLT-415 added the ENVIRONMENT_DOMAIN

We could use that for the SFS API rabbitmq AMQP_HOST and RABBITMQ_DEFAULT_VHOST

Nginx: Ping configuration is not optimal

https://wunder.slack.com/archives/CAC0EPL1G/p1598360110001200?thread_ts=1598359982.000900&cid=CAC0EPL1G

Need to add:

fastcgi_param SCRIPT_NAME /_ping.php;

Because in AWS we would like to use this:

// Simplified IP range for AWS Health Check.
if ($_SERVER['SCRIPT_NAME'] == '/_ping.php') {
  $settings['trusted_host_patterns'][] = '^[\d.]+$';
}

Allow for an external database

Currently the database values are always assumed to be coming from the mariadb valuyes. But we can allow for the possibility that these are set directly in values.drupal.env.

We simply need to NOT automatically write the database variables from mariadb if mariadb is disabled:

env:
{{- if or (not (hasKey .Values.mariadb "enabled")) .Values.mariadb.enabled }}
- name: DB_USER
  value: "{{ .Values.mariadb.db.user }}"
- name: DB_NAME
  value: "{{ .Values.mariadb.db.name }}"
- name: DB_HOST
  value: {{ .Release.Name }}-mariadb
- name: DB_PASS
  valueFrom:
    secretKeyRef:
      name: {{ .Release.Name }}-mariadb
      key: mariadb-password
{{- end }}
- name: HASH_SALT
  valueFrom:
    secretKeyRef:
      name: {{ .Release.Name }}-secrets-drupal
      key: hashsalt  

Nginx: .txt handling

https://wunder.slack.com/archives/CAC0EPL1G/p1597854998007200

location ~* ^/sites/.*/files/(?:.+\.(?:txt))

vs

location ~* /sites/.+/files/.+\.txt

Nginx: Refactor the configuration

Here are refactored configurations from Trimble. Wunder configuration can be remodeled according to this.

The fastcgi.conf is sorted alphabetically to weed out duplicates, added HTTP_PROXY, etc.

The drupal.conf is completely refactored according to the precedence of the location blocks, otherwise it is tough to understand which rules apply when. This file cannot be used verbatim, but Wunder configuration can follow the suite. In some places, some blocks are missing which are not needed in the Trimble context. In some places, useless fastcgi_params are removed. Etc.

drupal.conf.txt
fastcgi.conf.txt
nginx.conf.txt

Nginx: Conflicting location blocks for update.php

https://wunder.slack.com/archives/CAC0EPL1G/p1597851884005800

location ~* ^/update.php {
    include fastcgi.conf;
    fastcgi_param QUERY_STRING $args;
    fastcgi_param SCRIPT_NAME /update.php;
    fastcgi_param SCRIPT_FILENAME $document_root/update.php;
    fastcgi_pass php;
  }

  location ~ ^/(update.php|core/update.php) {
    return 404;
  }

GDPR Dump may generate real user data

GDPR dump is generating email addresses that actually may be in use.

For security reasons I suggest to specify a template for using commonly used "/dev/null" email addresses ending with @example.com.

This way we avoid a problem if we accidentally send mass-email to "random" persons while using real domains such as @gmail.com or @yahoo.com email addresses.

PHP-FPM: Why dev lib?

https://wunder.slack.com/archives/CAC0EPL1G/p1598536714009100

https://github.com/wunderio/drupal-php-fpm/blob/master/Dockerfile#L11

What is the purpose of imagemagick-dev?

Nginx: handling of /system/files

https://wunder.slack.com/archives/CAC0EPL1G/p1597855656007600

location ~* /system/files/

vs

location ~ /system/files/

Introduce separate envvars for SMTP host & port instead of SMTP_ADDRESS

Let's introduce separate envvars for SMTP host & port instead of SMTP_ADDRESS:

charts/drupal/tests/drupal_deployment_test.yaml

Line 89 in c3b3a3b

value: RELEASE-NAME-mailhog:1025

Consider this scenario:

/**
 * Default SMTP settings.
 *
 * Mailhog address in Silta SMTP_ADDRESS env-variable is
 * in form 'hostname:port'. We need them separately.
 */
if (getenv('SILTA_CLUSTER')) {
  $smtp_address_parts = explode(':', getenv('SMTP_ADDRESS'));
  if (!empty($smtp_address_parts[0]) && !empty($smtp_address_parts[1])) {
    $config['smtp.settings']['smtp_host'] = $smtp_address_parts[0];
    $config['smtp.settings']['smtp_port'] = $smtp_address_parts[1];
  }
}

DX: Simple chart README.md is confusing

By reading https://github.com/wunderio/charts/tree/master/simple README.md I didn't undestand why this is called simple directory while having Gatsby Helm Chart as a title of README.md.

The description says that it is used in combination with CI, but it doesn't explain why we need this chart in the first place.

Suggestions for DX:

• Make titles and directory name consistent to avoid confusion
• If mentioning deriving from drupal-project, explain why or avoid giving information that generates more questions
• Explain why this chart is needed, explain the problem first

DX: Repository README.md is quite minimal

There's not much information shared about this repository in README.md file.

Suggestions for the contents:

• Why this repository exists and who is it for?
• List of charts, and links to their README.md files
• Maybe a diagram that also shows the dependencies?

Nginx: Duplicate inclusion

https://wunder.slack.com/archives/CAC0EPL1G/p1598365362004100

server {
  include fastcgi.conf;
  location ... {
    include fastcgi.conf;
  }
}

In my experimentation, it seemed that the outer include didn't work, the variables were not available in the inner scope.

Nginx: httpoxy vulnerable

https://wunder.slack.com/archives/CAC0EPL1G/p1598441452006300?thread_ts=1598441403.006200&cid=CAC0EPL1G

I see:

# Mitigate HTTPoxy
# https://httpoxy.org/
fastcgi_param HTTP_PROXY '';

at the top level of the drupal conf.
Should it not be in fastcgi.conf?

At least I'm not able to create any fastcgi_param at the top level that is passed down to Php-fpm.

Nginx: Handling Php scripts

https://wunder.slack.com/archives/CAC0EPL1G/p1597852312006400?thread_ts=1597852274.006300&cid=CAC0EPL1G

        ## Disallow these update scripts as they are not used in current workflow, Drupal 7/8.
        location ~ ^/(update.php|core/update.php) {
            return 404;
        }
        ## Disallow these install cripts as they are not used in current workflow, Drupal 7/8.
        location ~ ^/(install.php|core/install.php) {
            return 404;
        }

It seems unnecessary, because of this:

        ## Any other attempt to access PHP files returns a 404.
        location ~* ^.+\.php$ {
            return 404;
        }

In case of the latter, the RX can be made faster:

location ~* \.php$ {

PHP FPM: Dockerfile: Irrelevant --virtual in apk add

https://wunder.slack.com/archives/CAC0EPL1G/p1598536515008500

https://github.com/wunderio/drupal-php-fpm/blob/master/Dockerfile#L17

    && apk add --no-cache --virtual .imagick-runtime-deps imagemagick libmemcached

why --virtual .imagick-runtime-deps is used, if it is not referenced later?

Multiline postinstall command for frontend chart

I'm using the frontend chart and have seen errors when I've added a multiline command. Should it be possible? In that case I suppose this line:

https://github.com/wunderio/charts/blob/master/frontend/templates/services-deployment.yaml#L174
{ $service.postinstall.command }}

should be changed to something like
{{- toYaml $service.postinstall.command | nindent 10 }}?

DX: Reference to private repository

In https://github.com/wunderio/charts/tree/master/silta-cluster there's a description:

This helm chart helps setting up resources for https://github.com/wunderio/silta-cluster

The repository link is private.

As wunderio/charts is public repository, it might be a good idea to refer/support repositories that are only public. If this chart is not needed otherwise than with wunderio/silta-cluster, then maybe this chart is not in the right repository?

Nginx: There are double definition of @empty

https://wunder.slack.com/archives/CAC0EPL1G/p1597851738004800

https://github.com/wunderio/charts/blob/master/drupal/templates/drupal-configmap.yaml
Lines: 541 and 594.

        location @empty {