feat: add ds records and name servers resources for domains #818

App13Pie · 2025-02-02T14:44:23Z

Description

Added two resources for OVH domains:

ovh_domain_name_servers: Resource to set external name servers for a domain.
ovh_domain_ds_records: Resource to set ds_records for a domain

TODO: I haven't updated the documentation.

Fixes #146 (issue)

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I created a public zone on GPC Cloud DNS to have valid values to pass down to the below tests.

You'll have to export these environment variables:

export OVH_ZONE_TEST="..."
export OVH_DOMAIN_NS1_HOST_TEST="..."
export OVH_DOMAIN_NS1_IP_TEST="..."
export OVH_DOMAIN_NS2_HOST_TEST="..."
export OVH_DOMAIN_NS3_HOST_TEST="..."
export OVH_DOMAIN_DS_RECORD_ALGORITHM_TEST="..."
export OVH_DOMAIN_DS_RECORD_PUBLIC_KEY_TEST="..."
export OVH_DOMAIN_DS_RECORD_TAG_TEST="..."

Test A: make testacc TESTARGS="-run TestAccDomainNameServers_Basic"

Test Configuration:

Terraform version: terraform version: Terraform v1.10.5
Existing HCL configuration you used:

resource "ovh_domain_name_servers" "test" {
	domain = "test.ovh"

	servers {
		host = "ns-cloud-a1.googledomains.com"
		ip = "216.239.32.106"
	}

	servers {
		host = "ns-cloud-a2.googledomains.com"
		ip = ""
    }
}

Test B: make testacc TESTARGS="-run TestAccDomainDsRecords_Basic"

Test Configuration:

Terraform version: terraform version: Terraform v1.10.5
Existing HCL configuration you used:

resource "ovh_domain_ds_records" "test" {
	domain = "test.ovh"

	ds_records {
		algorithm = "RSASHA256"
		flags = "KEY_SIGNING_KEY"
		public_key = "YOUR_PUBLIC_KEY"
		tag = 12345
	}
}

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings or issues
I have added acceptance tests that prove my fix is effective or that my feature works
New and existing acceptance tests pass locally with my changes
I ran succesfully go mod vendor if I added or modify go.mod file

amstuta · 2025-02-12T08:02:49Z

ovh/resource_domain_ds_records.go

+
+	log.Printf("[DEBUG] Will read domain name DS records: %s\n", domainName)
+
+	responseData := &[]int{}


Suggested change

responseData := &[]int{}

var responseData []int

amstuta · 2025-02-12T08:05:55Z

ovh/types_domain.go

@@ -0,0 +1,16 @@
+package ovh
+
+type DomainTask struct {


since this is the exact same struct as DomainZoneTask in file types_domain_zone.go, could you keep only one ? (keeping this one is fine by me since these tasks are common to all /domain section)

amstuta · 2025-02-12T08:08:09Z

ovh/domain_task.go

+	"time"
+)
+
+func waitDomainTask(client *ovh.Client, domainName string, taskId int) error {


we should move this in types_domain.go to keep consistency with the existing code

amstuta · 2025-02-12T08:12:20Z

ovh/resource_domain_ds_records.go

+		responseData := &DomainDsRecord{}
+		endpoint := fmt.Sprintf("/domain/%s/dsRecord/%d", url.PathEscape(domainName), dsRecordId)
+
+		if err := config.OVHClient.Get(endpoint, &responseData); err != nil {
+			return helpers.CheckDeleted(resourceData, err, endpoint)
+		}
+
+		domainDsRecords.DsRecords = append(domainDsRecords.DsRecords, *responseData)


Suggested change

responseData := &DomainDsRecord{}

endpoint := fmt.Sprintf("/domain/%s/dsRecord/%d", url.PathEscape(domainName), dsRecordId)

if err := config.OVHClient.Get(endpoint, &responseData); err != nil {

return helpers.CheckDeleted(resourceData, err, endpoint)

}

domainDsRecords.DsRecords = append(domainDsRecords.DsRecords, *responseData)

responseData := DomainDsRecord{}

endpoint := fmt.Sprintf("/domain/%s/dsRecord/%d", url.PathEscape(domainName), dsRecordId)

if err := config.OVHClient.Get(endpoint, &responseData); err != nil {

return helpers.CheckDeleted(resourceData, err, endpoint)

}

domainDsRecords.DsRecords = append(domainDsRecords.DsRecords, responseData)

amstuta · 2025-02-12T08:16:05Z

ovh/resource_domain_ds_records.go

+func resourceDomainDsRecordsUpdate(resourceData *schema.ResourceData, meta interface{}) error {
+	config := meta.(*Config)
+	domainName := resourceData.Get("domain").(string)
+	task := &DomainTask{}


Suggested change

task := &DomainTask{}

task := DomainTask{}

amstuta · 2025-02-12T08:31:48Z

ovh/resource_domain_name_servers.go

+	}
+
+	for _, nameServerId := range *responseData {
+		responseData := &DomainNameServer{}


Suggested change

responseData := &DomainNameServer{}

responseData := DomainNameServer{}

amstuta · 2025-02-12T08:35:15Z

ovh/resource_domain_name_servers.go

+		domainNameServers.Servers = append(domainNameServers.Servers, *responseData)
+	}
+
+	sort.Slice(domainNameServers.Servers, func(i, j int) bool {


not sure why we want to sort this array ? I think the field servers could be a TypeSet so we could avoid this sort

amstuta · 2025-02-12T08:35:43Z

ovh/resource_domain_name_servers.go

+func resourceDomainNameServersUpdate(resourceData *schema.ResourceData, meta interface{}) error {
+	config := meta.(*Config)
+	domainName := resourceData.Get("domain").(string)
+	task := &DomainTask{}


Suggested change

task := &DomainTask{}

task := DomainTask{}

amstuta · 2025-02-12T08:53:34Z

ovh/resource_domain_name_servers_test.go

+
+		provider := testAccProvider.Meta().(*Config)
+
+		responseData := &[]int{}


Suggested change

responseData := &[]int{}

var responseData []int

amstuta · 2025-02-12T08:53:50Z

ovh/resource_domain_name_servers_test.go

+		var domainNameServerList []DomainNameServer
+
+		for _, nameServerId := range *responseData {
+			responseData := &DomainNameServer{}


Suggested change

responseData := &DomainNameServer{}

responseData := DomainNameServer{}

App13Pie added 2 commits February 2, 2025 15:24

feat: add ds records and name servers resources for domains

6c1b61f

chore: added documentation for ds records and name servers

a038639

App13Pie marked this pull request as ready for review February 11, 2025 22:43

App13Pie requested a review from a team as a code owner February 11, 2025 22:43

amstuta reviewed Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add ds records and name servers resources for domains #818

feat: add ds records and name servers resources for domains #818

App13Pie commented Feb 2, 2025 •

edited

Loading

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025

amstuta Feb 12, 2025


		log.Printf("[DEBUG] Will read domain name DS records: %s\n", domainName)

		responseData := &[]int{}

	responseData := &DomainNameServer{}
	responseData := DomainNameServer{}


		provider := testAccProvider.Meta().(*Config)

		responseData := &[]int{}

feat: add ds records and name servers resources for domains #818

Are you sure you want to change the base?

feat: add ds records and name servers resources for domains #818

Conversation

App13Pie commented Feb 2, 2025 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

App13Pie commented Feb 2, 2025 •

edited

Loading