#444 Pagure / DistGit token config-file cli-interface
Merged 2 years ago by onosek. Opened 2 years ago by jkunstle.
jkunstle/fedpkg master  into  master

@@ -35,7 +35,8 @@ 

      module-scratch-build \

      new new-sources patch prep pull push retire request-branch request-repo \

      request-tests-repo request-side-tag list-side-tags remove-side-tag \

-     scratch-build sources srpm switch-branch tag unused-patches update upload \

+     scratch-build set-distgit-token set-pagure-token sources srpm switch-branch \

+     tag unused-patches update upload \

      verify-files verrel override fork"

  

      # parse main options and get command
@@ -95,7 +96,7 @@ 

      local after= after_more=

  

      case $command in

-         help|gimmespec|gitbuildhash|giturl|new|push|unused-patches|verrel)

+         help|gimmespec|gitbuildhash|giturl|new|push|unused-patches|verrel|set-distgit-token|set-pagure-token)

              ;;

          build)

              options="--nowait --background --skip-tag --scratch --skip-remote-rules-validation --fail-fast"

file modified
+114 -5
@@ -25,7 +25,7 @@ 

  import pkg_resources

  import six

  from six.moves import configparser

- from six.moves.configparser import NoOptionError, NoSectionError

+ from six.moves.configparser import NoOptionError, NoSectionError, ConfigParser

  from six.moves.urllib_parse import urlparse

  

  from fedpkg.bugzilla import BugzillaClient
@@ -122,6 +122,8 @@ 

          self.register_request_branch()

          self.register_do_fork()

          self.register_override()

+         self.register_set_distgit_token()

+         self.register_set_pagure_token()

  

      # Target registry goes here

      def register_update(self):
@@ -261,16 +263,20 @@ 

          pagure_section = '{0}.pagure'.format(self.name)

          pagure_url = config_get_safely(self.config, pagure_section, 'url')

          pagure_url_parsed = urlparse(pagure_url).netloc

+ 

          description = textwrap.dedent('''

              Request a new dist-git repository

  

              Before the operation, you need to generate a pagure.io API token at:

                  https://{1}/settings/token/new

  

-             ACL required:

-                 "Create a new ticket"

+                 ACL required:

+                     "Create a new ticket"

+ 

+             Update your token with the following command:

+                 fedpkg set-pagure-token <api_key_here>

  

-             Save the API token to local user configuration located at:

+             Command saves token to fedpkg config file:

                  ~/.config/rpkg/{0}.conf

  

              For example:
@@ -450,6 +456,7 @@ 

          help_msg = 'Create a new fork of the current repository'

          distgit_section = '{0}.distgit'.format(self.name)

          distgit_api_base_url = config_get_safely(self.config, distgit_section, "apibaseurl")

+ 

          description = textwrap.dedent('''

              Create a new fork of the current repository

  
@@ -459,7 +466,10 @@ 

              ACL required:

                  "Fork a project"

  

-             Save the API token to local user configuration located at:

+             Update your token with the following command:

+                 fedpkg set-distgit-token <api_key_here>

+ 

+             Command saves token to fedpkg config file:

                  ~/.config/rpkg/{0}.conf

  

              For example:
@@ -508,6 +518,38 @@ 

  

          parser.set_defaults(command=self.show_releases_info)

  

+     def register_set_distgit_token(self):

+         help_msg = \

+             'Updates the fedpkg.distgit API token in ~/.config/rpkg/{0}.conf file.\n\n\

+             Tokens are of length 64 and contain only uppercase and numerical values.'\

+             .format(self.name)

+ 

+         parser = self.subparsers.add_parser(

+             'set-distgit-token',

+             help=help_msg,

+             description=help_msg)

+         parser.add_argument(

+             'token',

+             help='The new API token.')

+ 

+         parser.set_defaults(command=self.set_distgit_token)

+ 

+     def register_set_pagure_token(self):

+         help_msg = \

+             'Updates the fedpkg.pagure API token in ~/.config/rpkg/{0}.conf file.\n\n\

+             Tokens are of length 64 and contain only uppercase and numerical values.'\

+             .format(self.name)

+ 

+         parser = self.subparsers.add_parser(

+             'set-pagure-token',

+             help=help_msg,

+             description=help_msg)

+         parser.add_argument(

+             'token',

+             help='The new API token.')

+ 

+         parser.set_defaults(command=self.set_pagure_token)

+ 

      def register_override(self):

          """Register command line parser for subcommand override

  
@@ -1318,6 +1360,73 @@ 

              print('Fedora: {0}'.format(_join(releases['fedora'])))

              print('EPEL: {0}'.format(_join(releases['epel'])))

  

+     def _check_token(self, token, token_type):

+ 

+         if token is None:

+             self.log.error("ERROR: No input.")

+             return False

+ 

+         match = re.search(r'^\s*[A-Z0-9]{64}\s*$', token)

+         if match is None:

+             self.log.error("ERROR: Token is not properly formatted.")

+             return False

+         else:

+             return True

+ 

+     def _set_token(self, token_type):

+ 

+         # Pop token off of the parse stack

+         TOKEN = self.args.token

+ 

+         # Get the path to the fedpkg config file.

+         PATH = os.path.join(os.path.expanduser('~'),

+                             '.config',

+                             'rpkg',

+                             '{0}.conf'.format(self.name))

+ 

+         # load new config parser

+         local_config = ConfigParser()

+         local_config.read(PATH)

+ 

+         # Ensure that user config file exists.

+         if not os.path.isfile(PATH):

+             self.log.error("ERROR: User config file not found at: {0}\n".format(PATH))

+             return

+ 

+         # Check that the user passed a valid token

+         if self._check_token(TOKEN, token_type):

+ 

+             print("updating config")

+ 

+             # Update the token in the config object.

+             section = "{0}.{1}".format(self.name, token_type)

+ 

+             # add the section if it doesn't already exist

+             if section not in local_config:

+                 local_config.add_section(section)

+ 

+             # set the distgit / pagure urls as needed (might already be set)

+             if(token_type == "pagure"):

+                 local_config.set(section, "url", "https://pagure.io/")

+             else:

+                 local_config.set(section, "apibaseurl", "https://src.fedoraproject.org")

+ 

+             # update the token

+             local_config.set(section, "token", TOKEN)

+ 

+             # Write the config to the user's config file.

+             with open(PATH, "w") as fp:

+                 try:

+                     local_config.write(fp)

+                 except configparser.Error:

+                     self.log.error("ERROR: Could not write to user config file.")

+ 

+     def set_pagure_token(self):

+         self._set_token("pagure")

+ 

+     def set_distgit_token(self):

+         self._set_token("distgit")

+ 

      def retire(self):

          """

          Runs the rpkg retire command after check. Check includes reading the state

file modified
+97
@@ -2406,3 +2406,100 @@ 

          self.retire_release("epel7", "pending")

          self.retire_release("epel7", None)

          self.retire_release("epel8", None)

+ 

+ 

+ class TestSetToken(CliTestCase):

+     """

+     Test the set-x-token cli command. Pagure / Distgit have the same tests.

+     """

+ 

+     def get_cli(self, cli_cmd, name='fedpkg', cfg=None):

+         with patch('sys.argv', new=cli_cmd):

+             return self.new_cli(name=name, cfg=cfg)

+ 

+     # Test: using lowercase characters rather than upper-case.

+     def test_token_input_mixed_lowercase_numerical(self):

+ 

+         LOWERCASE_TOKEN = "".join(["x" for _ in range(64)])

+         NUMERICAL_TOKEN = "".join([str(i % 10) for i in range(64)])

+ 

+         MIXED_LOWERCASE_NUMERICAL_TOKEN = ""

+         for i in range(32):

+             MIXED_LOWERCASE_NUMERICAL_TOKEN += LOWERCASE_TOKEN[i]

+             MIXED_LOWERCASE_NUMERICAL_TOKEN += NUMERICAL_TOKEN[i]

+ 

+         cli_cmd = ['fedpkg', 'set-pagure-token', MIXED_LOWERCASE_NUMERICAL_TOKEN]

+         cli = self.get_cli(cli_cmd)

+ 

+         try:

+             cli.set_pagure_token()

+         except rpkgError as error:

+             expected_error = "ERROR: Token is not properly formatted."

+             self.assertEqual(error, expected_error)

+ 

+     # Test: no input, none input.

+     def test_token_input_none(self):

+ 

+         cli_cmd = ["fedpkg", "set-pagure-token", None]

+         cli = self.get_cli(cli_cmd)

+ 

+         try:

+             cli.set_pagure_token()

+         except rpkgError as error:

+             expected_error = "ERROR: No input."

+             self.assertEqual(error, expected_error)

+ 

+         """

+         EXAMPLE:

+             cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path,

+                     '--name', 'nethack', 'request-branch', 'f27',

+                     '--all-releases']

+             cli = self.get_cli(cli_cmd)

+             expected_error = \

+                 'You cannot specify a branch with the "--all-releases" option'

+             try:

+                 cli.request_branch()

+                 assert False, 'rpkgError not raised'

+             except rpkgError as error:

+                 self.assertEqual(str(error), expected_error)

+         """

+ 

+     # Test: input is only lowercase.

+     def test_token_input_lowercase(self):

+ 

+         LOWERCASE_TOKEN = "".join(["x" for _ in range(64)])

+ 

+         cli_cmd = ['fedpkg', 'set-pagure-token', LOWERCASE_TOKEN]

+         cli = self.get_cli(cli_cmd)

+ 

+         try:

+             cli.set_pagure_token()

+         except rpkgError as error:

+             expected_error = "ERROR: Token is not properly formatted."

+             self.assertEqual(error, expected_error)

+ 

+     def test_token_input_too_short(self):

+ 

+         SHORT_TOKEN = "".join(['x' for _ in range(63)])

+ 

+         cli_cmd = ['fedpkg', 'set-pagure-token', SHORT_TOKEN]

+         cli = self.get_cli(cli_cmd)

+ 

+         try:

+             cli.set_pagure_token()

+         except rpkgError as error:

+             expected_error = "ERROR: Token is not properly formatted."

+             self.assertEqual(error, expected_error)

+ 

+     def test_token_input_too_long(self):

+ 

+         LONG_TOKEN = "".join(['x' for _ in range(65)])

+ 

+         cli_cmd = ['fedpkg', 'set-pagure-token', LONG_TOKEN]

+         cli = self.get_cli(cli_cmd)

+ 

+         try:

+             cli.set_pagure_token()

+         except rpkgError as error:

+             expected_error = "ERROR: Token is not properly formatted."

+             self.assertEqual(error, expected_error)

Added an interface that allows a user to update relevant expired
API keys via the command line interface.

Files:
cli.py: register_set_pagure_token
cli.py: register_set_distgit_token
cli.py: set_pagure_token
cli.py: set_distgit_token
cli.py: _set_token
cli.py: _check_token

Tests:
test_cli.py: TestSetToken (class)
test_cli.py: get_cli
test_cli.py: test_token_input_mixed_lowercase_numerical
test_cli.py: test_token_input_none
test_cli.py: test_token_input_lowercase
test_cli.py: test_token_input_too_short
test_cli.py: test_token_input_too_long

Fixes #192
Jira: RHELCMP-58

Signed-off-by: James Kunstle jkunstle@redhat.com

rebased onto 7d9781c4287baa236843f50e57bec4862a3d3fd6

2 years ago

rebased onto 3e9fdca8eb2a4faee841ff597f846a9a9b44d83e

2 years ago

rebased onto 324d52115b75efefaebedd46af3133185c98c491

2 years ago

Could this pattern "^\s[A-Z0-9]{64}\s$" simplify your processing?
or "^[A-Z0-9]{64}$" with .strip()

In most (maybe every) cases, when the config file is hardcoded, we could even get to the situation, that fedpkg-stage script is used. It has its own config name. You can use self.name (which contains fedpkg or fedpkg-stage) to distinguish the script.

self.name instead of const string.

Will this write the whole (global+user) config to the user config?
If yes, this would effectively do a lot of changes. I prefer to do just partial changes in the token section. But it needn't be much more complex. What about use ConfigParser for loading separate user config, update it and write?

We could also think about the detection of the expired token. Earlier I sent you a method on how we can verify the API functionality. If it fails, we can show a hint about the new methods to the user.

rebased onto cac91ae33718a5f14c0fa5ac68d437bdf8c80993

2 years ago

rebased onto 334e6404b058d1380ab4eead92e2acb4dcc7390a

2 years ago

I updated the code to reflect the optimizations that you suggested, apart from the final optimization concerning checking the api token. I think that I'll open another ticket later for this since it adds another layer of complexity to the function and will require writing a batch of Mock tests.

rebased onto 0536e12b56f2673f23c3765efa803f4e62cf5410

2 years ago

rebased onto ed129e99d3ac475c61c0b96d6a5e2ea95437e96b

2 years ago

rebased onto 95465caa6e3b559dd01e966300293843c473e9d6

2 years ago

rebased onto c1e0fe9a12e95661aa7d6c3ce5ff425acde2a2ad

2 years ago

rebased onto c5849556d3824a17b8696217364bf23b08ae84da

2 years ago

after_more seems to be useless here. Both commands could be moved to others on line 99.

rebased onto 8840b9f267bf9f1a79a45d6e034516d933e07bcf

2 years ago

rebased onto 0492474c7e612025eb493de836e20aa5a4a81e90

2 years ago

Made changes to the autocomplete script, included the names of the new functions on line 99 as requested.

rebased onto ced0e5a

2 years ago

Checked and it looks good.

Commit 92400b5 fixes this pull-request

Pull-Request has been merged by onosek

2 years ago