Skip to content

Conversation

@Shawn8901
Copy link
Contributor

@Shawn8901 Shawn8901 commented Jul 13, 2025

Adds vlagent to nixpkgs which is an agent to collect logs from compatible sources and forward them to a victorialogs instance.

With vlagent one can protect a victorialogs instance with basic auth and insert logs via non accessible endpoints from localhost to vlagent without any authentication.

That results in having a quite easy setup without tinkering with mTLS.

The pattern to build vlagent is similar to what is done for vmagent.

Requires #423260 to be merged before and i am planning to rebase this branch after merge.

I am already creating it as draft to gather some early feedback on the PR.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@Shawn8901
Copy link
Contributor Author

@sinavir fyi as you have requested vlagent in #423260 (comment)

@Shawn8901 Shawn8901 force-pushed the vlagent branch 2 times, most recently from 7e30d9c to 457afa4 Compare July 13, 2025 22:18
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 13, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 13, 2025
@Shawn8901 Shawn8901 force-pushed the vlagent branch 2 times, most recently from 7ff9f87 to 8472249 Compare July 14, 2025 17:55
@Shawn8901 Shawn8901 marked this pull request as ready for review July 14, 2025 17:56
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Jul 14, 2025
@NyCodeGHG
Copy link
Member

I'm not sure if an extra test is worth it for the remote write, I don't have a strong opinion but it could be moved into the other test with subtests so that it tests both scenarios, otherwise looks good to me

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 14, 2025
@Shawn8901
Copy link
Contributor Author

Shawn8901 commented Jul 14, 2025

yeah i can combine the two scenarios and write one from server machine and keep the remote one, ill take a look.

I was first thinking about that, but dropped initially that idea as it might then be hard to tell wether vlagent, journald upload or victorialogs is broken.
But on the other hand you are right that it might not be worth to keep it as two separat tests.

edit: ah okay i remember again another reason i have choosen 2 tests.
I wanted to include basic auth into the testing, as that is the main reason i am interested into vlagent.
But you can not insert data with systemd-journald-upload in case the victorialogs instance has basic auth enabled.
So i would kinda prefer to keep 2 tests, as i find both upload scenarios are something that we should test for.

@Shawn8901 Shawn8901 marked this pull request as draft July 15, 2025 20:15
@Shawn8901
Copy link
Contributor Author

Drafting, seems to have broken something on the latest changes, have to analyse it.

@Shawn8901 Shawn8901 force-pushed the vlagent branch 3 times, most recently from 73562ea to ef1ec23 Compare July 15, 2025 22:19
@Shawn8901 Shawn8901 marked this pull request as ready for review July 15, 2025 22:19
@Shawn8901
Copy link
Contributor Author

So i fixed it again. I hopefully found an acceptable inbetween solution.

I am now using escapeShellArgs for the "well-defined" parts and add escapeSystemdExecArgs or all extra arguments.

@Shawn8901 Shawn8901 requested a review from NyCodeGHG July 15, 2025 22:22
Copy link

@sinavir sinavir left a comment

Choose a reason for hiding this comment

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

Looks nice to me, I think maybe one could add maxDiskUsagePerUrl options because it is useful in my opinion but since we have extraArgs, it is just a suggestion

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jul 17, 2025
@Shawn8901 Shawn8901 force-pushed the vlagent branch 6 times, most recently from a8e8f32 to c105a76 Compare July 18, 2025 17:26
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5704

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5731

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5770

@SuperSandro2000 SuperSandro2000 merged commit b146c51 into NixOS:master Aug 17, 2025
26 of 27 checks passed
@Shawn8901 Shawn8901 deleted the vlagent branch August 18, 2025 05:20
@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: tests This PR has tests 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants