#650 Print friendly copr-cli error on http copr_url when https is enforced
Closed 5 years ago by praiskup. Opened 5 years ago by frostyx.
copr/ frostyx/copr friendly-error-on-http  into  master

file modified
+4 -2
@@ -51,6 +51,8 @@ 

  Any operation requiring credentials will fail!

  ==================================================

  

+ Hint: {1}

+ 

  """

  

  try:
@@ -64,8 +66,8 @@ 

  

          try:

              self.config = config_from_file(self.config_path)

-         except (CoprNoConfigException, CoprConfigException):

-             sys.stderr.write(no_config_warning.format(self.config_path))

+         except (CoprNoConfigException, CoprConfigException) as ex:

+             sys.stderr.write(no_config_warning.format(self.config_path, ex))

              self.config = {"copr_url": "http://copr.fedoraproject.org", "no_config": True}

  

          self.client = Client(self.config)

file modified
+4 -4
@@ -152,7 +152,7 @@ 

  

  @mock.patch('copr_cli.main.config_from_file', return_value=mock_config)

  def test_cancel_build_no_config(config_from_file, capsys):

-     config_from_file.side_effect = copr.v3.CoprNoConfigException()

+     config_from_file.side_effect = copr.v3.CoprNoConfigException("Dude, your config is missing")

  

      with pytest.raises(SystemExit) as err:

          main.main(argv=["cancel", "123400"])
@@ -163,7 +163,7 @@ 

      assert "Error: Operation requires api authentication" in err

      assert "File '~/.config/copr' is missing or incorrect" in err

  

-     expected_warning = no_config_warning.format("~/.config/copr")

+     expected_warning = no_config_warning.format("~/.config/copr", "Dude, your config is missing")

      assert expected_warning in err

  

  
@@ -191,7 +191,7 @@ 

      expected_output = read_res('list_projects_expected.txt')

  

      # no config

-     config_from_file.side_effect = copr.v3.CoprNoConfigException()

+     config_from_file.side_effect = copr.v3.CoprNoConfigException("Dude, your config is missing")

      control_response = [Munch(x) for x in response_data]

      get_list.return_value = control_response

  
@@ -199,7 +199,7 @@ 

      out, err = capsys.readouterr()

      assert expected_output in out

  

-     expected_warning = no_config_warning.format("~/.config/copr")

+     expected_warning = no_config_warning.format("~/.config/copr", "Dude, your config is missing")

      assert expected_warning in err

  

  

@@ -35,6 +35,9 @@ 

  username = {{ g.user.name }}

  token = {{ g.user.api_token }}

  copr_url = {{ ('https://' + config['PUBLIC_COPR_HOSTNAME'])| fix_url_https_frontend}}

+ {% if config['ENFORCE_PROTOCOL_FOR_FRONTEND_URL'] == 'http' %}

+ encrypted = False

I'd slightly prefer allow_unencrypted = True, but not a hard requirement.

+ {% endif %}

  # expiration date: {{ g.user.api_token_expiration }}

  </pre>

  

file modified
+10 -8
@@ -1,5 +1,4 @@ 

  import os

- import six

  import time

  import configparser

  from munch import Munch
@@ -21,21 +20,24 @@ 

  

      try:

          exists = raw_config.read(path)

-     except configparser.Error:

-         raise CoprConfigException()

+     except configparser.Error as ex:

+         raise CoprConfigException(str(ex))

  

      if not exists:

-         raise CoprNoConfigException()

+         raise CoprNoConfigException("There is no config file: {}".format(path))

  

      try:

          for field in ["username", "login", "token", "copr_url"]:

-             if six.PY3:

-                 config[field] = raw_config["copr-cli"].get(field, None)

-             else:

-                 config[field] = raw_config.get("copr-cli", field)

+             config[field] = raw_config["copr-cli"].get(field, None)

+         config["encrypted"] = raw_config["copr-cli"].getboolean("encrypted", True)

  

      except configparser.Error as err:

          raise CoprConfigException("Bad configuration file: {0}".format(err))

+ 

+     if config["encrypted"] and config["copr_url"].startswith("http://"):

+         raise CoprConfigException("The `copr_url` should not be http, please obtain "

+                                   "an up-to-date configuration from the Copr website")

+ 

      return config

  

  

Fixes #502

The first two commits are kinda general, not related to the actual issue. I made those changes to make the bugfix easier.

I'd slightly prefer allow_unencrypted = True, but not a hard requirement.

    [python] remove unnecessary PY3 condition

    The condition is IMHO not required. The command

        raw_config["copr-cli"].get(field, None)

    works for me in both python3 and python2.7. I am not sure
    what the situation is in python2.6 and lesser, but since
    the condition distinguishes only between 2 and 3, it shouldn't
    matter.

Well, we still have el6 support for cli/python. I suppose issue
would be caught by the testsuite on el6, right?

I suspect that since we depend on python-configparser, the condition
removal is really correct:

# rpm -qi python-configparser | grep Sum
Summary     : Backport of Python 3 configparser module

Thanks, +1.

rebased onto fd6aad0

5 years ago

Pull-Request has been closed by praiskup

5 years ago

[python] remove unnecessary PY3 condition

I've tested the problematic line in python2.6 on epel6 and it seems to work.

docker pull alanfranz/drb-epel-6-x86-64  # There is no official epel6 container?
...
[root@1e4fbf1937f5 /]# python
Python 2.6.6 (r266:84292, Aug 18 2016, 15:13:37)
>>> import os
>>> import configparser
>>> raw_config = configparser.ConfigParser()
>>> path = os.path.expanduser(os.path.join("~", ".config", "copr"))
>>> raw_config.read(path)
>>> raw_config["copr-cli"].get("copr_url", None)
u'http://copr-fe-dev.cloud.fedoraproject.org'

There is no official epel6 container?

Dunno, but I've used mock -r epel-6-x86_64 --install/--shell for this. FWIW, we can use podman nowadays instead of docker :thumbsup: