From 7f85002ec004d0f4bfe5023598047567273fec71 Mon Sep 17 00:00:00 2001 From: John Soo Date: Mon, 13 Mar 2023 16:44:29 -0700 Subject: [PATCH 1/5] nixos/minio: format with nixpkgs-fmt --- nixos/modules/services/web-servers/minio.nix | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/nixos/modules/services/web-servers/minio.nix b/nixos/modules/services/web-servers/minio.nix index 1a9eacb431b3..b2d48f961f14 100644 --- a/nixos/modules/services/web-servers/minio.nix +++ b/nixos/modules/services/web-servers/minio.nix @@ -60,7 +60,7 @@ in ''; }; - rootCredentialsFile = mkOption { + rootCredentialsFile = mkOption { type = types.nullOr types.path; default = null; description = lib.mdDoc '' @@ -98,7 +98,7 @@ in systemd.tmpfiles.rules = [ "d '${cfg.configDir}' - minio minio - -" - ] ++ (map (x: "d '" + x + "' - minio minio - - ") cfg.dataDir); + ] ++ (map (x: "d '" + x + "' - minio minio - - ") cfg.dataDir); systemd.services.minio = { description = "Minio Object Storage"; @@ -110,9 +110,10 @@ in User = "minio"; Group = "minio"; LimitNOFILE = 65536; - EnvironmentFile = if (cfg.rootCredentialsFile != null) then cfg.rootCredentialsFile - else if ((cfg.accessKey != "") || (cfg.secretKey != "")) then (legacyCredentials cfg) - else null; + EnvironmentFile = + if (cfg.rootCredentialsFile != null) then cfg.rootCredentialsFile + else if ((cfg.accessKey != "") || (cfg.secretKey != "")) then (legacyCredentials cfg) + else null; }; environment = { MINIO_REGION = "${cfg.region}"; From e30469a37bfdd898b0f359c758c18bc00167cbaf Mon Sep 17 00:00:00 2001 From: John Soo Date: Mon, 13 Mar 2023 16:50:04 -0700 Subject: [PATCH 2/5] nixos/minio: activate/restart service on credentials path changes Otherwise the `minio.service` service will fail either: * with a message that the EnvironmentFile does not exist * or silently with potentially stale credentials --- nixos/modules/services/web-servers/minio.nix | 73 ++++++++++++++------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/nixos/modules/services/web-servers/minio.nix b/nixos/modules/services/web-servers/minio.nix index b2d48f961f14..a1695e6a1edf 100644 --- a/nixos/modules/services/web-servers/minio.nix +++ b/nixos/modules/services/web-servers/minio.nix @@ -96,30 +96,59 @@ in config = mkIf cfg.enable { warnings = optional ((cfg.accessKey != "") || (cfg.secretKey != "")) "services.minio.`accessKey` and services.minio.`secretKey` are deprecated, please use services.minio.`rootCredentialsFile` instead."; - systemd.tmpfiles.rules = [ - "d '${cfg.configDir}' - minio minio - -" - ] ++ (map (x: "d '" + x + "' - minio minio - - ") cfg.dataDir); + systemd = lib.mkMerge [{ + tmpfiles.rules = [ + "d '${cfg.configDir}' - minio minio - -" + ] ++ (map (x: "d '" + x + "' - minio minio - - ") cfg.dataDir); - systemd.services.minio = { - description = "Minio Object Storage"; - after = [ "network-online.target" ]; - wantedBy = [ "multi-user.target" ]; - serviceConfig = { - ExecStart = "${cfg.package}/bin/minio server --json --address ${cfg.listenAddress} --console-address ${cfg.consoleAddress} --config-dir=${cfg.configDir} ${toString cfg.dataDir}"; - Type = "simple"; - User = "minio"; - Group = "minio"; - LimitNOFILE = 65536; - EnvironmentFile = - if (cfg.rootCredentialsFile != null) then cfg.rootCredentialsFile - else if ((cfg.accessKey != "") || (cfg.secretKey != "")) then (legacyCredentials cfg) - else null; + services.minio = { + description = "Minio Object Storage"; + after = [ "network-online.target" ]; + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + ExecStart = "${cfg.package}/bin/minio server --json --address ${cfg.listenAddress} --console-address ${cfg.consoleAddress} --config-dir=${cfg.configDir} ${toString cfg.dataDir}"; + Type = "simple"; + User = "minio"; + Group = "minio"; + LimitNOFILE = 65536; + EnvironmentFile = + if (cfg.rootCredentialsFile != null) then cfg.rootCredentialsFile + else if ((cfg.accessKey != "") || (cfg.secretKey != "")) then (legacyCredentials cfg) + else null; + }; + environment = { + MINIO_REGION = "${cfg.region}"; + MINIO_BROWSER = "${if cfg.browser then "on" else "off"}"; + }; }; - environment = { - MINIO_REGION = "${cfg.region}"; - MINIO_BROWSER = "${if cfg.browser then "on" else "off"}"; - }; - }; + } + + (lib.mkIf (cfg.rootCredentialsFile != null) { + services.minio.unitConfig.ConditionPathExists = cfg.rootCredentialsFile; + + paths.minio-root-credentials = { + wantedBy = [ "multi-user.target" ]; + + pathConfig = { + PathChanged = [ cfg.rootCredentialsFile ]; + Unit = "minio-restart.service"; + }; + }; + + services.minio-restart = { + description = "Restart MinIO"; + + script = '' + systemctl restart minio.service + ''; + + serviceConfig = { + Type = "oneshot"; + Restart = "on-failure"; + RestartSec = 5; + }; + }; + })]; users.users.minio = { group = "minio"; From 4400f7d72ff5ce35d3be07cb7fd6906f5bcb4c9c Mon Sep 17 00:00:00 2001 From: John Soo Date: Mon, 13 Mar 2023 18:00:43 -0700 Subject: [PATCH 3/5] nixosTests/minio: make sure editing root credentials restarts minio --- nixos/tests/minio.nix | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/nixos/tests/minio.nix b/nixos/tests/minio.nix index ad51f738d490..bbea6f523f34 100644 --- a/nixos/tests/minio.nix +++ b/nixos/tests/minio.nix @@ -18,6 +18,14 @@ let sio.seek(0) minioClient.put_object('test-bucket', 'test.txt', sio, sio_len, content_type='text/plain') ''; + rootCredentialsFile = "/etc/nixos/minio-root-credentials"; + credsPartial = pkgs.writeText "minio-credentials-partial" '' + MINIO_ROOT_USER=${accessKey} + ''; + credsFull = pkgs.writeText "minio-credentials-full" '' + MINIO_ROOT_USER=${accessKey} + MINIO_ROOT_PASSWORD=${secretKey} + ''; in { name = "minio"; meta = with pkgs.lib.maintainers; { @@ -28,10 +36,7 @@ in { machine = { pkgs, ... }: { services.minio = { enable = true; - rootCredentialsFile = pkgs.writeText "minio-credentials" '' - MINIO_ROOT_USER=${accessKey} - MINIO_ROOT_PASSWORD=${secretKey} - ''; + inherit rootCredentialsFile; }; environment.systemPackages = [ pkgs.minio-client ]; @@ -41,7 +46,15 @@ in { }; testScript = '' + import time + start_all() + # simulate manually editing root credentials file + machine.wait_for_unit("multi-user.target") + machine.copy_from_host("${credsPartial}", "${rootCredentialsFile}") + time.sleep(3) + machine.copy_from_host("${credsFull}", "${rootCredentialsFile}") + machine.wait_for_unit("minio.service") machine.wait_for_open_port(9000) From 29523c05e444e8f56e1f9ebed6a85b2168f80916 Mon Sep 17 00:00:00 2001 From: John Soo Date: Mon, 13 Mar 2023 18:01:16 -0700 Subject: [PATCH 4/5] nixosTests/minio: format with nixpkgs-fmt --- nixos/tests/minio.nix | 85 ++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/nixos/tests/minio.nix b/nixos/tests/minio.nix index bbea6f523f34..ece4864f771c 100644 --- a/nixos/tests/minio.nix +++ b/nixos/tests/minio.nix @@ -1,5 +1,5 @@ -import ./make-test-python.nix ({ pkgs, ...} : -let +import ./make-test-python.nix ({ pkgs, ... }: + let accessKey = "BKIKJAA5BMMU2RHO6IBB"; secretKey = "V7f1CwQqAcwo80UEIJEjc5gVQUSSx5ohQ9GSrr12"; minioPythonScript = pkgs.writeScript "minio-test.py" '' @@ -19,53 +19,54 @@ let minioClient.put_object('test-bucket', 'test.txt', sio, sio_len, content_type='text/plain') ''; rootCredentialsFile = "/etc/nixos/minio-root-credentials"; - credsPartial = pkgs.writeText "minio-credentials-partial" '' + credsPartial = pkgs.writeText "minio-credentials-partial" '' MINIO_ROOT_USER=${accessKey} ''; - credsFull = pkgs.writeText "minio-credentials-full" '' + credsFull = pkgs.writeText "minio-credentials-full" '' MINIO_ROOT_USER=${accessKey} MINIO_ROOT_PASSWORD=${secretKey} ''; -in { - name = "minio"; - meta = with pkgs.lib.maintainers; { - maintainers = [ bachp ]; - }; - - nodes = { - machine = { pkgs, ... }: { - services.minio = { - enable = true; - inherit rootCredentialsFile; - }; - environment.systemPackages = [ pkgs.minio-client ]; - - # Minio requires at least 1GiB of free disk space to run. - virtualisation.diskSize = 4 * 1024; + in + { + name = "minio"; + meta = with pkgs.lib.maintainers; { + maintainers = [ bachp ]; }; - }; - testScript = '' - import time + nodes = { + machine = { pkgs, ... }: { + services.minio = { + enable = true; + inherit rootCredentialsFile; + }; + environment.systemPackages = [ pkgs.minio-client ]; - start_all() - # simulate manually editing root credentials file - machine.wait_for_unit("multi-user.target") - machine.copy_from_host("${credsPartial}", "${rootCredentialsFile}") - time.sleep(3) - machine.copy_from_host("${credsFull}", "${rootCredentialsFile}") + # Minio requires at least 1GiB of free disk space to run. + virtualisation.diskSize = 4 * 1024; + }; + }; - machine.wait_for_unit("minio.service") - machine.wait_for_open_port(9000) + testScript = '' + import time - # Create a test bucket on the server - machine.succeed( - "mc config host add minio http://localhost:9000 ${accessKey} ${secretKey} --api s3v4" - ) - machine.succeed("mc mb minio/test-bucket") - machine.succeed("${minioPythonScript}") - assert "test-bucket" in machine.succeed("mc ls minio") - assert "Test from Python" in machine.succeed("mc cat minio/test-bucket/test.txt") - machine.shutdown() - ''; -}) + start_all() + # simulate manually editing root credentials file + machine.wait_for_unit("multi-user.target") + machine.copy_from_host("${credsPartial}", "${rootCredentialsFile}") + time.sleep(3) + machine.copy_from_host("${credsFull}", "${rootCredentialsFile}") + + machine.wait_for_unit("minio.service") + machine.wait_for_open_port(9000) + + # Create a test bucket on the server + machine.succeed( + "mc config host add minio http://localhost:9000 ${accessKey} ${secretKey} --api s3v4" + ) + machine.succeed("mc mb minio/test-bucket") + machine.succeed("${minioPythonScript}") + assert "test-bucket" in machine.succeed("mc ls minio") + assert "Test from Python" in machine.succeed("mc cat minio/test-bucket/test.txt") + machine.shutdown() + ''; + }) From 740fea3eddfd1b6a8a1f65f32ae3bd0336370f14 Mon Sep 17 00:00:00 2001 From: John Soo Date: Thu, 16 Mar 2023 09:45:09 -0700 Subject: [PATCH 5/5] fixup! nixos/minio: activate/restart service on credentials path changes --- nixos/modules/services/web-servers/minio.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nixos/modules/services/web-servers/minio.nix b/nixos/modules/services/web-servers/minio.nix index a1695e6a1edf..21bec4f63a87 100644 --- a/nixos/modules/services/web-servers/minio.nix +++ b/nixos/modules/services/web-servers/minio.nix @@ -124,8 +124,11 @@ in } (lib.mkIf (cfg.rootCredentialsFile != null) { + # The service will fail if the credentials file is missing services.minio.unitConfig.ConditionPathExists = cfg.rootCredentialsFile; + # The service will not restart if the credentials file has + # been changed. This can cause stale root credentials. paths.minio-root-credentials = { wantedBy = [ "multi-user.target" ];