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

FEATURE: M365_BUILDER add support for alternative MX hosts #3191

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ajh0912
Copy link

@ajh0912 ajh0912 commented Nov 4, 2024

Closes #3185

Could you review and advise if this approach is the right way?

I've added a new example for M365_BUILDER, with all services enabled and using the new mxHost parameter to point to an example DNSSEC enabled MX host.

@tlimoncelli
Copy link
Contributor

CC @juliusrickert @nikolamilekic for feedback

Copy link
Contributor

@juliusrickert juliusrickert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short, I advise against creating a custom solution for this.
Instead, I'd like to see documentation on how to solve the situation using existing mechanisms. (See code comment.)

skypeForBusiness: true,
mdm: true,
initialDomain: "example.onmicrosoft.com",
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing comma should be removed, I think.

value.domainGUID + '.mail.protection.outlook.com.'
)
);
r.push(MX(value.label, 0, value.domainGUID + '.' + value.mxHost + '.'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PowerShell(?) commands (link to highlight, might not work in all browsers) returns the whole MX hostname: contosotest-com.o-v1.mx.microsoft.

Therefore, and to make the helper more flexible in general, I advise to do one of the following:

  1. Document how to disable the MX and set the Microsoft-provided hostname. (preferred)
  2. Allow any MX to be set, possibly multiple MXs

Documentation for the first option could look like this:

D("example.com", REG_MY_PROVIDER, DnsProvider(DSP_MY_PROVIDER),
  M365_BUILDER("example.com", {
      mx: false,
      …
  }),
  MX('@', 0, 'example-com.<random>.mx.microsoft.') // Note the trailing dot.
);

@nikolamilekic
Copy link
Contributor

@tlimoncelli
I wish I could give constructive feedback on this, but I'm not sure I can.

I understand the challenges of supporting complex macros, but at the same time, I highly value the ability to glance at a DNSControl file and quickly get the idea of what is configured and what isn't (and correctly). For complex configurations such as that of M365 (or MTA-STS), I believe it is especially important to have a macro because I only need to read a single line of code and know what it pertains to instead of 20.

I think it will be of enormous help for the adoption of various complex DNS conventions and standards (such as MTA-STS) if there were an easy way to configure them. With the DMARC and SPF builders, DNSControl is already well on the way of becoming just such a tool. Wouldn't it be great if we went all the way? If the cost is community-supported maintenance of complex builders, would that be so bad? I'd much rather place trust on a community of experts to maintain a set of macros, than risk copy/paste mistakes from documentation.

@tlimoncelli
Copy link
Contributor

@nikolamilekic

Thank you for taking the time to write up your thoughts.

"Builders" are perhaps one of DNSControl's most unique and powerful features. They are complex to maintain, and I'd like to improve that.

One idea I had was to remove them from the system, so that people can maintain them independently. However I think that just takes a big problem and moves it elsewhere.

I'd rather make them easier to manage:

  1. Testable: Unit tests so we don't break existing use-cases.
  2. Isolated: Isolate the code to 1 place for easier management
  3. Versioned: Either semver or a policy of "no breaking changes until major releases"

I don't know how to do those things in JavaScript, but I do know how to do them all in Go. I have an idea for a way to make it possible to write these in Go, with proper testing, isolation, and versioning.

That's long term. As far as M365_BUILDER, I defer to @juliusrickert and would prefer documentation (rather than major changes) at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for mx.microsoft. in M365_BUILDER
4 participants