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

DXCDT-458: Handling 404 errors in data sources #698

Merged
merged 7 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions internal/auth0/404check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package auth0

import (
"context"
"errors"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

// CheckFor404Error accepts and executes a resource's read function within the context
// of a data source and will appropriately error when a resource is not found.
func CheckFor404Error(ctx context.Context, readFunc func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics, d *schema.ResourceData, m interface{}) diag.Diagnostics {
diags := readFunc(ctx, d, m)
if diags != nil {
return diags
}
if d.Id() == "" {
return diag.FromErr(errors.New("no resource found with that identifier (404)"))
}
return nil
}
Comment on lines +13 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aside from abstracting-out each read function and handling the errors within the respective instances (resource vs data source), detecting an empty ID is the simplest solution, albeit not the most elegant.

This is mostly just a proposal of what we could do but at this point I don't think doing a wider refactor would be that much more effort and establishes a better code pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is already a solid n+1, we can refactor further after everything's in v1 and we have time. 👍🏻

5 changes: 3 additions & 2 deletions internal/auth0/client/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

auth0 "github.com/auth0/terraform-provider-auth0/internal/auth0"
"github.com/auth0/terraform-provider-auth0/internal/config"
internalSchema "github.com/auth0/terraform-provider-auth0/internal/schema"
)
Expand Down Expand Up @@ -37,7 +38,7 @@ func readClientForDataSource(ctx context.Context, d *schema.ResourceData, m inte
clientID := d.Get("client_id").(string)
if clientID != "" {
d.SetId(clientID)
return readClient(ctx, d, m)
return auth0.CheckFor404Error(ctx, readClient, d, m)
}

name := d.Get("name").(string)
Expand All @@ -61,7 +62,7 @@ func readClientForDataSource(ctx context.Context, d *schema.ResourceData, m inte
for _, client := range clients.Clients {
if client.GetName() == name {
d.SetId(client.GetClientID())
return readClient(ctx, d, m)
return auth0.CheckFor404Error(ctx, readClient, d, m)
}
}

Expand Down
20 changes: 20 additions & 0 deletions internal/auth0/client/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client_test

import (
"fmt"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand Down Expand Up @@ -64,3 +65,22 @@ func TestAccDataClientById(t *testing.T) {
},
})
}

const testAccDataSourceClientNonexistentID = `
data "auth0_client" "test" {
client_id = "this-client-does-not-exist"
}
`

func TestAccDataSourceClientNonexistentID(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataSourceClientNonexistentID, t.Name()),
ExpectError: regexp.MustCompile(
`no resource found with that identifier \((404\))`,
),
},
},
})
}
5 changes: 3 additions & 2 deletions internal/auth0/connection/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/auth0/terraform-provider-auth0/internal/auth0"
"github.com/auth0/terraform-provider-auth0/internal/config"
internalSchema "github.com/auth0/terraform-provider-auth0/internal/schema"
)
Expand Down Expand Up @@ -40,7 +41,7 @@ func readConnectionForDataSource(ctx context.Context, data *schema.ResourceData,
connectionID := data.Get("connection_id").(string)
if connectionID != "" {
data.SetId(connectionID)
return readConnection(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readConnection, data, meta)
}

api := meta.(*config.Config).GetAPI()
Expand All @@ -59,7 +60,7 @@ func readConnectionForDataSource(ctx context.Context, data *schema.ResourceData,
for _, connection := range connections.Connections {
if connection.GetName() == name {
data.SetId(connection.GetID())
return readConnection(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readConnection, data, meta)
}
}

Expand Down
19 changes: 19 additions & 0 deletions internal/auth0/connection/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,22 @@ func TestAccDataSourceConnectionByID(t *testing.T) {
},
})
}

const testAccDataConnectionNonexistentID = `
data "auth0_connection" "test" {
connection_id = "con_xxxxxxxxxxxxxxxx"
}
`

func TestAccDataSourceConnectionNonexistentID(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataConnectionNonexistentID, t.Name()),
ExpectError: regexp.MustCompile(
`no resource found with that identifier \((404\))`,
),
},
},
})
}
5 changes: 0 additions & 5 deletions internal/auth0/organization/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package organization

import (
"context"
"net/http"

"github.com/auth0/go-auth0/management"
"github.com/hashicorp/go-multierror"
Expand Down Expand Up @@ -79,10 +78,6 @@ func readOrganizationForDataSource(ctx context.Context, data *schema.ResourceDat
if organizationID != "" {
foundOrganization, err = api.Organization.Read(ctx, organizationID)
if err != nil {
if mErr, ok := err.(management.Error); ok && mErr.Status() == http.StatusNotFound {
data.SetId("")
return nil
}
Comment on lines -82 to -85
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted because we want to handle 404s in the data source.

return diag.FromErr(err)
}
} else {
Expand Down
19 changes: 19 additions & 0 deletions internal/auth0/organization/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,22 @@ func TestAccDataSourceOrganizationByID(t *testing.T) {
},
})
}

const testAccDataSourceOrganizationNonexistentID = testAccGivenAnOrganizationWithConnectionsAndMembers + `
data "auth0_organization" "test" {
organization_id = "org_XXXXXXXXXXXXXXXX"
}
`

func TestAccDataSourceOrganizationNonexistentID(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataSourceOrganizationNonexistentID, t.Name()),
ExpectError: regexp.MustCompile(
"404 Not Found: No organization found by that id or name",
),
},
},
})
}
6 changes: 0 additions & 6 deletions internal/auth0/resourceserver/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package resourceserver

