-
Notifications
You must be signed in to change notification settings - Fork 549
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
fix: use imager incoming version for extension validation #9696
fix: use imager incoming version for extension validation #9696
Conversation
pkg/machinery/extensions/validate.go
Outdated
func (ext *Extension) Validate() error { | ||
if err := ext.validateConstraints(); err != nil { | ||
func (ext *Extension) Validate(talosVersion semver.Version) error { | ||
if err := ext.validateConstraints(talosVersion); err != nil { | ||
return err | ||
} | ||
|
||
return ext.validateContents() |
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.
how about validateContents()
- as it's different for different versions of Talos, it might still break?
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 guess then we need have something similar to quirks for extension allowed paths
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.
yep, either that, or a way to disable it in Validate()
- e.g. Image Factory works on actually approved/shipped versions, so contents should be fine?
2ba26a8
to
e2116d7
Compare
@@ -24,6 +24,8 @@ type Builder struct { | |||
Arch string | |||
// ExtensionTreePath is a path to the extracted extension tree. | |||
ExtensionTreePath string | |||
// ExtensionValidateContents enables validation of the extension contents. | |||
ExtensionValidateContents bool |
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 can drop this since we only use it for the deprecated .machine.install.extensions
https://github.com/siderolabs/talos/pull/9696/files#diff-9adbdc7691682c62b382f9040db20e4e0d54e5be12c74378bef14a586a4adf20R22
if constraint != "" { | ||
talosVersion, err := semver.ParseTolerant(version.Tag) | ||
// Validate the extension: compatibility, contents, etc. | ||
func (ext *Extension) Validate(opts ...ValidationOption) error { |
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.
did this, so Validate
is backword compatible, I could also negate the options, so Validate
works as is
e2116d7
to
0fb8078
Compare
Use the version coming from imager to validate extension constraints. Part of : siderolabs#9694 Signed-off-by: Noel Georgi <git@frezbo.dev>
0fb8078
to
682718d
Compare
/m |
Use the version coming from imager to validate extension constraints.
Part of : #9694