Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

fix: remove stale resources from state #91

Merged
kimdre merged 9 commits into from
Jul 12, 2024
Merged

fix: remove stale resources from state #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
34f66ee
fix: remove staled resources from state
kimdre Jul 12, 2024
1f20063
refactor: remove warning for stale/non-existent resources
kimdre Jul 12, 2024
a4042ed
test: add acc tests for stale resources
kimdre Jul 12, 2024
0bb483f
make fmt
jkroepke Jul 12, 2024
7a347b7
add error handling
jkroepke Jul 12, 2024
a515390
fix: return error if primary server not found
kimdre Jul 12, 2024
caffb81
refactor: adjust context
kimdre Jul 12, 2024
754387a
test: fix primary server stale test
kimdre Jul 12, 2024
c9dc6fb
fix: fix error
kimdre Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion internal/api/primary_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (c *Client) GetPrimaryServer(ctx context.Context, id string) (*PrimaryServe

switch resp.StatusCode {
case http.StatusNotFound:
return nil, nil
return nil, fmt.Errorf("primary server %s not found", id)
case http.StatusOK:
var response *PrimaryServerResponse

Expand All @@ -50,6 +50,27 @@ func (c *Client) GetPrimaryServer(ctx context.Context, id string) (*PrimaryServe
}
}

func (c *Client) GetPrimaryServers(ctx context.Context, zoneID string) ([]PrimaryServer, error) {
resp, err := c.request(ctx, http.MethodGet, "/api/v1/primary_servers?zone_id="+zoneID, nil)
if err != nil {
return nil, fmt.Errorf("error getting primary servers for zone %s: %w", zoneID, err)
}

switch resp.StatusCode {
case http.StatusOK:
var response PrimaryServersResponse

err = readAndParseJSONBody(resp, &response)
if err != nil {
return nil, err
}

return response.PrimaryServers, nil
default:
return nil, fmt.Errorf("http status %d unhandled", resp.StatusCode)
}
}

func (c *Client) CreatePrimaryServer(ctx context.Context, server CreatePrimaryServerRequest) (*PrimaryServer, error) {
resp, err := c.request(ctx, http.MethodPost, "/api/v1/primary_servers", server)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/provider/primary_server_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ func (r *primaryServerResource) Read(ctx context.Context, req resource.ReadReque

return nil
})
if err != nil {
if err != nil && fmt.Sprint(err) != fmt.Sprintf("primary server %s not found", state.ID.ValueString()) {
Copy link
Copy Markdown
Member

@jkroepke jkroepke Jul 12, 2024

Choose a reason for hiding this comment

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

@kimdre das ist echt schlechtes go :D

besser wäre es, im api packages

var ErrNotFound = errors.New("not found")

dann beim error

return nil, fmt.Errorf("primary server %s: %w", id, ErrNotFound)

und hier in der condition:

if err != nil && !errors.Is(err, api.ErrNotFound))

Copy link
Copy Markdown
Member

@jkroepke jkroepke Jul 12, 2024

Choose a reason for hiding this comment

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

undd as ganze auch für zones und records :D

Copy link
Copy Markdown
Member Author

@kimdre kimdre Jul 12, 2024

Choose a reason for hiding this comment

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

Jaja hatte da keine Zeit mehr für ne schöne Lösung 😂
#92

resp.Diagnostics.AddError("API Error", fmt.Sprintf("read primary server: %s", err))

return
}

if server == nil {
resp.Diagnostics.AddWarning("Resource Not Found", fmt.Sprintf("Primary server with id %s doesn't exist, removing it from state", state.ID))
resp.State.RemoveResource(ctx)

return
}
Expand Down
96 changes: 96 additions & 0 deletions internal/provider/primary_server_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package provider

import (
"context"
"fmt"
"net/http"
"regexp"
"strconv"
"strings"
"testing"

"github.com/germanbrew/terraform-provider-hetznerdns/internal/api"
"github.com/germanbrew/terraform-provider-hetznerdns/internal/utils"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
)
Expand Down Expand Up @@ -118,6 +123,97 @@ func TestAccPrimaryServer_TwoPrimaryServersResources(t *testing.T) {
})
}

