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

New feature: Show data result #264

Closed
tuxmea opened this issue Jan 18, 2024 Discussed in #241 · 7 comments
Closed

New feature: Show data result #264

tuxmea opened this issue Jan 18, 2024 Discussed in #241 · 7 comments
Assignees

Comments

@tuxmea
Copy link
Member

tuxmea commented Jan 18, 2024

Discussed in #241

Originally posted by tuxmea December 22, 2023

Request

As a Puppet admin I want to be able to see the result of the lookup.

Solution

After selecting a key a new button shows up (or gets enabled) with title "result".
Upon clicking an overlay windows presents the complete result, based on lookup_options.

@tuxmea tuxmea added enhancement New feature or request community labels Jan 18, 2024
@bastelfreak
Copy link
Member

@oneiros
Copy link
Collaborator

oneiros commented Jan 24, 2024

I spent quite some time browsing puppet's docs and code and must admit, I am none the wiser as to how to implement this.

I think there are two general possibilities:

  1. Call puppets lookup function
  2. Re-implement the whole lookup feature from scratch

The trouble with 1. is that the code is very convoluted and complex. I tried hard to find some lower level abstractions we could use, but found nothing suitable. So my best bet would be to use what the CLI does (see link above). The biggest obstacle is creating a "scope" for the lookup. The CLI code sets this up quite sensibly and we could probably reuse a lot of this. But as the very last step, it compiles a catalog (https://github.com/puppetlabs/puppet/blob/3192f4aab419b221402f738ed1be90de0d969be5/lib/puppet/application/lookup.rb#L413). I think this might be the reason you need to execute the command as root on a puppet server.

Obviously, we cannot do this, so the only way out I currently see is mocking/stubbing a Puppet::Parser::Scope object and see how far this would get us. This is probably tedious and I cannot guarantee it will work.

Also, according to the documentation, lookup will not just query hiera data but use other sources as well (though it is not clear, what those other data sources actually are 🤷‍♂️). So it is possible that the result would show things otherwise not visible in HDM. I am not sure this is ideal.

The alternative is to build our own implementation of lookup. This may of course take a lot of effort. And no matter how well we implement this, it will never be 100% accurate. Also, we would probably learn why the original code is so complex 🙂

If we go down that route, it might make sense to tackle #84.

Sorry, I have no clear recommendation which approach would be best.

When it comes to 1., I can not guarantee 100% that it is even possible, though I think it might be. Also, I have no idea how probable it is to get surprising results from lookup, that do not reflect what is visible in HDM.

When it comes to 2., it feels a lot like reinventing the wheel. I am unsure about the effort involved and fear a multitude of (possibly subtle) bugs when comparing to the original lookup function.

I really wish there was a more clear definition of what "hiera" actually is (and is not). Back when it was a seperate gem, things were a lot simpler, but now there is actually no such thing as "hiera". There is only lookup which is deeply entangled with puppet internals and seems to do more than what hiera used to do.

Maybe this is an opportunity for us to make such a definition, informed by how people use hiera data in practice. But maybe that is just wishful thinking on my part.

@tuxmea
Copy link
Member Author

tuxmea commented Jan 25, 2024

Hi David,

I doubt that we need to use the Puppet lookup code as I assume that this code is far to complex.
What we can do depends on the value for lookup_options and the specified merge behavior and the data types.

  1. lookup_option: merge: first
  • Result: take the top most value
  1. lookup_option: merge: unique
  • Check that all occurences of a key (in each hierarchy) is of data type Array.
  • Result: array merge of all key occurences
  1. lookup_option: merge: deep
  • Check that all occurences of a key (in each hierarchy) is of data type Hash
  • Hash deep merge starting from lowest to top hierarchy
  • Please note that subkeys can be overwritten.

@oneiros
Copy link
Collaborator

oneiros commented Jan 26, 2024

Quick question: If I try to lookup a simple scalar value, e.g. an integer, but request a merge strategy of unique or deep, will this result in an error?

@bastelfreak
Copy link
Member

@oneiros no, that's allowed for unique: https://www.puppet.com/docs/puppet/8/hiera_merging#merge_behaviors

A unique merge (also called an array merge) combines any number of array and scalar (string, number, boolean) values to return a merged, flattened array with all matching values for a key. All duplicate values are removed. The lookup fails if any of the values are hashes. The result is ordered from highest priority to lowest.

For deep I'm not sure in the moment and need to test it.

oneiros added a commit that referenced this issue Jan 30, 2024
oneiros added a commit that referenced this issue Jan 31, 2024
oneiros added a commit that referenced this issue Jan 31, 2024
oneiros added a commit that referenced this issue Jan 31, 2024
oneiros added a commit that referenced this issue Jan 31, 2024
oneiros added a commit that referenced this issue Jan 31, 2024
oneiros added a commit that referenced this issue Feb 1, 2024
oneiros added a commit that referenced this issue Feb 1, 2024
oneiros added a commit that referenced this issue Feb 1, 2024
oneiros added a commit that referenced this issue Feb 1, 2024
oneiros added a commit that referenced this issue Feb 2, 2024
oneiros added a commit that referenced this issue Feb 16, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
oneiros added a commit that referenced this issue Mar 1, 2024
@tuxmea
Copy link
Member Author

tuxmea commented Mar 13, 2024

AFAIK we can close this issue as this is implemented.

@tuxmea
Copy link
Member Author

tuxmea commented Mar 14, 2024

Implemented. Closing

@tuxmea tuxmea closed this as completed Mar 14, 2024
@rwaffen rwaffen added skip-changelog and removed enhancement New feature or request community labels Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants