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

Dns record fixes + expansion #29

Merged
merged 6 commits into from Mar 2, 2018

Conversation

Projects
None yet
3 participants
@benjvi
Contributor

benjvi commented Feb 26, 2018

  • Adds "data" input so that users can specify SRV, LOC records (fixes #2 )
  • Adds some computed api fields to give users more context ('created_on', 'modified_on','proxiable','metadata')
  • Force recreation on domain changes + removes redundant zone_id lookups (this will help with, but not resolve #21 )
  • Set id blank when record is not found ( fixes #7 )
  • Allow CAA records through validation ( fixes #26 )
  • Now runs tests in parallel
@catsby

catsby approved these changes Mar 2, 2018

Looks good to me! Two nitpicks that are not required

log.Printf("[WARN] Removing record from state because it's not found in API")
d.SetId("")
return nil
} else {

This comment has been minimized.

@catsby

catsby Mar 2, 2018

Contributor

nit: no need for else here because of the return.

if err!= nil {
  if stringsContains .... {
    /// 
    d.SetId("")
    return nil
  }
  
  return err
}

This comment has been minimized.

@catsby

catsby Mar 2, 2018

Contributor

Applied in f01532c

d.Set("created_on", record.CreatedOn.Format(time.RFC3339Nano))
d.Set("data", expandStringMap(record.Data))
d.Set("modified_on", record.ModifiedOn.Format(time.RFC3339Nano))
d.Set("metadata", expandStringMap(record.Meta))

This comment has been minimized.

@catsby

catsby Mar 2, 2018

Contributor

For non-scalar values (Maps, Lists, Sets), please check the error:

if err := d.Set("metadata", expandStringMap(record.Meta)); err != nil {
  log.Printf("[WARN] Error setting metadata: %s", err)  
}

This comment has been minimized.

@catsby

catsby Mar 2, 2018

Contributor

Applied in f01532c

@catsby catsby merged commit 1c64972 into terraform-providers:master Mar 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@EpiqSty

This comment has been minimized.

EpiqSty commented Mar 16, 2018

@catsby it's nice to hear, that #2 is fixed, but i am still getting errors while creating SRV records:

Initializing provider plugins...
- Checking for available provider plugins on https://releases.hashicorp.com...
- Downloading plugin for provider "cloudflare" (0.1.0)...

The following providers do not have any version constraints in configuration,
so the latest version was installed.

To prevent automatic upgrades to new major versions that may contain breaking
changes, it is recommended to add version = "..." constraints to the
corresponding provider blocks in configuration, with the constraint strings
suggested below.

* provider.cloudflare: version = "~> 0.1"

Terraform has been successfully initialized!

[.....]

cloudflare_record._sipfederationtls__tcp_example_com_SRV: Creating...
  domain:   "" => "example.com"
  hostname: "" => "<computed>"
  name:     "" => "_sipfederationtls._tcp"
  proxied:  "" => "false"
  ttl:      "" => "3600"
  type:     "" => "SRV"
  value:    "" => "0 0 5061 sip.example.com"
  zone_id:  "" => "<computed>"

Error: Error applying plan:

1 error(s) occurred:

* cloudflare_record._sipfederationtls__tcp_example_com_SRV: 1 error(s) occurred:

* cloudflare_record._sipfederationtls__tcp_example_com_SRV: Failed to create record: error from makeRequest: HTTP status 400: content "{\"success\":false,\"errors\":[{\"code\":1004,\"message\":\"DNS Validation Error\",\"error_chain\":[{\"code\":9036,\"message\":\"Invalid or missing data for %s record\"}]}],\"messages\":[],\"result\":null}"

Should I await until next provider release or I could get last revision and build provider on my own and use it as a custom provider?

Thanks in advance!

@catsby

This comment has been minimized.

Contributor

catsby commented Mar 16, 2018

Hey @EpiqSty – while the fix has been merged, there has been no new release since then. Here's a list of things coming in the next release:

You are welcome to pull down the master branch of this repository and build the provider yourself! This requires Go 1.9 setup on your system, but after cloning the repository simply run go install on the machine to install it. You'll then need to be in the directory of your configuration and run terraform init -upgrade to grab the compiled version.

@EpiqSty

This comment has been minimized.

EpiqSty commented Mar 16, 2018

@catsby , thanks for the quick reply!

Do you know anything about release plans?

I mean for manual usage upgrading from sources is kind of OK, but as we are using terraform in CI/CD environment that will requires adjustment of Jenkins jobs in order to get custom provider on the Jenkins slaves and so on.

So, this is a question about the release date, basically ;)

If it in the near future (several weeks) we could create such records locally ( using custom provider builded from sources) and push tfstates to our terraform repo.

But if we should wait for the next release several months, probably we should choose the option of using custom providers directly on the jenkins slaves.

@catsby

This comment has been minimized.

Contributor

catsby commented Mar 16, 2018

I do not know the release date, other than "as soon as possible". There are some other PRs and features that I believe are requested in the next release.

The release date is optimistically 0-7 days, pessimistically 2-3 weeks. I cannot promise either of those, though 😄

@EpiqSty

This comment has been minimized.

EpiqSty commented Mar 16, 2018

well, it's good enough - thanks ;)

@EpiqSty

This comment has been minimized.

EpiqSty commented Mar 20, 2018

@catsby I have tested creation of SRV records with latest version, build from sources, but get the same error.
Maybe for SRV records we should specify structured data?
I saw something related in your commit:
allow users to specify structured data for records 41ef9b8
But I can't find any examples, how should we define such data.
Should it be something like this ?

 resource "cloudflare_record" "_sipfederationtls__tcp_example_com_SRV" {
   domain  = "example.com"
   name    = "_sipfederationtls._tcp"
   type    = "SRV"
   value   = "0 0 5061 sip.example.com"
   ttl     = 1
   data { 
     priority = 1
     weight   = 1
     port     = 5061
     target   = "sip.example.com" 
     service  = "_sipfederationtls"
     proto    = "_tcp"
     name     = "example.com" }
 }

Unfortunately , Cloudflare API itself has no proper documentation about fields required for creating SRV records.
I have created support ticket about it, but at the moment they provided me just this advice:

In the meantime, what I would suggest is you can create a SRV record via our UI and then you can use the API to do a GET request on the DNS records, that should show you all the fields that are required.

@EpiqSty

This comment has been minimized.

EpiqSty commented Mar 20, 2018

Looks like I found the way how to define it:

resource "cloudflare_record" "_sipfederationtls__tcp_example_com_SRV" {
  domain  = "example.com"
  name    = "_sipfederationtls._tcp"
  type    = "SRV"
#  value   = "0 0 5061 sip.example.com"
  ttl     = 3600
  data {
    priority = 0
    weight   = 0
    port     = 5061
    target   = "sip.example.com"
    service  = "_sipfederationtls"
    proto    = "_tcp"
    name     = "example.com" }
}

[...]

cloudflare_record._sipfederationtls__tcp_example_com_SRV: Creating...
  created_on:    "" => "<computed>"
  data.%:        "" => "7"
  data.name:     "" => "example.com"
  data.port:     "" => "5061"
  data.priority: "" => "0"
  data.proto:    "" => "_tcp"
  data.service:  "" => "_sipfederationtls"
  data.target:   "" => "sip.example.com"
  data.weight:   "" => "0"
  domain:        "" => "example.com"
  hostname:      "" => "<computed>"
  metadata.%:    "" => "<computed>"
  modified_on:   "" => "<computed>"
  name:          "" => "_sipfederationtls._tcp"
  proxiable:     "" => "<computed>"
  proxied:       "" => "false"
  ttl:           "" => "3600"
  type:          "" => "SRV"
  value:         "" => "<computed>"
  zone_id:       "" => "<computed>"
cloudflare_record._sipfederationtls__tcp_example_com_SRV: Creation complete after 0s (ID: <removed>)

I left "value" commented just in order to mention, that SRV definition with "value" only, ends up with error "Invalid or missing data for %s record", but the same record, defined as data structure works well.

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