-
Notifications
You must be signed in to change notification settings - Fork 902
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
Support dependecies with version constraints in crank validate #5669
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Jiri Tyr <jiri.tyr@gmail.com>
@jtyr Unfortunately this won't make it into v1.16 - feature freeze for that release was two weeks ago and it's being released tomorrow. Sorry! |
|
||
// Compose new complete image string if any complient version was found | ||
image = fmt.Sprintf("%s:%s", imageBase, addVer) | ||
} |
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.
Would it make sense to split this logic out into a separate function?
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.
Done in the last commit. Please feel free to review it.
Signed-off-by: Jiri Tyr <jiri.tyr@gmail.com>
Perhaps @ezgidemirel would be a good first reviewer for this, given she implemented the |
@ezgidemirel please could your review this PR? |
for _, v := range vs { | ||
if c.Check(v) { | ||
addVer = v.Original() | ||
} | ||
} |
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.
instead of going through each version and finding the last one which satisfies the constraint, you may also loop in reverse and break after finding the first version satisfies it.
Description of your changes
This PR continuation of the PR #5570 where I have pointed out that the
crossplane beta validate
doesn't work withConfiguratons
that contain version constraints. This PR is trying to address that.I have:
Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Added backport release-x.y labels to auto-backport this PR.The PR was tested like this:
Clone the repo, apply the patch and build a new
crank
binary:Create directory structure for the test:
mkdir -p /tmp/{apis/foo.custom-api.example.org,test/{functions,validation}} cd /tmp/test
Create files:
Run local Docker registry:
Package and push the
foo
package:Validate the
Bar
composition using the upstreamcrank
binary:Validate the
Bar
composition using the patchedcrank
binary: