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

Dns record fixes + expansion #29

Merged
merged 6 commits into from
Mar 2, 2018

Conversation

benjvi
Copy link
Contributor

@benjvi benjvi commented Feb 26, 2018

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Applied in hashicorp@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))
Copy link
Contributor

Choose a reason for hiding this comment

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

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)  
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Applied in hashicorp@f01532c

@EpiqSty
Copy link

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
Copy link
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
Copy link

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
Copy link
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
Copy link

EpiqSty commented Mar 16, 2018

well, it's good enough - thanks ;)

@EpiqSty
Copy link

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
Copy link

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
Labels
None yet
Projects
None yet
3 participants