Skip to content
This repository has been archived by the owner on Mar 8, 2022. It is now read-only.

Support for hook secrets #205

Closed
ghost opened this issue Apr 15, 2020 · 16 comments · Fixed by #262
Closed

Support for hook secrets #205

ghost opened this issue Apr 15, 2020 · 16 comments · Fixed by #262
Labels
enhancement New feature or request preserve

Comments

@ghost
Copy link

ghost commented Apr 15, 2020

@xanderflood commented on Apr 8, 2020, 12:31 AM UTC:

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

The provider supports rules, rule configs, and hooks, but currently doesn't support hook secrets, which are the analog of configs for hooks.

A current workaround for this is simply to use templatefile with a hook resource. This is fully functional but somewhat less secure and requires the maintainer to store their hooks in files that are not necessarily valid JS, meaning for instance that they'll have issues with code analysis tools.

New or Affected Resource(s)

This would entail introducing a single new resource auth0_hook_secret. It's lifecycle callbacks would be implemented as follows:

  • Create: makes a call to this endpoint, with a single secret in the body, then sets the id of the resource
  • Read: makes a call to this endpoint (which gets all secrets for the hook) and then sets only the value for the desired secret
  • Update: makes a call to this endpoint, again with a single secret
  • Destroy: makes a call to this endpoint, passing an array containing only the name of the secret

Changes to the name field should force a new resource. This is equivalent to but simpler than correctly implementing name changes.

Also, there's a question about how to set the ID of a resource. Using {hook_id}{separator}{secret_name} has the advantage that it will be possible to do imports. However, since hook_id is itself an opaque string, choosing a separator is difficult. Moreover, imports seem unimportant for this resource since destroying and re-creating is generally low cost.

Potential Terraform Configuration

resource "auth0_hook_secret" "internal" {
  hook_id = auth0_hook.my-hook.id
  name = "mySecretName"
  value = "json-value"
}

This issue was moved by alexkappa from terraform-providers/terraform-provider-auth0#11.

@ghost ghost added the enhancement New feature or request label Apr 15, 2020
Repository owner deleted a comment Apr 15, 2020
Repository owner deleted a comment Apr 15, 2020
@alexkappa
Copy link
Owner

Thanks for suggesting the enhancement @xanderflood. Support in the SDK was added recently by @yinzara in go-auth0/auth0/pull/107.

We should be able to work on this soon.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity.
Stale issues will be closed after 5 days if no action is taken. If you
think this issue should not be closed, remove the stale label.

@github-actions github-actions bot added the stale label Jun 15, 2020
@xanderflood
Copy link

This issue should not be closed - it's low priority and may not go into development for a bit, but I think a long silence on this issue is expected 🤷‍♀️

@xanderflood
Copy link

It should be fairly straighforward and I'm happy to submit a PR for it myself when I have a bit more free time

@herry13
Copy link

herry13 commented Jun 15, 2020

Hi @alexkappa. I have implemented this feature in my branch https://github.com/herry13/terraform-provider-auth0/tree/herry/adding-resource-hook-secret. This is the first time I contribute to this project. Do you think my codes are good enough for a PR?

@alexkappa alexkappa added preserve and removed stale labels Jun 16, 2020
@xanderflood
Copy link

@herry13 I'm not affiliated with the project so I don't know what the normal procedures are, but if you go ahead and open a PR it'll provide us with a UI for comments and review. It looked pretty good to me when I skimmed it, though.

@yinzara
Copy link
Contributor

yinzara commented Jun 16, 2020

I can say that while I agree your implementation functions, hook secrets are managed as a single resource from the point of view of the Auth0 management API. Your implementation follows the patterns of rule_config (which are separate resources from the point of view of the Auth0 management API).

My branch uses a nested element of the auth0_hook resource to manage secrets:
https://github.com/yinzara/terraform-provider-auth0/blob/master/auth0/resource_auth0_hook.go

@herry13
Copy link

herry13 commented Jun 19, 2020

@yinzara I'm happy to ditch my branch. Can you please create a PR so that your implementation is available in the official Auth0 Terraform Provider? It would be nice if your PR can be merged ASAP because this feature is blocking my work.

@xanderflood
Copy link

@herry13 (again, I don't work here, but) in my experience with providers that aren't AWS, it can take a while (weeks to months) even after an update is merged before it gets mainlined into the Terraform provider binaries that are distributed by HashiCorp. If this is something you need urgently, you may want to consider compiling your own binary and hard-vendoring it into your project, and then going back on-label after this process has wrapped up, rather than waiting for this process to play out

@yinzara
Copy link
Contributor

yinzara commented Jun 19, 2020

Right now I'm guessing @alexkappa is preparing for the Auth0 Terraform webinars that are coming up soon. If he could give us some feedback as to which implementation he prefers, we can create some PRs but as @xanderflood says, I would suggest just using your custom built version as this process can take some time.

@herry13
Copy link

herry13 commented Jun 19, 2020

@xanderflood @yinzara Thank you guys for the advice. I really appreciate it. I will try an alternative solution for my work.

@alexkappa
Copy link
Owner

Hi everyone! First off, thank you for working on this feature. Unfortunately I didn't have the time to look into both approaches, but will do so soon and come back with my feedback. Thanks for your patience.

in my experience with providers that aren't AWS, it can take a while (weeks to months)

To offer some clarity into the process, the auth0 provider is not released as part of the terraform release schedule. Terraform provider developers coordinate with HashiCorp when a release is required and they usually are able to cut a release within hours of our request. The release cadence is mainly up to the individual provider developers (mostly).

It would be nice if your PR can be merged ASAP

I wish I could help you with this, but as with any new feature, I will need to make sure it works as intended for the entire lifetime of the project. It's therefore not something I can rush to accept. It must be done with reasonable caution. Feel free to use your own custom built as others have suggested, but there's no guarantee that the eventual feature would be compatible and you would manually need to migrate when it gets implemented.

@alexkappa
Copy link
Owner

After some review of the two approaches I am inclined to favor the one which combines the secrets into the auth0_hook resource. My guiding principle for this is staying as close to the Auth0 API as possible. Feel free to convince me otherwise though, there are certainly patterns in this provider where the former approach was used (e.g. rule configuration).

I would like to revisit the upsertHookSecrets function however. Perhaps we can explore ways to simplify it. Perhaps change it to insert/update? Perhaps there is a way to use the Diff() function to see which secrets have changed (added/removed)? We might need to change its schema for that though.

If you'd like, submit your PR so we can continue the conversation further.

@herrypio
Copy link

@alexkappa I agree with you that combining the secrets into the auth0_hook resource is a cleaner than having a separate resource for each secret. I will delete my branch. Thanks for the feedback.

@yinzara
Copy link
Contributor

yinzara commented Aug 20, 2020

Finally got to actually implementing the changes @alexkappa asked for. The implementation actually was kinda elegant when all was said and done. It makes exactly as many API calls as is necessary to retrieve existing keys, delete, update and/or create new secrets. In the end it will make between 1 and 4 API calls to properly update the resource.

@mcalster
Copy link
Contributor

Is there any time line on when this will be added to the provider?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request preserve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants