From a1069db985846f2edb22e8038037bdd5cdeb4588 Mon Sep 17 00:00:00 2001
From: Cristhian Garcia <crisgarta8@gmail.com>
Date: Fri, 26 Jan 2024 15:30:45 -0500
Subject: [PATCH 1/3] refactor: use patch for clickhouse settings

---
 .../clickhouse-named-collection-config        | 30 ++++++++++++
 tutoraspects/patches/clickhouse-server-config | 31 ++++++++++++
 tutoraspects/patches/clickhouse-user-config   | 18 +++++++
 .../patches/kustomization-configmapgenerator  |  2 +
 tutoraspects/plugin.py                        | 47 -------------------
 .../apps/clickhouse/config/docker_config.xml  |  2 +-
 .../clickhouse/config/named_collections.xml   |  5 ++
 .../apps/clickhouse/users/user_config.xml     |  2 +-
 .../jobs/init/clickhouse/init-clickhouse.sh   |  2 +-
 9 files changed, 89 insertions(+), 50 deletions(-)
 create mode 100644 tutoraspects/patches/clickhouse-named-collection-config
 create mode 100644 tutoraspects/patches/clickhouse-server-config
 create mode 100644 tutoraspects/patches/clickhouse-user-config
 create mode 100644 tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml

diff --git a/tutoraspects/patches/clickhouse-named-collection-config b/tutoraspects/patches/clickhouse-named-collection-config
new file mode 100644
index 000000000..1b38c94a8
--- /dev/null
+++ b/tutoraspects/patches/clickhouse-named-collection-config
@@ -0,0 +1,30 @@
+<!--
+This file includes named collections required for Aspects dictionaries to work.
+This will be read from a mount on
+/etc/clickhouse-server/config.d/named_collections.xml
+on startup. Named collections allow configuration to talk to external
+or internal services using stored credentials. We use them to create
+dictionaries on local queries, but credentials are still required.
+By placing them in named collections we can change the ClickHouse
+admin password without having to disrupt the dictionary caching.
+https://clickhouse.com/docs/en/operations/named-collections
+
+If you are creating your own dictionaries that reference other databases, you
+may need to add more named collections here first.
+-->
+<local_ch_xapi>
+    <host overridable="false">localhost</host>
+    <port overridable="false">{{ CLICKHOUSE_INTERNAL_NATIVE_PORT }}</port>
+    <database overridable="false">{{ ASPECTS_XAPI_DATABASE }}</database>
+    <user overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_USER }}</user>
+    <password overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_PASSWORD }}</password>
+    <secure overridable="false">{% if CLICKHOUSE_SECURE_CONNECTION %}1{% else %}0{% endif %}</secure>
+</local_ch_xapi>
+<local_ch_event_sink>
+    <host overridable="false">localhost</host>
+    <port overridable="false">{{ CLICKHOUSE_INTERNAL_NATIVE_PORT }}</port>
+    <database overridable="false">{{ ASPECTS_EVENT_SINK_DATABASE }}</database>
+    <user overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_USER }}</user>
+    <password overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_PASSWORD }}</password>
+    <secure overridable="false">{% if CLICKHOUSE_SECURE_CONNECTION %}1{% else %}0{% endif %}</secure>
+</local_ch_event_sink>
diff --git a/tutoraspects/patches/clickhouse-server-config b/tutoraspects/patches/clickhouse-server-config
new file mode 100644
index 000000000..7ec0b8ab3
--- /dev/null
+++ b/tutoraspects/patches/clickhouse-server-config
@@ -0,0 +1,31 @@
+<!--
+These are miscellaneous ClickHouse server settings, which get mounted to
+/etc/clickhouse-server/config.d/docker_config.xml
+
+See https://clickhouse.com/docs/en/operations/configuration-files for details.
+
+These default settings allow connecting to Clickhouse when run as a
+standalone docker container, instead of through docker-compose.
+-->
+
+<!--
+Port for HTTP API. See also 'https_port' for secure connections.
+This interface is also used by ODBC and JDBC drivers (DataGrip, Dbeaver, ...)
+and by most of web interfaces (embedded UI, Grafana, Redash, ...).
+-->
+<http_port>{{CLICKHOUSE_INTERNAL_HTTP_PORT}}</http_port>
+
+<!--
+Port for interaction by native protocol with:
+ - clickhouse-client and other native ClickHouse tools (clickhouse-benchmark,
+   clickhouse-copier);
+ - clickhouse-server with other clickhouse-servers for distributed query processing;
+ - ClickHouse drivers and applications supporting native protocol
+ (this protocol is also informally called as "the TCP protocol");
+See also 'tcp_port_secure' for secure connections.
+-->
+<tcp_port>{{CLICKHOUSE_INTERNAL_NATIVE_PORT}}</tcp_port>
+
+<listen_host>::</listen_host>
+<listen_host>0.0.0.0</listen_host>
+<listen_try>1</listen_try>
diff --git a/tutoraspects/patches/clickhouse-user-config b/tutoraspects/patches/clickhouse-user-config
new file mode 100644
index 000000000..535c8cbc1
--- /dev/null
+++ b/tutoraspects/patches/clickhouse-user-config
@@ -0,0 +1,18 @@
+<!--
+Override configuration in users.xml. ClickHouse will read this from a mount on
+/etc/clickhouse-server/users.d/ on startup.
+
+The http settings revert back to the value from versions pre-23.6,
+when the default was changed from 1Mb to 128Kb. This allows larger POSTs such
+as the event sink sends from the LMS.
+
+NOTE: Currently this patch does not impact Kubernetes deployments. ClickHouse
+writes to the user directory that we want to override causing errors if we try
+to mount this file there:
+-->
+<profiles>
+    <default>
+        <http_max_field_value_size>1048576</http_max_field_value_size>
+        <http_max_field_name_size>1048576</http_max_field_name_size>
+    </default>
+</profiles>
diff --git a/tutoraspects/patches/kustomization-configmapgenerator b/tutoraspects/patches/kustomization-configmapgenerator
index 5272ae682..e1a962f2a 100644
--- a/tutoraspects/patches/kustomization-configmapgenerator
+++ b/tutoraspects/patches/kustomization-configmapgenerator
@@ -37,10 +37,12 @@
 - name: clickhouse-settings
   files:
     - plugins/aspects/apps/clickhouse/config/docker_config.xml
+    - plugins/aspects/apps/clickhouse/config/named_collections.xml
     - plugins/aspects/apps/clickhouse/users/user_config.xml
   options:
     labels:
         app.kubernetes.io/name: clickhouse
+
 {% endif %}
 
 {% if RUN_RALPH %}
diff --git a/tutoraspects/plugin.py b/tutoraspects/plugin.py
index 0b7b5ee73..675c754c4 100644
--- a/tutoraspects/plugin.py
+++ b/tutoraspects/plugin.py
@@ -177,53 +177,6 @@
             "{% endif %}",
         ),
         ("CLICKHOUSE_K8S_VOLUME_SIZE", "10Gi"),
-        # This can be used to override some configuration values in
-        # via "docker_config.xml" file, which will be read from a
-        # mount on /etc/clickhouse-server/config.d/ on startup.
-        # See https://clickhouse.com/docs/en/operations/configuration-files
-        #
-        # This default allows connecting to Clickhouse when run as a
-        # standalone docker container, instead of through docker-compose.
-        (
-            "CLICKHOUSE_EXTRA_XML_CONFIG",
-            """
-    <!-- Port for HTTP API. See also 'https_port' for secure connections.
-         This interface is also used by ODBC and JDBC drivers (DataGrip, Dbeaver, ...)
-         and by most of web interfaces (embedded UI, Grafana, Redash, ...).
-    -->
-    <http_port>{{CLICKHOUSE_INTERNAL_HTTP_PORT}}</http_port>
-
-    <!-- Port for interaction by native protocol with:
-         - clickhouse-client and other native ClickHouse tools (clickhouse-benchmark,
-           clickhouse-copier);
-         - clickhouse-server with other clickhouse-servers for distributed query processing;
-         - ClickHouse drivers and applications supporting native protocol
-         (this protocol is also informally called as "the TCP protocol");
-         See also 'tcp_port_secure' for secure connections.
-    -->
-    <tcp_port>{{CLICKHOUSE_INTERNAL_NATIVE_PORT}}</tcp_port>
-
-    <listen_host>::</listen_host>
-    <listen_host>0.0.0.0</listen_host>
-    <listen_try>1</listen_try>
-        """,
-        ),
-        # Override configuration in users.xml. Similar to CLICKHOUSE_EXTRA_XML_CONFIG,
-        # this will be read from a mount on /etc/clickhouse-server/users.d/
-        # on startup
-        # The http settings revert back to the value from versions pre-23.6,
-        # when the default was changed from 1Mb to 128Kb
-        (
-            "CLICKHOUSE_EXTRA_USERS_XML_CONFIG",
-            """
-    <profiles>
-        <default>
-            <http_max_field_value_size>1048576</http_max_field_value_size>
-            <http_max_field_name_size>1048576</http_max_field_name_size>
-        </default>
-    </profiles>
-            """,
-        ),
         (
             "CLICKHOUSE_URL",
             "{{CLICKHOUSE_HOST}}:{{CLICKHOUSE_INTERNAL_HTTP_PORT}}",
diff --git a/tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml b/tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml
index f7c78ffbf..5939d129e 100644
--- a/tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml
+++ b/tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml
@@ -1,3 +1,3 @@
 <clickhouse>
-    {{ CLICKHOUSE_EXTRA_XML_CONFIG }}
+    {{patch("clickhouse-server-config")}}
 </clickhouse>
diff --git a/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml b/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml
new file mode 100644
index 000000000..a6a072135
--- /dev/null
+++ b/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml
@@ -0,0 +1,5 @@
+<clickhouse>
+     <named_collections>
+         {{patch("clickhouse-named-collection-config")}}
+     </named_collections>
+</clickhouse>
diff --git a/tutoraspects/templates/aspects/apps/clickhouse/users/user_config.xml b/tutoraspects/templates/aspects/apps/clickhouse/users/user_config.xml
index 0eed7a38c..ab5975552 100644
--- a/tutoraspects/templates/aspects/apps/clickhouse/users/user_config.xml
+++ b/tutoraspects/templates/aspects/apps/clickhouse/users/user_config.xml
@@ -1,3 +1,3 @@
 <clickhouse>
-    {{ CLICKHOUSE_EXTRA_USERS_XML_CONFIG }}
+    {{patch("clickhouse-user-config")}}
 </clickhouse>
diff --git a/tutoraspects/templates/aspects/jobs/init/clickhouse/init-clickhouse.sh b/tutoraspects/templates/aspects/jobs/init/clickhouse/init-clickhouse.sh
index fc58bf662..74fdee54d 100644
--- a/tutoraspects/templates/aspects/jobs/init/clickhouse/init-clickhouse.sh
+++ b/tutoraspects/templates/aspects/jobs/init/clickhouse/init-clickhouse.sh
@@ -61,7 +61,7 @@ GRANT {{ ON_CLUSTER }} SELECT ON system.events TO '{{ ASPECTS_CLICKHOUSE_REPORT_
 GRANT {{ ON_CLUSTER }} SELECT ON system.metrics TO '{{ ASPECTS_CLICKHOUSE_REPORT_USER }}';
 GRANT {{ ON_CLUSTER }} SELECT ON system.replication_queue TO '{{ ASPECTS_CLICKHOUSE_REPORT_USER }}';
 
-
+-- Patch from clickhouse-extra-sql follows...
 {{ patch("clickhouse-extra-sql") }}
 
 EOF

From 1b572a51471ea34c2fcd688022fca975bad54684 Mon Sep 17 00:00:00 2001
From: Cristhian Garcia <crisgarta8@gmail.com>
Date: Fri, 26 Jan 2024 15:33:22 -0500
Subject: [PATCH 2/3] fix: mount clickhouse settings by individual files

---
 .../clickhouse-named-collection-config        | 30 -------------------
 tutoraspects/patches/k8s-deployments          | 15 +++++++---
 .../patches/kustomization-configmapgenerator  | 11 ++++---
 .../clickhouse/config/named_collections.xml   |  5 ----
 .../{docker_config.xml => server_config.xml}  |  0
 5 files changed, 18 insertions(+), 43 deletions(-)
 delete mode 100644 tutoraspects/patches/clickhouse-named-collection-config
 delete mode 100644 tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml
 rename tutoraspects/templates/aspects/apps/clickhouse/config/{docker_config.xml => server_config.xml} (100%)

diff --git a/tutoraspects/patches/clickhouse-named-collection-config b/tutoraspects/patches/clickhouse-named-collection-config
deleted file mode 100644
index 1b38c94a8..000000000
--- a/tutoraspects/patches/clickhouse-named-collection-config
+++ /dev/null
@@ -1,30 +0,0 @@
-<!--
-This file includes named collections required for Aspects dictionaries to work.
-This will be read from a mount on
-/etc/clickhouse-server/config.d/named_collections.xml
-on startup. Named collections allow configuration to talk to external
-or internal services using stored credentials. We use them to create
-dictionaries on local queries, but credentials are still required.
-By placing them in named collections we can change the ClickHouse
-admin password without having to disrupt the dictionary caching.
-https://clickhouse.com/docs/en/operations/named-collections
-
-If you are creating your own dictionaries that reference other databases, you
-may need to add more named collections here first.
--->
-<local_ch_xapi>
-    <host overridable="false">localhost</host>
-    <port overridable="false">{{ CLICKHOUSE_INTERNAL_NATIVE_PORT }}</port>
-    <database overridable="false">{{ ASPECTS_XAPI_DATABASE }}</database>
-    <user overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_USER }}</user>
-    <password overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_PASSWORD }}</password>
-    <secure overridable="false">{% if CLICKHOUSE_SECURE_CONNECTION %}1{% else %}0{% endif %}</secure>
-</local_ch_xapi>
-<local_ch_event_sink>
-    <host overridable="false">localhost</host>
-    <port overridable="false">{{ CLICKHOUSE_INTERNAL_NATIVE_PORT }}</port>
-    <database overridable="false">{{ ASPECTS_EVENT_SINK_DATABASE }}</database>
-    <user overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_USER }}</user>
-    <password overridable="false">{{ ASPECTS_CLICKHOUSE_REPORT_PASSWORD }}</password>
-    <secure overridable="false">{% if CLICKHOUSE_SECURE_CONNECTION %}1{% else %}0{% endif %}</secure>
-</local_ch_event_sink>
diff --git a/tutoraspects/patches/k8s-deployments b/tutoraspects/patches/k8s-deployments
index 27c101e24..0d1550cf9 100644
--- a/tutoraspects/patches/k8s-deployments
+++ b/tutoraspects/patches/k8s-deployments
@@ -36,16 +36,23 @@ spec:
           volumeMounts:
             - mountPath: /var/lib/clickhouse/
               name: data
-            - mountPath: /etc/clickhouse-server/config.d/
-              name: settings
+            - mountPath: /etc/clickhouse-server/config.d/server_config.xml
+              name: server-settings
+              subPath: server_config.xml
+            - mountPath: /etc/clickhouse-server/users.d/user_config.xml
+              name: user-settings
+              subPath: user_config.xml
       restartPolicy: Always
       volumes:
         - name: data
           persistentVolumeClaim:
             claimName: clickhouse
-        - name: settings
+        - name: user-settings
           configMap:
-            name: clickhouse-settings
+            name: clickhouse-user-settings
+        - name: server-settings
+          configMap:
+            name: clickhouse-server-settings
 {% endif %}
 
 {% if RUN_RALPH %}
diff --git a/tutoraspects/patches/kustomization-configmapgenerator b/tutoraspects/patches/kustomization-configmapgenerator
index e1a962f2a..9747bbdc7 100644
--- a/tutoraspects/patches/kustomization-configmapgenerator
+++ b/tutoraspects/patches/kustomization-configmapgenerator
@@ -34,15 +34,18 @@
         app.kubernetes.io/name: aspects
 
 {% if RUN_CLICKHOUSE %}
-- name: clickhouse-settings
+- name: clickhouse-user-settings
   files:
-    - plugins/aspects/apps/clickhouse/config/docker_config.xml
-    - plugins/aspects/apps/clickhouse/config/named_collections.xml
     - plugins/aspects/apps/clickhouse/users/user_config.xml
   options:
     labels:
         app.kubernetes.io/name: clickhouse
-
+- name: clickhouse-server-settings
+  files:
+    - plugins/aspects/apps/clickhouse/config/server_config.xml
+  options:
+    labels:
+        app.kubernetes.io/name: clickhouse
 {% endif %}
 
 {% if RUN_RALPH %}
diff --git a/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml b/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml
deleted file mode 100644
index a6a072135..000000000
--- a/tutoraspects/templates/aspects/apps/clickhouse/config/named_collections.xml
+++ /dev/null
@@ -1,5 +0,0 @@
-<clickhouse>
-     <named_collections>
-         {{patch("clickhouse-named-collection-config")}}
-     </named_collections>
-</clickhouse>
diff --git a/tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml b/tutoraspects/templates/aspects/apps/clickhouse/config/server_config.xml
similarity index 100%
rename from tutoraspects/templates/aspects/apps/clickhouse/config/docker_config.xml
rename to tutoraspects/templates/aspects/apps/clickhouse/config/server_config.xml

From ad51310016cccf62da555ced456925b9f4b7848f Mon Sep 17 00:00:00 2001
From: Cristhian Garcia <crisgarta8@gmail.com>
Date: Mon, 29 Jan 2024 13:42:19 -0500
Subject: [PATCH 3/3] chore: add note on cloud providers

---
 tutoraspects/patches/clickhouse-server-config | 3 +++
 tutoraspects/patches/clickhouse-user-config   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/tutoraspects/patches/clickhouse-server-config b/tutoraspects/patches/clickhouse-server-config
index 7ec0b8ab3..aa5f7eb14 100644
--- a/tutoraspects/patches/clickhouse-server-config
+++ b/tutoraspects/patches/clickhouse-server-config
@@ -1,4 +1,7 @@
 <!--
+IMPORTANT: This patch is not applied to Clickhouse Cloud Providers.
+           make sure to contact your cloud provider support to apply this patch.
+
 These are miscellaneous ClickHouse server settings, which get mounted to
 /etc/clickhouse-server/config.d/docker_config.xml
 
diff --git a/tutoraspects/patches/clickhouse-user-config b/tutoraspects/patches/clickhouse-user-config
index 535c8cbc1..fbbb94b09 100644
--- a/tutoraspects/patches/clickhouse-user-config
+++ b/tutoraspects/patches/clickhouse-user-config
@@ -1,4 +1,7 @@
 <!--
+IMPORTANT: This patch is not applied to Clickhouse Cloud Providers.
+           make sure to contact your cloud provider support to apply this patch.
+
 Override configuration in users.xml. ClickHouse will read this from a mount on
 /etc/clickhouse-server/users.d/ on startup.