Skip to content
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

Jump to bottom

OCPBUGS-1357: Fix install-plugins.sh to not override locked plugin versions #1482

Merged
merged 2 commits into from
Sep 16, 2022
Merged

OCPBUGS-1357: Fix install-plugins.sh to not override locked plugin versions #1482

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
6a4dc68
Updating script to not override locked plugin versions
coreydaley Sep 14, 2022
a8e63cc
Updating bundle-plugins.txt via script updates
coreydaley Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 30 additions & 6 deletions 2/contrib/jenkins/install-plugins.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ copy_reference_file() {

REF_DIR=${REF:-/opt/openshift/plugins}
FAILED="$REF_DIR/failed-plugins.txt"
WARNING="$REF_DIR/warning-plugins.txt"
BUNDLE_PLUGINS=${BUNDLE_PLUGINS:-/opt/openshift/plugins/bundle-plugins.txt}
JENKINS_WAR=${JENKINS_WAR:-/usr/lib/jenkins/jenkins.war}
JENKINS_UC=${JENKINS_UC:-https://updates.jenkins.io}
Expand All @@ -172,9 +173,8 @@ function download() {
plugin="$1"
version="${2:-latest}"
ignoreLockFile="${3:-}"
lock="$(getLockFile "$plugin")"

if [[ $ignoreLockFile ]] || mkdir "$lock" &>/dev/null; then
if [[ $ignoreLockFile ]] || ! test -f $(getLockFile $plugin); then
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am storing the locked plugin version in the <plugin>.lock file for use when checking the versions that are being requested.

if ! doDownload "$plugin" "$version"; then
# some plugin don't follow the rules about artifact ID
# typically: docker-plugin
Expand All @@ -194,6 +194,13 @@ function download() {
fi

resolveDependencies "$plugin"
else
lockFile=$(getLockFile "$plugin")
lockedVersion=$(cat $lockFile)
echo "Plugin $plugin locked to version $lockedVersion, ignoring, requested version $version"
if versionLT "${lockedVersion}" "${version}"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

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 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

echo "Manual update from ${plugin}:${lockedVersion} to ${plugin}:${version} in base-plugins.txt may be needed." >> $WARNING
fi
fi
}

Expand Down Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

else
echo "Skipping already bundled dependency $d ($minVersion <= $versionInstalled)"
fi
Expand All @@ -328,7 +335,7 @@ function resolveDependencies() {
# version of the plugin
if versionLT "${previouslyDownloadedVersion}" "${minVersion}"; then
echo "Upgrading previously downloaded plugin $plugin at $previouslyDownloadedVersion to $minVersion"
download "$plugin" "$minVersion" "true"
download "$plugin" "$minVersion"
fi
fi
done
Expand Down Expand Up @@ -411,7 +418,7 @@ main() {
continue
fi
echo "Locking $plugin"
mkdir "$(getLockFile "${plugin%%:*}")"
echo "$(versionFromPlugin $plugin)" > "$(getLockFile "${plugin%%:*}")"
done

echo -e "\nAnalyzing war: $JENKINS_WAR"
Expand Down Expand Up @@ -449,8 +456,25 @@ main() {
echo "Installed plugins:"
installedPlugins

echo -e "\nVerifying Locked Plugins in Bundle..."
for plugin in `cat $@ | grep -v ^#`; do
if [ -z $plugin ]; then
continue
fi
if grep -q "^${plugin}$" $BUNDLE_PLUGINS; then
echo "Found $plugin"
else
echo "Missing $plugin"
echo "Plugin $plugin not found in $BUNDLE_PLUGINS" >> $FAILED
fi
done

if [[ -f $WARNING ]]; then
echo -e "\nSome warnings were encountered!\n$(<"$WARNING")" >&2
fi

if [[ -f $FAILED ]]; then
echo -e "\nSome plugins failed to download!\n$(<"$FAILED")" >&2
echo -e "\nSome errors were encountered!\n$(<"$FAILED")" >&2
exit 1
fi

Expand Down
10 changes: 5 additions & 5 deletions 2/contrib/openshift/base-plugins.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ cloudbees-bitbucket-branch-source:746.v350d2781c184
cloudbees-folder:6.16
config-file-provider:3.8.1
configuration-as-code-groovy:1.1
configuration-as-code:1.55.1
credentials:2.6.1.1
configuration-as-code:1414.v878271fc496f
credentials:1129.vef26f5df883c
credentials-binding:1.27.1
docker-commons:1.18
favorite:2.4.1
Expand Down Expand Up @@ -35,13 +35,13 @@ pipeline-build-step:2.16
pipeline-input-step:449.v77f0e8b_845c4
pipeline-utility-steps:2.12.0
prometheus:2.0.10
scm-api:2.6.5
scm-api:608.vfa_f971c5a_a_e9
script-security:1175.v4b_d517d6db_f0
snakeyaml-api:1.30.1
ssh-credentials:1.19
subversion:2.15.4
token-macro:267.vcdaea6462991
workflow-api:1144.v61c3180fa_03f
token-macro:293.v283932a_0a_b_49
workflow-api:1182.v41475e53ea_43
workflow-cps:2759.v87459c4eea_ca_
workflow-cps-global-lib:588.v576c103a_ff86
workflow-multibranch:711.vdfef37cda_816
Expand Down
4 changes: 2 additions & 2 deletions 2/contrib/openshift/bundle-plugins.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ ssh-credentials:1.19
sshd:3.0.4
structs:318.va_f3ccb_729b_71
subversion:2.15.4
token-macro:267.vcdaea6462991
token-macro:293.v283932a_0a_b_49
trilead-api:1.0.13
variant:1.4
workflow-api:1182.v41475e53ea_43
workflow-basic-steps:2.20
workflow-cps:2759.v87459c4eea_ca_
workflow-cps-global-lib:588.v576c103a_ff86
workflow-durable-task-step:2.35
workflow-job:2.41
workflow-job:1145.v7f2433caa07f
workflow-multibranch:711.vdfef37cda_816
workflow-scm-step:400.v6b_89a_1317c9a_
workflow-step-api:639.v6eca_cd8c04a_a_
Expand Down