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

nixos/mattermost: add secretFile option for declarative configs #117046

Merged
merged 0 commits into from Mar 2, 2022

Conversation

stuebinm
Copy link
Contributor

This adds an option services.mattermost.secretFile, which may be used in conjunction with services.mattermost.mutableConfig set to false to avoid placing secret parts of the configuration into the world-readable
nix store.

Motivation for this change

#116421

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@stuebinm
Copy link
Contributor Author

Huh, apparently I broke the manual. Since this is my first time committing to nixpkgs: where can I find documentation on how to write / test-build the manual? I couldn't find anything on that in the contributing page …

@Artturin
Copy link
Member

Artturin commented Jul 21, 2021

https://nixos.org/manual/nixos/stable/index.html#chap-contributing
you can also see why the test is failing by clicking details but its expired now

copying path '/nix/store/v7wl6gss5vc2gin9jj7h1zhsv2yskyfz-libxslt-1.1.34-bin' from 'https://cache.nixos.org'...
building '/nix/store/1lw3cf5z358kvcss5761kpk6ssb8l42a-options-docbook.xml.drv'...
building '/nix/store/zfkbi4n95yhmzprkvpn7bx2281jbvvdl-generated-docbook.drv'...
building '/nix/store/vmfidz0gkyf6rz3169mzlpj6fn4vfd6a-nixos-manual-combined.drv'...

manual-combined.xml:48310: element para: Relax-NG validity error : Did not expect element para there
 48306  </para><para><emphasis>Type:</emphasis> boolean</para><para><emphasis>Default:</emphasis> <literal>
 48307          false
 48308        </literal></para><para><emphasis>Declared by:</emphasis></para><simplelist><member><filename xlink:href="https://github.com/NixOS/nixpkgs/blob/release-21.05/nixos/modules/services/web-apps/mattermost.nix">
 48309                &lt;nixpkgs/nixos/modules/services/web-apps/mattermost.nix&gt;
 48310              </filename></member></simplelist></listitem></varlistentry><varlistentry><term xlink:href="#opt-services.mattermost.secretFile" xml:id="opt-services.mattermost.secretFile"><option>services.mattermost.secretFile</option></term><listitem><para>Path to a json file containing secret config values which should
 48311  not be written into the Nix store. If it is not null (the default)
 48312  and <xref linked="services.mattermost.mutableConfig">mutableConfig</xref>

manual-combined.xml:48310: element para: Relax-NG validity error : Expecting element example, got para
 48306  </para><para><emphasis>Type:</emphasis> boolean</para><para><emphasis>Default:</emphasis> <literal>
 48307          false
 48308        </literal></para><para><emphasis>Declared by:</emphasis></para><simplelist><member><filename xlink:href="https://github.com/NixOS/nixpkgs/blob/release-21.05/nixos/modules/services/web-apps/mattermost.nix">
 48309                &lt;nixpkgs/nixos/modules/services/web-apps/mattermost.nix&gt;
 48310              </filename></member></simplelist></listitem></varlistentry><varlistentry><term xlink:href="#opt-services.mattermost.secretFile" xml:id="opt-services.mattermost.secretFile"><option>services.mattermost.secretFile</option></term><listitem><para>Path to a json file containing secret config values which should
 48311  not be written into the Nix store. If it is not null (the default)
 48312  and <xref linked="services.mattermost.mutableConfig">mutableConfig</xref>

manual-combined.xml:48310: element para: Relax-NG validity error : Expecting element bridgehead, got para
 48306  </para><para><emphasis>Type:</emphasis> boolean</para><para><emphasis>Default:</emphasis> <literal>
 48307          false
 48308        </literal></para><para><emphasis>Declared by:</emphasis></para><simplelist><member><filename xlink:href="https://github.com/NixOS/nixpkgs/blob/release-21.05/nixos/modules/services/web-apps/mattermost.nix">
 48309                &lt;nixpkgs/nixos/modules/services/web-apps/mattermost.nix&gt;
 48310              </filename></member></simplelist></listitem></varlistentry><varlistentry><term xlink:href="#opt-services.mattermost.secretFile" xml:id="opt-services.mattermost.secretFile"><option>services.mattermost.secretFile</option></term><listitem><para>Path to a json file containing secret config values which should
 48311  not be written into the Nix store. If it is not null (the default)
 48312  and <xref linked="services.mattermost.mutableConfig">mutableConfig</xref>

manual-combined.xml:48310: element para: Relax-NG validity error : Element para has extra content: text
 48306  </para><para><emphasis>Type:</emphasis> boolean</para><para><emphasis>Default:</emphasis> <literal>
 48307          false
 48308        </literal></para><para><emphasis>Declared by:</emphasis></para><simplelist><member><filename xlink:href="https://github.com/NixOS/nixpkgs/blob/release-21.05/nixos/modules/services/web-apps/mattermost.nix">
 48309                &lt;nixpkgs/nixos/modules/services/web-apps/mattermost.nix&gt;
 48310              </filename></member></simplelist></listitem></varlistentry><varlistentry><term xlink:href="#opt-services.mattermost.secretFile" xml:id="opt-services.mattermost.secretFile"><option>services.mattermost.secretFile</option></term><listitem><para>Path to a json file containing secret config values which should
 48311  not be written into the Nix store. If it is not null (the default)
 48312  and <xref linked="services.mattermost.mutableConfig">mutableConfig</xref>

manual-combined.xml:48310: element para: Relax-NG validity error : Expecting element annotation, got para
 48306  </para><para><emphasis>Type:</emphasis> boolean</para><para><emphasis>Default:</emphasis> <literal>
 48307          false
 48308        </literal></para><para><emphasis>Declared by:</emphasis></para><simplelist><member><filename xlink:href="https://github.com/NixOS/nixpkgs/blob/release-21.05/nixos/modules/services/web-apps/mattermost.nix">
 48309                &lt;nixpkgs/nixos/modules/services/web-apps/mattermost.nix&gt;
 48310              </filename></member></simplelist></listitem></varlistentry><varlistentry><term xlink:href="#opt-services.mattermost.secretFile" xml:id="opt-services.mattermost.secretFile"><option>services.mattermost.secretFile</option></term><listitem><para>Path to a json file containing secret config values which should
 48311  not be written into the Nix store. If it is not null (the default)
 48312  and <xref linked="services.mattermost.mutableConfig">mutableConfig</xref>

manual-combined.xml fails to validate

nixos/modules/services/web-apps/mattermost.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/mattermost.nix Outdated Show resolved Hide resolved
@pinpox
Copy link
Member

pinpox commented Mar 1, 2022

@Artturin Is something still blocking this? I'm still looking for a good way to specify secrets in mattermost using the service.

@Artturin
Copy link
Member

Artturin commented Mar 2, 2022

needs a rebase and someone to test the pr https://nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

@pinpox
Copy link
Member

pinpox commented Mar 2, 2022

needs a rebase and someone to test the pr https://nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

Following the link I'm using this flake to test the module:

{
  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
  # inputs.pkgsReview.url = "github:Artturin/nixpkgs/pipewirejackldpath";
  inputs.pkgsReview.url = "github:stuebinm/nixpkgs/master";
  #inputs.pkgsReview.url = "/home/artturin/nixgits/my-nixpkgs";

  outputs = inputs@{ self, nixpkgs, pkgsReview }: {

    nixosConfigurations.vm = nixpkgs.lib.nixosSystem {
      system = "x86_64-linux";
      specialArgs = { inherit inputs; };
      modules = [
        ({ pkgs, ... }: {

          # Disable old module from upstream nixpkgs, if the module is not new
          disabledModules = [ "services/web-apps/mattermost.nix" ];
          imports = [

            # Include the module from the fork/fork you want to test
            "${inputs.pkgsReview}/nixos/modules/services/web-apps/mattermost.nix"

            # For virtualisation settings
            "${inputs.nixpkgs}/nixos/modules/virtualisation/qemu-vm.nix"
          ];

          # Test-Specific configuration for the module
          services.mattermost = {
            enable = true;
            mutableConfig = false;
          };

          # Documentation for these is in nixos/modules/virtualisation/qemu-vm.nix
          virtualisation = {
            memorySize = 1024 * 3;
            diskSize = 1024 * 3;
            cores = 4;
            msize = 104857600;
          };

          users.mutableUsers = false;
          users.users.root = { password = "root"; };
          users.users.user = {
            password = "user";
            isNormalUser = true;
            extraGroups = [ "wheel" ];
          };
        })
      ];
    };
    # So that we can just run 'nix run' instead of
    # 'nix build ".#nixosConfigurations.vm.config.system.build.vm" && ./result/bin/run-nixos-vm'
    defaultPackage.x86_64-linux =
      self.nixosConfigurations.vm.config.system.build.vm;
    defaultApp.x86_64-linux = {
      type = "app";
      program = "${self.defaultPackage.x86_64-linux}/bin/run-nixos-vm";
    };
  };
}

I run into an error when using nix run, not sure if I'm doing it wrong or the module has a problem.

✦ ×  nix run
warning: Git tree '/home/pinpox/review-mattermost' is dirty
error: access to canonical path '/nix/store/mp0llrzvhm7phgma5d13mv9l6ipyncsf-mattermost-webapp-6.3.3/config/config.json' is forbidden in restricted mode
(use '--show-trace' to show detailed location information)

I'd like to help test it, but need some guidance.

@pinpox
Copy link
Member

pinpox commented Mar 2, 2022

Huh, was it already fixed?

@stuebinm
Copy link
Contributor Author

stuebinm commented Mar 2, 2022

Did a rebase to current master; it seems to work fine on my end, but since a lot has happened since I originally opened this PR and I can't claim to understand the module in its current state, testing by someone else would be important

@pinpox hm, that's interesting. It seems like it's trying to import a file config.json right next to the module, but I have no idea where it does that. You can try running it with --impure to disable pure evaluation mode and see if it works then, though that's obviously not a good solution …

Also, does anyone know why me pushing the rebase apparently led to github automatically closing the PR and someone else merging it before I could reopen it? Seems weird …

@pinpox
Copy link
Member

pinpox commented Mar 2, 2022

Still getting an error with --impure

» nix run --impure
warning: Git tree '/home/pinpox/module-review-flake' is dirty
error: the string '{
         "ServiceSettings": {
           "SiteURL": "",
           "WebsocketURL": "",
      
[...] Redcated JSON [...]

         },
         "ExportSettings": {
           "Directory": "./export",
           "RetentionDays": 30
         }
       }' is not allowed to refer to a store path (such as '/nix/store/hk389ylvhy82v8f3w5pyjaajfavx3xdg-mattermost-server-6.3.3')

       at /nix/store/7pa8041q6j2s15ccm763lzcw4z8dxh59-source/nixos/modules/services/web-apps/mattermost.nix:9:19:

            8|
            9|   defaultConfig = builtins.fromJSON (builtins.replaceStrings [ "\\u0026" ] [ "&" ]
             |                   ^
           10|     (readFile "${pkgs.mattermost}/config/config.json")
(use '--show-trace' to show detailed location information)

Regarding the config.json file, wouldn't it be easier for the nixos module to just use the environment variables instead? The mattermost docs say these override settings from config.json anyway:

Starting from Mattermost v3.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.

Also, does anyone know why me pushing the rebase apparently led to github automatically closing the PR and someone else merging it before I could reopen it? Seems weird …

The PR now also says 0 files changed which seems wrong?

@stuebinm
Copy link
Contributor Author

stuebinm commented Mar 2, 2022

The PR now also says 0 files changed which seems wrong?

looks like I pushed the wrong branch 🙈

I'm not sure if I can reopen it now — github doesn't offer me a "reopen" button, and new commits in my repository don't seem to be showing up. Might open a new PR with the same content if I can't find another way …

@stuebinm
Copy link
Contributor Author

stuebinm commented Mar 2, 2022

new PR: #162479

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants