Skip to content

✨ Inject Ironic CA Cert in IPA #851

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

Open
diconico07 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨ Inject Ironic CA Cert in IPA #851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ available, it falls back to the default IPA images.
The following environment variables control IPA kernel boot parameters (all
have sensible defaults):

- `IRONIC_IPA_INSECURE` - Allow insecure connections (default: `1`)
- `IRONIC_IPA_INSECURE` - Allow insecure connections (default: `0` if Ironic
certificate can be verified locally, `1` otherwise)
- `IRONIC_IPA_DEBUG` - Enable debug mode (default: `1`)
- `IRONIC_IPA_INSPECTION_DHCP_ALL_INTERFACES` - Request DHCP on all interfaces
during inspection (default: `1`)
Expand All @@ -186,7 +187,8 @@ have sensible defaults):
- `IRONIC_JSON_RPC_PORT` - port used by the ironic json-rpc service (default to
6189).
- `WEBSERVER_CACERT_FILE` - Specifies the CA or CA bundle that will be used
by Ironic to verify disk and IPA images.
by Ironic to verify disk and IPA images. Will also be used by IPA to verify
disk images.

The following mountpoints can be passed in to customize run-time
functionality:
Expand Down
4 changes: 3 additions & 1 deletion ironic-config/inspector.ipxe.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
echo In inspector.ipxe

{%- macro kernel_cmdline(kernel_path, ramdisk_name) -%}
kernel --timeout 60000 {{ kernel_path }} ipa-insecure={{ env.IRONIC_IPA_INSECURE | default('1') }} ipa-inspection-collectors={{ env.IRONIC_IPA_COLLECTORS }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }} BOOTIF=${mac} ipa-debug={{ env.IRONIC_IPA_DEBUG | default('1') }} ipa-enable-vlan-interfaces={{ env.IRONIC_ENABLE_VLAN_INTERFACES }} ipa-inspection-dhcp-all-interfaces={{ env.IRONIC_IPA_INSPECTION_DHCP_ALL_INTERFACES | default('1') }} ipa-collect-lldp={{ env.IRONIC_IPA_COLLECT_LLDP | default('1') }} {{ env.INSPECTOR_EXTRA_ARGS }} initrd={{ ramdisk_name }} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} || goto retry_boot
kernel --timeout 60000 {{ kernel_path }} ipa-insecure={{ env.IRONIC_IPA_INSECURE | default('0') }} ipa-inspection-collectors={{ env.IRONIC_IPA_COLLECTORS }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }} BOOTIF=${mac} ipa-debug={{ env.IRONIC_IPA_DEBUG | default('1') }} ipa-enable-vlan-interfaces={{ env.IRONIC_ENABLE_VLAN_INTERFACES }} ipa-inspection-dhcp-all-interfaces={{ env.IRONIC_IPA_INSPECTION_DHCP_ALL_INTERFACES | default('1') }} ipa-collect-lldp={{ env.IRONIC_IPA_COLLECT_LLDP | default('1') }} {{ env.INSPECTOR_EXTRA_ARGS }} initrd={{ ramdisk_name }} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} || goto retry_boot
{%- endmacro -%}

{% if env.DEPLOY_KERNEL_BY_ARCH is defined %}
Expand Down Expand Up @@ -55,6 +55,7 @@ imgfree
# ironic-inspector-image and configuration in configure-ironic.sh
{{ kernel_cmdline('${ipa_kernel}', '${ipa_ramdisk_name}') }}
initrd --timeout 60000 ${ipa_ramdisk} || goto retry_boot
initrd --timeout 60000 {{ env.IRONIC_HTTP_URL }}/ipa-cacert-bundle || goto retry_boot
Copy link
Member

@tuminoid tuminoid Mar 11, 2026

Choose a reason for hiding this comment

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

...and eventually we end up here doing reboot loop?

boot
{% else %}
:retry_boot
Expand All @@ -63,5 +64,6 @@ imgfree
# ironic-inspector-image and configuration in configure-ironic.sh
{{ kernel_cmdline(env.IRONIC_HTTP_URL + '/images/ironic-python-agent.kernel', 'ironic-python-agent.initramfs') }}
initrd --timeout 60000 {{ env.IRONIC_HTTP_URL }}/images/ironic-python-agent.initramfs || goto retry_boot
initrd --timeout 60000 {{ env.IRONIC_HTTP_URL }}/ipa-cacert-bundle || goto retry_boot
boot
{% endif %}
25 changes: 17 additions & 8 deletions ironic-config/ipxe_config.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,28 @@ goto deploy

:deploy
imgfree
{%- set ipxe_tls_setup = false %}
{%- if ipxe_tls_setup %}
{%- if pxe_options.deployment_aki_path %}
{%- set aki_path_https_elements = pxe_options.deployment_aki_path.split(':') %}
{%- set aki_port_and_path = aki_path_https_elements[2].split('/') %}
{%- set aki_afterport = aki_port_and_path[1:]|join('/') %}
{%- set aki_path_https = ['https:', aki_path_https_elements[1], ':8084/', aki_afterport]|join %}
{%- set aki_path = ['https:', aki_path_https_elements[1], ':8084/', aki_afterport]|join %}
{%- endif %}
{%- if pxe_options.deployment_ari_path %}
{%- set ari_path_https_elements = pxe_options.deployment_ari_path.split(':') %}
{%- set ari_port_and_path = ari_path_https_elements[2].split('/') %}
{%- set ari_afterport = ari_port_and_path[1:]|join('/') %}
{%- set ari_path_https = ['https:', ari_path_https_elements[1], ':8084/', ari_afterport]|join %}
{%- set ari_path = ['https:', ari_path_https_elements[1], ':8084/', ari_afterport]|join %}
{%- endif %}
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path_https }} selinux=0 troubleshoot=0 text {{ pxe_options.pxe_append_params|default("", true) }} BOOTIF=${mac} initrd={{ pxe_options.initrd_filename|default("deploy_ramdisk", true) }} || goto retry
{%- else %}
{%- set ari_path = pxe_options.deployment_ari_path %}
{%- set aki_path = pxe_options.deployment_aki_path %}
{%- endif %}
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path }} selinux=0 troubleshoot=0 text {{ pxe_options.pxe_append_params|default("", true) }} BOOTIF=${mac} initrd={{ pxe_options.initrd_filename|default("deploy_ramdisk", true) }} || goto retry

initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path_https }} || goto retry
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path }} || goto retry
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %} ../ipa-cacert-bundle || goto retry
boot

:retry
Expand All @@ -39,17 +46,19 @@ poweroff

:boot_anaconda
imgfree
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path_https }} text {{ pxe_options.pxe_append_params|default("", true) }} inst.ks={{ pxe_options.ks_cfg_url }} {% if pxe_options.repo_url %}inst.repo={{ pxe_options.repo_url }}{% else %}inst.stage2={{ pxe_options.stage2_url }}{% endif %} initrd=ramdisk || goto boot_anaconda
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path_https }} || goto boot_anaconda
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path }} text {{ pxe_options.pxe_append_params|default("", true) }} inst.ks={{ pxe_options.ks_cfg_url }} {% if pxe_options.repo_url %}inst.repo={{ pxe_options.repo_url }}{% else %}inst.stage2={{ pxe_options.stage2_url }}{% endif %} initrd=ramdisk || goto boot_anaconda
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path }} || goto boot_anaconda
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %} ../ipa-cacert-bundle || goto boot_anaconda
boot

:boot_ramdisk
imgfree
{%- if pxe_options.boot_iso_url %}
sanboot {{ pxe_options.boot_iso_url }}
{%- else %}
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path_https }} root=/dev/ram0 text {{ pxe_options.pxe_append_params|default("", true) }} {{ pxe_options.ramdisk_opts|default('', true) }} initrd=ramdisk || goto boot_ramdisk
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path_https }} || goto boot_ramdisk
kernel {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ aki_path }} root=/dev/ram0 text {{ pxe_options.pxe_append_params|default("", true) }} {{ pxe_options.ramdisk_opts|default('', true) }} initrd=ramdisk || goto boot_ramdisk
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %}{{ ari_path }} || goto boot_ramdisk
initrd {% if pxe_options.ipxe_timeout > 0 %}--timeout {{ pxe_options.ipxe_timeout }} {% endif %} ../ipa-cacert-bundle || goto boot_ramdisk
boot
{%- endif %}

Expand Down
9 changes: 4 additions & 5 deletions ironic-config/ironic.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ deploy_logs_local_path = /shared/log/ironic/deploy
# See https://bugzilla.redhat.com/show_bug.cgi?id=1822763
max_command_attempts = 30
certificates_path = {{ env.IRONIC_GEN_CERT_DIR }}
api_ca_file = {{ env.IPA_CACERT_FILE }}
Comment on lines 56 to +57
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The api_ca_file configuration is unconditionally set to {{ env.IPA_CACERT_FILE }}, but this file may not exist in all scenarios (e.g., when no IPA CA certificates are provided, no default CA bundle exists, and no Ironic CA certificate is available). This could cause Ironic to fail if it expects a valid file path. Consider adding a conditional check similar to the webserver_verify_ca pattern above (lines 32-36) to only set this when the file exists.

Suggested change
certificates_path = {{ env.IRONIC_GEN_CERT_DIR }}
api_ca_file = {{ env.IPA_CACERT_FILE }}
certificates_path = {{ env.IRONIC_GEN_CERT_DIR }}
{% if env.IPA_CACERT_FILE %}
api_ca_file = {{ env.IPA_CACERT_FILE }}
{% endif %}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@diconico07 diconico07 Feb 11, 2026

Choose a reason for hiding this comment

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

to me an empty file would denote a configuration issue (default bundle path not correctly set) and thus I'm not sure we want to make it conditional here, but I'm not opposed to do so if you see other benefits to it.


[api]
{% if env.IRONIC_REVERSE_PROXY_SETUP == "true" %}
Expand Down Expand Up @@ -233,20 +234,18 @@ images_path = /shared/html/tmp
instance_master_path = /shared/html/master_images
tftp_master_path = /shared/tftpboot/master_images
tftp_root = /shared/tftpboot
kernel_append_params = nofb nomodeset vga=normal ipa-insecure=1 {% if env.ENABLE_FIPS_IPA %}fips={{ env.ENABLE_FIPS_IPA|trim }}{% endif %} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }}
kernel_append_params = nofb nomodeset vga=normal initrd=ipa-cacert-bundle ipa-insecure={{ env.IRONIC_IPA_INSECURE | default('0') }} {% if env.ENABLE_FIPS_IPA %}fips={{ env.ENABLE_FIPS_IPA|trim }}{% endif %} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }}
# This makes networking boot templates generated even for nodes using local
# boot (the default), ensuring that they boot correctly even if they start
# netbooting for some reason (e.g. with the noop management interface).
enable_netboot_fallback = true
# Enable the fallback path to in-band inspection
ipxe_fallback_script = inspector.ipxe
{% if env.IPXE_TLS_SETUP | lower == "true" %}
ipxe_config_template = /templates/ipxe_config.template
{% endif %}
ipxe_config_template = {{ env.IRONIC_CONF_DIR }}/ipxe_config.template

[redfish]
use_swift = false
kernel_append_params = nofb nomodeset vga=normal ipa-insecure=1 {% if env.ENABLE_FIPS_IPA %}fips={{ env.ENABLE_FIPS_IPA|trim }}{% endif %} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }}
kernel_append_params = nofb nomodeset vga=normal ipa-insecure={{ env.IRONIC_IPA_INSECURE | default('0') }} {% if env.ENABLE_FIPS_IPA %}fips={{ env.ENABLE_FIPS_IPA|trim }}{% endif %} {% if env.IRONIC_RAMDISK_SSH_KEY %}sshkey="{{ env.IRONIC_RAMDISK_SSH_KEY|trim }}"{% endif %} {{ env.IRONIC_KERNEL_PARAMS|trim }} systemd.journald.forward_to_console={{ env.IPA_FORWARD_CONSOLE | default('yes') }}
{% if env.BMC_TLS_ENABLED == "true" %}
# idrac uses the same options as the redfish driver
verify_ca = {{ env.BMC_CACERT_FILE }}
Expand Down
1 change: 1 addition & 0 deletions main-packages-list.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
cpio
dnsmasq
dosfstools
httpd
Expand Down
6 changes: 6 additions & 0 deletions scripts/configure-ironic.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ if [[ -f /proc/sys/crypto/fips_enabled ]]; then
export ENABLE_FIPS_IPA
fi

if [[ "${IPXE_TLS_SETUP,,}" == "true" ]]; then
sed 's/set ipxe_tls_setup = false/set ipxe_tls_setup = true/' /templates/ipxe_config.template > "${IRONIC_CONF_DIR}/ipxe_config.template"
else
cp /templates/ipxe_config.template "${IRONIC_CONF_DIR}/ipxe_config.template"
fi

# The original ironic.conf is empty, and can be found in ironic.conf_orig
render_j2_config "/etc/ironic/ironic.conf.j2" \
"${IRONIC_CONF_DIR}/ironic.conf"
Expand Down
1 change: 1 addition & 0 deletions scripts/rundnsmasq
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ if [[ "${DNS_IP:-}" == "provisioning" ]]; then
fi

mkdir -p /shared/tftpboot
mkdir -p /shared/html

# Copy files to shared mount
if [[ -r "${IPXE_CUSTOM_FIRMWARE_DIR}" ]]; then
Expand Down
8 changes: 8 additions & 0 deletions scripts/runironic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ if [[ "${IRONIC_SKIP_DBSYNC:-false}" != true ]]; then
run_ironic_dbsync
fi

generate_cacert_bundle_initrd /shared/html/ipa-cacert-bundle

if [[ "${IRONIC_IPA_INSECURE:-0}" -eq 0 ]] && [[ -n "${WEBSERVER_CACERT_FILE:-}" ]]; then
ipa_cert_update_enabled="true"
else
ipa_cert_update_enabled="false"
fi
configure_restart_on_certificate_update "${ipa_cert_update_enabled}" ironic "${WEBSERVER_CACERT_FILE:-}"
configure_restart_on_certificate_update "${IRONIC_TLS_SETUP}" ironic "${IRONIC_CERT_FILE}"

configure_ironic_auth
Expand Down
44 changes: 43 additions & 1 deletion scripts/tls-common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export IRONIC_SSL_PROTOCOL=${IRONIC_SSL_PROTOCOL:-"-ALL +TLSv1.2 +TLSv1.3"}
export IPXE_SSL_PROTOCOL=${IPXE_SSL_PROTOCOL:-"-ALL +TLSv1.2 +TLSv1.3"}
export IRONIC_VMEDIA_SSL_PROTOCOL=${IRONIC_VMEDIA_SSL_PROTOCOL:-"ALL"}

export DEFAULT_CACERT_BUNDLE=${DEFAULT_CACERT_BUNDLE:-"/etc/ssl/cert.pem"}

