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.

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

fix ConfigMap data with leading underscore (#44) #45

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions modules/k8s.nix
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,24 @@ with lib; let
)
cfg.api.defaults);

moduleToAttrs = value:
if isAttrs value
then mapAttrs (_n: moduleToAttrs) (filterAttrs (n: v: v != null && !(hasPrefix "_" n)) value)
else if isList value
then map moduleToAttrs value
else value;
moduleToAttrs = objType: propertyPath: value:
if isAttrs value then
let
# Fix https://github.com/hall/kubenix/issues/44
Copy link
Owner

Choose a reason for hiding this comment

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

nit: I'd argue most of this should comment probably go in the commit message details but I'd rather too many comments than too few so don't let that stop you 🙂

# The check for names starting with leading '_' has been here since forever.
# Not sure where it makes sense, but it definitely doesn't make sense for ConfigMap -> data/binaryData
# => To get a minimal invasive fix we added `objType` and `propertyPath` to make `moduleToAttrs` "context-aware".
# This way we can allow names with leading underscore exactly for ConfigMap -> data/binaryData
allowLeadingUnderscore = objType.group == "core" && objType.version == "v1" && objType.kind == "ConfigMap" &&
(propertyPath == [ "data" ] || propertyPath == [ "binaryData" ]);
Copy link
Owner

Choose a reason for hiding this comment

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

The underscore isn't a valid base64 character so we shouldn't need to check binaryData, yeah?

filterName = name: allowLeadingUnderscore || !(hasPrefix "_" name);
filteredAttrs = filterAttrs (n: v: v != null && filterName n) value;
in
mapAttrs (_n: moduleToAttrs objType (propertyPath ++ [ _n ])) filteredAttrs
else if isList value then
map (moduleToAttrs objType propertyPath) value
else
value;

apiOptions = { config, ... }: {
options = {
Expand Down Expand Up @@ -536,7 +548,7 @@ in

kubernetes.objects = flatten (mapAttrsToList
(_: type:
mapAttrsToList (_name: moduleToAttrs)
mapAttrsToList (_name: moduleToAttrs type [ ])
(optionalHashedNames' cfg.api.resources.${type.group}.${type.version}.${type.kind} type.kind)
)
cfg.api.types);
Expand Down
1 change: 1 addition & 0 deletions tests/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ let
./k8s/defaults.nix
./k8s/order.nix
./k8s/submodule.nix
./k8s/configmap.nix
Copy link
Owner

Choose a reason for hiding this comment

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

💯

# TODO: `importYaml` uses IFD which fails during `nix flake check` as it evaluates
# for all systems, not only the current one: https://github.com/hall/kubenix/issues/12
# ./k8s/imports.nix
Expand Down
20 changes: 20 additions & 0 deletions tests/k8s/configmap.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{ config, kubenix, ... }:
let
configMapData = (builtins.head config.kubernetes.objects).data;
in
{
imports = [ kubenix.modules.test kubenix.modules.k8s ];

test = {
name = "k8s-simple";
description = "Test that ConfigMap data keys can have a leading underscore (https://github.com/hall/kubenix/issues/44)";
assertions = [
{
message = "leading underscore in ConfigMap key should be preserved";
assertion = configMapData == { _FOO = "_bar"; };
}
];
};

kubernetes.resources.configMaps.foo.data._FOO = "_bar";
}
Loading