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

Throw error when encountering untracked action secrets #248

Merged
merged 4 commits into from
Jul 27, 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
55 changes: 55 additions & 0 deletions auth0/resource_auth0_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"

"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/resource"
Expand Down Expand Up @@ -160,6 +161,7 @@ func readAction(ctx context.Context, d *schema.ResourceData, m interface{}) diag
d.Set("dependencies", flattenActionDependencies(action.Dependencies)),
d.Set("runtime", action.Runtime),
)

if action.DeployedVersion != nil {
result = multierror.Append(result, d.Set("version_id", action.DeployedVersion.GetID()))
}
Expand All @@ -171,6 +173,12 @@ func updateAction(ctx context.Context, d *schema.ResourceData, m interface{}) di
action := expandAction(d)

api := m.(*management.Management)

diagnostics := preventErasingUnmanagedSecrets(d, api)
if diagnostics.HasError() {
return diagnostics
}

if err := api.Action.Update(d.Id(), action); err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -251,6 +259,53 @@ func deployAction(ctx context.Context, d *schema.ResourceData, m interface{}) di
return diag.FromErr(d.Set("version_id", actionVersion.GetID()))
}

func preventErasingUnmanagedSecrets(d *schema.ResourceData, api *management.Management) diag.Diagnostics {
if !d.HasChange("secrets") {
return nil
}

preUpdateAction, err := api.Action.Read(d.Id())
if err != nil {
return diag.FromErr(err)
}

// We need to also include the secrets that we're about to remove
// against the checks, not just the ones with which we are left.
oldSecrets, newSecrets := d.GetChange("secrets")
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "old" and "new" mean in this context? Is it the union of current, added and removed? If we're just trying to calculate all secrets that are and were managed, it might not make sense to continue to separate by "new" and "old" beyond this point because that really won't impact the logic below.

My suggestion, assuming my understanding is correct, is to consolidate these into a single slice to simplify the logic below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure that makes sense.

  • Why do we need to get the change and not just the actual current secrets?
  • If we use d.Get() we'll just get the secrets that are currently in the config, that means if we remove 1 we won't fetch it, but if we don't and the secret is actually on the action, we'll think that we're trying to wipe something that shouldn't get wiped, when actually we do want to wipe it. Hope this makes sense. Lmk if you have questions.

allSecrets := append(oldSecrets.([]interface{}), newSecrets.([]interface{})...)

return checkForUnmanagedActionSecrets(allSecrets, preUpdateAction.Secrets)
}

func checkForUnmanagedActionSecrets(
secretsFromConfig []interface{},
secretsFromAPI []*management.ActionSecret,
) diag.Diagnostics {
secretKeysInConfigMap := make(map[string]bool, len(secretsFromConfig))
for _, secret := range secretsFromConfig {
secretKeyName := secret.(map[string]interface{})["name"].(string)
secretKeysInConfigMap[secretKeyName] = true
}

var diagnostics diag.Diagnostics
for _, secret := range secretsFromAPI {
if _, ok := secretKeysInConfigMap[secret.GetName()]; !ok {
diagnostics = append(diagnostics, diag.Diagnostic{
Severity: diag.Error,
Summary: "Unmanaged Action Secret",
Detail: fmt.Sprintf("Detected an action secret not managed though Terraform: %s. If you proceed, "+
"this secret will get deleted. It is required to add this secret to your action configuration "+
"to prevent unintentionally destructive results.",
secret.GetName(),
),
AttributePath: cty.Path{cty.GetAttrStep{Name: "secrets"}},
})
}
}

return diagnostics
}

func expandAction(d *schema.ResourceData) *management.Action {
action := &management.Action{
Name: String(d, "name"),
Expand Down
90 changes: 88 additions & 2 deletions auth0/resource_auth0_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import (
"regexp"
"testing"

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

"github.com/auth0/terraform-provider-auth0/auth0/internal/template"
)
Expand All @@ -19,6 +24,11 @@ resource auth0_action my_action {
id = "post-login"
version = "v3"
}
secrets {
name = "foo"
value = "111111"
}
}
`

Expand All @@ -43,6 +53,11 @@ resource auth0_action my_action {
name = "foo"
value = "123456"
}
secrets {
name = "bar"
value = "654321"
}
}
`

Expand Down Expand Up @@ -79,7 +94,9 @@ func TestAccAction(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_action.my_action", "name", fmt.Sprintf("Test Action %s", t.Name())),
resource.TestCheckResourceAttr("auth0_action.my_action", "code", "exports.onExecutePostLogin = async (event, api) => {};"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.#", "0"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.#", "1"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.0.name", "foo"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.0.value", "111111"),
resource.TestCheckResourceAttr("auth0_action.my_action", "dependencies.#", "0"),
resource.TestCheckResourceAttr("auth0_action.my_action", "runtime", "node16"),
resource.TestCheckResourceAttr("auth0_action.my_action", "deploy", "false"),
Expand All @@ -103,9 +120,11 @@ func TestAccAction(t *testing.T) {
resource.TestCheckResourceAttr("auth0_action.my_action", "dependencies.#", "1"),
resource.TestCheckResourceAttr("auth0_action.my_action", "dependencies.0.name", "auth0"),
resource.TestCheckResourceAttr("auth0_action.my_action", "dependencies.0.version", "2.41.0"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.#", "1"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.#", "2"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.0.name", "foo"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.0.value", "123456"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.1.name", "bar"),
resource.TestCheckResourceAttr("auth0_action.my_action", "secrets.1.value", "654321"),
),
},
{
Expand Down Expand Up @@ -179,3 +198,70 @@ resource auth0_action my_action {
}
}
`

func TestCheckForUntrackedActionSecrets(t *testing.T) {
sergiught marked this conversation as resolved.
Show resolved Hide resolved
var testCases = []struct {
name string
givenSecretsInConfig []interface{}
givenActionSecrets []*management.ActionSecret
expectedDiagnostics diag.Diagnostics
}{
{
name: "action has no secrets",
givenSecretsInConfig: []interface{}{},
givenActionSecrets: []*management.ActionSecret{},
expectedDiagnostics: diag.Diagnostics(nil),
},
{
name: "action has no untracked secrets",
givenSecretsInConfig: []interface{}{
map[string]interface{}{
"name": "secretName",
},
},
givenActionSecrets: []*management.ActionSecret{
{
Name: auth0.String("secretName"),
},
},
expectedDiagnostics: diag.Diagnostics(nil),
},
{
name: "action has untracked secrets",
givenSecretsInConfig: []interface{}{
map[string]interface{}{
"name": "secretName",
},
},
givenActionSecrets: []*management.ActionSecret{
{
Name: auth0.String("secretName"),
},
{
Name: auth0.String("anotherSecretName"),
},
},
expectedDiagnostics: diag.Diagnostics{
{
Severity: diag.Error,
Summary: "Unmanaged Action Secret",
Detail: "Detected an action secret not managed though Terraform: anotherSecretName. " +
"If you proceed, this secret will get deleted. It is required to add this secret to " +
"your action configuration to prevent unintentionally destructive results.",
AttributePath: cty.Path{cty.GetAttrStep{Name: "secrets"}},
},
},
},
}

sergiught marked this conversation as resolved.
Show resolved Hide resolved
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
actualDiagnostics := checkForUnmanagedActionSecrets(
testCase.givenSecretsInConfig,
testCase.givenActionSecrets,
)

assert.Equal(t, testCase.expectedDiagnostics, actualDiagnostics)
})
}
}
Loading