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

allow Azure backend to read config from environment variables #749

Closed
savishy opened this issue Jun 9, 2021 · 6 comments
Closed

allow Azure backend to read config from environment variables #749

savishy opened this issue Jun 9, 2021 · 6 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@savishy
Copy link

savishy commented Jun 9, 2021

Is your feature request related to a problem? Please describe.
While configuring the Azure backend, one has to hardcode secrets into the Tempo YAML, which may cause a problem in some restrictive environments where automated scans of code repositories may reveal secret-like strings and cause InfoSec teams to panic.

Describe the solution you'd like
It would be great if we could avoid this somehow. One way out is to read via environment variables. An example of using with the Azure Storage Blobs Go SDK, is documented here:
https://github.com/Azure-Samples/storage-blobs-go-quickstart/blob/master/storage-quickstart.go

Here is one of the places where this change would go in. (There may be more of course. And we would probably have to add some logic to consider env-vars as well as config fields.)

c, err := blob.NewSharedKeyCredential(conf.StorageAccountName.String(), conf.StorageAccountKey.String())

Describe alternatives you've considered
Another alternative (if Tempo can support it) would be to read just the secret bits from a different config file. I imagine this would be tougher to support though.

Additional context

  • I have used the S3 backend earlier, this supports passing in secrets via environment variables, whereas the Azure backend does not.
@joe-elliott
Copy link
Member

It appears that you have to manually read env vars? I'd guess the two on this line are fairly standard:

https://github.com/Azure-Samples/storage-blobs-go-quickstart/blob/master/storage-quickstart.go#L52

The Azure backend is supported by the community. It is difficult for us to make changes and test them b/c we do not run in Azure, but I think a good place to add support is here:

If conf.StorageAccountName.String() == "" then load whatever is in the env var AZURE_STORAGE_ACCOUNT and put it in this field. Same with conf.StorageAccountKey.

If you have time to PR/test this change please do.

@josephwoodward
Copy link
Contributor

@joe-elliott @annanay25 I presume this can now be closed?

@annanay25
Copy link
Contributor

Let's wait on @savishy to confirm that the env variable expansion works for them. We can close after.

@josephwoodward
Copy link
Contributor

@annanay25 Sounds good 👍

@zalegrala
Copy link
Contributor

Did this work as expected?

@joe-elliott
Copy link
Member

Between these two PRs I think we can safely close this:

Allow env var interpolation into config: #788
Explicitly support AZURE_STORAGE_ACCOUNT and AZURE_STORAGE_KEY for credentials: #1147

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants