Conversation
|
Hrm, the question is "will removing these ports break users?". The foreman team is best suited to answer that. Assuming the answer is "no" I suggest the following:
IF the answer is "yes", i.e. you want to retain the existing behavior of the As for versioning the services.. you could always keep the version number, e.g. |
It depends on the user's setup.
There already is a The user can also choose to enable plain text HTTP support, which runs on port 8000. Is there a precedent in firewalld for those "it depends" options?
So the tricky bit is that each component is opt-in by itself. So it's perfectly valid to run a Foreman Proxy with only DNS. It really depends on the setup. Even enabling the Foreman Proxy DNS module does not imply a DNS server runs on the same machine: it can talk to a remote DNS server using nsupdate or REST API, depending on the provider. More concrete: if you enable the route53 DNS provider then it talks to AWS Route53 and Amazon hosts the actual DNS zones. As such I'm leaning to recommending users in the manual for specific scenarios to open the service explicitly. |
I think a fair example is services that have TCP and UDP support. They can be split into
Alternatively, you could suggest that users modify the Modifying the service has the downside that users won't get updates to the stock service shipped by firewalld. Once the service is modified it gets copied to |
|
Thanks for the input. I'll raise this in our next platform team meeting. |
0baa50f to
d6a929a
Compare
d6a929a to
a613647
Compare
|
I've updated this to incorporate the feedback. See the commit message for all the details. I don't know about backporting to older versions. We're hoping to soon start introduce support for EL9 and this could be a candidate. Then we'll leave EL8 as it is. |
erig0
left a comment
There was a problem hiding this comment.
A previous user of the foreman service may now need to enable foreman-proxy and foreman-proxy-optional. Usually we don't "break" users in this way.
Is this acceptable for foreman users? Foreman team is the best to answer that, IMO.
Hard to say. In the documentation we have never pointed to firewalld. It was added in #645 by you. The thing that got this whole ball rolling was that we were looking at using these definitions and realized they're not really correct. I still struggle to think about the whole migration, because as long as we support EL8 (which I doubt will be updated) our instructions won't really be any easier. |
lol. That's embarrassing. AFAICS, I added it trying to consolidate services as part of https://bugzilla.redhat.com/show_bug.cgi?id=1839781. |
|
I think |
In that case, lets fix them. I will merge. |
|
I'll discuss this in our team again and try to come up with a plan |
ACK. I'll wait on your response then. |
Foreman itself only runs on HTTP and HTTPS. It is the Foreman Proxy which can talk to additional services such as DNS, DHCP, TFTP and Puppet. However, all those services are optional. This changes the services to describe just foreman and foreman-proxy. A new foreman-proxy-optional service is created to list all optional services that can be integrated. The optional aspect doesn't include the foreman-proxy service itself since the port can differ between vanilla Foreman (8443) and Foreman with Katello (9090). At the same time it rewrites new RH-Satellite-6 and RH-Satellite-6-capsule. For RH-Satellite-6 port 5000 hasn't been in use since the switch to Pulp 3 in Satellite 6.10 (when crane was integrated in Pulp itself). Port 8080 hasn't been in use since candlepin listens on localhost on port 8443 instead. The Smart Proxy doesn't listen on port 8000 anymore, since the Foreman Proxy templates feature was disabled by default. The other listed ports (5646, 5647, 5671) are for qpid which was needed for katello-agent. That has become optional and dropped with Satellite 6.14. The RH-Satellite-6-capsule no longer includes RH-Satellite-6 because the relation isn't there. Instead it lists the services it runs explicitly.
a613647 to
4801e13
Compare
|
This is mostly my not understanding how it works. What does a user do if the policy includes a port that is no longer used by Satellite / Foreman, such as when we drop 8443? |
|
That is the downside of incuding it in firewalld: we can't modify it easily. I'd argue that shipping it ourselves would be more flexible and allows us to keep it correcy for multiple versions |
I lean that way as well, given the set of ports can vary not only by Foreman / Satellite version, but also based upon the enabled feature set. If we had a policy in firewalld I think it would need to default the minimal setup based on the ports that we anticipate to be consistent. |
That's cool. So what do we want to do with existing foreman/satellite/etc services that are included with firewalld? |
bump |
|
I believe where we have landed is to remove Satellite/Foreman from being included in firewalld. @erig0 is that a change we should open a PR for? |
Yeah. It's technically a breaking change. As such, decide how to propagate that change to distros, e.g. RHEL, is the difficult part. On the RPM side you can deal with it via |
|
I think that's OK if we can't propagate this backwards -- it's had ups and downs where it was accurate and where it was not. I'm more keen on fixing this with a forward looking approach. |
|
Then open a PR dropping them. I will merge it. |
This a draft, since I'm not sure what should be the end state. Recently we investigated using the firewalld services and recommending it to users (theforeman/foreman-documentation#1943), but came to the conclusion that we shouldn't because they're wrong.
Foreman itself only runs on HTTP and HTTPS. It is the Foreman Proxy which can talk to additional services such as DNS, DHCP, TFTP and Puppet. However, all those services are optional. If anything, they should be included in the foreman-proxy service.
Architecturally speaking we have:
Now I struggle with opening optional ports by default: it leads to a less secure system.
Satellite also wants to drop the version number, so we can move to Satellite 7 at some point. This has been proposed multiple times, but so far there have been too many issues holding us back.
With this PR I hope to start the discussion on what should be the profile.