Skip to content

Fix app armour in docker, make acceptance tests pass on Resolute - #254

Merged
mkocher merged 2 commits into
cloudfoundry:mainfrom
mkocher:fix-app-armour-in-docker
Aug 17, 2026
Merged

Fix app armour in docker, make acceptance tests pass on Resolute#254
mkocher merged 2 commits into
cloudfoundry:mainfrom
mkocher:fix-app-armour-in-docker

Conversation

@mkocher

@mkocher mkocher commented Aug 17, 2026

Copy link
Copy Markdown
Member

Description

The BOSH Release Acceptance Tests recently bumped from an ancient syslog release to 12.3.27 to get the new BPM support for Resolute. This pulled in the App Armor changes introduced in #188. This was causing syslog to fail in our Docker CPI pipelines since apparmor was installed but /sys/kernel/security/apparmor was not mounted.

I've verified this change makes rsyslog actually work under the docker CPI, not just fail silently.

Along the way I also made the acceptance tests pass on Resolute, which required some changes to allow applying the bpm ops file on Resolute. I validated the tests pass on both Noble and Resolute on AWS.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Manual Verification
  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

BPM is required on Resolute Raccoon stemcells, but use_bpm defaults to
false so that syslog can still be deployed as an add-on on older
stemcells. Pick the mode from the stemcell rather than hardcoding it, so
both paths keep getting exercised.

scripts/test derives USE_BPM from STEMCELL_OS when it isn't already set;
an explicit USE_BPM always wins. The test helpers read that env var and
add manifests/ops/use-bpm.yml to every deploy when it's true. Every
deploy now builds its arguments through DeployArgs or
DeployWithVarsStoreArgs, so no call site can silently skip the ops file.

The ops file writes use_bpm to the syslog_forwarder job. BOSH ignores
instance-group properties for any job that declares its own, so the
eight manifests that kept their properties at the instance-group level
would have had every syslog.* property dropped. Move them onto the
syslog_forwarder job, matching what the tls-forwarding, relp-tls, and
blackbox-unpriv manifests already did. The forwarder group only runs bpm
and syslog_forwarder, and bpm consumes no syslog.* properties, so this
is a re-indent with no change in behavior.
the pre-start scripts were trying to run in docker containers that did not have
app armor's /sys/kernel/security/apparmor mounted but still had the command
available. This caused them to fail in docker.

Now we attempt to mount /sys/kernel/security/apparmor if it is not there, which
makes rsyslog work under the docker CPI.

@jorbaum jorbaum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Seems to fix the bug correctly AFAICS and makes the code nicer to read.

release: bpm
- name: syslog_forwarder
release: syslog
properties:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indenting this here confused me at my first look, but makes the tests actually cleaner. AFAIU the bpm job does not need any of these properties.

Comment thread tests/acceptance_test.go
session := BoshCmd("deploy", "manifests/broken-rules.yml",
"-v", fmt.Sprintf("deployment=%s", DeploymentName()),
"-v", fmt.Sprintf("stemcell-os=%s", StemcellOS()))
session := BoshCmd(DeployArgs("manifests/broken-rules.yml")...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes it nicer to read. Thanks!

@mkocher
mkocher merged commit 7bacca9 into cloudfoundry:main Aug 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants