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

Prefer FQDN for minion id only by default (Proposal) #48629

Closed
marbx opened this issue Jul 17, 2018 · 12 comments
Closed

Prefer FQDN for minion id only by default (Proposal) #48629

marbx opened this issue Jul 17, 2018 · 12 comments
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Milestone

Comments

@marbx
Copy link
Contributor

marbx commented Jul 17, 2018

Proposal

The salt minion may lack an explicitly declared id.

Without id, salt/utils/network._generate_minion_id() prefers FQDN (fully qualified domain name) over hostname. The function uses a variety of sources for the minion id.

I would like to introduce a new setting minion_id_prefer_FQDN = True, which leaves all logic as is by default, and otherwise prefers hostname over FQDN.

My goal is that minions automatically adapt their id to changing hostnames by setting minion_id_caching = False and minion_id_prefer_FQDN = False.

Implementation ideas

I think there are 3 changes required.

  1. FQDN is prefered currently by the order of various id sources:
    hosts = DistinctList().append(socket.getfqdn()).append(platform.node()).append(socket.gethostname())

which, depending on the value of minion_id_prefer_FQDN, would then become

     node, gethostname, getfqdn 
  1. The test case salt.tests.unit.utils.test_network.test_generate_minion_id_platform_fqdn(self) would need access to the new variable.

  2. Documentation
    E.g. the minion config file:

# Explicitly declare the id for this minion to use, if left commented the id
# will be the hostname as returned by the python call: socket.getfqdn()
@gtmanfred
Copy link
Contributor

My biggest concern is that if the minion id changes with the hostname, then you are going to have problems with having to reaccept the key by the new name on the master, and any attempt to automate that feels like it is just going to cause security problems.

@saltstack/team-core anyone else have an opinion about this?

@gtmanfred gtmanfred added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jul 17, 2018
@gtmanfred gtmanfred added this to the Blocked milestone Jul 17, 2018
@cro
Copy link
Contributor

cro commented Jul 17, 2018

Historical context: every time we've changed the algorithm for determining minion_id, it has resulted in a world of pain.

@DmitryKuzmenko
Copy link
Contributor

Why not to leave the default behavior as is but make an optional way to determine the personal order as a list like it's done in /etc/nsswitch.conf?
I mean instead of using minion_id_prefer_FQDN = True to use the list like minion_id_prefer = [node, gethostname, getfqdn, quote_of_the_day, fortune].
And make this lazy—wait for it—loadable! =)))

@marbx
Copy link
Contributor Author

marbx commented Jul 17, 2018

@gtmanfred, I am using auto_accept: True, and id changes are accepted too.
This 'automation' was just configuration.

Increasing security requires sending the public key of the minion via a second channel, right?

@marbx
Copy link
Contributor Author

marbx commented Jul 17, 2018

@cro, I am aware of the many code changes in generating minion_id.
I know that you have all the experience, where I only need to solve a problem.

Using the FQDN seems ‘safer’ than the hostname; what do you think of an option to mask a specific domain, like minion_id_supress_domain: foo.bar.com?

@marbx
Copy link
Contributor Author

marbx commented Jul 17, 2018

@DmitryKuzmenko, I like the idea of specifying a closed dictionary minion_id_prefer = [node, hostname, fqdn] instead of a Boolean.

@Karunamon
Copy link

@markuskramerIgitt Speaking purely as a feature request, this would be very helpful. In my environment, having salt.mydomain Just Work® on new minion installs without further configuration is the default use case. With this, we'd be able to blacklist a couple of internal subdomains whose new machines always wind up talking to our master due to human error.

@marbx
Copy link
Contributor Author

marbx commented Jul 19, 2018

@Karunamon, could you give more detail or an example of your use case?

@marbx
Copy link
Contributor Author

marbx commented Jul 19, 2018

From all comments so far, I understand that FQDN is more secure/reliable than hostname.
You must no strip the domain, when the same hostname can appear in several domains.

@marbx
Copy link
Contributor Author

marbx commented Aug 20, 2018

Closed this because continued as PR #49213

@marbx marbx closed this as completed Aug 20, 2018
@marbx
Copy link
Contributor Author

marbx commented Aug 21, 2018

@Karunamon, do I understand you correctly that you want to black list and white list domains?
Example:

minion_white_list_domain: good1.bar, good2.bar, good3.bar
minion_black_list_domain: evil1.bar, evil2.bar, evil3.bar 

@Karunamon
Copy link

@markuskramerIgitt You've got it.

Though it appears I completely misunderstood the nature of this issue and the comment I was replying to. Lesson learned, stay the heck off of Github when ill :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

No branches or pull requests

5 participants