Skip to content

loosen version verification #3750

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 26, 2025
Merged

loosen version verification #3750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
996a114
loosen version verification
ucodery Sep 24, 2025
110d75d
Update readme.md
ucodery Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions internal/runners/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package install
import (
"errors"
"fmt"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -269,8 +268,6 @@ func (i *Install) resolveRequirements(packages captain.PackagesValue, ts time.Ti
return reqs, nil
}

var versionRe = regexp.MustCompile(`^\d(\.\d+)*$`)

func resolveVersion(req *requirement) error {
version := req.Requested.Version

Expand All @@ -283,7 +280,7 @@ func resolveVersion(req *requirement) error {
// Verify that the version provided can be resolved
// Note: if the requirement does not have an ingredient, it is being dynamically imported, so
// we cannot resolve its versions yet.
if versionRe.MatchString(version) && req.Resolved.ingredient != nil {
if req.Resolved.ingredient != nil {
Comment on lines -286 to +283
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex does seem like it was overkill here, but reading this code I think we still want to check if version is non-empty? In most cases we run state install without a specific version, in which case this it doesn't seem like we should run this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I ran a single test of state install namespace:name (no dynamic, no version) and that worked. But I would be okay to check for the empty string.

match := false
for _, knownVersion := range req.Resolved.ingredient.Versions {
if knownVersion.Version == version {
Expand Down
74 changes: 36 additions & 38 deletions pkg/platform/model/buildplanner/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package buildplanner
import (
"encoding/json"
"errors"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -172,25 +171,16 @@ func processBuildPlannerError(bpErr error, fallbackMessage string) error {
return &response.BuildPlannerError{Err: locale.NewExternalError("err_buildplanner", "{{.V0}}: Encountered unexpected error: {{.V1}}", fallbackMessage, bpErr.Error())}
}

var versionRe = regexp.MustCompile(`^\d+(\.\d+)*$`)

func isExactVersion(version string) bool {
return versionRe.MatchString(version)
func isRangeVersion(version string) bool {
return strings.Contains(version, "=") || strings.Contains(version, "<") || strings.Contains(version, ">")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strings.Contains(version, "=")

This doesn't indicate a range version does it? I suppose you could have >= but = alone could also apply to ==1.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, = alone doesn't indicate a version range, but == is a range, as are >=, <=, !=, and others that are less common like ~=. It is just an indicator, as = is never legal in a concrete version identifier.

these three characters are the minimum sufficient to determine if any of the universal version range operators are being used.

Copy link
Contributor

@Naatan Naatan Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I don't think I'm following, let me ask a different way;

Wouldn't isRangeVersion('==1.0') return true even though it should be false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one is a bit different, true, but IMO it does and should return true. There is a distinction between ==1.0 and 1.0, though maybe not to the state tool. The former is a constraint and does represent a range, but a range that includes only one version; the latter is a versions string.

So the practical reason to send even ==1.0 to the back end is that == should not be part of the version string. Sending it to the platform means we don't have to special case this one operator for stripping. This is good, as version strings get complex across so many ecosystems, e.g. in Rust = is the eq operator, not ==.

}

func isWildcardVersion(version string) bool {
return strings.Contains(version, ".x") || strings.Contains(version, ".X")
}

func VersionStringToRequirements(version string) ([]types.VersionRequirement, error) {
if isExactVersion(version) {
return []types.VersionRequirement{{
types.VersionRequirementComparatorKey: "eq",
types.VersionRequirementVersionKey: version,
}}, nil
}

if !isWildcardVersion(version) {
if isRangeVersion(version) {
// Ask the Platform to translate a string like ">=1.2,<1.3" into a list of requirements.
// Note that:
// - The given requirement name does not matter; it is not looked up.
Expand All @@ -210,33 +200,41 @@ func VersionStringToRequirements(version string) ([]types.VersionRequirement, er
return requirements, nil
}

// Construct version constraints to be >= given version, and < given version's last part + 1.
// For example, given a version number of 3.10.x, constraints should be >= 3.10, < 3.11.
// Given 2.x, constraints should be >= 2, < 3.
requirements := []types.VersionRequirement{}
parts := strings.Split(version, ".")
for i, part := range parts {
if part != "x" && part != "X" {
continue
}
if i == 0 {
return nil, locale.NewInputError("err_version_wildcard_start", "A version number cannot start with a wildcard")
}
requirements = append(requirements, types.VersionRequirement{
types.VersionRequirementComparatorKey: types.ComparatorGTE,
types.VersionRequirementVersionKey: strings.Join(parts[:i], "."),
})
previousPart, err := strconv.Atoi(parts[i-1])
if err != nil {
return nil, locale.WrapInputError(err, "err_version_number_expected", "Version parts are expected to be numeric")
if isWildcardVersion(version) {
// Construct version constraints to be >= given version, and < given version's last part + 1.
// For example, given a version number of 3.10.x, constraints should be >= 3.10, < 3.11.
// Given 2.x, constraints should be >= 2, < 3.
requirements := []types.VersionRequirement{}
parts := strings.Split(version, ".")
for i, part := range parts {
if part != "x" && part != "X" {
continue
}
if i == 0 {
return nil, locale.NewInputError("err_version_wildcard_start", "A version number cannot start with a wildcard")
}
requirements = append(requirements, types.VersionRequirement{
types.VersionRequirementComparatorKey: types.ComparatorGTE,
types.VersionRequirementVersionKey: strings.Join(parts[:i], "."),
})
previousPart, err := strconv.Atoi(parts[i-1])
if err != nil {
return nil, locale.WrapInputError(err, "err_version_number_expected", "Version parts are expected to be numeric")
}
parts[i-1] = strconv.Itoa(previousPart + 1)
requirements = append(requirements, types.VersionRequirement{
types.VersionRequirementComparatorKey: types.ComparatorLT,
types.VersionRequirementVersionKey: strings.Join(parts[:i], "."),
})
}
parts[i-1] = strconv.Itoa(previousPart + 1)
requirements = append(requirements, types.VersionRequirement{
types.VersionRequirementComparatorKey: types.ComparatorLT,
types.VersionRequirementVersionKey: strings.Join(parts[:i], "."),
})
return requirements, nil
}
return requirements, nil

return []types.VersionRequirement{{
types.VersionRequirementComparatorKey: "eq",
types.VersionRequirementVersionKey: version,
}}, nil

}

// pollBuildPlanned polls the buildplan until it has passed the planning stage (ie. it's either planned or further along).
Expand Down
3 changes: 2 additions & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ For usage information please refer to the [State Tool Documentation](http://docs

### Building & Testing

First run `state run install-deps` followed by `state run preprocess` if you are building for the first time.
First run `state run install-deps-dev` followed by `state run preprocess` if you are building for the first time.

* **Building:** `state run build`
* The built executable will be stored in the `build` directory
* If you modified assets or switched branches, you need to re-run `state run preprocess` first
* The first time you are building, or if you modified modules outside the primary state binary, run `state run build-all`
* **Testing:**
* **Unit tests\*:** `state run test`
* **Integration tests:** `state run integration-tests`
Expand Down
Loading