func TestAccPrimaryServer_StalePrimaryServersResources(t *testing.T) {
aZoneName := acctest.RandString(10) + ".online"
aZoneTTL := 3600

psAddress := "1.1.0.0"
psPort := 53

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
// Create and Read testing
{
Config: strings.Join(
[]string{
testAccZoneResourceConfig("test", aZoneName, aZoneTTL),
testAccPrimaryServerResourceConfigCreate("test", psAddress, psPort),
}, "\n",
),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttrSet("hetznerdns_primary_server.test", "id"),
resource.TestCheckResourceAttr("hetznerdns_primary_server.test", "address", psAddress),
resource.TestCheckResourceAttr("hetznerdns_primary_server.test", "port", strconv.Itoa(psPort)),
),
},
// ImportState testing
{
ResourceName: "hetznerdns_primary_server.test",
ImportState: true,
ImportStateVerify: true,
},
// Update and Read testing
{
Config: strings.Join(
[]string{
testAccZoneResourceConfig("test", aZoneName, aZoneTTL),
testAccPrimaryServerResourceConfigCreate("test", psAddress, psPort*2),
}, "\n",
),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("hetznerdns_primary_server.test", "port", strconv.Itoa(psPort*2)),
),
},
// Remove primary server from Hetzner DNS and check if it will be recreated by Terraform
{
PreConfig: func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var (
data hetznerDNSProviderModel
apiToken string
apiClient *api.Client
err error
)

apiToken = utils.ConfigureStringAttribute(data.ApiToken, "HETZNER_DNS_API_TOKEN", "")
httpClient := logging.NewLoggingHTTPTransport(http.DefaultTransport)
apiClient, err = api.New("https://dns.hetzner.com", apiToken, httpClient)
if err != nil {
t.Fatalf("Error while creating API apiClient: %s", err)
}
zone, err := apiClient.GetZoneByName(ctx, aZoneName)
if err != nil {
t.Fatalf("Error while fetching zone: %s", err)
} else if zone == nil {
t.Fatalf("Zone %s not found", aZoneName)
}

primaryServer, err := apiClient.GetPrimaryServers(ctx, zone.ID)
if err != nil {
t.Fatalf("Error while fetching primary server: %s", err)
} else if primaryServer == nil {
t.Fatalf("Primary server %s not found", zone.ID)
}

err = apiClient.DeletePrimaryServer(ctx, primaryServer[0].ID)
if err != nil {
t.Fatalf("Error while deleting primary server: %s", err)
}
},
// Check if the record is recreated
// ExpectNonEmptyPlan: true,
RefreshState: true,
ExpectError: regexp.MustCompile("hetznerdns_primary_server.test will be created"),
},
// Delete testing automatically occurs in TestCase
},
})
}

