Skip to content

WIP: fix: parse channel version for pre-apply check #580

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ekitagawa
Copy link
Contributor

Filter out suffix to better compare channel version and version in config.

@ekitagawa ekitagawa changed the title fix: parse channel version for pre-apply check WIP: fix: parse channel version for pre-apply check Jun 10, 2025
@james-nesbitt
Copy link
Collaborator

please include a test for the fips scenario; I will look at fixing the existing tests for your scenario

@ekitagawa
Copy link
Contributor Author

Just to be safe, I will actually run the test and harvest the sample input for the test class.

if !strings.HasPrefix(chanParts[1], config.Version) {
return fmt.Errorf("%w; MCR version does not match channel-version '%s' vs '%s'", ErrChannelDoesntMatchVersion, chanParts[1], config.Version)
configVersionParts := strings.Split(config.Version, ".")
configChannel := configVersionParts[0] + "." + configVersionParts[1] // take only the major and minor version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you limiting to major.minor versions and omitting the patch. The entire point to the channel validation is that in order to install a repo for a pinned patch version of MCR25, you need to use the major.minor.patch path for the repo information.

E.G.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants