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

Load StorageQueue SAS Tokens From Secret #514

Closed
1 task
barakAtSoluto opened this issue Apr 21, 2019 · 9 comments · Fixed by #535
Closed
1 task

Load StorageQueue SAS Tokens From Secret #514

barakAtSoluto opened this issue Apr 21, 2019 · 9 comments · Fixed by #535
Assignees
Labels
feature-request New feature requests metric-source:azure-storage All issues that relate to scraping metrics that depends on Azure Storage Management as a data source security All issues related to security specs-defined All issues where the specifications are defined and ready to be implemented
Milestone

Comments

@barakAtSoluto
Copy link

Currently the metrics configuration is loaded from a Config Map object.
When querying an Azure StorageQueue (or other storage account types I assume) you must provide sasToken.
There should be a way to pass that token in a more secure fashion.

Specification

  • SaS tokens are not passed in clear text via configMap
@tomkerkhove
Copy link
Owner

Fair ask.

Would it make sense if you can pass hard-coded value or a name of env variable to consume which is defined in the config file? Or how would you prefer this?

@tomkerkhove tomkerkhove added this to the v1.0.0 milestone Apr 21, 2019
@tomkerkhove tomkerkhove added feature-request New feature requests metric-source:azure-storage All issues that relate to scraping metrics that depends on Azure Storage Management as a data source security All issues related to security specs-required All issues where the specifications are still being defined and implementation should be halted labels Apr 21, 2019
@tomkerkhove tomkerkhove self-assigned this Apr 21, 2019
@tomkerkhove
Copy link
Owner

My initial thinking would be something along these lines.

Default should use an environment variable following this approach

name: demo_queue_size
description: "Amount of messages in the 'orders' queue"
resourceType: StorageQueue
accountName: promitor
queueName: orders
sasToken:
  environmentVariable: AZURE_STORAGE_SAS_TOKEN
azureMetricConfiguration:
  metricName: MessageCount
  aggregation:
    type: Total

However, in some scenarios a fixed one could make sense.

It would look like this then:

name: demo_queue_size
description: "Amount of messages in the 'orders' queue"
resourceType: StorageQueue
accountName: promitor
queueName: orders
sasToken:
  value: ?sv=2015-04-05&st=2015-04-29T22%3A18%3A26Z&se=2015-04-30T02%3A23%3A26Z&sr=b&sp=rw&sip=168.1.5.60-168.1.5.70&spr=https&sig=Z%2FRHIX5Xcg0Mq2rqI3OlWTjEg2tYkboXr1P9ZUXDtkk%3D
azureMetricConfiguration:
  metricName: MessageCount
  aggregation:
    type: Total

@barakAtSoluto
Copy link
Author

barakAtSoluto commented Apr 29, 2019

That could work i guess.
Another possible solution is to add an init container that will generate a secret's file.
We're using https://github.com/Soluto/kamus for this purpose so it would be nice to be able to integrate.
Also, maybe it's possible to generate tokens on the fly with - this. Though there might be an issue with the permissions you'll need to give your SP....

@tomkerkhove
Copy link
Owner

It's not really up to Promitor to generate access and should only be a consumer in my opinion.

For the time being, I think the configuration proposal from above should be good enough.

Kamus looks like a nice library but I think the sheer amount of people will not use this and don't want to enforce this and keep it as simple as possible, sorry!

@tomkerkhove tomkerkhove added specs-defined All issues where the specifications are defined and ready to be implemented and removed specs-required All issues where the specifications are still being defined and implementation should be halted labels Apr 29, 2019
@barakAtSoluto
Copy link
Author

barakAtSoluto commented Apr 29, 2019

Fair enough,
I was only giving Kamus as an example from something that can run as an init container and create the secret file.

I noticed the tokens are getting logged. That's also an issue...

@tomkerkhove
Copy link
Owner

tomkerkhove commented Apr 29, 2019

That's terrible, my bad - I wasn't aware of this and will fix it asap!

Tracking in #530

@tomkerkhove
Copy link
Owner

Feature is done, only docs remain.

@barakAtSoluto
Copy link
Author

Thanks @tomkerkhove, worked like a charm!

@tomkerkhove
Copy link
Owner

Happy to hear and thanks for the suggestion @barakAtSoluto!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature requests metric-source:azure-storage All issues that relate to scraping metrics that depends on Azure Storage Management as a data source security All issues related to security specs-defined All issues where the specifications are defined and ready to be implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants