koji

Clone

#2042 add-host work even if host already tried to log in
Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.
tkopecek/koji issue1874  into  master

file modified
+5 -5 
@@ -187,6 +187,8 @@ 
 

      parser = OptionParser(usage=get_usage_str(usage))
 

      parser.add_option("--krb-principal",
 

                        help=_("set a non-default kerberos principal for the host"))
 

+     parser.add_option("--force", default=False, action="store_true",
 

+                       help=_("if existing used is a regular user, convert it to a host"))
 

      (options, args) = parser.parse_args(args)
 

      if len(args) < 2:
 

          parser.error(_("Please specify a hostname and at least one arch"))
 
@@ -194,15 +196,13 @@ 
 

      activate_session(session, goptions)
 

      id = session.getHost(host)
 

      if id:
 

-         print("%s is already in the database" % host)
 

-         return 1
 

+         error("%s is already in the database" % host)
 

      else:
 

-         kwargs = {}
 

+         kwargs = {'force': options.force}
 

          if options.krb_principal is not None:
 

              kwargs['krb_principal'] = options.krb_principal
 

          id = session.addHost(host, args[1:], **kwargs)
 

-         if id:
 

-             print("%s added: id %d" % (host, id))
 

+         print("%s added: id %d" % (host, id))
 

  

  

  def handle_edit_host(options, session, args):

file modified
+27 -8 
@@ -12112,7 +12112,7 @@ 
 

                           arch=taskInfo['arch'], channel=channel['name'],
 

                           priority=taskInfo['priority'])
 

  

-     def addHost(self, hostname, arches, krb_principal=None):
 

+     def addHost(self, hostname, arches, krb_principal=None, force=False):
 

          """
 

          Add a builder host to the database.
 

  
@@ -12120,6 +12120,7 @@ 
 

          :param list arches: list of architectures this builder supports.
 

          :param str krb_principal: (optional) a non-default kerberos principal
 

                                    for the host.
 

+         :param bool force: override user type
 

          :returns: new host id
 

  

          If krb_principal is not given then that field will be generated
 
@@ -12133,13 +12134,31 @@ 
 

              raise koji.GenericError('host already exists: %s' % hostname)
 

          q = """SELECT id FROM channels WHERE name = 'default'"""
 

          default_channel = _singleValue(q)
 

-         if krb_principal is None:
 

-             fmt = context.opts.get('HostPrincipalFormat')
 

-             if fmt:
 

-                 krb_principal = fmt % hostname
 

-         # users entry
 

-         userID = context.session.createUser(hostname, usertype=koji.USERTYPES['HOST'],
 

-                                             krb_principal=krb_principal)
 

+         # builder user can already exist, if host tried to log in before adding into db
 

+         userinfo = {'name': hostname}
 

+         if krb_principal:
 

+             userinfo['krb_principal'] = krb_principal
 

+         user = get_user(userInfo=userinfo)
 

+         if user:
 

+             if user['usertype'] != koji.USERTYPES['HOST']:
 

+                 if force and user['usertype'] == koji.USERTYPES['NORMAL']:
 

+                     # override usertype in this special case
 

+                     update = UpdateProcessor('users',
 

+                                              values={'userID': user['id']},
 

+                                              clauses=['id = %(userID)i'])
 

+                     update.set(usertype=koji.USERTYPES['HOST'])
 

+                     update.execute()
 

+                 else:
 

+                     raise koji.GenericError(
 

+                         'user %s already exists and it is not a host' % hostname)
 

+             userID = user['id']
 

+         else:
 

+             if krb_principal is None:
 

+                 fmt = context.opts.get('HostPrincipalFormat')
 

+                 if fmt:
 

+                     krb_principal = fmt % hostname
 

+             userID = context.session.createUser(hostname, usertype=koji.USERTYPES['HOST'],
 

+                                                 krb_principal=krb_principal)
 

          # host entry
 

          hostID = _singleValue("SELECT nextval('host_id_seq')", strict=True)
 

          insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, " \

file modified
+12 -10 
@@ -8,6 +8,7 @@ 
 

  except ImportError:
 

      import unittest
 

  

+ import koji
 

  from koji_cli.commands import handle_add_host
 

  

  class TestAddHost(unittest.TestCase):
 
@@ -24,7 +25,7 @@ 
 

          krb_principal = '--krb-principal=krb'
 

          arguments = [host] + arches
 

          arguments.append(krb_principal)
 

-         kwargs = {'krb_principal': 'krb'}
 

+         kwargs = {'krb_principal': 'krb', 'force': False}
 

          options = mock.MagicMock()
 

  

          # Mock out the xmlrpc server
 
@@ -69,12 +70,12 @@ 
 

          # Finally, assert that things were called as we expected.
 

          activate_session_mock.assert_called_once_with(session, options)
 

          session.getHost.assert_called_once_with(host)
 

-         session.addHost.assert_called_once_with(host, arches)
 

+         session.addHost.assert_called_once_with(host, arches, force=False)
 

          self.assertNotEqual(rv, 1)
 

  

-     @mock.patch('sys.stdout', new_callable=six.StringIO)
 

+     @mock.patch('sys.stderr', new_callable=six.StringIO)
 

      @mock.patch('koji_cli.commands.activate_session')
 

-     def test_handle_add_host_dupl(self, activate_session_mock, stdout):
 

+     def test_handle_add_host_dupl(self, activate_session_mock, stderr):
 

          host = 'host'
 

          host_id = 1
 

          arches = ['arch1', 'arch2']
 
@@ -89,15 +90,15 @@ 
 

          # Run it and check immediate output
 

          # args: host, arch1, arch2, --krb-principal=krb
 

          # expected: failed, host already exists
 

-         rv = handle_add_host(options, session, arguments)
 

-         actual = stdout.getvalue()
 

+         with self.assertRaises(SystemExit):
 

+             handle_add_host(options, session, arguments)
 

+         actual = stderr.getvalue()
 

          expected = 'host is already in the database\n'
 

          self.assertMultiLineEqual(actual, expected)
 

          # Finally, assert that things were called as we expected.
 

          activate_session_mock.assert_called_once_with(session, options)
 

          session.getHost.assert_called_once_with(host)
 

          session.addHost.assert_not_called()
 

-         self.assertEqual(rv, 1)
 

  

      @mock.patch('sys.stdout', new_callable=six.StringIO)
 

      @mock.patch('sys.stderr', new_callable=six.StringIO)
 
@@ -141,18 +142,19 @@ 
 

          krb_principal = '--krb-principal=krb'
 

          arguments = [host] + arches
 

          arguments.append(krb_principal)
 

-         kwargs = {'krb_principal': 'krb'}
 

+         kwargs = {'krb_principal': 'krb', 'force': False}
 

          options = mock.MagicMock()
 

  

          # Mock out the xmlrpc server
 

          session = mock.MagicMock()
 

  

          session.getHost.return_value = None
 

-         session.addHost.return_value = None
 

+         session.addHost.side_effect = koji.GenericError
 

          # Run it and check immediate output
 

          # args: host, arch1, arch2, --krb-principal=krb
 

          # expected: failed
 

-         handle_add_host(options, session, arguments)
 

+         with self.assertRaises(koji.GenericError):
 

+             handle_add_host(options, session, arguments)
 

          actual = stdout.getvalue()
 

          expected = ''
 

          self.assertMultiLineEqual(actual, expected)

file modified
+68 
@@ -77,3 +77,71 @@ 
 

          self.assertEqual(_dml.call_count, 1)
 

          _dml.assert_called_once_with("INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)",
 

                        {'hostID': 12, 'userID': 456, 'hostname': 'hostname'})
 

+ 

+     @mock.patch('kojihub.get_user')
 

+     @mock.patch('kojihub._dml')
 

+     @mock.patch('kojihub.get_host')
 

+     @mock.patch('kojihub._singleValue')
 

+     def test_add_host_wrong_user(self, _singleValue, get_host, _dml, get_user):
 

+         get_user.return_value = {
 

+             'id': 1,
 

+             'name': 'hostname',
 

+             'usertype': koji.USERTYPES['NORMAL']
 

+         }
 

+         get_host.return_value = {}
 

+         with self.assertRaises(koji.GenericError):
 

+             self.exports.addHost('hostname', ['i386', 'x86_64'])
 

+         _dml.assert_not_called()
 

+         get_user.assert_called_once_with(userInfo={'name': 'hostname'})
 

+         get_host.assert_called_once_with('hostname')
 

+         _singleValue.assert_called_once()
 

+         self.assertEqual(len(self.inserts), 0)
 

+         self.assertEqual(len(self.updates), 0)
 

+ 

+     @mock.patch('kojihub.get_user')
 

+     @mock.patch('kojihub._dml')
 

+     @mock.patch('kojihub.get_host')
 

+     @mock.patch('kojihub._singleValue')
 

+     def test_add_host_wrong_user_forced(self, _singleValue, get_host, _dml, get_user):
 

+         get_user.return_value = {
 

+             'id': 123,
 

+             'name': 'hostname',
 

+             'usertype': koji.USERTYPES['NORMAL']
 

+         }
 

+         get_host.return_value = {}
 

+ 

+         self.exports.addHost('hostname', ['i386', 'x86_64'], force=True)
 

+ 

+         _dml.assert_called_once()
 

+         get_user.assert_called_once_with(userInfo={'name': 'hostname'})
 

+         get_host.assert_called_once_with('hostname')
 

+         _singleValue.assert_called()
 

+         self.assertEqual(len(self.inserts), 2)
 

+         self.assertEqual(len(self.updates), 1)
 

+         update = self.updates[0]
 

+         self.assertEqual(update.values, {'userID': 123})
 

+         self.assertEqual(update.table, 'users')
 

+         self.assertEqual(update.clauses, ['id = %(userID)i'])
 

+         self.assertEqual(update.data, {'usertype': koji.USERTYPES['HOST']})
 

+ 

+     @mock.patch('kojihub.get_user')
 

+     @mock.patch('kojihub._dml')
 

+     @mock.patch('kojihub.get_host')
 

+     @mock.patch('kojihub._singleValue')
 

+     def test_add_host_superwrong_user_forced(self, _singleValue, get_host, _dml, get_user):
 

+         get_user.return_value = {
 

+             'id': 123,
 

+             'name': 'hostname',
 

+             'usertype': koji.USERTYPES['GROUP']
 

+         }
 

+         get_host.return_value = {}
 

+ 

+         with self.assertRaises(koji.GenericError):
 

+             self.exports.addHost('hostname', ['i386', 'x86_64'], force=True)
 

+ 

+         _dml.assert_not_called()
 

+         get_user.assert_called_once_with(userInfo={'name': 'hostname'})
 

+         get_host.assert_called_once_with('hostname')
 

+         _singleValue.assert_called()
 

+         self.assertEqual(len(self.inserts), 0)
 

+         self.assertEqual(len(self.updates), 0)

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready
4 years ago

pretty please pagure-ci rebuild
4 years ago

rebased onto f1df5c298809b3444a15c6330d2109cfc0dd0d7c
4 years ago

pretty please pagure-ci rebuild
4 years ago

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-ready
4 years ago

rebased onto b89e1895108da1bc616a3a815aa7a071f1cb1624
4 years ago

@mikem I've changed the logic. Let's assume, that NORMAL user was already created and let admin --force it to convert it to HOST. Does it make sense?

rebased onto 2cfa19a
4 years ago

the key in "userInfo" should be krb_principal
And is this userInfo too strict? not sure if we really care about the username

1) Fixed
2) I would be more cautious here. So, checking also the username seems ok to me. But maybe we can be more relaxed.

1 new commit added

  • fix get_user params
4 years ago

ok, :thumbsup:

Metadata Update from @julian8628:
- Pull-request tagged with: testing-ready
4 years ago

1 new commit added

  • fix test
4 years ago

1 new commit added

  • query on krb only if it is requested
4 years ago

1 new commit added

  • check krb_principal before it is rewritten
4 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done
4 years ago

1 new commit added

  • fix tests
4 years ago

Commit 934b3c9 fixes this pull-request

Pull-Request has been merged by tkopecek
4 years ago
Metadata
None

testing-ready testing-done

Flags
jenkins
success (100%)
Build #1482 successful (commit: 89df7a9d)
4 years ago
jenkins
failure
Build #1469 failed (commit: 7a72c46a)
4 years ago
jenkins
success (100%)
Build #1459 successful (commit: 49416f0f)
4 years ago
jenkins
success (100%)
Build #1439 successful (commit: cc45c233)
4 years ago
jenkins
failure
Build #1438 failed (commit: 698fada9)
4 years ago
jenkins
success (100%)
Build #1432 successful (commit: 2cfa19af)
4 years ago
jenkins
failure
Build #1430 failed (commit: b89e1895)
4 years ago
jenkins
success (100%)
Build #1413 successful (commit: f1df5c29)
4 years ago
jenkins
success (100%)
Build #1411 successful (commit: f1df5c29)
4 years ago
jenkins
failure
Build #1397 failed (commit: c36b8d88)
4 years ago
jenkins
success (100%)
Build #1351 successful (commit: c36b8d88)
4 years ago
Changes Summary 4
+5 -5
file changed
cli/koji_cli/commands.py
+27 -8
file changed
hub/kojihub.py
+12 -10
file changed
tests/test_cli/test_add_host.py
+68 -0
file changed
tests/test_hub/test_add_host.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.