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

Implements domain check data source #9

Merged

Conversation

andreaskweber
Copy link
Contributor

@andreaskweber andreaskweber commented Oct 17, 2021

Hey @issyl0,

I've implemented the domain check data source. I've tested it according to your instructions in my local development environment. To successfully integrate the changes, your Improvmx client dependency must be updated to a new version. As soon as you've merged the other PR and tagged a new version, I'll update this PR for you to review and merge. I've also updated the docs.

Data source result piped to Terraform output:

  + test = {
      + domain                    = "foobar.de"
      + id                        = "foobar.de"
      + record_mx_actual_values   = [
          + "mx1.improvmx.com",
          + "mx2.improvmx.com",
        ]
      + record_mx_expected_values = [
          + "mx1.improvmx.com",
          + "mx2.improvmx.com",
        ]
      + record_mx_is_valid        = true
      + record_spf_actual_value   = "v=spf1 include:spf.improvmx.com ~all"
      + record_spf_expected_value = "v=spf1 include:spf.improvmx.com ~all"
      + record_spf_is_valid       = true
    }

Corresponding endpoint documentation: https://improvmx.com/api/#domain-check

I'm happy to receive your feedback! If you've any questions, let me know.

Thank you! 😃

Copy link
Owner

@issyl0 issyl0 left a comment

Choose a reason for hiding this comment

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

Thanks for the attention to detail here, the .gitignore and the readability improvements as well as the functionality itself!

I released v0.16.0 of the ImprovMX API client, so you can bump the version in go.mod and test this out again.

I've also updated the docs.

I don't see any docs updates? Or were you referring to the error messages?

@andreaskweber
Copy link
Contributor Author

andreaskweber commented Oct 17, 2021

Sorry, wasn't ready with the docs, that's because the PR was marked as draft. Now I'm done. I've also updated the client library version to the most recent version.

Copy link
Owner

@issyl0 issyl0 left a comment

Choose a reason for hiding this comment

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

I tested this with the following domain that didn't have DNS configured:

resource "improvmx_domain" "test" {
  domain = "test.issyl0.co.uk"
}

data "improvmx_domain_check" "test" {
  domain = "test.issyl0.co.uk"
}

output "test_configuration" {
  value = data.improvmx_domain_check.test
}

and I got:

improvmx_domain.test-issyl0: Refreshing state... [id=test.issyl0.co.uk]

Changes to Outputs:
  + test-issyl0-configuration = {
      + domain                    = "test.issyl0.co.uk"
      + id                        = "test.issyl0.co.uk"
      + record_mx_actual_values   = []
      + record_mx_expected_values = [
          + "mx1.improvmx.com",
          + "mx2.improvmx.com",
        ]
      + record_mx_is_valid        = false
      + record_spf_actual_value   = ""
      + record_spf_expected_value = "v=spf1 include:spf.improvmx.com ~all"
      + record_spf_is_valid       = false
      + records_are_valid         = false
    }

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

test-issyl0-configuration = {
  "domain" = "test.issyl0.co.uk"
  "id" = "test.issyl0.co.uk"
  "record_mx_actual_values" = tolist([])
  "record_mx_expected_values" = tolist([
    "mx1.improvmx.com",
    "mx2.improvmx.com",
  ])
  "record_mx_is_valid" = false
  "record_spf_actual_value" = ""
  "record_spf_expected_value" = "v=spf1 include:spf.improvmx.com ~all"
  "record_spf_is_valid" = false
  "records_are_valid" = false
}

I'm not sure about the toList([]) output vs the [] in the expected output, if that matters, but I can add the DNS MX records for the test.issyl0.co.uk domain using those outputs, which is what you're trying to achieve I think:

resource "dnsimple_record" "mx-1" {
  domain = "test.issyl0.co.uk"
  name   = ""
  type   = "MX"
  ttl    = 3600
  value  = data.improvmx_domain_check.test.record_mx_expected_values[0]
}

resource "dnsimple_record" "mx-2" {
  domain = "test.issyl0.co.uk"
  name   = ""
  type   = "MX"
  ttl    = 3600
  value  = data.improvmx_domain_check.test.record_mx_expected_values[1]
}

resource "dnsimple_record" "spf" {
  domain = "test.issyl0.co.uk"
  name   = ""
  type   = "TXT"
  ttl    = 3600
  value  = data.improvmx_domain_check.test.record_spf_expected_value
}

which, once terraform apply has run, the output gives:

test-configuration = {
  "domain" = "test.issyl0.co.uk"
  "id" = "test.issyl0.co.uk"
  "record_mx_actual_values" = tolist([
    "mx1.improvmx.com",
    "mx2.improvmx.com",
  ])
  "record_mx_expected_values" = tolist([
    "mx1.improvmx.com",
    "mx2.improvmx.com",
  ])
  "record_mx_is_valid" = true
  "record_spf_actual_value" = ""
  "record_spf_expected_value" = "v=spf1 include:spf.improvmx.com ~all"
  "record_spf_is_valid" = false
  "records_are_valid" = false
}

All that to say, this is great, thank you so much! 💪🏻 🎉

@issyl0 issyl0 merged commit 5fc1fd6 into issyl0:main Oct 17, 2021
issyl0 added a commit that referenced this pull request Oct 17, 2021
- This'll include #9. 🎉
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.

None yet

2 participants