Operator does not remove stale users configuration when applying an updated CHI manifest

[janeklb]

Consider a CHI manifest:

apiVersion: "clickhouse.altinity.com/v1"
kind: "ClickHouseInstallation"
metadata:
  name: "clickhouse-installation-1"
spec:
  configuration:
    users:
      datadog_agent/password: datadog_agent
      datadog_agent/networks/ip: "::/0"
      datadog_agent/profile: default
      datadog_agent/quota: default
  # ... etc.

Note that the datadog_agent's networks configuration has a single entry (ip)

When the operator finishes reconciling this CHI, you can find the following entry in the clickhouse container's /etc/clickhouse-server/users.d/chop-generated-users.xml:

        <datadog_agent>
            <networks>
                <host_regexp>(chi-clickhouse-installation-1-[^.]+\d+-\d+|clickhouse\-clickhouse-installation-1)\.transaction-tracing-data\.svc\.cluster\.local$</host_regexp>
                <ip>::1</ip>
                <ip>127.0.0.1</ip>
                <ip>::/0</ip>
            </networks>
            <password_sha256_hex>ae1dbda236a2e10c903ae908e6338bdf3c60aab5530b07414f10ba1eedb6c051</password_sha256_hex>
            <profile>default</profile>
            <quota>default</quota>
        </datadog_agent>

Note that the datadog_agent's networks configuration has the ip config we specified in the manifest (::/0).

It also has a host_regexp config and two ip configs, which seem to be inherited/copied from the default user. AFAICT this behaviour isn't documented and so seeing this was a little confusing; however, this is not what my issue is about.

Now, kubectl apply the following (updated) CHI manifest

apiVersion: "clickhouse.altinity.com/v1"
kind: "ClickHouseInstallation"
metadata:
  name: "clickhouse-installation-1"
spec:
  configuration:
    users:
      datadog_agent/password: datadog_agent
      datadog_agent/networks/host_regexp: \.datadog\.datadog\.svc\.cluster\.local$
      datadog_agent/profile: default
      datadog_agent/quota: default
  # ... etc.

Note the only change is that the networks configuration no longer has an ip; it now (only) has a host_regexp.

When the operator has finished reconciling the changes produced by this manifest, the chop-generated-users.xml

        <datadog_agent>
            <networks>
                <host_regexp>\.datadog\.datadog\.svc\.cluster\.local$</host_regexp>
                <ip>::1</ip>
                <ip>127.0.0.1</ip>
                <ip>::/0</ip>
            </networks>
            <password_sha256_hex>ae1dbda236a2e10c903ae908e6338bdf3c60aab5530b07414f10ba1eedb6c051</password_sha256_hex>
            <profile>default</profile>
            <quota>default</quota>
        </datadog_agent>

Note that the host_regexp entry has been added, but the original ip entry ::/0 still remains.

Is this intentional? It is not something I would have expected, and afaict it is not documented anywhere in the operator's spec (https://github.com/Altinity/clickhouse-operator/blob/master/tests/requirements/requirements.md).

If we fetch CHI resource from kubernetes, it will show that both entries now exist (and so it seems like the issues lies somewhere in the reconciliation loop, and not the "config file generation" process):

# truncated output from `kubectl get chi -o yaml`
          users:
            datadog_agent/networks/host_regexp: \.datadog\.datadog\.svc\.cluster\.local$
            datadog_agent/networks/ip:
            - ::1
            - 127.0.0.1
            - ::/0
            datadog_agent/password_sha256_hex: ae1dbda236a2e10c903ae908e6338bdf3c60aab5530b07414f10ba1eedb6c051
            datadog_agent/profile: default
            datadog_agent/quota: default

[janeklb]

Worth noting that if the ClickHouseInstallation resource is deleted and re-applied, the newly created pod's configuration will not have the old <ip>::/0</ip> config.

[sunsingerus]

AFAICT this behaviour isn't documented and so seeing this was a little confusing

Feel free to make PR with appropriate documentation changes. Any contribution is very welcome.

[janeklb]

AFAICT this behaviour isn't documented and so seeing this was a little confusing

Feel free to make PR with appropriate documentation changes. Any contribution is very welcome.

Happy to do so! Can you please confirm that this is indeed intended behaviour, or if it's just incidental? This will determine how it should be phrased in the docs..

[sunsingerus]

Additional networks/ip are specified in the operator's config as default values for user accounts, created by the operator. Configuration source is available in operator's config file template

      default:
        # Default values for ClickHouse user account(s) created by the operator
        #   1. user/profile - string
        #   2. user/quota - string
        #   3. user/networks/ip - multiple strings
        #   4. user/password - string
        # These values can be overwritten on per-user basis.
        profile: "default"
        quota: "default"
        networksIP:
          - "::1"
          - "127.0.0.1"
        password: "default"

These are default values and can be modified|overwritten if need be.

[sunsingerus]

The operator merges values provided in the CR (chi resource) with defaults from its configuration.

[asad-awadia]

so if i change the password of the default user using the following

  configuration:
    users:
      default/password_sha256_hex: 0x123f

it won't get picked up??

I keep getting Authentication type PLAINTEXT_PASSWORD is not allowed, check the setting allow_plaintext_password even though this the password is specified in sha256 hex

[janeklb]

Additional networks/ip are specified in the operator's config as default values for user accounts, created by the operator. Configuration source is available in operator's config file template

Ah - I missed this, thank you.

Please do note that the issue originally described is for something a little different though.