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-156] Add warnings for untracked hook secrets #189

Merged
merged 3 commits into from
Jun 16, 2022
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
4 changes: 2 additions & 2 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ indent_style = tab
[*.md]
trim_trailing_whitespace = false

[*.{yml, yaml, raml, yml.dist, feature, toml}]
[*.{yml, yaml, raml, yml.dist, feature, toml, tf}]
indent_size = 2

[GNUmakefile]
[Makefile]
indent_style = tab
99 changes: 67 additions & 32 deletions auth0/resource_auth0_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package auth0

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

"github.com/auth0/go-auth0"
"github.com/auth0/go-auth0/management"
"github.com/hashicorp/go-cty/cty"
"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 @@ -58,9 +59,10 @@ func newHook() *schema.Resource {
},
"secrets": {
Type: schema.TypeMap,
Elem: schema.TypeString,
Sensitive: true,
Optional: true,
Description: "The secrets associated with the hook",
Elem: schema.TypeString,
},
"enabled": {
Type: schema.TypeBool,
Expand All @@ -73,15 +75,15 @@ func newHook() *schema.Resource {
}

func createHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
hook := buildHook(d)
hook := expandHook(d)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming this so we stay consistent with the "expand" and "flatten" nomenclature.

api := m.(*management.Management)
if err := api.Hook.Create(hook); err != nil {
return diag.FromErr(err)
}

d.SetId(auth0.StringValue(hook.ID))
d.SetId(hook.GetID())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be a need anywhere in the codebase to use the auth0 pointer to value helper funcs, as we have accessor methods on pointer fields from the go-auth0 SDK.


if err := upsertHookSecrets(d, m); err != nil {
if err := upsertHookSecrets(ctx, d, m); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bringing the signature of this func in line with all the others, making sure we pass context first.

We should evolve go-auth0 in the future to accept the context, for higher control over the request timeouts.

return diag.FromErr(err)
}

Expand All @@ -101,6 +103,8 @@ func readHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
return diag.FromErr(err)
}

diagnostics := checkForUntrackedHookSecrets(ctx, d, m)

result := multierror.Append(
d.Set("name", hook.Name),
d.Set("dependencies", hook.Dependencies),
Expand All @@ -109,44 +113,27 @@ func readHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
d.Set("enabled", hook.Enabled),
)

return diag.FromErr(result.ErrorOrNil())
if err = result.ErrorOrNil(); err != nil {
diagnostics = append(diagnostics, diag.FromErr(err)...)
}

return diagnostics
}

func updateHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
hook := buildHook(d)
hook := expandHook(d)
api := m.(*management.Management)
if err := api.Hook.Update(d.Id(), hook); err != nil {
return diag.FromErr(err)
}

if err := upsertHookSecrets(d, m); err != nil {
if err := upsertHookSecrets(ctx, d, m); err != nil {
return diag.FromErr(err)
}

return readHook(ctx, d, m)
}

func upsertHookSecrets(d *schema.ResourceData, m interface{}) error {
if d.IsNewResource() || d.HasChange("secrets") {
secrets := Map(d, "secrets")
api := m.(*management.Management)
hookSecrets := toHookSecrets(secrets)
return api.Hook.ReplaceSecrets(d.Id(), hookSecrets)
}

return nil
}

func toHookSecrets(val map[string]interface{}) management.HookSecrets {
hookSecrets := management.HookSecrets{}
for key, value := range val {
if strVal, ok := value.(string); ok {
hookSecrets[key] = strVal
}
}
return hookSecrets
}

func deleteHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
api := m.(*management.Management)
if err := api.Hook.Delete(d.Id()); err != nil {
Expand All @@ -162,22 +149,70 @@ func deleteHook(ctx context.Context, d *schema.ResourceData, m interface{}) diag
return nil
}

func buildHook(d *schema.ResourceData) *management.Hook {
func checkForUntrackedHookSecrets(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
secretsFromConfig := Map(d, "secrets")

api := m.(*management.Management)
secretsFromAPI, err := api.Hook.Secrets(d.Id())
if err != nil {
return diag.FromErr(err)
}

var warnings diag.Diagnostics
for key := range secretsFromAPI {
if _, ok := secretsFromConfig[key]; !ok {
warnings = append(warnings, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unexpected Hook Secrets",
Detail: fmt.Sprintf("Found unexpected hook secrets with key: %s. ", key) +
"To prevent issues, manage them through terraform. If you've just imported this resource " +
"(and your secrets match), to make this warning disappear, run a terraform apply.",
AttributePath: cty.Path{cty.GetAttrStep{Name: "secrets"}},
})
}
}
Comment on lines +161 to +173
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Secrets are not part of the response of the GET Hook details. To fetch them we need to use the following endpoint https://auth0.com/docs/api/management/v2#!/Hooks/get_secrets. However the actual values are never shown, only the presence of certain keys. To aid our users, this will now throw warnings in the terminal with suggestions on what secrets to start tracking. e.g.

╷
│ Warning: Unexpected Hook Secrets
│
│   with auth0_hook.my_hook,
│   on main.tf line 22, in resource "auth0_hook" "my_hook":
│   22:   secrets = {
│   23:     foo = "foo"
│   24:     car = "car"
│   25:   }
│
│ Found unexpected hook secrets with key: test. To prevent issues, manage them through terraform. If you've just imported this
│ resource (and your secrets match), to make this warning disappear, run a terraform apply.


return warnings
}

func upsertHookSecrets(ctx context.Context, d *schema.ResourceData, m interface{}) error {
if d.IsNewResource() || d.HasChange("secrets") {
hookSecrets := expandHookSecrets(d)
api := m.(*management.Management)
return api.Hook.ReplaceSecrets(d.Id(), hookSecrets)
}

return nil
}

func expandHook(d *schema.ResourceData) *management.Hook {
hook := &management.Hook{
Name: String(d, "name"),
Script: String(d, "script"),
TriggerID: String(d, "trigger_id", IsNewResource()),
Enabled: Bool(d, "enabled"),
}

deps := Map(d, "dependencies")
if deps != nil {
if deps := Map(d, "dependencies"); deps != nil {
hook.Dependencies = &deps
}

return hook
}

func expandHookSecrets(d *schema.ResourceData) management.HookSecrets {
hookSecrets := management.HookSecrets{}
secrets := Map(d, "secrets")

for key, value := range secrets {
if strVal, ok := value.(string); ok {
hookSecrets[key] = strVal
}
}

return hookSecrets
}

func validateHookName() schema.SchemaValidateDiagFunc {
hookNameValidation := validation.StringMatch(
regexp.MustCompile(`^[^\s-][\w -]+[^\s-]$`),
Expand Down
76 changes: 32 additions & 44 deletions auth0/resource_auth0_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/stretchr/testify/assert"
)

func TestAccHook(t *testing.T) {
Expand All @@ -15,7 +16,7 @@ func TestAccHook(t *testing.T) {
ProviderFactories: testProviders(httpRecorder),
Steps: []resource.TestStep{
{
Config: testAccHookCreate,
Config: fmt.Sprintf(testAccHookCreate, ""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_hook.my_hook", "name", "pre-user-reg-hook"),
resource.TestCheckResourceAttr("auth0_hook.my_hook", "script", "function (user, context, callback) { callback(null, { user }); }"),
Expand All @@ -33,6 +34,7 @@ resource "auth0_hook" "my_hook" {
script = "function (user, context, callback) { callback(null, { user }); }"
trigger_id = "pre-user-registration"
enabled = true
%s
}
`

Expand All @@ -43,7 +45,7 @@ func TestAccHookSecrets(t *testing.T) {
ProviderFactories: testProviders(httpRecorder),
Steps: []resource.TestStep{
{
Config: testAccHookSecrets("alpha"),
Config: fmt.Sprintf(testAccHookCreate, testAccHookSecrets),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_hook.my_hook", "name", "pre-user-reg-hook"),
resource.TestCheckResourceAttr("auth0_hook.my_hook", "dependencies.auth0", "2.30.0"),
Expand All @@ -55,7 +57,7 @@ func TestAccHookSecrets(t *testing.T) {
),
},
{
Config: testAccHookSecrets2("gamma", "kappa"),
Config: fmt.Sprintf(testAccHookCreate, testAccHookSecretsUpdate),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_hook.my_hook", "name", "pre-user-reg-hook"),
resource.TestCheckResourceAttr("auth0_hook.my_hook", "dependencies.auth0", "2.30.0"),
Expand All @@ -67,7 +69,7 @@ func TestAccHookSecrets(t *testing.T) {
),
},
{
Config: testAccHookSecrets("delta"),
Config: fmt.Sprintf(testAccHookCreate, testAccHookSecretsUpdateAndRemoval),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_hook.my_hook", "name", "pre-user-reg-hook"),
resource.TestCheckResourceAttr("auth0_hook.my_hook", "script", "function (user, context, callback) { callback(null, { user }); }"),
Expand All @@ -81,58 +83,44 @@ func TestAccHookSecrets(t *testing.T) {
})
}

func testAccHookSecrets(fooValue string) string {
return fmt.Sprintf(`
resource "auth0_hook" "my_hook" {
name = "pre-user-reg-hook"
script = "function (user, context, callback) { callback(null, { user }); }"
trigger_id = "pre-user-registration"
enabled = true
const testAccHookSecrets = `
dependencies = {
auth0 = "2.30.0"
}
secrets = {
foo = "%s"
secrets = {
foo = "alpha"
}
}
`, fooValue)
}
`

func testAccHookSecrets2(fooValue string, barValue string) string {
return fmt.Sprintf(`
resource "auth0_hook" "my_hook" {
name = "pre-user-reg-hook"
script = "function (user, context, callback) { callback(null, { user }); }"
trigger_id = "pre-user-registration"
const testAccHookSecretsUpdate = `
dependencies = {
auth0 = "2.30.0"
}
enabled = true
secrets = {
foo = "%s"
bar = "%s"
foo = "gamma"
bar = "kappa"
}
}
`, fooValue, barValue)
}
`

const testAccHookSecretsUpdateAndRemoval = `
dependencies = {
auth0 = "2.30.0"
}
secrets = {
foo = "delta"
}
`

func TestHookNameRegexp(t *testing.T) {
for name, valid := range map[string]bool{
"my-hook-1": true,
"hook 2 name with spaces": true,
" hook with a space prefix": false,
"hook with a space suffix ": false,
" ": false,
" ": false,
for givenHookName, expectedError := range map[string]bool{
"my-hook-1": false,
"hook 2 name with spaces": false,
" hook with a space prefix": true,
"hook with a space suffix ": true,
" ": true,
" ": true,
} {
fn := validateHookName()

validationResult := fn(name, cty.Path{cty.GetAttrStep{Name: "name"}})
if validationResult.HasError() && valid {
t.Fatalf("Expected %q to be valid, but got validation errors %v", name, validationResult)
}
if !validationResult.HasError() && !valid {
t.Fatalf("Expected %q to be invalid, but got no validation errors.", name)
}
validationResult := validateHookName()(givenHookName, cty.Path{cty.GetAttrStep{Name: "name"}})
assert.Equal(t, expectedError, validationResult.HasError())
}
}
Loading