kapicorp / kapitan

Generic templated configuration management for Kubernetes, Terraform and other things
https://kapitan.dev
Apache License 2.0
1.82k stars 199 forks source link

Prune empty file behaviour #491

Open ademariag opened 4 years ago

ademariag commented 4 years ago

Describe the bug/feature

NOTE: Offline chat with team decided that the above behaviour is acceptable. Reporting for posterity.

With the jsonnet output, so we have 2 ways to generate files: let’s call them single file and multi file mode.

relevant code

single file

a jsonnet file called main.jsonnet that produces a content that "is not a json dict" will add that content in a file main.yml

Note: this condition (not a dict) can never normally be invoked, and it only happens in the rare situation where prune is enabled AND the produced json file is

{}

resulting into a Null object. In this case, we take the filename of the jsonnet itself: in my case, main.yml, and generate it as an empty file.

multi file mode (default mode)

a jsonnet file that produces a dict of values, for instance

{ 
  "manifest": { 
    foo: true
  }
}

And we write out the “manifest.yml” file with content:

{
  "foo": true
}

If the content of the "manifest" key is empty, AND prune is enabled, kapitan will NOT generate the file

My points:

For a real life example where this behaviour is a problem, see:

https://github.com/kapicorp/kapitan-reference/blob/bc92712591c23ba46f342ef11564586fa8c58e85/components/generators/ingresses/main.jsonnet#L17

and in general with every generator

To Reproduce Steps to reproduce the behavior:

  1. Create files as described in the above examples
  2. run kapitan compile
  3. observe behaviour as described

Expected behavior

Single mode file should EITHER not exist, or be consistent with multi file and not create an empty file when prune is on.

Additional context Offline chat with team decided that the above behaviour is acceptable.

th31nitiate commented 4 years ago

I had a look at this locally. When prune is set to true the following function is called prune_empty(). This is what then performs pruning by updating the output object to a null value.

It is quite difficult to understand because the function does some recursion to check the dict. I believe the null object or empty dict returned by prune_empty() prevents if not isinstance(output_obj, dict): form executing, thus the file is no longer generated when this option is enabled.

To get the described expected behaviour you would need to to make prune the default or modify this function if not isinstance(output_obj, dict):

On issue #443 Adrian states that the issue should be resolved in the latest release.

ademariag commented 4 years ago

thanks @th31nitiate , issue #443 solves the crash, but highlights the behaviour here explained. The workaround for this new issue is the same as the one I had for #443, https://github.com/kapicorp/kapitan-reference/blob/bc92712591c23ba46f342ef11564586fa8c58e85/components/generators/ingresses/main.jsonnet#L17

th31nitiate commented 4 years ago

I see what you mean. The repository looks like a great Kapitan reference.

It would be a difficult one to solve without changing the default behaviour in my opinion, but I haven't really taken time to look over the Kapitan code.

I was wondering if you know how to run Kapitan locally while developing without having to compile the binary ?

I know that was a bit of silly question. I managed to find out how to execute specific functions and classes by looking at these tests examples. The reason I was asking was because of the default runtime parameters.

uberspot commented 4 years ago

The fix for this should be straightforward. Swapping the order of the if block starting here https://github.com/deepmind/kapitan/blob/master/kapitan/inputs/jsonnet.py#L52 with the next one.

The question this issue is trying to answer is whether users expect prune to also not-generate empty files in single file mode as it does for multiple file mode or not.

What would users expect behavior-wise?

ademariag commented 4 years ago

TBH I looked at it and I don't think you can activate the single mode at all!

for JSON to be valid, the file needs to be:

{ "key": "value"}

or even just:

{}

which in both cases would be a "dict", which would make it ignore the if not isinstance(output_obj, dict) check.

This means that the only moment that check is evaluated is when, with prune: true, {} becomes NoneType.

Test

adding the following code

 56         print("output_obj is of type: ", type(output_obj))
 57         return

With Prune:

output_obj is of type:  <class 'NoneType'> <- prune makes it NoneType, and this triggers the "single file" mode

Without Prune:

output_obj is of type:  <class 'dict'> <- because it is {}

This mean that the infamous SINGLE FILE mode cannot ever be used in real cases.

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 1 year with no activity. Remove the stale label or comment if this issue is still relevant for you. If not, please close it yourself.