k8sgpt: 0.3.30 -> 0.3.41

[mrgiles]

Things done

This commit updates k8sgpt to release 0.3.41

Release: v0.3.41

Note: On my local system, the command nixpkgs-review passed the buildPhase but failed the checkPhase (trivy integration test). I wanted to try and see if I can run the nixpkgs-review command pointing to this PR, which is one of the options available. I will test this first with a draft PR.

This is the local build log file: k8sgpt.log.

• Built on platform(s)
  • [x] 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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• [ ] 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/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release 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.

---

Add a :+1: reaction to pull requests you find important.

[mrgiles]

The nixpkgs-review command using the PR also failed. Maybe somebody else can run nixpkgs-review on their system and see if it succeeds? Thanks.

[mrgiles]

After doing some troubleshooting and research focused on the build issue, here' what I got.

The error I got (it's in the attached log file):

Running phase: checkPhase
...
panic: Get "https://aquasecurity.github.io/helm-charts/index.yaml": dial tcp: lookup aquasecurity.github.io on [::1]:53: read udp [::1]:47967->[::1]:53: read: connection refused

This is related on how Go is resolving Internet host names. I tested several DNS client configurations, with and wothout stub resolver, with and without IPv6, etc, I couldn't figure out a working solution. So I forced Go to use the 8.8.8.8 resolver by adding a preBuildPhases section to the build file for my build tests and this solved the issue for me.

This is the code added, it's not included in this PR because this error might just happen on my system. But, if you think it needs to be included, I can add it.

  preBuildPhases = [ "setDNS" ];
  setDNS = ''
    export GODNS=8.8.8.8
    export GODEBUG=netdns=go
  '';

I'm attaching the k8sgpt.log here but basically, now it shows that the checkPhase runs successfully and it builds the k8sgpt binary:

buildPhase completed in 2 minutes 9 seconds
checkPhase completed in 2 minutes 0 seconds

Result of nixpkgs-review run on x86_64-linux 1

1 package built:

• k8sgpt

@developer-guy, @kranurag7, what do you recommend doing? I thought of opening an upstream issue and see if anybody else is having this issue but, when I tested it outside Nix, just using their Makefile, it worked. So a Nix issue might not be relevant to them. Thanks

[kranurag7]

I know nix sets CGO_ENABLED=1 out of the box and we are explicitly setting it to CGO_ENABLED = 0 so not sure if removing this would help here. Can you please check this out? Generally using the default is preferred which is CGO_ENABLED=1.

ref: https://nixos.org/manual/nixpkgs/stable/#var-go-CGO_ENABLED

[mrgiles]

Thanks for checking @kranurag7.

I tried with CGO_ENABLED = 1. The build failed very early on. It didn't even start the Running Phase:... portion, so no logs.

Applying `nixpkgs` diff...
error: patch failed: pkgs/applications/networking/cluster/k8sgpt/default.nix:13
error: pkgs/applications/networking/cluster/k8sgpt/default.nix: patch does not apply

[mrgiles]

I've also tried removing the CGO_ENABLED = 0; line to see if it made any difference but I've got the same result.

[mrgiles]

I'm going to move it to Ready for review, to see if it passes the automatic online build process, since it may just fail on my local system.

[kranurag7]

I'm going to move it to Ready for review, to see if it passes the automatic online build process, since it may just fail on my local system.

Thanks for working on it. Looks like the checks are passing. If it still doesn't work on your system which you can check by fetching the binary from CI cache then we can export the DNS specific variables which seems to be working on your system and won't be breaking for others as well.

One more request, can you please add yourself as maintainer here, at this point, you've really gone deep in order to fix the issue and understand it better than me.

[mrgiles]

Hi @kranurag7. I've run several tests on my system, including setting up different stub resolvers and DNS configurations based on the error that I get during the checkPhase of the package build but no luck, so far.

I'll open an issue upstream and ask the developers of k8sgpt. I've looked on their repo but couldn't find a similar issue. I 'd like to have all the facts before adding those variables to the nix file.

I'll continue working on this tomorrow, and I'll add myself as maintainer. Thanks.

[mrgiles]

I've been testing different solutions. I think that skipping the trivy test is less disruptive than introducing those Go variables that I proposed before. I can confirm that the k8sgpt binary builds locally and that it works, including the trivy integration.

I'm stuck now with failing new nixpkgs-review tests because of this PR that I've just found while looking for why my tests were failing. Once this is solved, I will re-run the build tests and post the results.

I didn't post the issue upstream because I tested building k8sgpt directly from their repo and it works, so it seems like it's a Nix packages build issue.

[mrgiles]

They solved the lisp-modules issue. Now the k8sgpt package builds fine.

See build log file: k8sgpt-x86_64-linux.log

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 348479

---

x86_64-linux

:white_check_mark: 1 package built:

• k8sgpt

[kranurag7]

Thank you for sticking through it @mrgiles I trust you and happy to get the changes in. 🥇

[kranurag7]

@NixOS/nixpkgs-merge-bot merge

[nixpkgs-merge-bot[bot]]

@kranurag7 merge not permitted (#305350): pkgs/applications/networking/cluster/k8sgpt/default.nix is not in pkgs/by-name/

[kranurag7]

looks like I cannot merge this patch, I'm sure nixpkgs maintainer will take another looks and get this changes in. Thanks again for spending your time on this during the weekend. I hope this works smooth now.

[mrgiles]

Hey @kranurag7, thanks for checking.

I tried to fix the merge error myself (based on the error that you got when merging) by moving the package to pkgs/by-name and made a mess. I left everything as it was before the latest change. Oops!

I hope the Nix Packages maintainer can fix this one.
Do you need to run the merge command again?

[kranurag7]

@NixOS/nixpkgs-merge-bot merge

[nixpkgs-merge-bot[bot]]

@kranurag7 merge not permitted (#305350): pkgs/applications/networking/cluster/k8sgpt/default.nix is not in pkgs/by-name/

[kranurag7]

Thanks for your work @mrgiles let's leave this to nixpkgs maintainer for merging it. I hope you get a good sleep now. This was quite a lot of debugging for a go package but kudos you made it to the end. :)

[mrgiles]

nixpkgs-merge-bot: merge not permitted pkgs/applications/networking/cluster/k8sgpt/default.nix is not in pkgs/by-name/

I'm giving the merge issue another try (I'm stubborn). Seems like all checks have passed, we should be able to merge. After merge, we'll still have to pass the Hydra tests. If that fails, then I'll rollback the latest changes. Thanks.

[kranurag7]

go for it @mrgiles 🚀

[mrgiles]

@NixOS/nixpkgs-merge-bot merge

[nixpkgs-merge-bot[bot]]

@mrgiles merge not permitted (#305350): pkgs/applications/networking/cluster/k8sgpt/default.nix is not in pkgs/by-name/ pkgs/top-level/all-packages.nix is not in pkgs/by-name/

[mrgiles]

Same thing... Oh, well. I'll rollback the changes. At least I gave a try. Thanks for checking @kranurag7

[kranurag7]

Same thing... Oh, well. I'll rollback the changes. At least I gave a try. Thanks for checking @kranurag7

Thank you Mr. Hacker. No worries at all, at this point, I'll say go to bed. It's late for you. I'm really worried about your health if you keep stretching like this. Let me try to get some help from other maintainers on this. :)

// cc @devusb (gentle ping) I saw you used the bot command to merge kubectl-gadget patch https://github.com/NixOS/nixpkgs/pull/348479 Is there some requirement that @mrgiles can follow to get this merged for him? He has been working for days on this. I'll really appreciate if you can give some guidance here.

[mrgiles]

I think I've found out why the PR merge run by the nixpks-merge-bot fails. The error message is quite obvious but I didn't understand why it didn't fail for other cases. Those cases were migrated by hand, not by the bot.

The key is in the bot's merging_strategy.py code. It basically states that any file that is part of the PR will be checked to see if it is a file present under the pkgs/by-name directory. If it isn't, it fails with a list of those offending files.

pkgs/applications/networking/cluster/k8sgpt/default.nix is not in pkgs/by-name/ pkgs/top-level/all-packages.nix is not in pkgs/by-name/

The reason it failed for my PR is that I've changed the old package file and I've also deleted its definition from the pkgs/top-level/all-packages.nix, so the bot detected those 2 changed files that aren't under pkgs/by-name and failed to merge.

The way the bot is coded is to only merge packages that didn't exist before. For existing packages, there is the manual process or an automated process that it's still being developed, I think.

I'd like to know if I make the changes to include this pkg under by-name, if they can still manually merge it. I've just asked in an existing Issue related to this topic, so let's see if we get some answers. Thanks.

[gepbird]

@mrgiles if you can, you should move this package to by-name manually (as you can see the automated migration is not done yet). But even if you do it in this PR, you won't be able to merge this PR as it touches files outside of by-name, and this is by design. Imagine an attacker changing one of his packages in by-name, and also adding malicious changes outside by-name, he shouldn't be able to merge that. In a future PR where k8sgpt is already in by-name and you make a version bump for example, you can use the nixpkgs-merge-bot to merge it yourself.

[mrgiles]

Thank you for your comments @gepbird! They helped me to better understand the process. Now I can see more clearly which of my assumptions were correct and which were wrong. I've restored the changes that I made before to move the package under by-name. Let's give this another try!

[emilazy]

I haven’t reviewed this PR but I just wanted to clarify a little:

In a future PR where k8sgpt is already in by-name and you make a version bump for example, you can use the nixpkgs-merge-bot to merge it yourself.

This only applies to automatic version bumps by @r-ryantm. Currently a committer’s approval is required for all human‐authored changes, so a PR like this one would not be eligible at this time even if the package was already in pkgs/by-name. Allowing package maintainers to merge automatic updates to pkgs/by-name packages is a very restrictive trial programme and can’t be used for anything other than trivial automatic version bumps.

[mrgiles]

I've renamed the commits as requested. Thanks for reviewing @gepbird

[mrgiles]

Let's see if this works. I've started it from scratch this time. Thanks.