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

Environment variables for nested dictionary values in config.json do not work #13963

Open
neilharris123 opened this issue Feb 29, 2020 · 1 comment

Comments

@neilharris123
Copy link

@neilharris123 neilharris123 commented Feb 29, 2020

I've been trying to add Mattermost environment variables for values in the "PluginSettings" section of the config.json file.
For example, in the following example, if I wanted to add a variable for encryptionkey so that it overrides the value in config.json, according to the logic in the documentation, I would need to add and environment variable with the name MM_PLUGINSETTINGS_PLUGINS_GITHUB_ENCRYPTIONKEY .

"PluginSettings": {
    ...
    "Plugins": {
        "github": {
            "encryptionkey": "<your encryption key>",
            "githuboauthclientid": "<your oauth client id>",
            "githuboauthclientsecret": "<your oauth client secrete",
            "githuborg": "<your github org>"
            "webhooksecret": "<your webhook secret>"
        },
    ...
    "PluginStates": {
        ...
        "github": {
            "Enable": true
        },
        ...
    }
},

However, this doesn't work. The variable is passed to the container, but it has no affect on the actual configuration. This is the same if I try to create a variable for any of these nested keys.

Is this a limitation with the capabilities of Mattermost? The ability to do this is vital, because, without it, sensitive data would need to be added to the config.json file, and this isn't a good practice.

@lieut-data
Copy link
Member

@lieut-data lieut-data commented Mar 2, 2020

Hi @neilharris123! Unfortunately, this is indeed a known limitation, and I agree it prevents best practices here. I thought we had a ticket tracking this, but couldn't find one, so created https://mattermost.atlassian.net/browse/MM-22831.

agnivade added a commit that referenced this issue Mar 3, 2020
We get the original map from viper config. And then we populate the map
with those keys which do not have a period in them.
After we have unmarshalled the original values, we restore the original
values which did not have periods.

This allows us to atleast override values which don't have period in them.

Not a complete fix, because that needs to be done from within viper itself.
But it fixes the most common cases which don't have period in them.

Fixes #13963
agnivade added a commit that referenced this issue Mar 4, 2020
We get the original map from viper config. And then we populate the map
with those keys which do not have a period in them.
After we have unmarshalled the original values, we restore the original
values which did not have periods.

This allows us to atleast override values which don't have period in them.

But there is a subtle gotcha here, for the PluginSettings.Plugins section,
we can have nested maps of any depth. The current code is just able to handle
maps of depth 1. Anything more than that, will be flattened. This is technically
a regression, but I believe such a case was never needed.

Not a complete fix, because that needs to be done from within viper itself.
But it fixes the most common cases which don't have period in them.

Fixes #13963
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants