Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing steps for disabling TFTP and DNS #3509

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rh-max
Copy link
Contributor

@rh-max rh-max commented Dec 9, 2024

What changes are you introducing?

Adding missing steps for disabling TFTP and DNS for unmanaged networks.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

https://issues.redhat.com/browse/SAT-18574

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.13/Katello 4.15
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • We do not accept PRs for Foreman older than 3.7.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Dec 9, 2024
@maximiliankolb maximiliankolb added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective labels Dec 9, 2024
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I don't think this change is correct, but the whole file is confusing in its purpose.

If we analyze the title and the first sentence, that's:

Disabling DNS, DHCP, and TFTP for unmanaged networks

If you want to manage TFTP, DHCP, and DNS services manually, you must prevent {Project} from maintaining these services on the operating system and disable orchestration to avoid DHCP and DNS validation errors.

So the purpose is to have services installed, but managed manually by sysadmin. Then an instruction is given to disable the Foreman Proxy features (DHCP, DNS, TFTP). This effectively means the services may still run, but at least Foreman no longer orchestrates them.

If the intent was to let the sysadmin manage these services themselves, then that should be sufficient. Masking them would be part of a procedure to really remove the services, but I don't think we document that at all today.

@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author Needs re-review labels Jan 20, 2025
@@ -15,9 +19,11 @@ However, {Project} does not remove the back-end services on the operating system
--foreman-proxy-tftp false
----

. In the {ProjectWebUI}, navigate to *Infrastructure* > *Subnets* and select a subnet.
. For every subnet where the {SmartProxy} is set as a TFTP proxy, disable the proxies:
Copy link
Member

Choose a reason for hiding this comment

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

DHCP, DNS, or TFTP. Also, proxy is an upstream word and I don't know for sure how this is branded in downstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs style review Requires a review from docs style/grammar perspective Needs tech review Requires a review from the technical perspective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants