Use environment variables to configure Mattermost

Mattermost now supports reading from environment variables to override what is in config.json. We should move to this method of passing configuration settings to MM instead of generating config.json. This is because Mattermost also writes data here, and if a reconfigure then occurs these newer settings can be lost.

More information on the current issue is here: https://gitlab.com/gitlab-org/omnibus-gitlab/issues/1688

mentioned in issue #1688 (closed)

added For Scheduling label

added up-for-grabs label

@marin @joshlambert So the idea is that omnibus will no longer be creating/touching config.json, but only set necessary environment variables. Right?

Only the ones that are crucial for running Mattermost such as database connection details and external url.

@marin Oh. Ok. So my understanding is this: Through /etc/gitlab/gitlab.rb file, users can set only a very few necessary settings that we will be converting and providing as environment variables to mattermost. Every other setting that we currently support via gitlab.rb will have to be done through Mattermost (this calls for a proper deprecation procedure). Thus, our cookbook will no longer be dealing with config.json file, only the environment variables. Right?

Correct.

According to Mattermost docs, there are certain settings that aren't configurable by mattermost system console, but only by manually updating config.json file. The current situation is that users are able to configure them using gitlab.rb file because we are generating config.json file. But once we stop generating/touching config.json file, user will no longer have this option. They will have to manually edit the config.json file. This means

1. Users now have three sources of truth for mattermost - gitlab.rb, Mattermost system console and config.json (though the last two are interleaved). Do we really want that?
2. How do we handle backup of config.json file? Do we add that to the files we backup or do we advice users to manually back it up?

@balasankarc Which settings are not configurable from the system console?

@marin The ones specified in the link I gave above - https://docs.mattermost.com/administration/config-settings.html#settings-configurable-only-in-config-json

mentioned in merge request !1677 (merged)

Users now have three sources of truth for mattermost

@balasankarc Is that really true though? If we tell users that Omnibus will ONLY handle a handful of settings, it will be up to them to keep the rest updated.

We only need 3 things: site_url, sql_data_source and path to config.json file. By default, we can pre-fill the current path to config.json. We would stop managing it and we would have to clearly note down that change. I think that the current situation is actually worse than telling users it is up to them to back this config up.

It would also be a good time to have this breaking change for GitLab 10. /cc @joshlambert

Is that really true though?

It is no longer true if we decide to handle only those three variables. I am not against such a breaking change especially because it will reduce confusion and "GitLab overwrote my changes to config.json" issues. Provided we communicate this clearly, in big block letters, we should definitely do this.

So the plan will be this

1. Provide site_url and sql_data_source as environment variable to mattermost. Note: We will not be using config.json anymore.
2. Provide an option for users to set other variables using environment variable, as done in !1677 (merged) .
3. No longer touch config.json file in our recipes.
4. Add docs to users on "how to set other mattermost settings" - using config.json file and using mattermost['env'] setting in omnibus.rb

@marin WDYT?

PS: Even if we document it, I am pretty sure we will be hearing a few "Configuration settings in omnibus aren't applied anymore". So, let's do this breaking change in 10.0.

Provide site_url and sql_data_source as environment variable to mattermost.

Also a path to config.json, by default one we are currently setting. We probably need few more settings by default but those 3 are absolute minimum

Other than that, everything else sounds correct.

@joshlambert are you ok with us doing this for 10.0 and any other input?

mentioned in issue gitlab-mattermost#103

Yes, let's get this done in v10. To confirm the behavior would be:

1. For existing users upon upgrading to v10, all of their current settings will remain intact in config.json. However we will no longer update or touch that file, so all future updates will need to be done by the user.
2. For new users, we will configure the 3 settings outlined here and nothing else.
3. Other edits which are supported by ENV vars can be performed using mattermost['ENV']="...".

To confirm as well, config.json will not be overwritten on MM package updates right?

changed milestone to %10.0

added Scheduled and removed For Scheduling labels

To confirm as well, config.json will not be overwritten on MM package updates right?

File is generated after package installation so it is not going to be touched by package upgrades.

I compared our config.json with the upstream one (v4.0.3). We have different values for the following settings compared to the upstream one. The settings starting with * indicate the ones that are not present in upstream config.json, but we still ship.

@marin I believe we will have to handle these settings as most of them are GitLab specific. The starred ones can be obviously deleted.

{
    "ServiceSettings": {
        "SiteURL": <provided value>,
        "ListenAddress": "127.0.0.1:8065",
        "EnableIncomingWebhooks": false,
        "EnableOutgoingWebhooks": false,
        "EnablePostUsernameOverride": true,
        "EnablePostIconOverride": true
    },
    "TeamSettings": {
        "SiteName": "GitLab Mattermost",
        "MaxUsersPerTeam": 150,
        * "RestrictTeamNames": true,
        "TeammateNameDisplay" : "full_name"
    },
    "SqlSettings": {
        "DriverName": "postgres",
        "DataSource": "user=gitlab_mattermost host=/var/opt/gitlab/postgresql port=5432 dbname=mattermost_production",
        "DataSourceReplicas": ["user=gitlab_mattermost host=/var/opt/gitlab/postgresql port=5432 dbname=mattermost_production"],
        "MaxIdleConns": 10,
        "MaxOpenConns": 10,
        "AtRestEncryptKey": "<generated secret>"
    },
    "LogSettings": {
        "ConsoleLevel": "INFO",
        "FileLevel": "ERROR",
        "FileLocation": "/var/log/gitlab/mattermost/mattermost.log"
    },
    "FileSettings": {
        "Directory": "/var/opt/gitlab/mattermost/data",
        "EnablePublicLink": true,
        "PublicLinkSalt": "<generated salt>",
        "AmazonS3Region": "",
        "AmazonS3Endpoint": "",
        * "AmazonS3BucketEndpoint": "",
        * "AmazonS3LocationConstraint": false,
        * "AmazonS3LowercaseBucket": false
    },
    "EmailSettings": {
        "EnableSignUpWithEmail": false,
        "EnableSignInWithUsername": false,
        "SendEmailNotifications": false,
        "FeedbackEmail": "",
        "SMTPServer": "",
        "SMTPPort": "",
        "InviteSalt": "<generated salt>"
    },
    "SupportSettings": {
        "TermsOfServiceLink": "/static/help/terms.html",
        "PrivacyPolicyLink": "/static/help/privacy.html",
        "AboutLink": "/static/help/about.html",
        "HelpLink": "",
        "ReportAProblemLink": "/static/help/report_problem.html",
        "SupportEmail": "support@example.com"
    },
    "GitLabSettings": {
        "Enable": true,
        "Secret": "<generated secret>",
        "Id": "<generated id>",
        "AuthEndpoint": "<gitlab url>/oauth/authorize",
        "TokenEndpoint": "<gitlab url>/oauth/token",
        "UserApiEndpoint": "<gitlab url>/api/v4/user"
    }
}

/cc @gitlab-build-team

@balasankarc Hrm, not seeing any *'s?

@joshlambert

  * "RestrictTeamNames": true,
  * "AmazonS3BucketEndpoint": "",
  * "AmazonS3LocationConstraint": false,
  * "AmazonS3LowercaseBucket": false

These are probably baggage from previous versions which weren't properly cleaned. I haven't dug much on them.

Update: I did a more aggressive cleaning up to see which all configuration we can drop and here are the results

{
    "ServiceSettings": {
        "SiteURL": "<%= @service_site_url %>",
        "ListenAddress": "<%= @service_address %>:<%= @service_port %>",
        "MaximumLoginAttempts": <%= @service_maximum_login_attempts %>
    },
    "TeamSettings": {
        "SiteName": "<%= @team_site_name %>",
        "MaxUsersPerTeam": <%= @team_max_users_per_team %>,
        "EnableUserCreation": <%= @team_enable_user_creation %>
    },
    "SqlSettings": {
        "DriverName": "<%= @sql_driver_name %>",
        "DataSource": "<%= @sql_data_source %>",
        "DataSourceReplicas": [<%= @sql_data_source_replicas.map{ |dsr| "\"#{dsr}\"" }.join(',') %>],
        "MaxIdleConns": <%= @sql_max_idle_conns %>,
        "MaxOpenConns": <%= @sql_max_open_conns %>,
        "AtRestEncryptKey": "<%= @sql_at_rest_encrypt_key %>"
    },
    "LogSettings": {
        "FileLocation": "<%= @log_file_directory %>/mattermost.log"
    },
    "FileSettings": {
        "DriverName": "<%= @file_driver_name %>",
        "Directory": "<%= @file_directory %>",
        "PublicLinkSalt": "<%= @file_public_link_salt %>"
    },
    "RateLimitSettings": {
        "PerSec": <%= @ratelimit_per_sec %>,
        "MemoryStoreSize": <%= @ratelimit_memory_store_size %>
    },
    "GitLabSettings": {
        "Enable": <%= @gitlab_enable %>,
        "Secret": "<%= @gitlab_secret %>",
        "Id": "<%= @gitlab_id %>",
        "AuthEndpoint": "<%= @gitlab_auth_endpoint %>",
        "TokenEndpoint": "<%= @gitlab_token_endpoint %>",
        "UserApiEndpoint": "<%= @gitlab_user_api_endpoint %>"
    }

However, there are a few configuration settings in the above list which I think are there because of some issue from Mattermost side. The logic of Mattermost is that if something is not specified in config.json, it will be fallen back to the default values. However, that is not happening with these configurations

• ServiceSettings
  • ListenAddress
  • MaximumLoginAttempts
• TeamSettings
  • MaxUsersPerTeam
  • EnableUserCreation
• SqlSettings
  • MaxIdleConns
  • MaxOpenConns
• FileSettings
  • DriverName
  • PublicLinkSalt (Theoretically, this should be used only if EnablePublicLink (whose default value is false) is set to true.
• RateLimitSettings
  • PerSec
  • MemoryStoreSize

Not setting values for these configurations in config.json raises Go errors and causes Mattermost not to start. So, we have to set some value to these configurations.

I will be trying out the native Mattermost (not the one bundled with GitLab) and see if these configurations fall back correctly there. If not, I will raise issues in Mattermost issue tracker to dig more on this.

The remaining configurations are GitLab specific and need to be set via env variables for proper integration between GitLab and Mattermost. So if the issues with the above configuration settings are solved, the possible variables we need to set will be the following (Reference: Mattermost Documentation)

• MM_SERVICESETTINGS_SITEURL
• MM_TEAMSETTINGS_SITENAME
• MM_SQLSETTINGS_DRIVERNAME
• MM_SQLSETTINGS_DATASOURCE
• MM_SQLSETTINGS_DATASOURCEREPLICAS
• MM_SQLSETTINGS_ATRESTENCRYPTKEY
• MM_LOGSETTINGS_FILELOCATION
• MM_FILESETTINGS_DIRECTORY
• MM_GITLABSETTINGS_ENABLE
• MM_GITLABSETTINGS_SECRET
• MM_GITLABSETTINGS_ID
• MM_GITLABSETTINGS_AUTHENDPOINT
• MM_GITLABSETTINGS_TOKENENDPOINT
• MM_GITLABSETTINGS_USERAPIENDPOINT

I will confirm this and update here asap.

There are settings like "EnableCustomEmoji" which can not be properly configured without getting wiped after each reconfigure.

@marin I've figured out an overall idea about how to do this, which I would be happy to discuss and polish. I will try my best to describe it clearly here. (Or we can do a call)

Plan

• Merge https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1677 to add env support for Mattermost.

• Remove every setting provided in gitlab.rb.template except the following 13 ones

  mattermost['service_site_url']
  mattermost['team_site_name']
  mattermost['sql_driver_name']
  mattermost['sql_data_source']
  mattermost['sql_data_source_replicas']
  mattermost['log_file_directory']
  mattermost['file_directory']
  mattermost['gitlab_enable']
  mattermost['gitlab_secret']
  mattermost['gitlab_id']
  mattermost['gitlab_auth_endpoint']
  mattermost['gitlab_token_endpoint']
  mattermost['gitlab_user_api_endpoint']

• Delete default values set to every setting other than the above mentiond ones.

• In mattermost recipe, define mattermost_env (similar to rails_env in gitlab-rails recipe)

  mattermost_env = {
          'MM_SERVICESETTINGS_SITEURL' => node['gitlab']['mattermost']['service_site_url'],
          'MM_TEAMSETTINGS_SITENAME' => node['gitlab']['mattermost']['team_site_name'],
          'MM_SQLSETTINGS_DRIVERNAME' => node['gitlab']['mattermost']['sql_driver_name'],
          'MM_SQLSETTINGS_DATASOURCE' => node['gitlab']['mattermost']['sql_data_source'],
          .
          .
          .
  }

• Merge it with any of the user specified env variables and write it to env file

  env_dir File.join(mattermost_home, 'env') do
    variables(
      mattermost_env.merge(node['gitlab']['mattermost']['env'])
    )
    restarts ["service[mattermost]"]
  end

• Ship the upstream provided config.json file to /var/opt/gitlab/mattermost/config.json instead of generating it from a template using variables

  • Change the config.json.erb template to original upstream config.json. We no longer needs to interpolate variables, so can drop the erb format.
    • To avoid confusion, maybe delete the 13 settings that can be set via gitlab.rb from config.json (the ones mentioned above). Else users may set them in config.json and expect them to work.
    • We will need to update config.json as more configuration is added on Mattermost side. But we are still doing that, anyway, so that is not a problem.
  • In Mattermost recipe, change the necessary block to use :create_if_missing action so that changes that users make are not overwritten by reconfigure.
  • This makes sure of two things
    • An upgrade won't overwrite config.json as it is copied after installation process
    • A reconfigure won't overwrite config.json as it uses :create_if_missing action

Changes applicable to users

• Any configuration other than the 13 ones mentioned above that was specified gitlab.rb will no longer have an effect, This is a breaking change.
• Such configurations need to go in through either of the following
  • Mattermost system console
  • Manually editing /var/opt/gitlab/mattermost/config.json
  • As specially formatted (MM_<category>_<setting> - as specified in MM docs) env variables in mattermost['env']
• Reconfigure will no longer modify config.json file. Neither will upgrades. If users want to use new features via config.json, they will have to add them themselves. In short, we will no longer be taking care of config.json.

If the plan looks sane enough, I will open an MR and we can discuss it there.

PS: Regarding the issue of missing configurations not falling back to default values, mentioned in https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2377#note_38157608, I was able to reproduce it with native mattermost installation also. I opened an issue at Mattermost upstream repo - https://github.com/mattermost/platform/issues/7276. A ticket has been filed at their internal tracker also - https://mattermost.atlassian.net/browse/PLT-7472 . But it will not affect us as we are shipping the upstream config.json, so there won't be any missing values.

mentioned in merge request !1877

assigned to @balasankarc

@marin Opened https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1877 based on the above idea.

@balasankarc The plan sounds good to me apart from one thing:

Ship the upstream provided config.json file to /var/opt/gitlab/mattermost/config.json instead of generating it from a template using variables.

I don't think we should do anything with the template. We should not provide a template or fetch anything from upstream. This should remain a manual task for the user.

If we are no longer touching that file why would we ship it?

@joshlambert Let's move the discussion to the MR. Checkout https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1877#note_38905212

@esethna2 @jasonblais We are running into an issue and we need your help. In https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1877 we are getting to the point of having MM configured through environmental variables and removing config.json as a dependency from the package. However, it seems that it is impossible to start mattermost without providing a config.json file. How do we resolve this?

We considered just adding the template file that a user can copy and edit but it will be again very confusing if the values in config.json do not reflect what was configured in the package. We also can't populate that file because this is what we are doing right now.

Any ideas?

Hey @marin

One option is to copy over a default config.json if no file already exists. If a config.json file already exists then don't copy.

The environment variables take precedence over the config.json file and can still be set. Let me know if this helps?

@jasonblais My concern with that is that users will be similarly confused (like they are with the current situation) if the changes in config.json they make do not take effect. The simplest way I can think of for getting around that is if a comment is included in config.json that states ENV variables take presedence over whatever is configured in the config file. Is it possible to include this in Mattermost itself?

Ok, so I'm a little late to this party, sorry. I have been managing updates on the Mattermost bosh release, and have encountered a similar problem, and I think I have a common solution. When put into HA mode, MM already has the ability to make the system console read only. What if we request 2 things from MM themselves:

1. make the flag for turning the system console into read-only available in both HA mode and normal mode.
2. add an attribute in config.json that will provide a banner on all system console pages indicating where to make changes.

Eg. "The Mattermost System Console is currently in read only mode. To make changes to the configuration please modify #{mattermost.console.whereToChange}" Default for mattermost.console.whereToChange would default to "#{path to config file} file directly"

Then gitlab could set it to "please update the gitlab.rb file at #{path to gitlab.rb"

My BOSH release would use "please update your BOSH deployment manifest."

This prevents the unintentional override of values, avoids confusion for the end user, and allows flexibility for any automated deployment method.

Thoughts?

@jasonblais With https://github.com/mattermost/platform/issues/7276 in, the config.json file can be even empty and Mattermost will fall back to default values, right? If that is indeed the case, can't the hard requirement of a config.json file be dropped?

PS: Sorry for the delayed response. Am away on a vacation.

Chatted with a few developers on our end, sorry for the delay.

We don't think it's a good idea to prevent users from changing settings from the System Console. We have upcoming features (like plugins) that'll need to be configured through the System Console and may not be able to be configured through environment variables. There's also some other settings like auto-generated salts that will be changed by Mattermost regardless of what we disable in the UI. These will both require that we keep the config.json file around. As long as GitLab is providing the environment variables and there's no permission issues, Mattermost should be able to create and populate the config.json itself, so nothing will need to be added for that.

In terms of having the environment variables overwriting the config.json, we don't think it's a problem as long as it's only overwriting the ones that have been set by the user in gitlab.rb. The main problem with how it works currently is that it overwrites all of the user's changes made from the System Console, even when they haven't specified the setting in gitlab.rb.

Let me know if there are any questions or concerns?

So how's that going to work with:

1. High availability which currently locks the system console anyway.
2. Everything as code advocates that want to be able to destroy and recreate an entire system in a completely automated way.
3. Blue/green deploys using chef/puppet/user data/etc.

One of the the things we value about mattermost is the ability to maintain source control of our entire configuration and track approval of each change. Once you introduce configurations that can't be managed outside the UI you make automated deploy and configuration management tools dramatically harder.

Is it possible to consider plugins as data rather than configuration? Save them in the db rather than the configuration file? I know this is the approach gitlab takes, and it works well. It would solve all three of the points above, while still allowing users to make changes to things like plugins later. The biggest challenge I can see is needing to some how differentiate between configuration and data in the UI. But that could be a s simple as ensuring each page has either configuration or data, not both. Then the config pages would be read only with a banner indicating such, and the other pages, like integrations, would not be locked down.

As for things like salt values, I go back to the fact that HA systems already have read-only configurations. I see no reason to treat things differently than they are now, but you could also simply throw an error on startup if read only is true and those values are missing.

Thoughts?

@jasonblais

As long as GitLab is providing the environment variables and there's no permission issues, Mattermost should be able to create and populate the config.json itself, so nothing will need to be added for that.

I have to clarify something. If I pass a non-existing location to mattermost, and if mattermost user has permission to create/edit there, will mattermost automatically create a config.json file? It doesn't seem to work for me. I have the following to run mattermost

exec chpst -e /var/opt/gitlab/mattermost/env \
  -P \
  -U mattermost \
  -u mattermost \
  /opt/gitlab/embedded/bin/mattermost \
  --config /var/opt/gitlab/mattermost/config.json

Here, /var/opt/gitlab/mattermost/config.json doesn't exist. Mattermost is not creating that file, but returns the following error

2017-09-15_06:58:16.49056 Error opening config file=/var/opt/gitlab/mattermost/config.json, err=Config File "config" Not Found in "[/var/opt/gitlab/mattermost /opt/gitlab/embedded/service/mattermost/config /opt/gitlab/embedded/service/config /opt/gitlab/embedded/config /opt/gitlab/embedded/service/mattermost]"
2017-09-15_06:58:17.71420 Error opening config file=/var/opt/gitlab/mattermost/config.json, err=Config File "config" Not Found in "[/var/opt/gitlab/mattermost /opt/gitlab/embedded/service/mattermost/config /opt/gitlab/embedded/service/config /opt/gitlab/embedded/config /opt/gitlab/embedded/service/mattermost]"

Mattermost user has permission to create a file in /var/opt/gitlab/mattermost.

@twk3 @joshlambert It seems config.json is definitely needed and can't be skipped. If that is the case, I would prefer us shipping a curated version of config.json (removing the settings that are already provided via gitlab.rb) than the upstream one as it can cause confusion among users. That is, even if they set a value in config.json, it won't be applied if that value is one of the 13 that we provide via gitlab.rb, because env variable takes precedence.

This was the state at ad12c0cd49cc2c3d7fedc8e7e7566ce8d47c3fff, which was reversed by 77c82449933a2a5cb36ae9636a72208d0f7738ff

Shipping a curated version of config.json doesn't make it any worse than the existing situation (we are already shipping a config.json.erb file that needs to be updated with each mattermost release version), just that it will reduce the profit we gain by this change. In short, the good result of this change will be user's config.json will no longer be overwritten by reconfigure.

WDYT?

removed up-for-grabs label

Thanks @balasankarc, let me know if this is accurate:

• Mattermost does not ship a config.json in their package
• We need to create some config.json for the service to start

If so a few questions:

• Do we have to maintain this between releases? Why if it is a simple bare bones config?
• Can it be empty, just touch the file?

If there is going to be maintenance overhead if we ship our own, but not overhead if we use the upstream, I would vote we use upstream.

@joshlambert

• Mattermost does not ship a config.json in their package

Incorrect. They do ship a config.json file that we can reuse. But that will contain those settings also, which we are already overriding using gitlab.rb (and thus, via env variables). So they can be potentially confusing to users - "Hey, some settings aren't applied even if I changed them in config.json". Because, env variables take precedence and they should've changed them in gitlab.rb. Sure, we can document it and expect users to do the right thing, but I can see us and support getting tickets about this.

Which is why I suggest we take that upstream provided one, remove the redundant settings, and then ship that.

• We need to create some config.json for the service to start

Correct. We need to create a valid, non-empty config.json file.

Do we have to maintain this between releases? Why if it is a simple bare bones config?

We may or may not maintain this. We can provide the upstream one as of today and ask users to modify it as per their need. Again, we will have to prune the redundant settings. I am fine with that too.

Can it be empty, just touch the file?

With the existing Mattermost version, it can't be empty. It will report "unexpected EOF in JSON file" and fail to start. I am yet to test with the new Mattermost, but I assume nothing has changed there.

Does mattermost auto-update the config file when upgrading? If not we will still need omnibus to update the file in certain circumstances. (name of a key changes, etc)

@twk3

Does mattermost auto-update the config file when upgrading? If not we will still need omnibus to update the file in certain circumstances. (name of a key changes, etc)

We will update the source config.json file (the one which we ship in templates folder and will be copying to /var/opt/gitlab/mattermost/config.json as part of reconfigure if not exists), so that for new installations, users get the correct stuff. For existing installations, we will not be touching users' existing config.json file as part of reconfigure. Users will have to update it themselves as per directions from mattermost.

In short our options are this

1. Ship the upstream config.json without doing anything and copy it to /var/opt/gitlab/mattermost/config.json if that file doesn't exist. Expect users to understand that certain values should be changed in gitlab.rb and not in config.json
2. Maintain a curated config.json and copy it to /var/opt/gitlab/mattermost/config.json if that file doesn't exist. This file may or may not be updated with new releases as situation demands. I am leaning towards updating it whenever we can.

Personally, I prefer the latter, until we reach a state where Mattermost will be automatically generating config.json file if not found using values provided as env variables and default values.

Based on my testing, that doesn't seem to be the case as of now. I am building a new package against mattermost 4.2.0 to see if it has improved in latest version.

@jasonblais, we had a team discussion around path forward on this and one of the items that came up was the migration of config.json values that is required from time to time on MM upgrades.

With the MM console now editing portions of config.json, does the console handle these config migrations? Or is this currently left to the end users to edit?

@balasankarc what if we dropped our default mattermost values for all but those 13, but in addition to those 13, still allowed users to configure all variables through gitlab.rb, and use the ENV variables for those as well. That way we are only overwriting variables that the users have explicitly set in gitlab.rb (plus our 13), and the rest work properly from the console, or can be configured manually through config.json. (Which is how we will recommend configuring mattermost?)

I think this would be the best way move forward with our current plan, while still supporting the exiting config options of our current users without generating too much confusion/support issues. Then once we have a more solid plan in place we can start deprecating the gitlab.rb settings.

cc\ @joshlambert

@twk3 let me try to play back the impacts for existing and new users.

For existing installations:

• The config.json already has the default values, as well as any specific values that were set.
• Going forward, we will continue to set the specifically configured values but not the default ones.
• In this scenario, I imagine we would continue to write to config.json rather than use ENV vars because we are anyway modifying the file and there is less confusion with one source of truth?

For new installations:

• config.json is the curated version mentioned here: https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2377#note_40485237
• Default values from gitlab.rb will not be set
• Anything specific from gitlab.rb will be written into the config.json

In both cases during gitlab-ctl reconfigure we would then selectively edit the parameters that were specifically set in gitlab.rb. Is that accurate?

If so, a few thoughts:

• I like the reduced surface area of conflicts between things set in the MM console, config.json, and gitlab.rb. It will still be confusing, but hopefully less so since someone specifically set something in gitlab.rb.
• We still have the issue with breaking changes in MM, since we don't totally replace the file.
• How much effort is it to selectively edit config.json in this manner?

@joshlambert I'm not suggesting editing mattermost config.json, at least not for the moment.

• We stop changing config.json using omnibus, as planned, but ship a curated copy of it. (And still have to figure out what to do about the breaking config changes there). (Existing installs will use the current version)
• We set our 13 env variables (most of them calculated from other ares in gitlab.rb/secrets)
• We set env variables for any other mattermost settings the users has in their /etc/gitlab/gitlab.rb
• We drop pretty much all our defaults for mattermost from attributes/default.rb
• Recommend users change /var/opt/gitlab/mattermost/config.json or use the web console to configure mattermost rather than using gitlab.rb (other than for the gitlab integration settings)
• Throw deprecation warnings for mattermost settings in gitlab.rb, so users know they will have to move to setting config.json
• In GitLab 11, drop support for setting mattermost settings in gitlab.rb (other than the gitlab integration ones)

@twk3 Okay, I thought we were trying to continue to support via gitlab.rb but since there is not full coverage with ENV vars, we'd have to still edit.

Instead we'd basically say that we'd only continue to support configuration within gitlab.rb for those options that offer ENV var configuration. Do we know what those gaps are?

@twk3 I agree that we should have a deprecation period where existing configuration will work, but users will be advised to move away from it.

Regarding to "provide any setting user has defined in gitlab.rb as env variable", it is possible, but we will probably need to have a mapping between the gitlab.rb keys and the config.json keys. I don't think we are completely consistent in naming the gitlab.rb keys to programmatically compute MM_<CATEGORY>_<SETTING> string from it. I will confirm if that is possible or not.

As this seems to be the better way forward, I will try to do that mapping and implement the logic.

/cc @joshlambert

I compared gitlab.rb and config.json.erb and found out the following keys doesn't follow the pattern (i.e we can't compute MM_<CATEGORY>_<STRING> variable using a general string-formatting logic)

mattermost['service_lets_encrypt_cert_cache_file']
mattermost['log_console_enable']
mattermost['email_enable_batching']
mattermost['ratelimit_enable_rate_limiter']
mattermost['webrtc_gateway_stun_uri']
mattermost['webrtc_gateway_turn_uri']
mattermost['webrtc_gateway_turn_username']
mattermost['webrtc_gateway_turn_shared_key']

That is way better than what I anticipated. Since this is a relatively temporary thing that will probably go away with next major release of GitLab, I think we can compute the rest of the settings programmatically and add these 8 ones manually.

@balasankarc are you indicating that there is a corresponding Environment Variable for every one of the settings in gitlab.rb?

If so, that would be awesome. We could just:

• Ship the curated config.json, and set it up if one does not already exist
• For existing config.json, leave it alone and we will override via Environment Variables from this point forward
• For all of the current settings in gitlab.rb, we will pass them via ENV Var going forward.
• Document the switch from config.json to environment var

@joshlambert As per Mattermost docs, there is. :)

Environment Variables Starting in Mattermost version 3.8, you can use environment variables to manage the configuration. Environment variables override settings in config.json. If a change to a setting in config.json requires a restart for it to take effect, then changes to the corresponding environment variable also require a server restart.

The name of the environment variable for any setting can be derived from the name of that setting in config.json.

For example, to derive the name of the Site URL setting:

1. Find the setting in config.json. In this case, ServiceSettings.SiteURL.
2. Add MM_ to the beginning and convert all characters to uppercase and replace the . with _. For example, MM_SERVICESETTINGS_SITEURL.
3. The setting becomes export MM_SERVICESETTINGS_SITEURL="http://example.com"

I gave it a try and it seems everything worked with env variables except MM_SQLSETTINGS_DATASOURCEREPLICAS. Since that value requires an array as input, I am not entirely sure how to pass it via env variables.

@jasonblais Can someone from Mattermost shed some light on an example value for MM_SQLSETTINGS_DATASOURCEREPLICAS ?

For example, consider this snippet from config.json

    "SqlSettings": {
        "DriverName": "postgres",
        "DataSource": "user=gitlab_mattermost host=/var/opt/gitlab/postgresql port=5432 dbname=mattermost_production",
        "DataSourceReplicas": ["user=gitlab_mattermost host=/var/opt/gitlab/postgresql port=5432 dbname=mattermost_production"],
        "MaxIdleConns": 10,
        "MaxOpenConns": 10,
        "Trace": false,
        "AtRestEncryptKey": "581b5a39b4df2f2b2629926eeaa99270",
        "DataSourceSearchReplicas": []

We use chpst to pass an env variable directory to mattermost execution. So, what should be the value in the file /var/opt/gitlab/mattermost/env/MM_SQLSETTINGS_DATASOURCEREPLICAS as per the above config.json? What format does Mattermost expect?

/cc @gitlab-build-team

@balasankarc At initial glance that should work.

1. What kind of an error did you get for the DataSourceReplicas field?
2. Do you have any errors if you don't have DataSourceReplicas specified?

@jasonblais It will work if I don't have DataSourceReplicas specified. But, I want to know how to convert it to an env variable in case someone has enabled it. Hence I need an example value in the correct format to put into MM_SQLSETTINGS_DATASOURCEREPLICAS. Specifically, how does Mattermost parse an array that is passed as an environment variable (which probably will be a string)?

My current value and the error is below (this value works if it is in config.json)

root@99363c5d2179:/var/opt/gitlab/mattermost/env# cat MM_SQLSETTINGS_DATASOURCEREPLICAS 
["user=gitlab_mattermost host=/var/opt/gitlab/postgresql port=5432 dbname=mattermost_production"]
root@99363c5d2179:
root@99363c5d2179:
root@99363c5d2179:/var/opt/gitlab/mattermost/env# gitlab-ctl tail mattermost
==> /var/log/gitlab/mattermost/current <==
2017-09-18_12:55:15.31871 [2017/09/18 12:55:15 UTC] [INFO] Enterprise Enabled: false
2017-09-18_12:55:15.31872 [2017/09/18 12:55:15 UTC] [INFO] Current working directory is /opt/gitlab/embedded/service/mattermost
2017-09-18_12:55:15.31899 [2017/09/18 12:55:15 UTC] [INFO] Loaded config file from /var/opt/gitlab/mattermost/config.json
2017-09-18_12:55:15.31945 [2017/09/18 12:55:15 UTC] [INFO] Able to write files to local storage.
2017-09-18_12:55:15.31947 [2017/09/18 12:55:15 UTC] [INFO] Server is initializing...
2017-09-18_12:55:15.31973 [2017/09/18 12:55:15 UTC] [INFO] Pinging SQL master database
2017-09-18_12:55:15.32785 [2017/09/18 12:55:15 UTC] [INFO] Pinging SQL replica-0 database
2017-09-18_12:55:15.33187 [2017/09/18 12:55:15 UTC] [EROR] Failed to ping DB retrying in 10 seconds err=pq: role "mattermost" does not exist
2017-09-18_12:55:25.33313 [2017/09/18 12:55:25 UTC] [INFO] Pinging SQL replica-0 database
2017-09-18_12:55:25.33525 [2017/09/18 12:55:25 UTC] [EROR] Failed to ping DB retrying in 10 seconds err=pq: role "mattermost" does not exist

==> /var/log/gitlab/mattermost/mattermost.log <==
[2017/09/18 12:55:15 UTC] [INFO] Enterprise Enabled: false
[2017/09/18 12:55:15 UTC] [INFO] Current working directory is /opt/gitlab/embedded/service/mattermost
[2017/09/18 12:55:15 UTC] [INFO] Loaded config file from /var/opt/gitlab/mattermost/config.json
[2017/09/18 12:55:15 UTC] [INFO] Able to write files to local storage.
[2017/09/18 12:55:15 UTC] [INFO] Server is initializing...
[2017/09/18 12:55:15 UTC] [INFO] Pinging SQL master database
[2017/09/18 12:55:15 UTC] [INFO] Pinging SQL replica-0 database
[2017/09/18 12:55:15 UTC] [EROR] Failed to ping DB retrying in 10 seconds err=pq: role "mattermost" does not exist
[2017/09/18 12:55:25 UTC] [INFO] Pinging SQL replica-0 database
[2017/09/18 12:55:25 UTC] [EROR] Failed to ping DB retrying in 10 seconds err=pq: role "mattermost" does not exist

Additionally, I have another doubt. DataSourceReplicas and DataSourceSearchReplicas are marked Enterprise edition feature in https://docs.mattermost.com/administration/config-settings.html#read-replicas-enterprise-edition . Why are they even considered in Team edition?

Digging around in the codebase, it appears that this can only be a string. https://github.com/mattermost/mattermost-server/blob/c357949856196b65d7dfd9d6203ad0980edbf895/utils/config.go#L200-L203 shows that it uses viper.AutomaticEnv, so we should apply knowledge from https://github.com/spf13/viper#working-with-environment-variables. Perhaps the only option I see is a JSON encoded string literal.

@balasankarc @WarheadsSE

I don't think you need to set the read replica: if it's empty it should copy over the master database into the read replica.

We created a ticket for the issue where the server doesn't start if DataSourceReplicas is set on an unlicensed server: https://mattermost.atlassian.net/browse/PLT-7675

@jasonblais Yeah. GitLab doesn't need to set read replica for GitLab-Mattermost integration to work. But that is a valid setting and if a user has set it, we need to correctly place it as an env variable or else Mattermost won't start. That is why I would like to know what format does Mattermost expect for that setting, when provided as an env variable.

OTOH If it is indeed an Enterprise-Edition-only setting, Mattermost should ignore it being present in a Team Edition instance (so that server will start) and GitLab can probably drop it from the list of options provided via gitlab.rb. Thanks for raising the ticket regarding this (which isn't publicly accessible, but that's ok).

Thanks @balasankarc, I'd suggest looking into the JSON encoding through the Viper library: https://github.com/spf13/viper#working-with-environment-variables

If that doesn't work, let us know and we can investigate it further.

@balasankarc @joshlambert @WarheadsSE

We just tested this on our end. You can set the first item in the array with MM_SQLSETTINGS_DATASOURCEREPLICAS, and use that field for SqlSetting.DatasourceReplicas.

Let me know if this helps?

@jasonblais Actually, I wanted an example value to know how you set it via env variables. Anyway, it doesn't matter now as @it33 confirmed via Slack that it is an EE-only setting and GitLab can skip it. He has also opened https://gitlab.com/mattermost/proposals/issues/17 to discuss about it (for Mattermost's EE customers).

The optimal solution would be for Mattermost to simply ignore EE-only settings if defined in a Team Edition instance. I believe the ticket you raised talks about it.

When put into HA mode, MM already has the ability to make the system console read only.

@tejaycar1 I liked this proposal but as you later noted, it is not going to work with the upcoming changes :(

@balasankarc @joshlambert @twk3 I propose the following process:

Actionable items now:

• Adding a deprecation notice now for all but 13 configuration options we need to get MM up and running. Notice would say that in GitLab 11 these settings will no longer be controlled by omnibus.
• In the deprecation notice we link to our docs which will explain how you can use env variables to control most of the things and how you can make the switch right now. We should also link to MM docs related to configuration.
• We announce this in a blog post

GitLab 11:

• If MM team does not want to make a change to config file at startup (either by creating a file if it doesn't exist or not panicking at start), we should create a curated config.json file once. Existing config.json we should ignore
• We remove all other default settings
• We no longer touch config.json and it is up to the user to maintain it from there on.

What do you think?

I am fine with dropping support for config.json provided we give a deprecation period,notice and proper docs.