import (
"context"
"net/http"
"net/url"

"github.com/auth0/go-auth0/management"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -49,10 +47,6 @@ func readResourceServerForDataSource(ctx context.Context, data *schema.ResourceD
api := meta.(*config.Config).GetAPI()
resourceServer, err := api.ResourceServer.Read(ctx, resourceServerID)
if err != nil {
if mErr, ok := err.(management.Error); ok && mErr.Status() == http.StatusNotFound {
data.SetId("")
return nil
}
Comment on lines -52 to -55
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted because we want to handle 404s in the data source. This fixes #604 .

return diag.FromErr(err)
}

Expand Down
21 changes: 21 additions & 0 deletions internal/auth0/resourceserver/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,24 @@ func TestAccDataResourceServerAuth0APIManagement(t *testing.T) {
},
})
}

const testAccDataResourceServerNonexistentIdentifier = testAccGivenAResourceServer + `
data "auth0_resource_server" "test" {
depends_on = [ auth0_resource_server.my_api ]

identifier = "this-resource-server-does-not-exist"
}
`

func TestAccDataSourceResourceServerNonexistentIdentifier(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataResourceServerNonexistentIdentifier, t.Name()),
ExpectError: regexp.MustCompile(
`404 Not Found: The resource server does not exist`,
),
},
},
})
}
5 changes: 3 additions & 2 deletions internal/auth0/role/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/auth0/terraform-provider-auth0/internal/auth0"
"github.com/auth0/terraform-provider-auth0/internal/config"
internalSchema "github.com/auth0/terraform-provider-auth0/internal/schema"
)
Expand Down Expand Up @@ -40,7 +41,7 @@ func readRoleForDataSource(ctx context.Context, data *schema.ResourceData, meta
roleID := data.Get("role_id").(string)
if roleID != "" {
data.SetId(roleID)
return readRole(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readRole, data, meta)
}

api := meta.(*config.Config).GetAPI()
Expand All @@ -60,7 +61,7 @@ func readRoleForDataSource(ctx context.Context, data *schema.ResourceData, meta
for _, role := range roles.Roles {
if role.GetName() == name {
data.SetId(role.GetID())
return readRole(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readRole, data, meta)
}
}

Expand Down
20 changes: 20 additions & 0 deletions internal/auth0/role/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,23 @@ func TestAccDataSourceRoleByID(t *testing.T) {
},
})
}

const testAccDataSourceNonexistentRole = testAccGivenAResourceServer + `
data "auth0_role" "test" {
name = "this-role-does-not-exist"
}
`

func TestAccDataSourceRoleDoesNotExist(t *testing.T) {
acctest.Test(t, resource.TestCase{
PreventPostDestroyRefresh: true,
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataSourceNonexistentRole, t.Name()),
ExpectError: regexp.MustCompile(
`No role found with "name" = "this-role-does-not-exist"`,
),
},
},
})
}
3 changes: 2 additions & 1 deletion internal/auth0/tenant/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/auth0/terraform-provider-auth0/internal/auth0"
"github.com/auth0/terraform-provider-auth0/internal/config"
internalSchema "github.com/auth0/terraform-provider-auth0/internal/schema"
)
Expand Down Expand Up @@ -59,5 +60,5 @@ func readTenantForDataSource(ctx context.Context, data *schema.ResourceData, met
return diag.FromErr(result.ErrorOrNil())
}

return readTenant(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readTenant, data, meta)
}
3 changes: 2 additions & 1 deletion internal/auth0/user/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/auth0/terraform-provider-auth0/internal/auth0"
internalSchema "github.com/auth0/terraform-provider-auth0/internal/schema"
)

Expand All @@ -32,5 +33,5 @@ func dataSourceSchema() map[string]*schema.Schema {
func readUserForDataSource(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics {
userID := data.Get("user_id").(string)
data.SetId(userID)
return readUser(ctx, data, meta)
return auth0.CheckFor404Error(ctx, readUser, data, meta)
}
22 changes: 22 additions & 0 deletions internal/auth0/user/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package user_test

import (
"fmt"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -80,3 +81,24 @@ func TestAccDataSourceUser(t *testing.T) {
},
})
}

const testAccDataSourceUserDoesNotExist = `
data "auth0_user" "test" {
user_id = "auth0|this-user-id-does-not-exist"
}
`

func TestAccDataSourceUserDoesNotExist(t *testing.T) {
testName := strings.ToLower(t.Name())

acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: acctest.ParseTestName(testAccDataSourceUserDoesNotExist, testName),
ExpectError: regexp.MustCompile(
`no resource found with that identifier \((404\))`,
),
},
},
})
}
39 changes: 39 additions & 0 deletions test/data/recordings/TestAccDataSourceClientNonexistentID.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
version: 2
interactions:
- id: 0
request:
proto: HTTP/1.1
proto_major: 1
proto_minor: 1
content_length: 5
transfer_encoding: []
trailer: {}
host: terraform-provider-auth0-dev.eu.auth0.com
remote_addr: ""
request_uri: ""
body: |
null
form: {}
headers:
Content-Type:
- application/json
User-Agent:
- Go-Auth0/1.0.0-beta.0
url: https://terraform-provider-auth0-dev.eu.auth0.com/api/v2/clients/this-client-does-not-exist
method: GET
response:
proto: HTTP/2.0
proto_major: 2
proto_minor: 0
transfer_encoding: []
trailer: {}
content_length: -1
uncompressed: true
body: '{"signing_keys":[{"cert":"[REDACTED]"}]}'
headers:
Content-Type:
- application/json; charset=utf-8
status: 404 Not Found
code: 404
duration: 138.304ms
Loading