(#16856) Add support for Data-in-Modules#1821
Closed
hlindberg wants to merge 146 commits intopuppetlabs:masterfrom
Closed
(#16856) Add support for Data-in-Modules#1821hlindberg wants to merge 146 commits intopuppetlabs:masterfrom
hlindberg wants to merge 146 commits intopuppetlabs:masterfrom
Conversation
This is the initial check in of a type model, a factory and a type calculator that can answer question about types (assignable?, commonality), and infer type given an instance. The type model is required by the bindings model.
This is w.i.p and only somewhat tested code.
The infer method did not handle inference of a type (it resulted in a Ruby type with reference to the type class). This commit updates the type calculator with an infer of PObjectType. This means that all types, including the meta-type PType is typed as PType. Tests added. (This is all esoteric, but needs to be done correctly).
Without this it is more difficult to create a type for a class.
This adds w.i.p on a BindingsChecker - a validator of a bindings model. It contains: - Issue module - Checker/Validator - TypeFactory method to create a PCollectionType (used in checker) - Additions to Bindings model (multibind producers)
This commit adds two public methods to the TypeCalculator and a slew of more unit tests. It also changes the behavior of the assignable? method slightly to make it more stringent about types and instances. The new method instance?(t, o) answers the question "is the object o an instance of the Puppet Type t". This method is necessary since the assignable?(t, t2) method no longer infers the type of its second argument. Instead it will always assume that both arguments are either Puppet types or ruby classes and it will convert any ruby class into its corresponding Puppet Type using the new method for_ruby_class(c). This new method returns the Puppet Type for the given Ruby class. It will return PType for the ruby class 'Class' and 'Array' and 'Hash' are assumed to be array of data and hash with literal keys and data values.
The name "type" reads better with the other method names. Later it will be able to produce the type of a PType (which plays the same role in the puppet typesystem).
The previous implementation did the wrong thing for classes that are mapped to distinct Puppet types. It now relays to the TypeCalculator.type when calling TypeFactory.ruby(o) with a class (as this is the most common case).
The Puppet Type System will treat all Ruby Integers as PIntegerType regardless of if they are Fixnum or Bignum since the boundary between those two depends on the architecture. A fixnum might be limited to 31 bits in which case anything larger than that will be bignum. A bignum may potentially be larger than 64 bit. Integers that large are currently not supported.
This implementation processes: - effective categories - bindings in layers and produces a Hash[Key, InjectorEntry] that can be fed to an Injector.
This is the first cut of the Binder which is configured from bindings data. Once configured it is used to instantiate an Injector (t.b.d).
… class This is in preparation for creating a BindingsValidatorFactory. The common super class removes the need for copying behavior from the current ValidatorFactory.
…oadable With this commit, all pops related files are required in the pops.rb file. The types.rb was removed since it was no longer needed.
In this commit, the injector and binder are completed up to the point where a binder can be configured, and injector created based on the binder, and a single value be looked up! Adds binder and injector spec tests with a few test cases.
In order to support custom producers it was not enough to just return a lambda as the producer. This is now instead an instance of Puppet::Pops::Binder::Producer.
This adds support for a custom producer. In the model the producer is defined with a ProducerProducerDescriptor which references another ProducerDescriptor responsible for producing an instance of Puppet::Pops::Binder::Producer (typically an InstanceProducerDescriptor that references a class derived from Pupet::Pops::Binder::Producer). This enables custom producers with custom instantiation protocol. This is of value when each production requires a different set of arguments, or where the producer needs to be setup (establish a connection or similar).
This adds more injector tests and corrects several small things to make them run. key factory was given a method to return its type calculator, when a key factory is used, it is important to use the same type calculator.
…ding model This commit adds the initial implementation of the following classes: BindingsChecker Checks the validity of a bindings model BindingIssues Contains all declaration of all issues that the BindersChecker can report BindingsLabelProvider String representation of all binder model elements BindingsValidatorFactory The factory for creating the checker and associate it with the correct support classes. Unit tests are provided for the checker and factory. They do in turn use the label provider and issue.
lookup_producer was overridden by a method with the same name that created a lookup-producer instead of looking one up. There were typos in the lookup_producer that were also fixed.
This adds the ability to optionally give a block to or just result. The lookup returns what the block returns if block is present. This is good because it make it easier to handle default values when a value is missing, or to conveniently invoke a custom producer method without requiring a temporary variable. Includes tests.
This producer is useful to perform a first found lookup sequence which can be used to describe a lookup with default value and similar use cases.
Also adds category.value to the Category label on the format "Category 'categorization/value' e.g. Category 'environment/dev'.
Nil does not work when returned to puppet logic.
There was a design flaw in the model that made it impossible to bind a multibind in another. This prevented setting up complex structures inside the data. This fix makes any binding be a potential MultibindingContribution by simply setting its multibind_id.
This helps to detect what is wrong with the issue. Before this it was not possible to see which issue caused the exception and debugging was always required. Now the output contain the issue code.
This also makes it possible to leave out sections of the hash to get the default values.
Contains small API adjustments wrt. using varargs intead of requiring that arguments were wrapped in an array. Also adds ability to use a ruby lambda for producer transformer option.
…ash key. The scheme handler support is now done via injection. This is good because it reuses existing functionality rather than providing yet another way of loading user code. A special injector only used while creating the real injector is created (called boot_injector). The scheme handlers are now broken out into separate ruby files (they were earlier in the BindingsComposer). The broken out handlers are now lazily loaded by an injector. A flaw was found in the type system; types where not usable as hash keys. This commit also fixes this by adding hash and eql methods to the types.
This makes it possible to specify scheme_handlers and hiera_backends by binding scheme/symbolic name to a runtime class name. These are loaded by the injector when required. Adds tests, and fixes issues.
…n to load. A setting of nil is the default which optionally loads $confdir/binder_config.yaml, and a default internal config if this file is missing. If set it must point to an existing file.
Also adds tests for some exceptional cases. Adds ability to specify override and abstract in the BindingsFactory.
The confdir-hiera scheme no has logic to skip a hiera-1 hiera.yaml if it cannot be loaded. This commit also fixes a problems with re-registration of injector boot extensions. They were made in a static way causing the next run to again register a duplicate set of bindings which caused a conflicting bindings error. The boot bindings (for the request) should not be system wide, and should therefore be associated with the environment and overwritten on each request. The new mechanism changes how the bindings from the binder_conf.yaml gets contributed to the boot injector.
The problem was that the bindings system switched to the future parser when evaluating puppet logic by modifying the Puppet[:parser] setting back and forth. This caused the compiler to (somewhere) be made aware of a change "in the environment" - this caused tests to fail as the environment was cleared as a consequence. The fix is a simple change, the ast_transformer now creates an ArithmeticOperator2 instead of ArithmeticOperator. Only future parser creates those instances. The ArithmeticOperator2 naturally allows concatenation. With that change, all switching of Puppet[:parser] could be removed. This is a much better solution because the settings are kept unchanged.
The Puppet Binder a.k.a "Data-in-Modules" is based on Rgen, and is required to be able to use the data-in-module/hiera2 features. The setting :binder must be set to true, or :parser set to 'future' for the bindings system, data-in-modules, and hiera2 to be in effect. The bindings system is now "booted" by the compiler on demand if the settings dictate this. Implicit lookup of undefined class parameters will not use injector unless active. Explicit lookup using lookup will raise an error unless the bindings system is active. Tests toggle the activation
In Ruby 1.8.7, the YAML implementation does odd things on syntax errors in yaml source input (it is not reported and ends up being interpted the wrong way). The test that checks for YAML syntax error is now modified to handle RUBY 1.8.7 braindead YAML by checking for a different issue code from the validator.
…luated When the scope gets a request for e.g. ::fqdn it goes on a search along the milky way galaxy to find the scope for the class named "" (which is the special "top scope class"), as the compiler is evaluating expressions before having evaluated all of the things that eventually contributes variables to the top scope, it can not find the scope for the class named "". The class "" still exists at this point, but there is no maping from name "" to the scope (and it cannot be registered until it is fully evaluated since the registration also means that the class has been evaluated (CATCH-22). To overcome the problem, scope now identifies the special corner case of request is for ""::name, "" exists, but there is no scope for it, and the request for ""::name was made to topscope, it is safe to then perform the lookup as a local scope lookup. At the same time this is fixed, there is a performance difference between lookup of $::fqdn, and $fqdn and the defaults is changed to use $fqdn.
… bindings When bindings are not activated (with --binder, or --parser future), the lookup function tried to use classes in the Pops name space and fails on uninitialized constant. This fix changes that to raise a more meaningful message.
A series of small fixes to readability in comments and logic in bindings_composer.
…er_config Before this fix, the information was Syntax error line, pos, but no file. This fix uses the file (or a reference to the ruby file that defined the defaults), and sets that in a syntax error exception if file information is not already set.
|
CLA signed by all contributors. |
The lookup function test was missing because the setting --binder was missing, and lookup only works when this feature is turned on).
YAML in 2.0.0 is a different implementation and throws a different type of SyntaxError than in earlier Rubies. This fix uses the same mechanism as the Yaml Ruby indirector for handling exceptions from YAML loading/parsing.
Contributor
|
Hmm, I'm unsure about the API for hiera2 backends. I think it would make more sense to have a lookup(path, key) instead of read_data(directory, file_name), then just initialize the backend with the correct path or something. It seems very file based backend centric. For example I'm not really sure how to implement either of my two hiera1 backends using this API, But maybe I'm understanding it wrong? |
Contributor
Author
|
The hiera2 backends are there to match the file/path based binding-provider schemes (module-hiera, and confdir-hiera). There are several options wrt. integration; hiera2 backend (typically file based data), scheme (open ended, get a set of bindings for the request), and producer (produce an object/value for a key), and naturally also using the Ruby bindings internal DSL support (which is the easiest to use). Update: Found the psql one, looks like a candidate to do with a scheme, read everything in the table and translate into a bindings model. Bake the connection related information into the URI. Update again: Also found the puppetdb query support. It is different in that it looks up individual values. This behaves like a Producer. (The first version of 'data-in-modules' have no way to define which producer to use when using hiera2 json/yaml format. An extended format would be needed that interprets special characters in the key (like you are already doing), or by having a value that has a special encoding to make it be recognized as a query. Happy to help out with how to implement; and also to see how the base implementation can be extended to make integrations like these more convenient. At the moment, support for puppetdb-query is most easily done with bindings described in Ruby i.e. using the ´Puppet::Bindings.newbindings(name) { }` since it is possible to just call out to get the value for the keys you want bound; no new scheme required, no new bckend, etc. just supply the bindings in Ruby. |
Contributor
|
hmm, if you were to supply the bindings using ruby, where do you actually do that? I mean there's no ruby files in the hiera-data directory. I'm probably missing something here, might be more clear with some more usage documentation than just the code :) But yeah, essentially the issue I had was how to make backends that cannot enumerate all keys up front. Could imagine other backends where that might be hard, like DNS. |
Contributor
Author
|
@dalen yes, it will be clearer after documentation is made available. Will deal with that asap. |
|
Closing this PR in favor of PR #1827 which adds a few cleanup-ish commits. Please carry on with review comments there. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This (mega) PR consists of support for "Data-in-Modules" (ARM-9) which consists in turn of Hiera-2 and Puppet Bindings (as described in ARM-8).