Skip to content

Comments

Add network overrides and build a qemu image#15599

Closed
binujp wants to merge 6 commits intotomls/base/mainfrom
bphilip/network-override
Closed

Add network overrides and build a qemu image#15599
binujp wants to merge 6 commits intotomls/base/mainfrom
bphilip/network-override

Conversation

@binujp
Copy link
Contributor

@binujp binujp commented Jan 28, 2026

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./LICENSES-AND-NOTICES/SPECS/data/licenses.json, ./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md, ./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge

Summary

What does the PR accomplish, why was it needed?

We have to make sure systemd-networkd is managing all interfaces. Add a package to deliver the override.

Brought in logic to build a vm-image with the current package selection. This is based on Nan Liu's changes which accomplished the same.

Change Log
  • Change
  • Change
  • Change
Does this affect the toolchain?

YES/NO

Associated issues
  • #xxxx
Links to CVEs
Test Methodology
  • Pipeline build id: xxxx

@binujp binujp requested review from Copilot and reubeno and removed request for Copilot January 28, 2026 07:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds systemd-networkd configuration for managing all network interfaces and introduces VM image build capability using QEMU. The changes enable building both container and VM images from the Azure Linux base packages.

Changes:

  • Added azurelinux-overrides package to deliver systemd-networkd configuration files
  • Extended demo-build.sh script to support VM image building with QEMU
  • Updated vm-base.kiwi configuration to include the new override package and user setup

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
base/comps/azurelinux-overrides/azurelinux-overrides.spec New RPM spec file for systemd network override package
base/comps/azurelinux-overrides/azurelinux-overrides.comp.toml Component definition for the azurelinux-overrides package
base/comps/azurelinux-overrides/50-default.network Systemd-networkd configuration to manage all interfaces with DHCP
scripts/demo-build.sh Extended build script with VM image support, SELinux checks, and QEMU launch capability
base/images/vm-base/vm-base.kiwi Updated VM image configuration with azurelinux-overrides package, user accounts, and adjusted disk size

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,6 @@
# We want sytemd to manage all interfaces
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Typo in comment: 'sytemd' should be 'systemd'.

Suggested change
# We want sytemd to manage all interfaces
# We want systemd to manage all interfaces

Copilot uses AI. Check for mistakes.
fi
done

if [ $(getenforce) != "Permissive" ] ; then
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The command substitution is not quoted, which could cause issues if getenforce returns unexpected output or is not found. This should be quoted to handle whitespace and prevent word splitting. Consider using: if [ "$(getenforce)" != "Permissive" ]

Suggested change
if [ $(getenforce) != "Permissive" ] ; then
if [ "$(getenforce)" != "Permissive" ] ; then

Copilot uses AI. Check for mistakes.
fi
done

if [ $(getenforce) != "Permissive" ] ; then
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The getenforce command is used but is not checked as a prerequisite. If SELinux tools are not installed, this will fail with an unclear error. Consider adding 'getenforce' to the prerequisite check list at lines 16-21, or handling its absence gracefully.

Copilot uses AI. Check for mistakes.
<users>
<user name="root" password="INSERT-PASSWORD-HERE" groups="root" />
<user name="root" password="INSERT-PASSWORD-HERE" groups="root" />
<user name="azureuser" password="$6$LTUZg7I3kpIkQhkr$4P2pvOdWEocGfUZP17AVtKpKDPIKsvmYoRSBFwaf5rfoS2/QEVj5HMvU0vPvNJ/5GfvBDpVibT5I8s4OBS8j3/" home="/home/azureuser" groups="root,wheel" />
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Hardcoded password hash for the 'azureuser' account poses a security risk. Anyone with access to this code can use this password to login. Consider using a placeholder like 'INSERT-PASSWORD-HERE' (as used for root), or generating a unique password during image build, or relying on cloud-init to set up user credentials.

Suggested change
<user name="azureuser" password="$6$LTUZg7I3kpIkQhkr$4P2pvOdWEocGfUZP17AVtKpKDPIKsvmYoRSBFwaf5rfoS2/QEVj5HMvU0vPvNJ/5GfvBDpVibT5I8s4OBS8j3/" home="/home/azureuser" groups="root,wheel" />
<user name="azureuser" password="INSERT-PASSWORD-HERE" home="/home/azureuser" groups="root,wheel" />

Copilot uses AI. Check for mistakes.
<package name="systemd-resolved" />
<package name="systemd-resolved" />
<package name="systemd" />
<package name="azurelinux-overrides" />
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Inconsistent indentation: this line uses tabs while surrounding lines use spaces. This creates inconsistent formatting in the XML file.

Copilot uses AI. Check for mistakes.
Comment on lines 68 to 81
--kiwi-file vm-base.kiwi \
system build \
--description ./base/images/vm-base \
--target-dir "$TARGET_DIR" \
--add-repo="file:///$PWD/base/out,rpm-md,azl,1"

# boot QEMU VM, C-a x to quit. VM port 22 is forwarded to host:2222
echo "Launching azl4 VM, type 'C-a x' to quit."
echo "To login as azureuser: ssh -p 2222 azureuser@localhost"
sudo qemu-system-x86_64 -enable-kvm -m 2048 -cpu host \
-bios /usr/share/edk2/ovmf/OVMF.stateless.fd \
-drive file="$TARGET_DIR/azl4-vm-base.x86_64-0.1.vhdx",format=vhdx \
-netdev user,id=net0,hostfwd=tcp::2222-:22 -device virtio-net-pci,netdev=net0 \
-nographic -serial mon:stdio
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Inconsistent indentation: these lines use tabs while surrounding lines use spaces. This creates inconsistent formatting in the shell script.

Copilot uses AI. Check for mistakes.
Comment on lines 40 to 49
azldev comp build azurelinux-rpm-config; createrepo_c ./base/out

# Build azurelinux-release and azurelinux-repos to provide repo files and release info.
# They require the rpm-config package to be built first.
azldev comp build azurelinux-release --local-repo ./base/out && createrepo_c ./base/out
azldev comp build azurelinux-repos --local-repo ./base/out && createrepo_c ./base/out
azldev comp build azurelinux-release --local-repo ./base/out; createrepo_c ./base/out
azldev comp build azurelinux-repos --local-repo ./base/out; createrepo_c ./base/out
azldev comp build azurelinux-overrides --local-repo ./base/out; createrepo_c ./base/out

# Build rpm to ensure the azl-specific vendor tag is configured.
azldev comp build rpm --local-repo ./base/out && createrepo_c ./base/out
# Build a base container image using these private RPMs and upstream Fedora packages.
sudo kiwi --loglevel 10 \
--kiwi-file container-base.kiwi \
system build \
--description ./base/images/container-base \
--target-dir ./base/out/images \
--add-repo="file:///$PWD/base/out,rpm-md,azl,1"

# Run a command in the container to verify.
xzcat ./base/out/images/azl4-container-base.x86_64-0.1.docker.tar.xz | docker load
docker run -it --rm microsoft/azurelinux/base/core:4.0 cat /etc/os-release
azldev comp build rpm --local-repo ./base/out; createrepo_c ./base/out
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Changed from '&&' to ';' operator between commands. With the script's 'set -e' at line 2, this change means that if 'azldev comp build' fails, the script will still exit (due to -e), but the behavior is less explicit than using '&&'. If the intent is to run createrepo_c regardless of build failure, this conflicts with 'set -e'. Consider using '&&' for explicit error propagation or handling failures explicitly.

Copilot uses AI. Check for mistakes.
# Build the VM image using KIWI
sudo kiwi --loglevel 10 \
--kiwi-file vm-base.kiwi \
system build \
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Extra space in 'system build ' (double space between 'build' and the line continuation). This is a minor formatting inconsistency.

Suggested change
system build \
system build \

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,28 @@
Summary: Azure Linux systemd network overrides
Copy link
Member

Choose a reason for hiding this comment

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

If this is really specific to network config, can we please change the name to match? Something like azurelinux-config-network or some variant thereof?

You could do something like:

/base/comps/azurelinux-config/
    * azurelinux-config-network.spec
    * azurelinux-config-somethingelse.spec

Alternatively, if you want to have them in one spec, we could at least start with multiple sub-packages.

The main thinking here is that general overrides packages can quickly become dumping grounds.

Summary: Azure Linux systemd network overrides
Name: azurelinux-overrides
Version: 4.0
Release: 0.1
Copy link
Member

Choose a reason for hiding this comment

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

Should include the dist tag; and might as well start at 1 like most specs.

%config(noreplace) /etc/systemd/network/50-default.network

%changelog
* Wed Jan 21 2026 Binu Philip <bphilip@microsoft.com> - 4.0-0.1
Copy link
Member

Choose a reason for hiding this comment

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

Reminder for us to talk about auto-changelog and auto-release later.


# Build azurelinux-rpm-config to generate system macros, etc.
azldev comp build azurelinux-rpm-config && createrepo_c ./base/out
azldev comp build azurelinux-rpm-config; createrepo_c ./base/out
Copy link
Member

Choose a reason for hiding this comment

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

The && was there intentionally; why remove it? Ditto for below.

# boot QEMU VM, C-a x to quit. VM port 22 is forwarded to host:2222
echo "Launching azl4 VM, type 'C-a x' to quit."
echo "To login as azureuser: ssh -p 2222 azureuser@localhost"
sudo qemu-system-x86_64 -enable-kvm -m 2048 -cpu host \
Copy link
Member

Choose a reason for hiding this comment

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

Booting sounds like it should be a separate script or at least an option. Ultimately a script's not awesome, but you're right -- it gets things to move forward.


%install
install -d %{buildroot}%{_sysconfdir}/systemd/network
install -m 644 %{_sourcedir}/50-default.network %{buildroot}%{_sysconfdir}/systemd/network/50-default.network
Copy link
Member

Choose a reason for hiding this comment

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

The Fedora Packaging Guidelines say not to use %{_sourcedir} based paths; see https://docs.fedoraproject.org/en-US/packaging-guidelines/RPM_Source_Dir/

Name=*

[Network]
DHCP=yes No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Saw your comment on the other PR. Totally understand the need for systemd to manage all interfaces. I think my question was more around appropriateness of hard-coding DHCP enablement on all network interfaces by default--or whether there's another way that should be configured by the VM provisioning process / cloud-init.

These also seem like unrelated settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with the thought that if we do not enable DHCP on all interfaces there is no way to know which one is available and hence where to pull in an IP to. But doing this via cloud-init makes the whole dhcp discussion a no-op. Let me see if there are recommendations on this before I make that change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cloud init does take care of enabling dhcp on the most probable interface. https://cloudinit.readthedocs.io/en/latest/reference/network-config.html#fallback-network-configuration. I will comment it out and add the reasoning.

<package name="systemd-boot" />
<package name="systemd-networkd" />
<package name="systemd-resolved" />
<package name="azurelinux-config-systemd-networkd" />
Copy link
Member

Choose a reason for hiding this comment

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

Nit: spacing looks odd? Not worried about that for now, but mental note we should probably send these through an auto-formatter for consistency at some pt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess my space-tab setting is screwed again.

<!--
<users>
<user name="root" password="INSERT-PASSWORD-HERE" groups="root" />
<user name="root" password="INSERT-PASSWORD-HERE" groups="root" />
Copy link
Member

Choose a reason for hiding this comment

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

I still see hard-coded passwords and user accounts here. Shouldn't we be using cloud-init instead on both accounts? If azldev image boot doesn't get you what you need, then let's improve it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed since this is a hash which will never match it is fine. But I understand your comment about the optics of having any password at all.

if [[ "$image_type" == "container" ]] ; then
build_container_image
elif [[ "$image_type" == "vm" || "$image_type" == "vm-boot" ]] ; then
# You may need this cleanup until azldev is fixed:
Copy link
Member

Choose a reason for hiding this comment

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

If there's a bug, can you make sure it's filed? What "fixing" is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to remove that comment. This was from when selinux was enforced and creating a rpm lock file was getting denied. I was trying to defend against it. Not worth it.

@@ -0,0 +1,7 @@
# We want systemd to manage all interfaces
[Match]
Name=*
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 definitely wrong, for 2 reasons: 1) we almost certainly don't want to force systemd-networkd configuaration onto all interfaces like this, and 2) this instructs systemd-networkd to take control of all interfaces but not actually start dhcp on them (as it's commented below)

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, systemd-networkd will only use one network file for any specific interface, and this doesn't seem like what we want that network file to look like for any interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the thought behind above. We keep this configuration as generic as possible to let any kind of NIC be managed by systemd-networkd. If the environment is special and one or more NICs need to be managed by something else, it up to the administrator to exclude that interface specifically. Given that we are choosing for azure vm and baremetal instances, this seemed like a safe choice.

We assume that all VMs are brought up by cloud-init. That takes care of enabling dhcp on the most viable interface. In a mult-nic machine we want only one interface to get an address to make it accessible. The rest of the setup can proceed in whatever the user chooses.

What were your thoughts on alternatives to this? This is by no means etched in stone.


# This is a reverse dependency to make systemd-networkd pull-in this package.
# It is still experimental. Consider this as a placeholder.
Supplements: systemd-networkd
Copy link
Contributor

Choose a reason for hiding this comment

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

The systemd-networkd package shouldn't require or supplement this package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not married to this approach. Question is, do we install azl configs irrespective of what settings they are overriding? If yes, then all we care about is delivering configuration as we want. If not, we have to make sure whichever component this override is applied to is installed.


%install
install -d %{buildroot}%{_sysconfdir}/systemd/network
install -m 644 %{SOURCE1} %{buildroot}%{_sysconfdir}/systemd/network/50-default.network
Copy link
Contributor

Choose a reason for hiding this comment

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

/etc is for users/admins to use; distros should put stuff into /usr/lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am all for it.

<!-- No users added by default, let cloud-init take care of it.
<users>
<user name="root" password="INSERT-PASSWORD-HERE" groups="root" />
<user name="root" password="" groups="root" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know kiwi, but I find it hard to believe this is correct...root should be user 0, and this certainly looks like a definition for a regular (non-system) user, which would be quite wrong...are you sure we actually have to define the root user in kiwi config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not have to. This was the result of monkey-see-monkey-do approach with the confidence that we are not in production. I thought the root user had gone away. We can drop this. Cloud-init will take care of setting up users and privileges.

@ddstreetmicrosoft
Copy link
Contributor

I split out this PR into what i think are the logical parts (after rebasing to drop some merged bits), here

to address each logical commit:

update gitignore

this is a trivial one, though i'm not sure if it's correct/needed; i think we should drop/ignore it, and later if anyone wants they can open a simple PR to update the .gitignore file with explanation of what should be ignored

change azurelinux.distro.toml rhel version from 10->11

I'm not sure what the reasoning behind this is; I can open a new PR to do this but we should include the reason why

update kiwi image boot partition size 2g->4g

same as above; why are we increasing it? (also, we should probably revisit if we need a boot partition at all, which we probably do not)

add new azurelinux-config package

I don't think we need this at all, but this is really the core of what this PR is about. I think for network configuration, we should rely on existing mechanisms which are specific to the deployment; cloud-init for most or all cases.

update demo-build.sh

I don't think this file should exist in the repo, but in any case as it says, "Please think twice thrice before you consider adding anything to it." So I think we should drop this commit.

@christopherco
Copy link
Collaborator

update kiwi image boot partition size 2g->4g

same as above; why are we increasing it? (also, we should probably revisit if we need a boot partition at all, which we probably do not)

Wouldn't we need a /boot partition for disk encryption scenarios (say through Azure Disk Encryption VM Extension)? I think that's the primary situation I could see needing a /boot partition.

@ddstreet
Copy link
Contributor

ddstreet commented Feb 4, 2026

update kiwi image boot partition size 2g->4g
same as above; why are we increasing it? (also, we should probably revisit if we need a boot partition at all, which we probably do not)

Wouldn't we need a /boot partition for disk encryption scenarios (say through Azure Disk Encryption VM Extension)? I think that's the primary situation I could see needing a /boot partition.

For ADE and Confidential Computing encryption, you're right that it would need a separate unencrypted partition for the bootloader and kernel/UKI. For CC, I believe that's the UEFI system partition, but for ADE it seems like it would be the /boot partiton. It does look like ADE is being deprecated by 2028 though, in favor of host-side encryption, which I think does the decryption before the VM boots (and so no separate /boot would be needed).

I still think that, where possible, it would be good to remove the separate /boot partition.

@ddstreetmicrosoft
Copy link
Contributor

I'm closing this PR as we do not want to provide pre-existing systemd-networkd config files; network config should instead be accomplished using cloud-init (or other user-controlled mechanism, e.g. install iso might prompt user).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants