nix-community / authentik-nix

Nix flake with package, NixOS module and basic VM test for authentik. Trying to provide an alternative deployment mode to the officially supported docker-compose approach. Not affiliated with or officially supported by the authentik project [maintainer=@willibutz]
MIT License
74 stars 15 forks source link

Allow applying additional patches #27

Closed Ma27 closed 1 month ago

Ma27 commented 4 months ago

Not very pretty, but useful when having to apply local patches for the Python code. Can be done like this:

(self: super: {
  inherit (authentik-nix.lib.mkAuthentikScope {
    pkgs = self;
    extraPatches = [
      ./hackhack.patch
    ];
  }) authentikComponents;
})

This cannot be used currently for Go patches since I left it out for the gopkgs part intentionally due to the filterSource stuff since it's somewhat likely that a patch contains stuff that's filtered out by the filterSource expression. Not sure what to do about that.

cc @fpletz @WilliButz

quentinmit commented 3 months ago

Why not just remove the inherit authentik-src lines from components/default.nix? Then authentik-src could just be overridden in the scope, which would allow both different versions of Authentik as well as patches.

Ma27 commented 3 months ago

I'm not sure I understand? The authentik-src should be overridable already or do you mean putting it into the scope to allow overriding it for all components?

quentinmit commented 3 months ago

I'm not sure I understand? The authentik-src should be overridable already or do you mean putting it into the scope to allow overriding it for all components?

Yeah, right now you need to do something like authentikComponents = mapAttrs (_: c: c.override (old: { authentik-src = applyPatches { src = old.authentik-src; ... }; })) prev.authentikComponents to override it for each component separately.

It's already in the scope, it's just not being pulled from there for the packages that have inherit authentik-src

WilliButz commented 2 months ago

I'm not quite sure here. Wouldn't it be sensible to make authentik-src a top-level member of the custom scope and let the other components access it from there? I don't think it is necessary to apply patches to authentik-src per component, instead it should be fine to use one patched source for all components by default.

Thoughts on this? :)

quentinmit commented 2 months ago

I think my concrete suggestion would be to remove all the inherit lines inside callPackage in components/default.nix, and add inherit buildNapalmPackage mkPoetryEnv defaultPoetryOverrides authentikPoetryOverrides to the top level of the scope.

Then all the packages will get their arguments from the scope, and you can either override them globally with .overrideScope or you can override them for a specific package with .frontend.override.

Ma27 commented 2 months ago

I don't think it is necessary to apply patches to authentik-src per component, instead it should be fine to use one patched source for all components by default.

I think the problem with this is that other components access the source on eval-time resulting in IFD which I'd like to avoid. Can probably solved with e.g. dynamic derivations or if flakes ever get the ability to apply patches to inputs.

Modifying the source path itself is equivalent to just overriding the flake input, so in that case we don't need a change.

So to me it seems either patches being applied everywhere or (for now) or the status quo. Since you're the maintainer, I'd leave this up to you :)

Then all the packages will get their arguments from the scope, and you can either override them globally with .overrideScope or you can override them for a specific package with .frontend.override.

Sounds reasonable to me. If @WilliButz agrees, I'd update the PR for that, but let's discuss the patching part first.

WilliButz commented 2 months ago

I don't think it is necessary to apply patches to authentik-src per component, instead it should be fine to use one patched source for all components by default.

I think the problem with this is that other components access the source on eval-time resulting in IFD which I'd like to avoid. Can probably solved with e.g. dynamic derivations or if flakes ever get the ability to apply patches to inputs.

Good point! I wouldn't want to introduce IFD for this.

Modifying the source path itself is equivalent to just overriding the flake input, so in that case we don't need a change.

So to me it seems either patches being applied everywhere or (for now) or the status quo. Since you're the maintainer, I'd leave this up to you :)

I think it should be fine to apply patches for the components that need patching, of which there is only one at the moment.

Then all the packages will get their arguments from the scope, and you can either override them globally with .overrideScope or you can override them for a specific package with .frontend.override.

Sounds reasonable to me. If @WilliButz agrees, I'd update the PR for that, but let's discuss the patching part first.

Yes, aside from the issue with applying patches to the authentik-src, I think the suggestion from @quentinmit is still valuable. Making all those attributes direct members of the scope seems cleaner than the current approach as it would allow users to override authentik-src for all components, either when instantiating a custom scope (with IFD), or going via flake input override (without IFD), as mentioned by @Ma27

Ma27 commented 1 month ago

Yes, aside from the issue with applying patches to the authentik-src, I think the suggestion from @quentinmit is still valuable\

Agreed. I'm not in a need for such a change currently and taking care of that has no real priority for me. To make this explicit, I'll close this PR now, sorry.