[DXCDT-156] Add warnings for untracked hook secrets #189
Conversation
sergiught
force-pushed
the
patch/DXCDT-156-ISSUE-40
branch
from
5f91234
to
b609fb5
Compare
Codecov Report
@@ Coverage Diff @@
## main hashicorp/terraform-plugin-sdk#189 +/- ##
==========================================
- Coverage 84.02% 83.83% -0.20%
==========================================
Files 35 35
Lines 6029 6056 +27
==========================================
+ Hits 5066 5077 +11
- Misses 762 775 +13
- Partials 201 204 +3
Continue to review full report at Codecov.
|
| @@ -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) | |||
There was a problem hiding this comment.
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()) |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
| 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"}}, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
╵
Description
While taking a look at #40, I wasn't able to reproduce the issue with the secrets. I am suspecting that the issue was actually present in the go-auth0 SDK in this func https://github.com/auth0/go-auth0/blob/fcf36e271b507a9af6e2c281013f93181a9d8753/management/hook.go#L143 that eventually got fixed through a newer version.
However, we made the hook resource implementation more robust with this PR and we are now throwing out warnings in case we encounter secrets that aren't being tracked through terraform. e.g.
When we import hooks the first time, even if the secrets match 100% with the secrets found in the dashboard, the warnings will still be present as we only update the state file with the secrets on a create / update (terraform apply). This is due to the fact that the values aren't being shown while fetching the secrets.
At the present moment with the current version of our terraform sdk, we aren't able to have warnings tested through test steps. There's an issue tracking this however over here: hashicorp/terraform-plugin-testing#69
Checklist
Note: Checklist required to be completed before a PR is considered to be reviewable.
Auth0 Code of Conduct
Auth0 General Contribution Guidelines
Changes include test coverage?
Does the description provide the correct amount of context?
Have you updated the documentation?
Is this code ready for production?