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

NGINX Plus R33 related fixes #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

NGINX Plus R33 related fixes #3

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ed2b93
Added support for NGINX Plus R33
thresheek Oct 23, 2024
9909f0b
Allow to redefine REPOPREFIX
thresheek Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 34 additions & 13 deletions ngxunprivinst.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ set -e
# For RPM-based distros, make sure that you have rpm2cpio installed.
##
# Usage: ./ngxunprivinst.sh fetch -c <cert_file> -k <key_file> [-v <version>]
# ./ngxunprivinst.sh (install|upgrade) [-y] -p <path> <file> <file> ...
# ./ngxunprivinst.sh (install|upgrade) [-y] -p <path> -j <license> <file> <file> ...
# ./ngxunprivinst.sh list -c <cert_file> -k <key_file>
#
# fetch - download Nginx Plus and modules packages
Expand All @@ -21,6 +21,7 @@ set -e
# cert_file - path to your subscription certificate file
# key_file - path to your subscription private key file
# path - nginx prefix path
# license - path to your subscription license.jwt file
# version - nginx package version (default: latest available)
# -y - answers "yes" to all questions
##
Expand All @@ -30,6 +31,7 @@ PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:
NGXUSER=`id -nu`
NGXCERT=
NGXKEY=
NGXLICENSE=
NGXPATH=
CURDIR=`pwd`
WGET="wget -q"
Expand All @@ -56,12 +58,13 @@ else
shift
fi

args=`getopt c:k:p:v:y $*`
args=`getopt c:j:k:p:v:y $*`

for opt
do
case "$opt" in
-c) NGXCERT=$2; shift; shift;;
-j) NGXLICENSE=$2; shift; shift;;
-k) NGXKEY=$2; shift; shift;;
-p) NGXPATH=$2; shift; shift;;
-v) VERSION=$2; shift; shift;;
Expand All @@ -83,6 +86,11 @@ if [ "$NGXPATH" = '' ] && ( [ "$ACTION" = 'install' ] || [ "$ACTION" = 'upgrade'
fi
fi

if [ "$NGXLICENSE" = '' ] && ( [ "$ACTION" = 'install' ] || [ "$ACTION" = 'upgrade' ] ) ; then
Copy link

Choose a reason for hiding this comment

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

Hmm, I’m not sure it’s worth overcomplicating the logic, but this approach lacks backward compatibility. Let’s say I want to install r32, but I still have to provide the license file…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. The problem is that when we parse the CLI opts we don't really know which version we're going to install.

Dropping the requirement for the -j option to be specified means R33 will not be installed/launched easily by default, which this script is kinda supposed to do. I'm not sure on how to deal with this one, to be honest.

Copy link

Choose a reason for hiding this comment

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

I sent you a pr where I implemented this approach:

JWTREQ="NO"

if [ "$ACTION" = 'install' ] || [ "$ACTION" = 'upgrade' ]; then
    for pkg in $FILES; do
        pkgname=$(basename "$pkg")
        version=$(echo "$pkgname" | sed -n 's/^nginx-plus[_-]\([0-9]\+\).*$/\1/p')
        if [ -n "$version" ]; then
            if [ "$version" -ge 33 ]; then
                JWTREQ="YES"
            fi
            break
        fi
    done
fi

if [ "$NGXLICENSE" = '' ] && ( [ "$ACTION" = 'install' ] || [ "$ACTION" = 'upgrade' ] ) && [ "$JWTREQ" = 'YES' ]; then
    echo "-j option is mandatory for install/upgrade with NGINX Plus version >=33"
    exit 1
fi

Packages in the repository are named based on a template, so conclusions can be drawn from the filename: nginx-plus_<major>-<minor>~.... This won’t work in all cases, but in the vast majority of instances users will be obtaining files via fetch param I think.

echo "-j option is mandatory for install/upgrade"
exit 1
fi

FILES=$*

if [ -z "$FILES" ]; then
Expand All @@ -95,26 +103,28 @@ fi
ARCH=x86_64
[ `uname -m` = "aarch64" ] && ARCH=aarch64

[ -z $REPOPREFIX ] && REPOPREFIX=https://pkgs.nginx.com/plus

if [ -f /etc/redhat-release ]; then
RELEASE=`grep -Eo 'release [0-9]{1}' /etc/redhat-release | cut -d' ' -f2`
REPOURL=https://pkgs.nginx.com/plus/centos/$RELEASE/$ARCH/RPMS/
REPOURL=$REPOPREFIX/centos/$RELEASE/$ARCH/RPMS/
DISTRO="RHEL/CentOS"
SUFFIX="el"
elif [ -f /etc/os-release ] && fgrep SLES /etc/os-release; then
RELEASE=`grep -Eo 'VERSION="[0-9]{2}' /etc/os-release | cut -d'"' -f2`
REPOURL=https://pkgs.nginx.com/plus/sles/$RELEASE/$ARCH/RPMS/
REPOURL=$REPOPREFIX/sles/$RELEASE/$ARCH/RPMS/
DISTRO="SLES"
SUFFIX="sles"
elif [ -f /etc/os-release ] && fgrep -q -i amazon /etc/os-release; then
RELEASE=`grep -Eo 'VERSION=".+"' /etc/os-release | cut -d'"' -f2`
if [ "$RELEASE" = "2" ]; then
REPOURL=https://pkgs.nginx.com/plus/amzn2/2/$ARCH/RPMS/
REPOURL=$REPOPREFIX/amzn2/2/$ARCH/RPMS/
SUFFIX="amzn2"
elif [ "$RELEASE" = "2023" ]; then
REPOURL=https://pkgs.nginx.com/plus/amzn/2023/$ARCH/RPMS/
REPOURL=$REPOPREFIX/amzn/2023/$ARCH/RPMS/
SUFFIX="amzn2023"
else
REPOURL=https://pkgs.nginx.com/plus/amzn/latest/$ARCH/RPMS/
REPOURL=$REPOPREFIX/amzn/latest/$ARCH/RPMS/
SUFFIX="amzn1"
RELEASE="1"
fi
Expand All @@ -124,10 +134,10 @@ elif [ -f /usr/bin/dpkg ]; then
[ `uname -m` = "aarch64" ] && ARCH=arm64
DISTRO=`grep -E "^ID=" /etc/os-release | cut -d '=' -f2 | tr '[:upper:]' '[:lower:]'`
RELEASE=`grep VERSION_CODENAME /etc/os-release | cut -d '=' -f2`
REPOURL=https://pkgs.nginx.com/plus/$DISTRO/pool/nginx-plus/n/
REPOURL=$REPOPREFIX/$DISTRO/pool/nginx-plus/n/
elif [ -x /sbin/apk ]; then
RELEASE=`grep -Eo 'VERSION_ID=[0-9]\.[0-9]{1,2}' /etc/os-release | cut -d'=' -f2`
REPOURL=https://pkgs.nginx.com/plus/alpine/v$RELEASE/main/$ARCH/
REPOURL=$REPOPREFIX/alpine/v$RELEASE/main/$ARCH/
DISTRO="alpine"
else
echo "Cannot determine your operating system."
Expand All @@ -144,7 +154,7 @@ if [ "$ACTION" = 'fetch' ] || [ "$ACTION" = 'list' ]; then
ldd $(which wget) | grep -q libgnutls || \
echo "" | openssl s_client -servername pkgs.nginx.com -cert $NGXCERT -key $NGXKEY -connect pkgs.nginx.com:443 >/dev/null 2>&1 || \
WGET='wget -q --ciphers DEFAULT@SECLEVEL=1'
if ! $WGET -O /dev/null --certificate=$NGXCERT --private-key=$NGXKEY https://pkgs.nginx.com/plus/ ; then
if ! $WGET -O /dev/null --certificate=$NGXCERT --private-key=$NGXKEY $REPOPREFIX/ ; then
echo "Cannot connect to pkgs.nginx.com, please check certificate and key."
exit 1
fi
Expand Down Expand Up @@ -244,6 +254,7 @@ fetch() {
prepare() {
mkdir -p $ABSPATH
TMPDIR=`mktemp -dq /tmp/nginx-prefix.XXXXXXXX`
cp $NGXLICENSE $TMPDIR/license.jwt
if [ "$DISTRO" = "debian" ] || [ "$DISTRO" = "ubuntu" ]; then
for PKG in $FILES; do
dpkg -x $PKG $TMPDIR
Expand Down Expand Up @@ -329,7 +340,11 @@ extract() {
exit 1
fi
TARGETVER=$($ABSPATH/usr/sbin/nginx -v 2>&1 | cut -d '(' -f 2 | cut -d ')' -f 1 | cut -d'-' -f 3 | tr -d 'r')
if [ $TARGETVER -ge 31 ]; then
if [ $TARGETVER -ge 33 ]; then
mv $TMPDIR/license.jwt $ABSPATH/etc/nginx/license.jwt
echo "mgmt { license_token $ABSPATH/etc/nginx/license.jwt; }" >> $ABSPATH/etc/nginx/nginx.conf
Copy link

@route443 route443 Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In r33, the uuid_file directive was obsolete and replaced by state_path directive. The path can remain the same as for uuid_file, I think, however, the state file will be created automatically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm, do you mean we also need a state_path set here for R33+?

Copy link

Choose a reason for hiding this comment

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

Unfortunately, yes, this is another change that breaks similar scenarios. Actually, we could keep the uuid_file directive, but nginx will log a warning that this directive is deprecated. Starting from 33, state_path will indicate the path to the state file.

fi
if [ $TARGETVER -ge 31 -a $TARGETVER -lt 33 ]; then
echo "mgmt { uuid_file $ABSPATH/var/lib/nginx/nginx.id; }" >> $ABSPATH/etc/nginx/nginx.conf
fi
echo "Installation finished. You may run nginx with this command:"
Expand Down Expand Up @@ -367,7 +382,13 @@ upgrade() {
[ -d $TMPDIR/usr/lib64/ ] && cp -a $TMPDIR/usr/lib64/* $ABSPATH/usr/lib64/
check_modules_deps
TARGETVER=$($ABSPATH/usr/sbin/nginx -v 2>&1 | cut -d '(' -f 2 | cut -d ')' -f 1 | cut -d'-' -f 3 | tr -d 'r')
if [ $TARGETVER -ge 31 ]; then
if [ $TARGETVER -ge 33 ]; then
if ! $ABSPATH/usr/sbin/nginx -p $ABSPATH/etc/nginx -c nginx.conf -T 2>&1 | grep 'license_token' | grep -vE '^(.*)#.*license_token' >/dev/null; then
Copy link

Choose a reason for hiding this comment

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

As I understand it, with this kind of check, I’ll end up with a broken conf if I already have mgmt{} block, say, from a previous ver? And will get somthing like:

mgmt {uuid_file foo;}
mgmt {license_token bar;}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, yes, upgrade from R31/R32 to R33 will be broken if the configuration already has an mgmt block. I'll fix it.

Copy link

Choose a reason for hiding this comment

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

I can’t commit directly to this PR, so I sent you these changes for review in your fork.

cp $NGXLICENSE $ABSPATH/etc/nginx/license.jwt
echo "mgmt { license_token $ABSPATH/etc/nginx/license.jwt; }" >> $ABSPATH/etc/nginx/nginx.conf
fi
fi
if [ $TARGETVER -ge 31 -a $TARGETVER -lt 33 ]; then
if ! $ABSPATH/usr/sbin/nginx -p $ABSPATH/etc/nginx -c nginx.conf -T 2>&1 | grep 'uuid_file' | grep -vE '^(.*)#.*uuid_file' >/dev/null; then
echo "mgmt { uuid_file $ABSPATH/var/lib/nginx/nginx.id; }" >> $ABSPATH/etc/nginx/nginx.conf
fi
Expand All @@ -390,7 +411,7 @@ upgrade() {

list() {
if [ "$DISTRO" = 'ubuntu' ] || [ "$DISTRO" = 'debian' ]; then
REPOURL=https://pkgs.nginx.com/plus/$DISTRO/pool/nginx-plus/n/nginx-plus
REPOURL=$REPOPREFIX/$DISTRO/pool/nginx-plus/n/nginx-plus
fi
echo "Versions available for $DISTRO $RELEASE $ARCH:"
if [ "$DISTRO" = 'alpine' ] ; then
Expand Down