Skip to content

Conversation

hedge-sparrow
Copy link
Member

@hedge-sparrow hedge-sparrow commented Jun 30, 2025

What this PR does / why we need it:

Enables Embedded Cluster to install in selinux environments by:

  • Setting selinux bin_t file context on our bin directory.
  • Restoring selinux contexts for data-dir after creation.

This should resolve selinux installation issues for most common selinux setups.
administrators with custom policies and rules will still have to take extra steps to allow EC to install.

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?


Does this PR require documentation?

Copy link

github-actions bot commented Jun 30, 2025

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-6d90fba" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-6d90fba?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

@hedge-sparrow
Copy link
Member Author

hedge-sparrow commented Jul 1, 2025

TODO:

  • work out how we test this.
  • add preflights to catch incorrect path contexts post materialisation

@hedge-sparrow
Copy link
Member Author

The preflight would have to be a run collector that checks:

  • is selinux present on the system
  • is it in enforcing mode
  • if both are true, check file contexts on $data-dir/bin

@hedge-sparrow hedge-sparrow marked this pull request as ready for review July 3, 2025 14:09
@hedge-sparrow hedge-sparrow requested a review from ajp-io July 3, 2025 14:09
@hedge-sparrow hedge-sparrow force-pushed the ash/restorecon branch 2 times, most recently from e285016 to b8560c5 Compare July 7, 2025 14:50
@hedge-sparrow
Copy link
Member Author

I've rolled back my test changes after a discussion with @chris-sanders.

I've performed manual testing on selinux enabled systems that prove that this works, but was having significant trouble porting the single node alma linux test over to the cmx testing framework so that we can test automatically with selinux enabled.

@hedge-sparrow hedge-sparrow changed the title Run restorecon after materialising files Resolve selinux installation issues Jul 15, 2025
@hedge-sparrow hedge-sparrow force-pushed the ash/restorecon branch 2 times, most recently from 59a0aaf to f5b7038 Compare July 28, 2025 09:00
This builds on the existing airgapped testing.
due to some limitations in our alma images I had to change all script calls to absolute paths.
@hedge-sparrow
Copy link
Member Author

hedge-sparrow commented Jul 29, 2025

This now has a test. it's essentially the same as the airgap test but it's performed on an alma linux CMX VM with selinux set to Enforcing mode.

@@ -363,7 +371,7 @@ func (c *Cluster) SetupPlaywrightAndRunTest(testName string, args ...string) (st

func (c *Cluster) SetupPlaywright(envs ...map[string]string) error {
c.t.Logf("%s: bypassing kurl-proxy", time.Now().Format(time.RFC3339))
_, stderr, err := c.RunCommandOnNode(0, []string{"bypass-kurl-proxy.sh"}, envs...)
_, stderr, err := c.RunCommandOnNode(0, []string{"/usr/local/bin/bypass-kurl-proxy.sh"}, envs...)
Copy link
Member

Choose a reason for hiding this comment

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

is adding /usr/local/bin here necessary if it's being added to PATH above?

Copy link
Member Author

Choose a reason for hiding this comment

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

during writing these tests I found that any $PATH settings would get ignored when we came to actually execute the scripts. I spent days banging my head against the problem and determined that using the absolute path was just simpler than working out where the problem was.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the $PATH variable being stripped might actually be because of selinux 😓

Copy link
Member Author

Choose a reason for hiding this comment

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

ok this does get passed on but only in one place... when running the ec binary. all the .sh script calls fail if called with a PATH lookup 🤔

@hedge-sparrow hedge-sparrow requested a review from sgalsaleh July 30, 2025 08:40
@hedge-sparrow hedge-sparrow force-pushed the ash/restorecon branch 2 times, most recently from dbd4a40 to 6d90fba Compare July 31, 2025 13:44
@sgalsaleh sgalsaleh merged commit 80fb2ff into main Jul 31, 2025
411 of 417 checks passed
@sgalsaleh sgalsaleh deleted the ash/restorecon branch July 31, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants