OCPBUGS-1357: Fix install-plugins.sh to not override locked plugin versions #1482
Conversation
coreydaley
commented
Sep 14, 2022
coreydaley
commented
- Do no override locked plugin versions
- Verify that locked plugin versions exist in the bundle-plugins.txt file
|
/assign @akram @jitendar-singh |
openshift-ci
bot
added
the
approved
|
/retest |
coreydaley
changed the title
|
/jira refresh |
|
@coreydaley: An error was encountered querying GitHub for users with public email (jitsingh@redhat.com) for bug OCPBUGS-1357 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details. Full error message.
Post "http://ghproxy/graphql": dial tcp 172.30.229.2:80: connect: connection refused
Please contact an administrator to resolve this issue, then request a bug refresh with In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/jira refresh |
|
@coreydaley: An error was encountered querying GitHub for users with public email (jitsingh@redhat.com) for bug OCPBUGS-1357 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details. Full error message.
Post "http://ghproxy/graphql": dial tcp 172.30.229.2:80: i/o timeout
Please contact an administrator to resolve this issue, then request a bug refresh with In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@coreydaley: An error was encountered querying GitHub for users with public email (jitsingh@redhat.com) for bug OCPBUGS-1357 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details. Full error message.
Post "http://ghproxy/graphql": dial tcp 172.30.229.2:80: i/o timeout
Please contact an administrator to resolve this issue, then request a bug refresh with In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/severity-important
|
@coreydaley: This pull request references Jira Issue OCPBUGS-1357, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
@akram All tests are passing, please take a look and let me know if you have any questions about what the issue was. |
|
@coreydaley: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
|
||
| if [[ $ignoreLockFile ]] || mkdir "$lock" &>/dev/null; then | ||
| if [[ $ignoreLockFile ]] || ! test -f $(getLockFile $plugin); then |
There was a problem hiding this comment.
is there any reason to change the lockfile from a directory to a file?
I don't know the historical reason of having a directory, but, if we can minimize changes to this old code, that would be more reinsuring.
There was a problem hiding this comment.
I am storing the locked plugin version in the <plugin>.lock file for use when checking the versions that are being requested.
| lockFile=$(getLockFile "$plugin") | ||
| lockedVersion=$(cat $lockFile) | ||
| echo "Plugin $plugin locked to version $lockedVersion, ignoring, requested version $version" | ||
| if versionLT "${lockedVersion}" "${version}"; then |
There was a problem hiding this comment.
what will we do with the warning message?
and during CI, we will not check it for sure. Should we fail in case of warning ?
There was a problem hiding this comment.
The problem is that sometimes the versions are like 2.6.5 and sometimes they switch to 1123.456.asdflasdf, and the script does not account for 2.6.5 could be newer than 1123.456.asdflasdf, so if we show the warnings when someone updates the base-plugins.txt and then runs the install script, they can sort it out manually and update the locked version if needed, which it probably is. But specific plugins we want locked to a newer version that has CVE fixes.
| @@ -310,7 +317,7 @@ function resolveDependencies() { | |||
| # download the dependence; passing "true" is needed for "download" to replace the existing dependency | |||
| if versionLT "${versionInstalled}" "${minVersion}"; then | |||
| echo "Upgrading bundled dependency $d ($minVersion > $versionInstalled)" | |||
| download "$plugin" "$minVersion" "true" | |||
| download "$plugin" "$minVersion" | |||
There was a problem hiding this comment.
Was the bug here? I don't see a signature change to the download function, however, it was used only with 2 arguments previously, the 2nd one being a boolean. no regression ?
There was a problem hiding this comment.
Since this was passing true, it was overriding the locked versions of plugins and potentially installed a different version, which may or may not be correct, or what we wanted.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akram, coreydaley The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@coreydaley: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-1357 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
