feat: add check version requirement #4308
Open
+438
−55
Conversation
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
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
✅ Deploy Preview for zarf-docs canceled.
|
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
AustinAbro321
changed the title
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
mkcp
reviewed
Oct 23, 2025
| // List of migrations tracked in the zarf.yaml build data. | ||
| const ( | ||
| // This should be updated when a breaking change is introduced to the Zarf package structure. See: https://github.com/zarf-dev/zarf/releases/tag/v0.27.0 | ||
| LastNonBreakingVersion = "v0.27.0" |
mkcp
reviewed
Oct 23, 2025
mkcp
reviewed
Oct 23, 2025
| CurrentVersion string | ||
| } | ||
|
|
||
| func (e *VersionRequirementsError) Error() string { |
There was a problem hiding this comment.
Non-blocking, more of a discussion item. There's an alternative path for this where we use a NewVersionRequirementsError(...) VersionRequirementsError constructor function, and calculate all of the necessary fields upfront. Then the error method can assemble the message string from the pre-calculated fields. I expect that this approach would be a bit more readable, but it is otherwise equivalent.
There was a problem hiding this comment.
Not sure if this is what you mean but I moved the logic to determine the highest required version out of the error message, which I agree is better
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
Signed-off-by: Austin Abro <austinabro321@gmail.com>
| // SPDX-FileCopyrightText: 2021-Present The Zarf Authors | ||
|
|
||
| // Package requirements validates that Zarf meets the version requirements defined by the package | ||
| package requirements |
There was a problem hiding this comment.
Couple Questions:
- Do we have a expected example of how to utilize these when necessary?
- Are we using build data as state to manage collecting these requirements throughout the create lifecycle?
- Should we add documentation to support more of a user-facing warning "skipping the version check may introduce...." .
There was a problem hiding this comment.
- I see the setting of the data as something that will be done at create time in the recordPackageMetadata function. For instance, you may have a block like this:
var versionRequirements []v1alpha1.VersionRequirement
for _, comp := range pkg.Components {
for _, v := range comp.Charts {
if v.Version == "" {
versionRequirements = append(versionRequirements, v1alpha1.VersionRequirement{
Version: "v0.65.0",
Reason: "the internal file structure of charts has changed in the v1beta1 schema. Zarf is backwards compatible with these changes from v0.65.0+",
})
break
}
}
}
pkg.Build.VersionRequirements = versionRequirements- I think this will be defined by whats in the package / general changes to Zarf, so we can always set it towards the end of create
- I think so, especially when we have real use cases in the defined. Interested in your thoughts on how that might be structured.
There was a problem hiding this comment.
Let me know if there's a better place to document this example, for now, I linked this block to issue #2245. As that's the first use case I can think of
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Commands that do not have an impact on the cluster such as inspect do not implement the version check. Additionally, Publish is very generic and not effected by the actual layout.
This removed
.build.LastNonBreakingChangewhich was set but not usedThis is what the error message will look like to the user. I created got this by manually setting
.build.VersionRequirementsthen creating and deploying that packageRelated Issue
Fixes #4256
Checklist before merging