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

Allow for private CA on the Agent side #3494

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Allow for private CA on the Agent side #3494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3add046
Allow for private CA on the Agent side
dbutenhof Jul 11, 2023
abbce29
Some updates ...
dbutenhof Jul 14, 2023
8b0a349
Cleanup and commentary
dbutenhof Jul 17, 2023
f25cacf
Review comments & cleanup
dbutenhof Jul 19, 2023
a088575
Minor tweak I missed
dbutenhof Jul 19, 2023
aab4a16
Cleanup
dbutenhof Jul 21, 2023
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
1 change: 1 addition & 0 deletions agent/config/pbench-agent-default.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ ssh_opts = -o BatchMode=yes -o StrictHostKeyChecking=no
api_version = 1
rest_endpoint = api/v%(api_version)s
server_rest_url = https://%(pbench_web_server)s/%(rest_endpoint)s
#server_ca =

[pbench/tools]
light-tool-set = vmstat
Expand Down
67 changes: 67 additions & 0 deletions agent/containers/images/container_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#!/bin/bash

# This is an essentially "trivial" script to interactively build a full set of
# Pbench Agent containers for a platform specified by the PB_AGENT_DISTRO
# environment variable, defaulting to fedora-38.
#
# These are the same commands used in the CI; however, the CI builds the
# RPMs in the common build.sh script while the Jenkins Pipeline.gy builds the
# CI container on top of that. Here we encapsulate the steps used to get a
# functional result to make this more convenient interactively.
#
# WORKSPACE_TMP is assumed to be the root of the work area, and by default will
# be set to ${HOME}.
#
# If you want to clean the make targets for RPM and container builds, use:
#
# agent/containers/images/container_build.sh --clean

export PB_AGENT_DISTRO=${PB_AGENT_DISTRO:-fedora-38}
export WORKSPACE_TMP=${WORKSPACE_TMP:-${HOME}}

function usage {
printf "Build a Pbench Agent container for the distribution named by the\n"
printf "PB_AGENT_DISTRO environment variable, which defaults to '${PB_AGENT_DISTRO}'.\n"
printf "\nThe following options are available:\n"
printf "\n"
printf -- "\t-c|--clean\n"
printf "\t\tRemove old RPM and container image targets before building.\n"
printf -- "\t-h|--help\n"
printf "\t\tPrint this usage message and terminate.\n"
}

opts=$(getopt -q -o ch --longoptions "clean,help" -n "${0}" -- "${@}")
if [[ ${?} -ne 0 ]]; then
printf -- "%s %s\n\n\tunrecognized option specified\n\n" "${0}" "${*}" >&2
usage >&2
exit 1
fi
eval set -- "${opts}"
rpm_clean=
image_clean=
while true; do
arg=${1}
shift
case "${arg}" in
-c|--clean)
rpm_clean=distclean
image_clean=clean
;;
-h|--help)
usage
exit 0
;;
--)
break
;;
*)
printf -- "${0}: unrecognized command line argument, '${arg}'\n" >&2
usage >&2
webbnh marked this conversation as resolved.
Show resolved Hide resolved
exit 1
;;
esac
done

make -C agent/rpm ${rpm_clean} ${PB_AGENT_DISTRO}-rpm
make -C agent/containers/images CI=1 CI_RPM_ROOT=${WORKSPACE_TMP} \
${image_clean} ${PB_AGENT_DISTRO}-everything
71 changes: 32 additions & 39 deletions contrib/containerized-pbench/pbench
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,62 @@
#! /bin/bash
#
# This script is a wrapper to facilitate the invocation of a Pbench Agent
# command using a containerized deployment of the Pbench Agent. Simply prefix
# command using a containerized deployment of the Pbench Agent. Simply prefix
# a Pbench Agent command line with the path to this script to run it inside a
# container, without needing to install the Agent on the host system.
#
# Invocation options are provided as environment variables:
# PB_AGENT_IMAGE_NAME: the full image name for the containerized Pbench Agent
# _PBENCH_AGENT_CONFIG: the location of the Pbench Agent configuration file
# PB_AGENT_RUN_DIR: the directory for use as the Pbench Agent "run directory"
# PB_AGENT_SERVER_LOC: the host and port for the Pbench Server
# PB_AGENT_PODMAN_OPTIONS: Additional options to be supplied to Podman run
# PB_AGENT_IMAGE_NAME: the full image name for the containerized Pbench Agent
# PB_AGENT_RUN_DIR: the directory for use as the Pbench Agent "run directory"
# PB_AGENT_CA: a CA bundle to verify Pbench Server PUTs
# PB_AGENT_PODMAN_OPTIONS: Additional options to be supplied to Podman run
#
# In all cases, reasonable defaults are supplied if the environment variables
# are not defined.
#
# This script checks for the presence of a `~/.ssh` directory, an existing
# Pbench Agent configuration file, and a Pbench Agent "run directory" and maps
# them into the container if they exist. If the configuration file is missing
# but the location of the Pbench Server is available, then this script will
# generate the configuration file, and the script creates the run directory if
# it does not exist. The script then invokes the Pbench Agent container with
# these options and any others which the user has specified and passes in the
# command to be executed.
# This script manages a persistent host Pbench Agent "run directory", which
# defaults to /var/tmp/{USER}/pbench-agent/run, and maps that directory into
# the container so that multiple runs can be generated and uploaded at once.
#
# To upload results to a Pbench Server, use this script to execute the
# pbench-results-move command within the container, specifying either --relay
# with the address of a Pbench Relay Server, or --server with the address of a
# Pbench Server and --token to specify a Pbench Server API key for user
# authentication.
#
# To use a server with a certificate signed by the Pbench development CA bundle
# define the environment variable PB_AGENT_CA to cause the CA to be mapped into
# the container and defined using REQUESTS_CA_BUNDLE:
#
# PB_AGENT_CA=server/pbenchinacan/etc/pki/tls/certs/pbench_CA.crt \
# contrib/containerized-pbench/pbench pbench-results-move \
# --server https://<server>:8443 --token <api-token>

image_name=${PB_AGENT_IMAGE_NAME:-quay.io/pbench/pbench-agent-all-centos-8:main}
config_file=${_PBENCH_AGENT_CONFIG:-${HOME}/.config/pbench/pbench-agent.cfg}
pbench_run_dir=${PB_AGENT_RUN_DIR:-/var/tmp/${USER}/pbench-agent/run}
pbench_server=${PB_AGENT_SERVER_LOC}
ca=${PB_AGENT_CA:-${REQUESTS_CA_BUNDLE}}
if [[ ${ca} ]]; then
pbench_ca=$(realpath ${ca}) # expand path outside container
fi
container_ca=/etc/pki/tls/certs/pbench_CA.crt # path inside container
webbnh marked this conversation as resolved.
Show resolved Hide resolved
other_options=${PB_AGENT_PODMAN_OPTIONS}

if [[ $# == 0 || $1 == "help" || $1 == "-h" || $1 == "--help" ]]; then
echo "Usage: ${0} <Pbench Agent Command> [<arg>...]" >&2
exit 2
fi

if [[ -d "${HOME}/.ssh" && -r "${HOME}/.ssh" ]]; then
other_options="--security-opt=label=disable -v ${HOME}/.ssh:/root/.ssh ${other_options}"
fi

if [[ -f "${config_file}" && -r "${config_file}" ]]; then
other_options="-v ${config_file}:/opt/pbench-agent/config/pbench-agent.cfg:z ${other_options}"
elif [[ -n "${pbench_server}" ]]; then
echo "Warning: the Pbench Agent config file is missing; attempting to generate one in ${config_file}" >&2
webbnh marked this conversation as resolved.
Show resolved Hide resolved
# TODO: this should be handled by a separate Pbench Agent "configuration wizard".
mkdir -p $(dirname ${config_file})
cat > ${config_file} <<- EOF
[DEFAULT]
pbench_install_dir = /opt/pbench-agent
pbench_web_server = ${pbench_server}
[config]
path = %(pbench_install_dir)s/config
files = pbench-agent-default.cfg
EOF
else
echo "Warning: the Pbench Agent config file (e.g., ${config_file}) is missing or inaccessible -- using default configuration." >&2
fi

mkdir -p ${pbench_run_dir}
other_options="-v ${pbench_run_dir}:/var/lib/pbench-agent:z ${other_options}"
if [[ -f "${pbench_ca}" ]]; then
other_options="-v ${pbench_ca}:${container_ca}:Z ${other_options}"
other_options="-e REQUESTS_CA_BUNDLE=${container_ca} ${other_options}"
fi

podman run \
-it \
--rm \
--network host \
--name pbench-agent \
-v ${pbench_run_dir}:/var/lib/pbench-agent:Z \
${other_options} \
${image_name} "${@}"
11 changes: 10 additions & 1 deletion lib/pbench/agent/results.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,13 @@ def __init__(
if server:
path = config.get("results", "rest_endpoint")
uri = f"{server}/{path}"
self.ca = None
else:
uri = config.get("results", "server_rest_url")

# If the "server_ca" config variable isn't defined, we expect to verify
# using a registered CA.
self.ca = config.get("results", "server_ca", fallback=None)
Comment on lines 406 to +415
Copy link
Member

Choose a reason for hiding this comment

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

Is this code up-to-date with our current thinking?

As the code now stands (AFAICT), if we specify server, then we ignore the server_ca option in the config file, and we'll depend on REQUESTS_CA_BUNDLE if an override is needed; if, instead, we pull the uri from the config file, then we'll honor server_ca if it is provided...why the bifurcation? Why not use the config file CA value in both cases if it is provided? Or, why have the server_ca option at all when the user can use REQUESTS_CA_BUNDLE?

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 config file defines a server and optionally a CA to go with that server. If you don't create a custom config file and use --server, there's not going to be a server_ca anyway. But if you have a config file, even with a server_ca, we can't necessarily assume that the --server target cert uses that CA.

So basically, config file server goes with config file CA; while --server goes with REQUESTS_CA_BUNDLE.

Copy link
Member

Choose a reason for hiding this comment

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

I'm unclear why we need this change and why it's better than just relying on REQUESTS_CA_BUNDLE for both cases. 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

So your idea is that if someone installs the Agent RPM and relies on a server configured in pbench-agent.cfg so they can just pbench-results-move (e.g., the passthrough server) we should still require them to put a customized REQUESTS_CA_BUNDLE in the environment?

Granted, in the long run, we don't want to force users to rely on private CA certs at all, but this still seems worth doing.

Copy link
Member

Choose a reason for hiding this comment

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

My thinking is that, in production, access to the Pbench Server will require only publicly-available and trusted CAs (for some definition of "public"...e.g., "Red Hat internal", perhaps...), and so no specific action (e.g., defining REQUESTS_CA_BUNDLE) on the part of the user would be required.

For Staging, we should be operating in a similar environment (i.e., behind the IntLab proxies and using IntLab-signed or RH-IT-signed certs).

So, it's only for func-test and development that we would need to be using our private CA. "Normal users" wouldn't need it, unless they are working with bits that are too hot for Staging.

And, I think we have func-test already covered. So, it all really boils down to how do we support our developers. And, for that, I think we can rely on defining REQUESTS_CA_BUNDLE (and, we can probably do it implicitly inside a wrapper script or two, so it doesn't have to be reflected at all in the actual Agent or Server code).

self.uri = f"{uri}/upload/{{name}}"
self.headers.update({"Authorization": f"Bearer {token}"})

Expand All @@ -430,7 +435,11 @@ def push(self, tarball: Path, tarball_md5: str) -> requests.Response:
tar_uri = self.uri.format(name=tarball.name)
with tarball.open("rb") as f:
return requests.put(
tar_uri, data=f, headers=self.headers, params=self.params
tar_uri,
data=f,
headers=self.headers,
params=self.params,
verify=self.ca,
)


Expand Down