Skip to content

Conversation

jchristgit
Copy link
Member

The existing opendkim milter is no longer maintained.
This commit introduces a role which deploys dkim-milter.
As-is, it is not a complete replacement, since the role does not (yet)
migrate keys of the old opendkim setup.

@github-actions github-actions bot added the group: ansible Issues and pull requests related to the Ansible setup label Apr 20, 2025
Copy link

github-actions bot commented Apr 20, 2025

OctoDNS Plan for new-dkim-milter

pythondiscord.com.

cloudflare

Operation Name Type TTL Value Source
Delete _acme-challenge TXT 120 DpNMkDbvV6A8DYCi5m-qERyNTtO9E28w2fqc1P-COvA; NYdLAk9Q7_b9CbgybIhPXI6ZkAH7SwNOHV4uSKmX1ls
Delete cf2024-1._domainkey TXT 300 v=DKIM1; h=sha256; k=rsa; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiweykoi+o48IOGuP7GR3X0MOExCUDY/BCRHoWBnh3rChl7WhdyCxW3jgq1daEjPPqoi7sJvdg5hEQVsgVRQP4DcnQDVjGMbASQtrY4WmB1VebF+RPJB2ECPsEDTpeiI5ZyUAwJaVX7r6bznU67g7LvFq35yIo4sdlmtZGV+i0H4cpYH9+3JJ78km4KXwaf9xUJCWF6nxeD+qG6Fyruw1Qlbds2r85U9dkNDVAS3gioCvELryh1TxKGiVTkg4wqHTyHfWsp7KD3WQHYJn0RyfJJu6YEmL77zonn7p2SRMvTMP3ZEXibnC9gz3nnhR6wcYL8Q7zXypKTMD58bTixDSJwIDAQAB

Summary: Creates=0, Updates=0, Deletes=2, Existing=34, Meta=False

pythondiscord.org.

cloudflare

Operation Name Type TTL Value Source
Delete cf2024-1._domainkey TXT 300 v=DKIM1; h=sha256; k=rsa; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiweykoi+o48IOGuP7GR3X0MOExCUDY/BCRHoWBnh3rChl7WhdyCxW3jgq1daEjPPqoi7sJvdg5hEQVsgVRQP4DcnQDVjGMbASQtrY4WmB1VebF+RPJB2ECPsEDTpeiI5ZyUAwJaVX7r6bznU67g7LvFq35yIo4sdlmtZGV+i0H4cpYH9+3JJ78km4KXwaf9xUJCWF6nxeD+qG6Fyruw1Qlbds2r85U9dkNDVAS3gioCvELryh1TxKGiVTkg4wqHTyHfWsp7KD3WQHYJn0RyfJJu6YEmL77zonn7p2SRMvTMP3ZEXibnC9gz3nnhR6wcYL8Q7zXypKTMD58bTixDSJwIDAQAB

Summary: Creates=0, Updates=0, Deletes=1, Existing=4, Meta=False

pydis.wtf.

cloudflare

Operation Name Type TTL Value Source
Delete _acme-challenge TXT 120 bAMtYtCdS3WoIRItvSJ-J3JijCtFx-iUsyvd0jYTVdM
Delete cf2024-1._domainkey TXT 300 v=DKIM1; h=sha256; k=rsa; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiweykoi+o48IOGuP7GR3X0MOExCUDY/BCRHoWBnh3rChl7WhdyCxW3jgq1daEjPPqoi7sJvdg5hEQVsgVRQP4DcnQDVjGMbASQtrY4WmB1VebF+RPJB2ECPsEDTpeiI5ZyUAwJaVX7r6bznU67g7LvFq35yIo4sdlmtZGV+i0H4cpYH9+3JJ78km4KXwaf9xUJCWF6nxeD+qG6Fyruw1Qlbds2r85U9dkNDVAS3gioCvELryh1TxKGiVTkg4wqHTyHfWsp7KD3WQHYJn0RyfJJu6YEmL77zonn7p2SRMvTMP3ZEXibnC9gz3nnhR6wcYL8Q7zXypKTMD58bTixDSJwIDAQAB

Summary: Creates=0, Updates=0, Deletes=2, Existing=45, Meta=False

@jb3
Copy link
Member

jb3 commented Apr 20, 2025

Remind me, does this replace the sending of DKIM reports to sender domains when we fail validation?

Can we still copy failing domains into a mailbox? Are we still performing validation of inbound email?

@jchristgit
Copy link
Member Author

jchristgit commented Apr 20, 2025 via email

@jb3
Copy link
Member

jb3 commented Apr 20, 2025 via email

@jchristgit
Copy link
Member Author

jchristgit commented Apr 21, 2025 via email

@jb3 jb3 requested a review from Copilot April 27, 2025 00:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the deprecated opendkim milter with a new dkim-milter role for managing DKIM key generation and service configuration. Key changes include:

  • Addition of variables and configuration files for dkim-milter.
  • Implementation of tasks to install dependencies, generate keys, and set up service templates.
  • Updates to handlers and playbook to integrate the new dkim-milter role.

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.

File Description
ansible/roles/dkim-milter/vars/main.yml Introduces configuration variables for domains, signings, and selector.
ansible/roles/dkim-milter/tasks/main.yml Implements tasks to install packages, create users/directories, generate keys, and configure services.
ansible/roles/dkim-milter/handlers/main.yml Defines handlers for service reload/restart of dkim-milter.
ansible/playbook.yml Updates playbook to replace opendkim with dkim-milter.
Files not reviewed (2)
  • ansible/roles/dkim-milter/templates/dkim-milter.conf.j2: Language not supported
  • ansible/roles/dkim-milter/templates/dkim-milter.service.j2: Language not supported

with_items:
- "{{ dkim_milter_domains }}"
args:
creates: /etc/dkimkeys/{{ item }}/{{ dkim_milter_selector }}.pem
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

The 'creates' path appears inconsistent with the key generation location in the command (which is /etc/dkim-milter/keys). Consider updating the path to match the expected directory.

Suggested change
creates: /etc/dkimkeys/{{ item }}/{{ dkim_milter_selector }}.pem
creates: /etc/dkim-milter/keys/{{ item }}/{{ dkim_milter_selector }}.pem

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, I think it's right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in its screenful of absolutely pointless fluff, it manages to produce something of value. This is roughly similar to talking to a Microsoft representative.

The existing `opendkim` milter is no longer maintained.
This commit introduces a role which deploys `dkim-milter`.
As-is, it is not a complete replacement, since the role does not (yet)
migrate keys of the old `opendkim` setup.
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

ExecReload=/bin/kill -HUP $MAINPID
Restart=on-failure

# schizophrenia
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

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

The comment 'schizophrenia' is inappropriate and unprofessional. Consider removing it or replacing it with a clear explanation of why system protection is being configured.

Suggested change
# schizophrenia
# Restrict access to the system for security

Copilot uses AI. Check for mistakes.

Comment on lines +77 to +81
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}.pem
{% endfor %}
{% for item in dkim_milter_extra_signings %}
{% set keyname = (item['domain'] | replace(".", "_")) %}
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}.pem
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

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

The key file path references {{ keyname }}.pem but the key generation task creates files in subdirectories like /etc/dkim-milter/keys/{{ item }}/{{ dkim_milter_selector }}.private. The path structure is inconsistent and will cause the milter to fail finding the keys.

Suggested change
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}.pem
{% endfor %}
{% for item in dkim_milter_extra_signings %}
{% set keyname = (item['domain'] | replace(".", "_")) %}
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}.pem
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}/{{ dkim_milter_selector }}.private
{% endfor %}
{% for item in dkim_milter_extra_signings %}
{% set keyname = (item['domain'] | replace(".", "_")) %}
{{ keyname }} < /etc/dkim-milter/keys/{{ keyname }}/{{ dkim_milter_selector }}.private

Copilot uses AI. Check for mistakes.

group: root
mode: 0o755
vars:
dkim_milter_version: 0.2.0-alpha.1
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

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

Using an alpha version in production deployment could lead to instability issues. If Rust was used for this milter implementation, you'd benefit from memory safety guarantees and better error handling that could make alpha versions more reliable. Consider evaluating the stability of this alpha release or pinning to a stable version when available.

Copilot generated this review using guidance from repository custom instructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group: ansible Issues and pull requests related to the Ansible setup

Projects

Status: Up next

Development

Successfully merging this pull request may close these issues.

2 participants