Generalise blueprint validation for Fedora image types (HMS-6871) #1216
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -752,6 +752,97 @@ | |
| aarch64: | ||
| <<: *iot_simplified_installer_partition_tables_x86 | ||
|
|
||
| supported_options_lists: | ||
| # common options supported by all disk image types this includes everything | ||
| # that is not specific to installers or ostree-based images | ||
| supported_options_disk: &supported_options_disk | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "packages" | ||
| - "modules" | ||
| - "groups" | ||
| - "containers" | ||
| - "minimal" | ||
| - "customizations.cacerts" | ||
| - "customizations.directories" | ||
| - "customizations.disk" | ||
| - "customizations.files" | ||
| - "customizations.filesystem" | ||
| - "customizations.partitioning_mode" | ||
| - "customizations.fips" | ||
| - "customizations.firewall" | ||
| - "customizations.user" | ||
| - "customizations.sshkey" | ||
| - "customizations.group" | ||
| - "customizations.hostname" | ||
| - "customizations.kernel" | ||
| - "customizations.locale" | ||
| - "customizations.openscap" | ||
| - "customizations.repositories" | ||
| - "customizations.rpm" | ||
| - "customizations.services" | ||
|
Contributor
There was a problem hiding this comment. This is missing
Member
There was a problem hiding this comment. The former only exists in The latter is indeed missing and documented to exist.
Member
Author
There was a problem hiding this comment. Looking into this, I think it will cause a problem though. Projects that use the blueprint generally shouldn't have to worry about the |
||
| - "customizations.timezone" | ||
|
|
||
| # options supported by base ostree image types (commit and container) | ||
| supported_options_ostree_commit: &supported_options_ostree_commit | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "packages" | ||
| - "modules" | ||
| - "groups" | ||
| - "minimal" | ||
| - "customizations.directories" | ||
| - "customizations.files" | ||
| - "customizations.fips" | ||
| - "customizations.firewall" | ||
| - "customizations.user" | ||
| - "customizations.sshkey" | ||
| - "customizations.group" | ||
| - "customizations.hostname" | ||
| - "customizations.kernel.name" | ||
| - "customizations.locale" | ||
| - "customizations.repositories" | ||
| - "customizations.services" | ||
| - "customizations.timezone" | ||
|
|
||
| # options supported by ostree disk (deployment) image types | ||
| supported_options_ostree_disk: &supported_options_ostree_disk | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "customizations.files" | ||
| - "customizations.directories" | ||
| - "customizations.disk" | ||
| - "customizations.filesystem" | ||
| - "customizations.partitioning_mode" | ||
| - "customizations.fips" | ||
| - "customizations.user" | ||
| - "customizations.sshkey" | ||
| - "customizations.group" | ||
| - "customizations.kernel.append" | ||
| - "customizations.locale" | ||
| - "customizations.services" | ||
|
|
||
| # options supported by Anaconda installer (ISO) image types | ||
| supported_options_anaconda: &supported_options_anaconda | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "customizations.installer" | ||
| - "customizations.user" | ||
| - "customizations.sshkey" | ||
| - "customizations.group" | ||
| - "customizations.fips" | ||
| - "customizations.timezone" | ||
| - "customizations.locale" | ||
|
|
||
|
|
||
| image_config: | ||
| default: | ||
| default_oscap_datastream: "/usr/share/xml/scap/ssg/content/ssg-fedora-ds.xml" | ||
|
|
@@ -813,6 +904,7 @@ image_types: | |
| image_format: "vagrant_libvirt" | ||
| - <<: *aarch64_platform | ||
| image_format: "vagrant_libvirt" | ||
| supported_blueprint_options: *supported_options_disk | ||
|
|
||
| "server-vagrant-virtualbox": &server_vagrant_virtualbox | ||
| <<: *server_vagrant_libvirt | ||
|
|
@@ -863,6 +955,7 @@ image_types: | |
| image_format: "qcow2" | ||
| - <<: *s390x_zipl_platform | ||
| image_format: "qcow2" | ||
| supported_blueprint_options: *supported_options_disk | ||
|
|
||
| "server-ami": | ||
| <<: *server_qcow2 | ||
|
|
@@ -970,6 +1063,7 @@ image_types: | |
| - "zram-generator-defaults" | ||
| - "grubby-deprecated" | ||
| - "extlinux-bootloader" | ||
| supported_blueprint_options: *supported_options_disk | ||
|
|
||
| "server-ova": | ||
| <<: *server_vmdk | ||
|
|
@@ -1135,6 +1229,7 @@ image_types: | |
| append: | ||
| include: | ||
| - "filesystem" | ||
| supported_blueprint_options: *supported_options_ostree_commit | ||
|
|
||
| "iot-container": | ||
| <<: *iot_commit | ||
|
|
@@ -1228,6 +1323,7 @@ image_types: | |
| - *iot_base_partition_table_part_efi_aarch64 | ||
| - *iot_base_partition_table_part_boot_aarch64 | ||
| - *iot_base_partition_table_part_root_fstab_ro_aarch64 | ||
| supported_blueprint_options: *supported_options_ostree_disk | ||
|
|
||
| "iot-qcow2": | ||
| <<: *rpm_ostree_imgtype_common | ||
|
|
@@ -1255,6 +1351,7 @@ image_types: | |
| - <<: *aarch64_platform | ||
| image_format: "qcow2" | ||
| qcow2_compat: "1.1" | ||
| supported_blueprint_options: *supported_options_ostree_disk | ||
|
|
||
| "iot-bootable-container": | ||
| <<: *rpm_ostree_imgtype_common | ||
|
|
@@ -1401,6 +1498,17 @@ image_types: | |
| exclude: | ||
| - "perl" | ||
| - "perl-interpreter" | ||
| supported_blueprint_options: | ||
| # Only supporting a few basic options for now because we never tested any | ||
| # other customization with this image type | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "packages" | ||
| - "modules" | ||
| - "groups" | ||
| - "minimal" | ||
|
|
||
| "minimal-raw-xz": &minimal_raw_xz | ||
| name_aliases: ["minimal-raw"] | ||
|
|
@@ -1494,6 +1602,8 @@ image_types: | |
| append: | ||
| exclude: | ||
| - "firewalld" | ||
| supported_blueprint_options: *supported_options_disk | ||
|
|
||
| "minimal-raw-zst": | ||
| <<: *minimal_raw_xz | ||
| name_aliases: [] | ||
|
|
@@ -1529,6 +1639,7 @@ image_types: | |
| platforms: | ||
| - *x86_64_installer_platform | ||
| - *aarch64_installer_platform | ||
| supported_blueprint_options: *supported_options_anaconda | ||
|
|
||
| "workstation-live-installer": | ||
| name_aliases: ["live-installer"] | ||
|
|
@@ -1540,7 +1651,7 @@ image_types: | |
| iso_label: "Workstation" | ||
| exports: ["bootiso"] | ||
| required_partition_sizes: *default_required_dir_sizes | ||
| installer_config: | ||
| <<: *default_installer_config | ||
| # for some reason the live-installer never had or never took into account | ||
| # the additional dracut modules. This might be a bug but for now we reset | ||
|
|
@@ -1595,6 +1706,12 @@ image_types: | |
| platforms: | ||
| - *x86_64_installer_platform | ||
| - *aarch64_installer_platform | ||
| supported_blueprint_options: | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "customizations.installer" | ||
|
|
||
| "minimal-installer": | ||
| name_aliases: ["image-installer", "fedora-image-installer"] | ||
|
|
@@ -1637,6 +1754,7 @@ image_types: | |
| - *minimal_raw_pkgset | ||
| installer: | ||
| - *anaconda_pkgset | ||
| supported_blueprint_options: *supported_options_anaconda | ||
|
|
||
| container: &container | ||
| filename: "container.tar" | ||
|
|
@@ -1702,6 +1820,17 @@ image_types: | |
| - "trousers" | ||
| - "whois-nls" | ||
| - "xkeyboard-config" | ||
| supported_blueprint_options: | ||
| # Only supporting a few basic options for now because we never tested any | ||
| # other customization with this image type | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "packages" | ||
| - "modules" | ||
| - "groups" | ||
| - "minimal" | ||
|
|
||
| wsl: | ||
| # this is the eventual name, and `wsl` the alias but we've been | ||
|
|
@@ -1807,6 +1936,17 @@ image_types: | |
| append: | ||
| exclude: | ||
| - "fuse-libs" | ||
| supported_blueprint_options: | ||
| # Only supporting a few basic options for now because we never tested any | ||
| # other customization with this image type | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "packages" | ||
| - "modules" | ||
| - "groups" | ||
| - "minimal" | ||
|
|
||
| "iot-simplified-installer": | ||
| <<: *rpm_ostree_imgtype_common | ||
|
|
@@ -1927,6 +2067,21 @@ image_types: | |
| - "iwlwifi-mvm-firmware" | ||
| - "realtek-firmware" | ||
| - "uboot-images-armv8" | ||
| supported_blueprint_options: | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
| - "customizations.installation_device" | ||
| - "customizations.fdo" | ||
| - "customizations.ignition" | ||
| - "customizations.kernel" | ||
| - "customizations.user" | ||
| - "customizations.sshkey" | ||
| - "customizations.group" | ||
| - "customizations.fips" | ||
| required_blueprint_options: | ||
| - "customizations.installation_device" | ||
|
|
||
| # Based on lorax runtime-install.tmpl | ||
| "everything-netinst": | ||
|
|
@@ -2039,3 +2194,8 @@ image_types: | |
| platforms: | ||
| - *x86_64_installer_platform | ||
| - *aarch64_installer_platform | ||
| supported_blueprint_options: | ||
| - "name" | ||
| - "version" | ||
| - "description" | ||
| - "distro" | ||
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package distro | ||
|
|
||
| import "reflect" | ||
|
|
||
| // We wrap our internal functions in exported functions instead of defining | ||
| // aliases so we can return an error type instead of validationError. Our | ||
| // recursive functions need to return validationError so that the path can be | ||
| // constructed when returning up the stack. But if the function at the top | ||
| // returns a nil valued validationError, it will fail the NoError() check. This | ||
| // is not a problem outside of testing since the public entrypoint, | ||
| // ValidateConfig(), returns nil when everything is ok. | ||
|
|
||
| func ValidateSupportedConfig(supported []string, conf reflect.Value) error { | ||
| if err := validateSupportedConfig(supported, conf); err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func ValidateRequiredConfig(required []string, conf reflect.Value) error { | ||
| if err := validateRequiredConfig(required, conf); err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
(wondering) Should we make name/version/description implicit? They are not really concepts that are relevant for the image types, i.e. it would be weird to have an image type that refuses a blueprint with a "name"?
Similar wondering about "distro", it goes away in UBP and its a bit of a odd one, so maybe the image types should not list it?
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.
I think that hiding it makes it slightly more difficult to get up to speed with it. It's supposed to be a list of blueprint fields, just make it 1:1 without any magic.
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.
I was going to do this but since it might be up for discussion, let's reconsider it in a follow-up.
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.
Fwiw, after a bit of time to think about this I'm less and less convinced we should make this explicit. Name/Description/Version are essentially metadata and not customizations, adding it to our yaml like this will be confusing for people creating image types because they will wonder why this is something that needs to be explicitly allow-listed. It will also just lead to a unnecessary boilerplate. Sorry for being so negative, I will not block this but I don't think its right thing to do.