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.

Sign up for GitHub

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

Jump to bottom

hp-ams: init at 2.8.3 #129423

Closed
wants to merge 1 commit into from
Closed

hp-ams: init at 2.8.3 #129423

wants to merge 1 commit into from

Conversation

RealityAnomaly
Copy link
Member

@RealityAnomaly RealityAnomaly commented Jul 6, 2021 •
edited

Motivation for this change

Adds support for the HP Agentless Management Service (for HP/HPE ProLiant servers) which interfaces with the iLO Management Processor to collect rich information and SNMP packets from the host OS.

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/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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.

@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/555

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 129423 at 49281390 run on x86_64-linux 1

2 packages built successfully:
  • hp-ams
  • nixos-install-tools
1 suggestion:
  • warning: missing-phase-hooks

    installPhase should probably contain runHook preInstall and runHook postInstall.

    Near pkgs/os-specific/linux/hp-ams/default.nix:18:3:

       |
18 |   installPhase = ''
   |   ^

@roberth
Copy link
Member

roberth commented Jul 7, 2021

This introduces generic code for systemd hardening. A review from @NixOS/systemd will be greatly appreciated.

@flokli
Copy link
Contributor

flokli commented Jul 7, 2021

There already has been another approach on adding certain sandboxing levels/hardening profiles in #87661, and I still think we shouldn't ship "sandboxing levels", but do this granular and per service - also to avoid sudden breakages if there's a new sandboxing options and we want to add it to this list.

Please apply that list explicitly to the hp-ams service unit, instead of generalizing it.

@RealityAnomaly
Copy link
Member Author

RealityAnomaly commented Jul 7, 2021 •
edited

Alright, I have done this.

@stale
Copy link

stale bot commented Jan 6, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 6, 2022
@roberth
Copy link
Member

roberth commented Jan 6, 2022

Apologies for the late response. Please move the release notes over to the upcoming release; then this can be merged.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 6, 2022
flokli
flokli reviewed Jan 6, 2022
View reviewed changes
nixos/modules/services/hardware/hp-ams.nix
Comment on lines +94 to +95
# fake this for amsHelper, otherwise it will segfault
environment.etc."debian_version".text = "10.9";
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding this to /etc/debian_version, can we use BindPaths= or use libredirect (see nixos/modules/services/networking/pppd.nix) to only make this visible for hp-ams?

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 10, 2022
SuperSandro2000
SuperSandro2000 reviewed Jan 27, 2023
View reviewed changes
nixos/doc/manual/release-notes/rl-2111.section.md
@@ -14,6 +14,8 @@ In addition to numerous new and upgraded packages, this release has the followin

- [geoipupdate](https://github.com/maxmind/geoipupdate), a GeoIP database updater from MaxMind. Available as [services.geoipupdate](options.html#opt-services.geoipupdate.enable).

- [hp-ams](https://buy.hpe.com/uk/en/software/server-management-software/server-ilo-management/ilo-management-engine/hpe-agentless-management/p/5219980), the HP Agentless Management Service for ProLiant servers. Available as [services.hardware.hp-ams](options.html#opt-services.hardware.hp-ams.enable).
Copy link
Member

Choose a reason for hiding this comment

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

This should be moved to the next release

pkgs/os-specific/linux/hp-ams/default.nix
Comment on lines +19 to +20
mkdir -p $out
dpkg -x $src $out
Copy link
Member

Choose a reason for hiding this comment

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

This should be done in unpackPhase

pkgs/os-specific/linux/hp-ams/default.nix
version = "2.8.3";

src = fetchurl {
url = "http://downloads.linux.hpe.com/SDR/repo/mcp/debian/pool/non-free/hp-ams_2.8.3-3056.1ubuntu16_amd64.deb";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
url = "http://downloads.linux.hpe.com/SDR/repo/mcp/debian/pool/non-free/hp-ams_2.8.3-3056.1ubuntu16_amd64.deb";
url = "http://downloads.linux.hpe.com/SDR/repo/mcp/debian/pool/non-free/hp-ams_${version}-3056.1ubuntu16_amd64.deb";

pkgs/os-specific/linux/hp-ams/default.nix

dontUnpack = true;
dontBuild = true;
dontStrip = true;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dontStrip = true;

or does this break things?

@SuperSandro2000
Copy link
Member

Closing this due to no response from the author in the last year.

@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Jan 27, 2023
@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@edolstra edolstra Awaiting requested review from edolstra

@infinisil infinisil Awaiting requested review from infinisil

@nbp nbp Awaiting requested review from nbp

@ryantm ryantm Awaiting requested review from ryantm

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@RealityAnomaly @nixos-discourse @r-rmcgibbo @roberth @flokli @SuperSandro2000