func testAccPrimaryServerResourceConfigCreate(resourceName, psAddress string, psPort int) string {
return fmt.Sprintf(`
resource "hetznerdns_primary_server" "%s" {
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/record_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (r *recordResource) Read(ctx context.Context, req resource.ReadRequest, res
}

if record == nil {
resp.Diagnostics.AddWarning("Resource Not Found", fmt.Sprintf("DNS record with id %s doesn't exist, removing it from state", state.ID))
resp.State.RemoveResource(ctx)

return
}
Expand Down
100 changes: 100 additions & 0 deletions internal/provider/record_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package provider

import (
"context"
"fmt"
"net/http"
"regexp"
"strconv"
"strings"
"testing"

"github.com/germanbrew/terraform-provider-hetznerdns/internal/api"
"github.com/germanbrew/terraform-provider-hetznerdns/internal/utils"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
)
Expand Down Expand Up @@ -234,6 +239,101 @@ func TestAccRecord_ResourcesDKIM(t *testing.T) {
})
}

func TestAccRecord_StaleResources(t *testing.T) {
zoneName := acctest.RandString(10) + ".online"
aZoneTTL := 60

value := "192.168.1.1"
aName := acctest.RandString(10)
aType := "A"
ttl := aZoneTTL * 2

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
// Create and Read testing
{
Config: strings.Join(
[]string{
testAccZoneResourceConfig("test", zoneName, aZoneTTL),
testAccRecordResourceConfigWithTTL("record1", aName, aType, value, ttl),
}, "\n",
),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet(
"hetznerdns_record.record1", "id"),
resource.TestCheckResourceAttr(
"hetznerdns_record.record1", "type", aType),
resource.TestCheckResourceAttr(
"hetznerdns_record.record1", "name", aName),
resource.TestCheckResourceAttr(
"hetznerdns_record.record1", "value", value),
resource.TestCheckResourceAttr(
"hetznerdns_record.record1", "ttl", strconv.Itoa(ttl)),
),
},
// ImportState testing
{
ResourceName: "hetznerdns_record.record1",
ImportState: true,
ImportStateVerify: true,
},
// Update and Read testing
{
Config: strings.Join(
[]string{
testAccZoneResourceConfig("test", zoneName, aZoneTTL),
testAccRecordResourceConfigWithTTL("record1", aName, aType, value, ttl*2),
}, "\n",
),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"hetznerdns_record.record1", "ttl", strconv.Itoa(ttl*2)),
),
},
// Remove record from Hetzner DNS and check if it will be recreated by Terraform
{
PreConfig: func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var (
data hetznerDNSProviderModel
apiToken string
apiClient *api.Client
err error
)

apiToken = utils.ConfigureStringAttribute(data.ApiToken, "HETZNER_DNS_API_TOKEN", "")
httpClient := logging.NewLoggingHTTPTransport(http.DefaultTransport)
apiClient, err = api.New("https://dns.hetzner.com", apiToken, httpClient)
if err != nil {
t.Fatalf("Error while creating API apiClient: %s", err)
}
zone, err := apiClient.GetZoneByName(ctx, zoneName)
if err != nil {
t.Fatalf("Error while fetching zone: %s", err)
}
record, err := apiClient.GetRecordByName(ctx, zone.ID, aName)
if err != nil {
t.Fatalf("Error while fetching record: %s", err)
}
err = apiClient.DeleteRecord(ctx, record.ID)
if err != nil {
t.Fatalf("Error while deleting record: %s", err)
}
},
// Check if the record is recreated
// ExpectNonEmptyPlan: true,
RefreshState: true,
ExpectError: regexp.MustCompile("hetznerdns_record.record1 will be created"),
},
// Delete testing automatically occurs in TestCase
},
})
}

func testAccRecordResourceConfigWithTTL(resourceName, name, recordType, value string, ttl int) string {
return fmt.Sprintf(`
resource "hetznerdns_record" "%s" {
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/zone_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (r *zoneResource) Read(ctx context.Context, req resource.ReadRequest, resp
}

if zone == nil {
resp.Diagnostics.AddWarning("Resource Not Found", fmt.Sprintf("DNS zone with id %s doesn't exist, removing it from state", state.ID))
resp.State.RemoveResource(ctx)

return
}
Expand Down
67 changes: 67 additions & 0 deletions internal/provider/zone_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package provider

import (
"context"
"fmt"
"net/http"
"regexp"
"strconv"
"testing"

"github.com/germanbrew/terraform-provider-hetznerdns/internal/api"
"github.com/germanbrew/terraform-provider-hetznerdns/internal/utils"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
)
Expand Down Expand Up @@ -117,6 +122,68 @@ func TestAccZone_ZoneExists(t *testing.T) {
})
}

func TestAccZone_StaleZone(t *testing.T) {
aZoneName := acctest.RandString(10) + ".online"
aZoneTTL := 60

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
// Create and Read testing
{
Config: testAccZoneResourceConfig("test", aZoneName, aZoneTTL),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttrSet("hetznerdns_zone.test", "id"),
resource.TestCheckResourceAttr("hetznerdns_zone.test", "name", aZoneName),
resource.TestCheckResourceAttr("hetznerdns_zone.test", "ttl", strconv.Itoa(aZoneTTL)),
resource.TestCheckResourceAttrSet("hetznerdns_zone.test", "ns.#"),
),
},
// ImportState testing
{
ResourceName: "hetznerdns_zone.test",
ImportState: true,
ImportStateVerify: true,
},
// Remove zone from Hetzner DNS and check if it will be recreated by Terraform
{
PreConfig: func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var (
data hetznerDNSProviderModel
apiToken string
apiClient *api.Client
err error
)

apiToken = utils.ConfigureStringAttribute(data.ApiToken, "HETZNER_DNS_API_TOKEN", "")
httpClient := logging.NewLoggingHTTPTransport(http.DefaultTransport)
apiClient, err = api.New("https://dns.hetzner.com", apiToken, httpClient)
if err != nil {
t.Fatalf("Error while creating API apiClient: %s", err)
}
zone, err := apiClient.GetZoneByName(ctx, aZoneName)
if err != nil {
t.Fatalf("Error while fetching zone: %s", err)
}
err = apiClient.DeleteZone(ctx, zone.ID)
if err != nil {
t.Fatalf("Error while deleting zone: %s", err)
}
},
// Check if the zone is recreated
// ExpectNonEmptyPlan: true,
RefreshState: true,
ExpectError: regexp.MustCompile("hetznerdns_zone.test will be created"),
},
// Delete testing automatically occurs in TestCase
},
})
}

func testAccZoneResourceConfig(resourceName string, name string, ttl int) string {
return fmt.Sprintf(`
resource "hetznerdns_zone" "%[1]s" {
Expand Down