Add functions to query and validate minimum spec version

[elezar]

This change adds support for determining the minimum spec version required for a specified spec. This allows a minimum version spec to be created if newer fields are not used.

Closes #91

[klihub]

Wouldn't it be a more sustainable/easier to understand and also easier to maintain implementation to define a compatibility-check function for each valid version we support ?

It would be enough if each of those functions would codify and check the extra conditions compared to the previous version, assuming the Spec passed verification of that (and consequently all previous functions).

Then MinimumRequiredVersion(spec *cdi.Spec) (string, error) could be implemented by something like

var (
  sortedVersions []string
  versionValidator = map[string]func(*cdi.Spec) error{
      "0.0.0": validate000,
      ...
)

func MinimumRequiredVersion(spec *cdi.Spec) (string, error) {
    for _, v := range sortVersions() {
        if err := versionValidator[v](spec); err != nil {
            return "", fmt.Errorf("no valid version found: %w", err)
        }
        return v, nil
    }
} 

func sortVersions() []string {
    if sortedVersions == nil {
        for v, _ := range validVersions {
            sortedVersions = append(sortedVersions, v)
        }
        semver.Sort(sortedVersions)
    }
    return sortedVersions
}

Moreover, something like IsCompatible(spec *cdi.Spec, version string) could be implemented with something like this:

func IsCompatibleVersion(spec *cdi.Spec, version string) error {
    for _, v := range sortVersions() {
        if err := versionValidator[v](spec); err != nil {
            return err
        }
        if v == version {
            return nil
        }
    }

    return fmt.Errorf("can't verify compatibility, unknown version %s", version)
}

[elezar]

@klihub I think I'm missing the implementation of validate000 in your example to fully understand your suggestion.

The way I've been thinking about this was about adding fields and as such this can be seen as implementing, for example, a:

requiresV050(*Spec) 

The issue with vaildation as you describe it (at least from my current point of view), is that older functions need to be updated with new fields with each modification.

Rewriting the checks as require* functions, I have:

// requiresV050 returns true if the spec uses v0.5.0 features
func requiresV050(spec *cdi.Spec) bool {
	var edits []*cdi.ContainerEdits

	for _, d := range spec.Devices {
		// The v0.5.0 spec allowed device names to start with a digit instead of requiring a letter
		if len(d.Name) > 0 && !isLetter(rune(d.Name[0])) {
			return true
		}

		edits = append(edits, &d.ContainerEdits)
	}

	edits = append(edits, &spec.ContainerEdits)
	for _, e := range edits {
		for _, dn := range e.DeviceNodes {
			if dn.HostPath != "" && dn.HostPath != dn.Path {
				return true
			}
		}
	}
	return false
}

// requiresV040 returns true if the spec uses v0.4.0 features
func requiresV040(spec *cdi.Spec) bool {
	var edits []*cdi.ContainerEdits

	for _, d := range spec.Devices {
		edits = append(edits, &d.ContainerEdits)
	}

	edits = append(edits, &spec.ContainerEdits)
	for _, e := range edits {
		for _, m := range e.Mounts {
			if m.Type != "" {
				return true
			}
		}
	}
	return false
}

Which may be easier to understand than the current implementation. I can work to clean up the edits manipulation a little to.

[elezar]

@klihub I reworked this to match something like you suggested. Not sure if I'm still missing something though.

I think we can still combine the validSpecVersions and the required map that I create. I will do this in a follow-up.

[klihub]

@klihub I think I'm missing the implementation of validate000 in your example to fully understand your suggestion.

Sorry, I omitted all the validating function details, because I didn't have time to find out what those really would be. But it looks to me that you got what I was after... even if my vague suggestion/idea was not well thought through (for instance I think I got it totally wrong that version check could/should be done from lowest to highest, requiring that all previous version checks succeed).

Which may be easier to understand than the current implementation. I can work to clean up the edits manipulation a little to.

If we want to do automatic version-picking in case it is unspecified, then I think it probably would be easier to maintain (and understand) the code if manage to codify the version-specific requirements to per-version validation functions, something along the lines of what is being done here.

[klihub]

LGTM.

[bart0sh]

/lgtm