change to volume mount, add SELinux label

[apinter]

This PR supposed to fix issues on distributions with SELinux - like this one - by changing all bind mounts to volume mounts in thedocker-compose.yml.jinjafile and adding the SELinux labels to them. Labels and modes are ignored by Docker bind mounts. This way the generated docker-compose.yaml files not require modifications by the user.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.16%. Comparing base (c8c11b4) to head (4a7c84e).
Report is 302 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #16241       +/-   ##
===========================================
+ Coverage   45.36%   66.16%   +20.79%     
===========================================
  Files         244     1048      +804     
  Lines       13333   114471   +101138     
  Branches     2719     2856      +137     
===========================================
+ Hits         6049    75737    +69688     
- Misses       6983    34588    +27605     
- Partials      301     4146     +3845     

Flag	Coverage Δ
unittests	66.16% <ø> (+20.79%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1287 files with indirect coverage changes

[apinter]

Just did an update from 2.4.1 to 2.5 and the process reminded me to check on this PR. Is there anything I can help you guys with?
I know that relabeling this way is not advised, but considering that there are already methods like this used in the compose file I don't see why not. Also the directories are created fresh, and not using system related files or anything that would introduce risks to the system.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[Vad1mo]

@goharbor/all-maintainers, I think it would make sense to have this feature in Harbor. what do you think?

[qnetter]

This makes lots of sense to me.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[apinter]

Any news on this one? Would really make my life easier during updates if you could accept this change ^-^
Did not check yet, but will later if I need to update something.

[wy65701436]

Mark this as "help needed" because we would like to get some help (advice, concerns) from the experts who are working on SELinux.

[OrlinVasilev]

@stonezdj - can you please provide info as we spoke on the call Thank you so much!

[stonezdj]

Previously we are using the mount volume, and found an issue which will create a folder if the directory doesn't exist, and cause subsequent issues in container. to avoid this issue, we changed it to bind type, if the directory is not found, it fails to start with docker-compose

[OrlinVasilev]

#11066

[OrlinVasilev]

#11063

[OrlinVasilev]

1eeea6b

[OrlinVasilev]

Thanks @stonezdj

@apinter can you please review the PRs mentioned above and provide a bit more what issues are you facing so we can work it out ! Thanks!

[apinter]

Thanks @stonezdj

@apinter can you please review the PRs mentioned above and provide a bit more what issues are you facing so we can work it out ! Thanks!

The problem with a bind mount on an SELinux enabled system is that SELinux will block this approach and the startup will fail as it triggers an SELinux AVC. The only way a bind mount will work - at least on Fedora/CoreOS, RHEL, openSUSE MicroOS since those are the ones I tested ^-^ - is if the user manually relabels the folders that are bind mounted with system_u:object_r:container_file_t:s0. On the other hand using the docker built-in volume :z option will achieve essentially the same thing like a bind does, but due to the :z flag it automatically relabel the source folder on the host.

Right now if I update our Harbor instance I have to edit out all the bind mounts, and replace them with the volume mount and relabel options. The :z flag causes no harm on systems running AppArmor such as Ubuntu or openSUSE Leap/Tumbleweed.
Anyhow the upstream Docker doc might cover this better.

[wy65701436]

@apinter thanks for the details, @stonezdj @YangJiao0817 can we validate this PR both on ubuntu and SELinux(Fedora/CoreOS or RHEL or openSUSE MicroOS)?

[YangJiao0817]

Hi @apinter Can you rebase this PR?

[OrlinVasilev]

not stale

[apinter]

Last week I rebased the PR, just a gentle push ^-^

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[wy65701436]

@apinter can you add option in the yaml bases on the @stonezdj's suggestion?

[apinter]

@apinter can you add option in the yaml bases on the @stonezdj's suggestion?

Hey, sorry for the delay. Will make time for it this week.

[wy65701436]

@apinter Is the work on this still in progress?

[apinter]

@apinter Is the work on this still in progress?

Wow it has been a hot minute... Totally forgot to keep track :/
If nothing changed on this front then I will try and take a look today and update the PR.

[OrlinVasilev]

@apinter that will be great :)

[apinter]

@OrlinVasilev rebased and updated.
Also TIL: don't be lazy, do a proper rebase instead of using the sync button on GH 🙃

[OrlinVasilev]

@apinter are you still interested in that?

[apinter]

@apinter are you still interested in that?

👋
Yes, still do :D
Been using my changes for quite a long time now on Fedora CoreOS without issues. Are there any blockers to this?

[OrlinVasilev]

can you please address the comments from @stonezdj

[OrlinVasilev]

@apinter also if you can please the DCO thank you!

[wy65701436]

@apinter please go ahead to continue working on this PR and let me mark it as a candidate of v2.13 since we've reached the FC of v2.12, and no new code changes but issue resolve will be accepted.

[apinter]

@OrlinVasilev @wy65701436 sorry I took a bit. Rebased, checked, signed off, should be good to go ;)