-
Notifications
You must be signed in to change notification settings - Fork 9
Base Configs Support - Phase 1 #455
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
base: main
Are you sure you want to change the base?
Conversation
}, | ||
} | ||
|
||
current := &imagecustomizerapi.Config{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try creating some configs without the artifacts path field the test would fail with null pointer exception at ResolveOverrideFields_
|
||
// ResolvedConfig represents a resolved config and chain of base configs. | ||
type ResolvedConfig struct { | ||
Config *imagecustomizerapi.Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't reuse the imagecustomizerapi.Config
type for the resolved/merged/overridden fields. Instead put all the resolved fields directly in ResolvedConfig
.
This will make it explicit which fields are actually resolved. That is, you won't have a bunch of empty values within the Config
field.
osPackages []OsPackage | ||
cosiBootMetadata *CosiBootloader | ||
|
||
baseConfigs []imagecustomizerapi.BaseConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store the full resolvedConfig
in ImageCustomizerParameters
. In the future, we can gradually move features from using imagecustomizerapi.Config
to using ResolvedConfig
instead.
} | ||
|
||
func (r *ResolvedConfig) resolveOverrideFields() { | ||
for _, cfg := range r.InheritanceChain { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, for interlaced operations and simple merges, following the InheritanceChain
ordering is probably fine.
Though, it's possible that for more complex merges, we may want to merge following the tree structure. So, it might be good to store the tree as well as the chain.
This PR introduces hierarchical configuration support in Image Customizer, allowing users to compose configs on top of one another via the .baseConfigs field.
Implement circular dependency detection
Create config inheritance chain resolution
include fields below:
Checklist