Skip to content

Adds CNSPRCY service#870

Merged
eljamm merged 1 commit into
ngi-nix:mainfrom
ta1hia:cnsprcy-service
Jun 20, 2025
Merged

Adds CNSPRCY service#870
eljamm merged 1 commit into
ngi-nix:mainfrom
ta1hia:cnsprcy-service

Conversation

@ta1hia
Copy link
Copy Markdown
Contributor

@ta1hia ta1hia commented Apr 18, 2025

Adds a service module for CNSPRCY and a more fleshed out basic test. I also went ahead and reorganized the CNSPRCY project directory to follow the project template.

ngi-nix/projects#49

@ta1hia ta1hia force-pushed the cnsprcy-service branch 4 times, most recently from fc785bf to 7411eb5 Compare April 23, 2025 21:05
Comment thread projects/CNSPRCY/programs/cnsprcy/module.nix
Comment thread projects/CNSPRCY/default.nix
Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is shaping up nicely. Is it still a draft, @ta1hia, or is it ready for review?

Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/default.nix Outdated
description = "CNSPRCY service";
wantedBy = [ "multi-user.target" ];
after = [ "nss-user-lookup.target" ];
preStart = "printf 'n\n${cfg.hostname}\ny\n' | ${pkgs.cnsprcy}/bin/cnspr config init";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is hostname only used to be printed in the service logs? I don't think that's necessary as options need to be something that change the service's behaviour rather than for debugging. Although we may have options that control the service's own debugging settings.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eljamm the cnspr config init command kicks off an interactive prompt for generating the initial CNSPRCY config file and the printf feeds some responses to the prompt, including setting an identifier for the current machine.

i considered constructing the config from scratch during options setting, but i hit the need for interactively generating keys using the cnspr command later down the road anyway so this approach seemed less finicky to me just to get things going. let me know if you think otherwise!

regardless i should at the very least add a comment here!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eljamm the cnspr config init command kicks off an interactive prompt for generating the initial CNSPRCY config file and the printf feeds some responses to the prompt, including setting an identifier for the current machine.

Ah, I see. That's a really clever solution. From what I've seen, we can also use the config init flags to directly set the hostname with -n and we can even choose where to put the database file with -p, so we can put it directly under stateDir:

Usage: cnspr config init [OPTIONS]

Options:
  -i, --id <ID>      ID to use for this node (omit to generate a new ID)
  -n, --name <NAME>  Name for this node
  -p, --path <PATH>  Where to store the new database
  -h, --help         Print help

Don't know how to make the serve command use it, though.

i considered constructing the config from scratch during options setting, but i hit the need for interactively generating keys using the cnspr command later down the road anyway so this approach seemed less finicky to me just to get things going. let me know if you think otherwise!

Yeah, it's fine to dynamically create it like this for now. We can iterate on the module later on and generate the config using options. It should be fairly easy to do since the config file uses the TOML format, which is supported in nixpkgs (also see how it's used in modules).

Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
@ta1hia ta1hia force-pushed the cnsprcy-service branch from 324b54e to 1880a44 Compare May 1, 2025 16:24
@ta1hia ta1hia marked this pull request as ready for review May 1, 2025 16:24
@ta1hia ta1hia changed the title draft: cnsprcy service Adds CNSPRCY service May 1, 2025
@ta1hia ta1hia force-pushed the cnsprcy-service branch 2 times, most recently from 4d540ea to 22b6e15 Compare May 5, 2025 22:06
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
description = "CNSPRCY service";
wantedBy = [ "multi-user.target" ];
after = [ "nss-user-lookup.target" ];
preStart = "printf 'n\n${cfg.hostname}\ny\n' | ${pkgs.cnsprcy}/bin/cnspr config init";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eljamm the cnspr config init command kicks off an interactive prompt for generating the initial CNSPRCY config file and the printf feeds some responses to the prompt, including setting an identifier for the current machine.

Ah, I see. That's a really clever solution. From what I've seen, we can also use the config init flags to directly set the hostname with -n and we can even choose where to put the database file with -p, so we can put it directly under stateDir:

Usage: cnspr config init [OPTIONS]

Options:
  -i, --id <ID>      ID to use for this node (omit to generate a new ID)
  -n, --name <NAME>  Name for this node
  -p, --path <PATH>  Where to store the new database
  -h, --help         Print help

Don't know how to make the serve command use it, though.

i considered constructing the config from scratch during options setting, but i hit the need for interactively generating keys using the cnspr command later down the road anyway so this approach seemed less finicky to me just to get things going. let me know if you think otherwise!

Yeah, it's fine to dynamically create it like this for now. We can iterate on the module later on and generate the config using options. It should be fairly easy to do since the config file uses the TOML format, which is supported in nixpkgs (also see how it's used in modules).

@ta1hia ta1hia force-pushed the cnsprcy-service branch from 31bcbc1 to 8f922d6 Compare May 9, 2025 00:20
@ta1hia
Copy link
Copy Markdown
Contributor Author

ta1hia commented May 9, 2025

@eljamm thank you for the review! Regarding the config initialization/generation discussion, I included a TODO comment with next step suggestions but left the preStart approach as-is for this PR. I applied all the other suggestions you made. lmk if you have any other notes!

Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. Could you please clean up the history, next? You can either rebase or just git reset main and make new commits.

Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
Comment thread projects/CNSPRCY/services/cnsprcy/module.nix Outdated
@ta1hia ta1hia force-pushed the cnsprcy-service branch from 5a424c7 to 71e9526 Compare May 11, 2025 14:18
@ta1hia
Copy link
Copy Markdown
Contributor Author

ta1hia commented May 11, 2025

Thank you @eljamm! Just did the git rebase :)

@eljamm eljamm force-pushed the cnsprcy-service branch from 71e9526 to cd0331f Compare June 20, 2025 17:35
Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ta1hia, sorry for taking so long to get back to you on this and thanks for your great efforts!

@eljamm eljamm merged commit 2073daf into ngi-nix:main Jun 20, 2025
14 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Nix@NGI Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants