close
The Wayback Machine - https://web.archive.org/web/20200914053929/https://github.com/github/octodns/issues/416
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

incorrect MX record formatting not detected, leaves python error #416

Closed
splk3 opened this issue Nov 1, 2019 · 4 comments
Closed

incorrect MX record formatting not detected, leaves python error #416

splk3 opened this issue Nov 1, 2019 · 4 comments

Comments

@splk3
Copy link

@splk3 splk3 commented Nov 1, 2019

Platform: Ubuntu 16.04
octodns version: 0.9.6 and 0.9.8

If an MX record is created and accidentally uses the value: format, octodns-validate will exit with python errors, instead of detecting the misconfiguration and providing corrective output.

Incorrect MX record format (for example, if someone copies an A record for format):

testmx:
  - ttl: 900
    type: A
    value: 10.10.10.100
  - ttl: 900
    type: MX
    value: mail.testmx.com.

Since this is missing values for exchange and preference, I would expect that octodns-validate would say that those entries are required for records of type MX. Instead, errors such as below are out put by python:

Traceback (most recent call last):
  File "/usr/local/bin/octodns-validate", line 11, in <module>
    sys.exit(main())
  File "/usr/local/lib/python2.7/dist-packages/octodns/cmds/validate.py", line 24, in main
    manager.validate_configs()
  File "/usr/local/lib/python2.7/dist-packages/octodns/manager.py", line 406, in validate_configs
    source.populate(zone)
  File "/usr/local/lib/python2.7/dist-packages/octodns/provider/yaml.py", line 77, in populate
    self._populate_from_file(filename, zone, lenient)
  File "/usr/local/lib/python2.7/dist-packages/octodns/provider/yaml.py", line 61, in _populate_from_file
    lenient=lenient)
  File "/usr/local/lib/python2.7/dist-packages/octodns/record/__init__.py", line 99, in new
    reasons = _class.validate(name, data)
  File "/usr/local/lib/python2.7/dist-packages/octodns/record/__init__.py", line 261, in validate
    reasons.extend(cls._value_type.validate(values, cls._type))
  File "/usr/local/lib/python2.7/dist-packages/octodns/record/__init__.py", line 823, in validate
    int(value['preference'])
TypeError: string indices must be integers, not unicode

When the record is corrected to include those values, as shown below, the errors go away and octodns completes successfully.

testmx:
  - ttl: 900
    type: A
    value: 10.10.10.100
  - ttl: 900
    type: MX
    value: 
      exchange: mail.testmx.com.
      preference: 10
@ross
Copy link
Contributor

@ross ross commented Nov 1, 2019

Since this is missing values for exchange and preference, I would expect that octodns-validate would say that those entries are required for records of type MX. Instead, errors such as below are out put by python:

Good call. Should be a fairly simple PR to the validation stuff to make sure that the data that comes in is a dict, i think in octodns/record/__init..py MxValue if you want to make a pass at it. I'll throw it onto my todo list if not.

@splk3
Copy link
Author

@splk3 splk3 commented Dec 11, 2019

will give it a go if I can find some time over holidays. thanks.

@siddharths2710
Copy link

@siddharths2710 siddharths2710 commented Aug 30, 2020

Is this still a valid issue? I see that the validate function handles the exchange and preference attributes:

class MxValue(EqualityTupleMixin):

    @classmethod
    def validate(cls, data, _type):
        if not isinstance(data, (list, tuple)):
            data = (data,)
        reasons = []
        for value in data:
            try:
                try:
                    int(value['preference'])
                except KeyError:
                    int(value['priority'])
            except KeyError:
                reasons.append('missing preference')
            except ValueError:
                reasons.append('invalid preference "{}"'
                               .format(value['preference']))
            exchange = None
            try:
                exchange = value.get('exchange', None) or value['value']
                if not exchange.endswith('.'):
                    reasons.append('MX value "{}" missing trailing .'
                                   .format(exchange))
            except KeyError:
                reasons.append('missing exchange')
        return reasons
@ross
Copy link
Contributor

@ross ross commented Aug 31, 2020

Yeah. It looks stale.

@ross ross closed this Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.