Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ include cf_remote/nt-discovery.sh
include cf_remote/Vagrantfile
include cf_remote/default_provision.sh
include cf_remote/demo.sql
include cf_remote/remote-download.sh

2 changes: 2 additions & 0 deletions cf_remote/nt-discovery.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ run_command "command -v apt" "APT" "Cannot find apt"
run_command "command -v pkg" "PKG" "Cannot find pkg"
run_command "command -v zypper" "ZYPPER" "Cannot find zypper"
run_command "command -v curl" "CURL" "Cannot find curl"
run_command "command -v wget" "WGET" "Cannot find wget"
run_command "command -v sha256sum" "SHA256SUM" "Cannot find sha256sum"

# ip

Expand Down
70 changes: 70 additions & 0 deletions cf_remote/remote-download.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/bin/bash

set -e

# Arg parsing

INSECURE=0
HAS_WGET=0
CHECKSUM=""

while getopts c:IW option; do
case "${option}" in
I)
INSECURE=1
;;
W)
HAS_WGET=1
;;
c)
CHECKSUM=$OPTARG
;;
*)
echo "Usage: $0 [-I] [-c checksum] package_url"
Copy link
Contributor

Choose a reason for hiding this comment

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

-W is not mentioned in the usage string

exit 1
;;
esac
done

shift $((OPTIND - 1))

PACKAGE=$1
if [ -z "$PACKAGE" ]; then
echo "Usage: $0 [-I] [-c checksum] package_url"
exit 1;
fi

# temp file

tmpfile=$(mktemp)
cleanup() {
rm -f "$tmpfile"
}
trap cleanup EXIT QUIT TERM INT

# Download

if [ "$HAS_WGET" -eq 1 ]; then
wget -nv -O "$tmpfile" "$PACKAGE"
else
curl --fail -sS -o "$tmpfile" "$PACKAGE"
fi

# Checksum

filename="$(basename "$PACKAGE")"

if [ -n "$CHECKSUM" ]; then
hash="$(sha256sum "$tmpfile" | awk '{print $1}')"

if [[ "$CHECKSUM" != "$hash" ]]; then
if [ "$INSECURE" -eq 0 ]; then
echo "Package '$PACKAGE' doesn't match the expected checksum '$CHECKSUM'. Run with --insecure to skip"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit confusing. I'm guessing the --insecure flag references an option in cf-remote? Because in this script the equivalent is the -I option? If I were to execute this script manually – which you should probably not do, but is something I would do when debugging – then I might try the --insecure flag after getting this error.

If cf-remote also supports the short option, then maybe you should use that in the error message? Or maybe you should change this script to parse long options?

exit 1
fi
echo "Package '$PACKAGE' doesn't match the expected checksum '$CHECKSUM'. Continuing due to insecure flag"
fi
fi

mv "$tmpfile" "$filename"

148 changes: 125 additions & 23 deletions cf_remote/remote.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python3
import sys
import re

from os.path import basename, dirname, join, exists
from collections import OrderedDict
from typing import Union
Expand Down Expand Up @@ -276,7 +277,17 @@ def get_info(host, *, users=None, connection=None):
data["role"] = "hub" if discovery.get("NTD_CFHUB") else "client"

data["bin"] = {}
for bin in ["dpkg", "rpm", "yum", "apt", "pkg", "zypper", "curl"]:
for bin in [
"dpkg",
"rpm",
"yum",
"apt",
"pkg",
"zypper",
"curl",
"wget",
"sha256sum",
]:
path = discovery.get("NTD_{}".format(bin.upper()))
if path:
data["bin"][bin] = path
Expand Down Expand Up @@ -432,7 +443,7 @@ def bootstrap_host(host_data, policy_server, *, connection=None, trust_server=Tr
def _package_from_list(tags, extension, packages):
artifacts = [Artifact(None, p) for p in packages]
artifact = filter_artifacts(artifacts, tags, extension)[-1]
return artifact.url
return artifact.url, artifact


def _package_from_releases(
Expand All @@ -445,7 +456,7 @@ def _package_from_releases(
release = releases.pick_version(version)
if release is None:
print("Could not find a release for version {}".format(version))
return None
return None, None

release.init_download()

Expand All @@ -455,7 +466,7 @@ def _package_from_releases(
version, edition
)
)
return None
return None, None

artifacts = release.find(tags, extension)
if not artifacts:
Expand All @@ -464,13 +475,16 @@ def _package_from_releases(
"hub" if "hub" in tags else "client"
)
)
return None
return None, None
artifact = artifacts[-1]
if remote_download:
return artifact.url
return artifact.url, artifact
else:
return download_package(
artifact.url, checksum=artifact.checksum, insecure=insecure
return (
download_package(
artifact.url, checksum=artifact.checksum, insecure=insecure
),
artifact,
)


Expand Down Expand Up @@ -514,13 +528,103 @@ def get_package_from_host_info(
tags.extend(tag for tag in package_tags if tag != "msi")

if packages is None: # No command line argument given
package = _package_from_releases(
package, artifact = _package_from_releases(
tags, extension, version, edition, remote_download, insecure
)
else:
package = _package_from_list(tags, extension, packages)
package, artifact = _package_from_list(tags, extension, packages)

return package, artifact


return package
"""
Remotely download package on host.

This function checks for avalaible binaries, copies remote-download.sh to the host, then runs it to download the package.
The script ensures package integrity by comparing checksums.
"""
Comment on lines +540 to +545
Copy link
Contributor

Choose a reason for hiding this comment

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

Docstrings go inside the function, as the first statement after the def line



def _remote_download(
host, package, artifact, pkg_binary, insecure=False, connection=None
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something, but the pkg_binary variable name is a bit confusing. Especially without a docstring explaining what it is. From the name I would guess it would be a string containing dpkg or rpm for the hosts packaging tool binary. However, it appears to be a list of installed programs we can use on the remote host. Maybe the name should be available_programs, detected_programs, found_programs, installed_tools, or something like that?

):

if not pkg_binary:
log.error("No binary could be found on host '{}'.format(host)")
Copy link
Contributor

Choose a reason for hiding this comment

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

The .format(...) should be on the outside of the quotes

return None

if "sha256sum" not in pkg_binary:
if not insecure:
log.error(
"Cannot check file integrity. sha256sum is not installed on host '{}'. Run with --insecure to skip".format(
host
)
)
return None
log.warning(
"Cannot check file integrity. sha256sum is not installed on host '{}'. Continuing due to insecure flag".format(
host
)
)

if "wget" in pkg_binary:
has_wget = True
elif "curl" in pkg_binary:
has_wget = False
else:
log.error(
"Cannot download remotely. wget and/or curl are not installed on host '{}'".format(
host
)
)
return None
Comment on lines +570 to +580
Copy link
Contributor

Choose a reason for hiding this comment

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

Could consider:

Suggested change
if "wget" in pkg_binary:
has_wget = True
elif "curl" in pkg_binary:
has_wget = False
else:
log.error(
"Cannot download remotely. wget and/or curl are not installed on host '{}'".format(
host
)
)
return None
if not any(bin in pkg_binary for bin in ("wget", "curl"))
log.error(
"Cannot download remotely. wget and curl are not installed on host '{}'".format(
host
)
)
return None
has_wget = "wget" in pkg_binary

Also the log message should say and not and/or.


if not (artifact and artifact.checksum):
if not insecure:
log.error(
"Cannot check file integrity on '{}'. No artifact associated with package '{}' found. Run with --insecure to skip".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the term artifact refer to? A docstring would be very helpful.

host, package
)
)
return None
log.warning(
"Cannot check file integrity on '{}'. No artifact associated with package '{}' found. Continuing due to insecure flag".format(
host, package
)
)

cf_remote_dir = dirname(__file__)
script_path = join(cf_remote_dir, "remote-download.sh")
if not exists(script_path):
sys.exit("%s does not exist" % script_path)
scp(
script_path,
host,
connection,
hide=True,
)

args = ""
if insecure:
args += "-I "
if has_wget:
args += "-W "
if artifact:
args += "-c {} ".format("hhh")
args += package

ret = ssh_cmd(connection, "bash remote-download.sh {}".format(args), errors=True)

if ret is None:
return None
if insecure:
warning = re.search(r"Package.*", ret)
if warning:
log.warning(warning.group(0))

log.debug("Successfully remotely downloaded package on host '{}'".format(host))
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like Package '{package_name}' downloaded successfully on host '{hostname}', or Package download completed on host '{hostname}'?


return basename(package)


@auto_connect
Expand Down Expand Up @@ -552,9 +656,10 @@ def install_host(
elif packages and len(packages) == 1:
package = packages[0]

artifact = None
if not package:
try:
package = get_package_from_host_info(
package, artifact = get_package_from_host_info(
data.get("package_tags"),
data.get("bin"),
data.get("arch"),
Expand All @@ -574,19 +679,16 @@ def install_host(
return 1

if remote_download:
if ("bin" not in data) or ("curl" not in data["bin"]):
log.error(
"Couldn't download remotely. Curl is not installed on host '%s'" % host
)
return 1

print("Downloading '%s' on '%s' using curl" % (package, host))
r = ssh_cmd(
cmd="curl --fail -O {}".format(package), connection=connection, errors=True
package = _remote_download(
host,
package,
artifact,
data.get("bin"),
connection=connection,
insecure=insecure,
)
if r is None:
if package is None:
return 1
package = basename(package)
elif not getattr(connection, "is_local", False):
scp(package, host, connection=connection)
package = basename(package)
Expand Down
2 changes: 1 addition & 1 deletion cf_remote/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ def parse_envfile(text):
if not key:
return error_and_none("Invalid env file format: Key missing")

if not re.fullmatch(r"([A-Z]+\_?)+", key):
if not re.fullmatch(r"[A-Z][A-Z0-9]*(\_[A-Z0-9]+)*", key):
return error_and_none("Invalid env file format: Invalid key")

if not (val.startswith('"') and val.endswith('"')):
Expand Down
Loading