# Node image storage is using the same cert and port as the API
export IRONIC_CERT_FILE=/certs/ironic/tls.crt
export IRONIC_KEY_FILE=/certs/ironic/tls.key
Expand All @@ -22,7 +24,8 @@ export RESTART_CONTAINER_CERTIFICATE_UPDATED=${RESTART_CONTAINER_CERTIFICATE_UPD
export MARIADB_CACERT_FILE=/certs/ca/mariadb/tls.crt
export BMC_CACERTS_PATH=/certs/ca/bmc
export BMC_CACERT_FILE=/conf/bmc-tls.pem
export IRONIC_CACERT_FILE=/certs/ca/ironic/tls.crt
export IRONIC_CACERT_FILE=${IRONIC_CACERT_FILE:-"/certs/ca/ironic/tls.crt"}
export IPA_CACERT_FILE=/conf/ipa-tls.pem

export IPXE_TLS_PORT="${IPXE_TLS_PORT:-8084}"

Expand Down Expand Up @@ -129,3 +132,42 @@ if ls "${BMC_CACERTS_PATH}"/* > /dev/null 2>&1; then
else
export BMC_TLS_ENABLED="false"
fi

if [[ -f "${WEBSERVER_CACERT_FILE:-}" ]]; then
copy_atomic "${WEBSERVER_CACERT_FILE}" "${IPA_CACERT_FILE}"
elif [[ -f "${DEFAULT_CACERT_BUNDLE}" ]]; then
Copy link
Member

@dtantsur dtantsur Mar 20, 2026

Choose a reason for hiding this comment

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

I cannot make up my mind about this aspect (hence my somewhat erratic comments below). While unlikely, some folks may have CA certificates built into their IPA images (in fact, I know people who do that, albeit outside of Metal3 context). This change will start overriding them with a generic bundle from CentOS Stream.

I'm leaning towards preferring an opt-in approach, even if it involves setting WEBSERVER_CACERT_FILE=/etc/ssl/cert.pem in the Ironic environment.

copy_atomic "${DEFAULT_CACERT_BUNDLE}" "${IPA_CACERT_FILE}"
fi
Copy link
Member

@tuminoid tuminoid Mar 11, 2026

Choose a reason for hiding this comment

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

There is no else here...

Copy link
Contributor Author

@diconico07 diconico07 Mar 11, 2026

Choose a reason for hiding this comment

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

would a

else
    echo "Missing TLS CA Bundle"
    exit 1

be okay for you, or should I find another way ?

Copy link
Member

@dtantsur dtantsur Mar 20, 2026

Choose a reason for hiding this comment

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

I'm not sure about making a CA bundle required on the Ironic side. Someone may have it embedded in the ramdisk.

In fact, this is most certainly going to break OpenShift because we don't provide IRONIC_CACERT_FILE.

Copy link
Member

@dtantsur dtantsur Mar 20, 2026

Choose a reason for hiding this comment

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

So yeah, the way the code is written now, it's better to fail explicitly. Otherwise, IPA_CACERT_FILE may not exist at all or may contain only the Ironic CA.


if [[ -f "${IRONIC_CACERT_FILE}" ]]; then
cat "${IRONIC_CACERT_FILE}" >> "${IPA_CACERT_FILE}"
fi

if ! openssl verify -CAfile "${IPA_CACERT_FILE}" "${IRONIC_CERT_FILE}" > /dev/null 2>&1; then
# if we are unable to verify the Ironic cert file set IPA_INSECURE to true
Copy link
Member

@dtantsur dtantsur Mar 20, 2026

Choose a reason for hiding this comment

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

Let's issue a warning message here

export IRONIC_IPA_INSECURE="1"
Copy link
Member

@tuminoid tuminoid Mar 11, 2026

Choose a reason for hiding this comment

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

...which leads to IPA being insecure, fine...

Copy link
Member

@dtantsur dtantsur Mar 20, 2026

Choose a reason for hiding this comment

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

Ah, okay, so OpenShift should fall back here.

fi

generate_cacert_bundle_initrd()
(
set -euo pipefail

local output_path="$1"
local temp_dir

temp_dir="$(mktemp -d)"
trap 'rm -rf "${temp_dir}"' EXIT

chmod 0755 "${temp_dir}"

cd "${temp_dir}"

mkdir -p etc/ironic-python-agent.d etc/ironic-python-agent
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The mkdir -p commands for creating the directory structure do not have error checking. If the directory creation fails (e.g., due to permissions), the subsequent cp command will fail. While the function will return an error due to the subshell ( usage and bash's error propagation, it would be clearer to check the result explicitly or ensure the script has proper error handling enabled with set -e within the subshell.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The generate_cacert_bundle_initrd function may fail silently if the IPA_CACERT_FILE does not exist. This can happen when there are no IPA CA certificates and no default CA bundle available. The function should check if the source file exists before attempting to copy it, or handle the error explicitly.

Suggested change
mkdir -p etc/ironic-python-agent.d etc/ironic-python-agent
mkdir -p etc/ironic-python-agent.d etc/ironic-python-agent
if [ ! -f "${IPA_CACERT_FILE}" ]; then
echo "ERROR: IPA CA certificate bundle '${IPA_CACERT_FILE}' not found; cannot generate initrd bundle." >&2
return 1
fi

Copilot uses AI. Check for mistakes.
cp "${IPA_CACERT_FILE}" etc/ironic-python-agent/ironic.crt
Copy link
Member

@tuminoid tuminoid Mar 11, 2026

Choose a reason for hiding this comment

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

...then we end up here copying non-existent files, and doing bad configs....

cat > etc/ironic-python-agent.d/ironic-tls.conf <<EOF
[DEFAULT]
cafile = /etc/ironic-python-agent/ironic.crt
EOF

find . -print0 | sort -z | cpio -0 -o -H newc -R +0:+0 --reproducible >> "${output_path}"
)
Loading