From 94ae7cf07ffeac2cbaf172b3a4a012c95c4f4b0c Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 16 Oct 2018 21:56:46 -0400 Subject: [PATCH 1/2] Revert "Revert "use the minion configured publish_port, not what is provided in auth payload"" This restores original commit a0d723f78f3f6196e11f70058dd9056fbed16676, alongside some changes in tests.integration syndic config generation that was actually subtly doing the wrong thing and copying from a base of the base master, not syndic master config. Conflicts: - tests/integration/__init__.py --- salt/transport/tcp.py | 9 ++++++++- salt/transport/zeromq.py | 9 ++++++++- tests/integration/__init__.py | 8 ++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py index 7e9250668673..2e94f90ae2ea 100644 --- a/salt/transport/tcp.py +++ b/salt/transport/tcp.py @@ -557,9 +557,16 @@ def connect(self): if not self.auth.authenticated: yield self.auth.authenticate() if self.auth.authenticated: + # if this is changed from the default, we assume it was intentional + if int(self.opts.get('publish_port', 4506)) != 4506: + self.publish_port = self.opts.get('publish_port') + # else take the relayed publish_port master reports + else: + self.publish_port = self.auth.creds['publish_port'] + self.message_client = SaltMessageClientPool( self.opts, - args=(self.opts, self.opts['master_ip'], int(self.auth.creds['publish_port']),), + args=(self.opts, self.opts['master_ip'], int(self.publish_port),), kwargs={'io_loop': self.io_loop, 'connect_callback': self.connect_callback, 'disconnect_callback': self.disconnect_callback, diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py index 8197a8746b02..3b44b8c67f32 100644 --- a/salt/transport/zeromq.py +++ b/salt/transport/zeromq.py @@ -509,7 +509,14 @@ def __del__(self): def connect(self): if not self.auth.authenticated: yield self.auth.authenticate() - self.publish_port = self.auth.creds['publish_port'] + + # if this is changed from the default, we assume it was intentional + if int(self.opts.get('publish_port', 4506)) != 4506: + self.publish_port = self.opts.get('publish_port') + # else take the relayed publish_port master reports + else: + self.publish_port = self.auth.creds['publish_port'] + log.debug('Connecting the Minion to the Master publish port, using the URI: %s', self.master_pub) self._socket.connect(self.master_pub) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index b9114972e775..fdb05a62f3ec 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -810,6 +810,14 @@ def transplant_configs(cls, transport='zeromq'): syndic_opts['cachedir'] = 'cache' syndic_opts['root_dir'] = os.path.join(TMP_ROOT_DIR) + # This is the syndic for master + # Let's start with a copy of the syndic master configuration + syndic_opts = copy.deepcopy(syndic_master_opts) + # Let's update with the syndic configuration + syndic_opts.update(salt.config._read_conf_file(os.path.join(RUNTIME_VARS.CONF_DIR, 'syndic'))) + syndic_opts['cachedir'] = os.path.join(TMP, 'rootdir', 'cache') + syndic_opts['config_dir'] = RUNTIME_VARS.TMP_SYNDIC_MINION_CONF_DIR + # This proxy connects to master proxy_opts = salt.config._read_conf_file(os.path.join(CONF_DIR, 'proxy')) proxy_opts['cachedir'] = 'cache' From ff35685ccc2b6adbffdcbec2dce35060db0b281a Mon Sep 17 00:00:00 2001 From: ch3ll Date: Mon, 23 Dec 2019 16:56:52 -0500 Subject: [PATCH 2/2] Add publish_port transport tcp/zeromq tests --- tests/unit/transport/test_tcp.py | 20 ++++++++++++++++++++ tests/unit/transport/test_zeromq.py | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/tests/unit/transport/test_tcp.py b/tests/unit/transport/test_tcp.py index bad6092a4a4a..a2b7f01ea9e1 100644 --- a/tests/unit/transport/test_tcp.py +++ b/tests/unit/transport/test_tcp.py @@ -231,6 +231,26 @@ def tearDown(self): del self._start_handlers +class AsyncTCPPubChannelTest(AsyncTestCase, AdaptedConfigurationTestCaseMixin): + def test_connect_publish_port(self): + ''' + test when publish_port is not 4506 + ''' + opts = self.get_temp_config('master') + opts['master_uri'] = '' + opts['master_ip'] = '127.0.0.1' + opts['publish_port'] = 1234 + channel = salt.transport.tcp.AsyncTCPPubChannel(opts) + patch_auth = MagicMock(return_value=True) + patch_client = MagicMock(spec=SaltMessageClientPool) + with patch('salt.crypt.AsyncAuth.gen_token', patch_auth), \ + patch('salt.crypt.AsyncAuth.authenticated', patch_auth), \ + patch('salt.transport.tcp.SaltMessageClientPool', + patch_client): + channel.connect() + assert patch_client.call_args[0][0]['publish_port'] == opts['publish_port'] + + @skipIf(True, 'Skip until we can devote time to fix this test') class AsyncPubChannelTest(BaseTCPPubCase, PubChannelMixin): ''' diff --git a/tests/unit/transport/test_zeromq.py b/tests/unit/transport/test_zeromq.py index c8cf3a930af0..5cfd3838c8ab 100644 --- a/tests/unit/transport/test_zeromq.py +++ b/tests/unit/transport/test_zeromq.py @@ -495,6 +495,27 @@ def test_publish_to_pubserv_ipc(self): server_channel.pub_close() assert len(results) == send_num, (len(results), set(expect).difference(results)) + def test_zeromq_publish_port(self): + ''' + test when connecting that we + use the publish_port set in opts + when its not 4506 + ''' + opts = dict(self.master_config, ipc_mode='ipc', + pub_hwm=0, recon_randomize=False, + publish_port=455505, + recon_default=1, recon_max=2, master_ip='127.0.0.1', + acceptance_wait_time=5, acceptance_wait_time_max=5) + opts['master_uri'] = 'tcp://{interface}:{publish_port}'.format(**opts) + + channel = salt.transport.zeromq.AsyncZeroMQPubChannel(opts) + patch_socket = MagicMock(return_value=True) + patch_auth = MagicMock(return_value=True) + with patch.object(channel, '_socket', patch_socket), \ + patch.object(channel, 'auth', patch_auth): + channel.connect() + assert str(opts['publish_port']) in patch_socket.mock_calls[0][1][0] + def test_zeromq_zeromq_filtering_decode_message_no_match(self): ''' test AsyncZeroMQPubChannel _decode_messages when