grafana / redshift-datasource

Apache License 2.0
8 stars 11 forks source link

Add support for AWS Secrets Manager #77

Closed andresmgot closed 2 years ago

andresmgot commented 3 years ago

It should be possible to use Secrets Manager to list and use an existing secret to query Amazon Redshift, as the AWS console allows you to:

Screenshot from 2021-09-06 12-11-26

Note that the only possible method to authenticate at the moment is using temporary credentials.

It's not the goal of this issue to add the possibility to store new secrets.

andresmgot commented 3 years ago

Implementation Details

AWS Secrets Manager

Secrets Manager enables you to replace hardcoded credentials in your code, including passwords, with an API call to Secrets Manager to retrieve the secret programmatically. This helps ensure the secret can't be compromised by someone examining your code, because the secret no longer exists in the code. Also, you can configure Secrets Manager to automatically rotate the secret for you according to a specified schedule. This enables you to replace long-term secrets with short-term ones, significantly reducing the risk of compromise.

Support

While the service is generic, a subset of AWS databases and services have full support for rotation (source):

API Request

When using a secret, an API request should include:

From: https://docs.aws.amazon.com/sdk-for-go/api/service/redshiftdataapiservice/#RedshiftDataAPIService.ExecuteStatement

Note that the database user is no longer needed nor the permissions to to call the redshift:GetClusterCredentials operation.

Additional permissions

In order to be able to list and get the information from the managed secret, the additional policies required are:

secretsmanager:GetSecretValue
secretsmanager:ListSecrets

Proposed implementation

The following options are ordered from the simplest to implement to the most complex. The more complex solutions benefits from a better UX and maintainability of the code. Estimations are roughly made.

[EDIT]

After some feedback, the options below have been reduced to two options.

Option 1. Add an new input field in Redshift to include the managed secret ARN

In the Redshift section, add a radio button to select between temporary credentials (current approach) and AWS Secret Manager. Then, if Secret Manager is selected, add a plain text input field in which the user can paste the secret ARN so the database user is optional:

Screenshot from 2021-09-15 11-25-30

Note that this wouldn't require extra permissions since no API call is involved.

Estimation: 16h

Option 2. Temporary credentials vs AWS Secret Manager selection

Similar to the above, the main difference is that rather than having the user to copy-paste the secret ARN, we would list the different managed secrets (only the ones related to Redshift) so the user can select one. This is basically to provide the same UX than the AWS console:

Screenshot from 2021-09-06 12-55-09

Note that this option requires the additional permissions listed above.

Estimation: 24-32h

Future Work (not for the first release)

Add support for AWS Secret Manager in the Grafana AWS SDK

Since this is a feature that may be useful for other AWS databases/services (see the list above) we should consider moving this feature up to the Grafana AWS SDK package. If we do so, we can add an optional input there (like in option 1 and 2) and if that value is set, automatically use that information to fill the Redshift details. Something like:

Screenshot from 2021-09-06 13-09-43

Estimation: ~40h

@sunker WDYT?

sunker commented 3 years ago

Thanks for a well written issue @andresmgot.

Ideally, the time we put into this is something we can benefit from in future plugin development, and that would suggest option 4. However, I'm not sure about the UX in that mockup. It's a bit unclear which fields one needs to fill in to switch between the two auth modes Temporary credentials and AWS Secret Manager. Maybe the @grafana/aws-sdk could expose yet another component, for example DatabaseAuthenticationConfig.tsx, that would be displayed underneath the ConnectionConfig. This new component would just render the radio buttons and field as suggested in option 3 above. The awsds go package (in grafana-aws-sdk) could provide methods for listing secrets and secret value. Rough estimate: 40h

If we don't have time / want to prioritize this at this point, I think we should go with option 1. Possibly we could also add a radio button that allows the user to select between the two modes. Just to make it clear what auth method they are actually using.

andresmgot commented 3 years ago

Ideally, the time we put into this is something we can benefit from in future plugin development, and that would suggest option 4. However, I'm not sure about the UX in that mockup. It's a bit unclear which fields one needs to fill in to switch between the two auth modes Temporary credentials and AWS Secret Manager. Maybe the @grafana/aws-sdk could expose yet another component, for example DatabaseAuthenticationConfig.tsx, that would be displayed underneath the ConnectionConfig. This new component would just render the radio buttons and field as suggested in option 3 above. The awsds go package (in grafana-aws-sdk) could provide methods for listing secrets and secret value. Rough estimate: 40h

The problem is that the naming "Temporary credentials" is only used by Redshift the documentation for the rest of databases/services don't mention that.

Also the handling of the secret vary depending on the data source. For Redshift, the API exposes a field to set the secret ARN but for other, the data source code needs to manually retrieve the secret, parse its JSON and extract the required credentials.

If we don't have time / want to prioritize this at this point, I think we should go with option 1. Possibly we could also add a radio button that allows the user to select between the two modes. Just to make it clear what auth method they are actually using.

:+1: Agree. Let's check this with Marc.

sunker commented 3 years ago

The problem is that the naming "Temporary credentials" is only used by Redshift the documentation for the rest of databases/services don't mention that.

Maybe the grafana/aws-sdk could just expose a SecretsManagerComponent, and the redshift ConfigEditor handles the radiobuttons.

Or we just do all of this in the Redshift data source for now. Possibly parts of this could be moved into the SDK in the future when we're working on other AWS database plugins.

Also the handling of the secret vary depending on the data source. For Redshift, the API exposes a field to set the secret ARN but for other, the data source code needs to manually retrieve the secret, parse its JSON and extract the required credentials.

Not sure I follow. My understanding was that for redshift, we need to parse the json returned in the SecretString prop returned in the aws secretsmanager get-secret-value method in order to get dbClusterIdentifier\ and username. I get that for other data sources we might need other fields than for example dbClusterIdentifier\. So they all need to parse the secret string differently.

andresmgot commented 3 years ago

Not sure I follow. My understanding was that for redshift, we need to parse the json returned in the SecretString prop returned in the aws secretsmanager get-secret-value method in order to get dbClusterIdentifier\ and username. I get that for other data sources we might need other fields than for example dbClusterIdentifier\. So they all need to parse the secret string differently.

I'm talking here about the query API. For Redshift, if we know the database name, the cluster identifier and the secret ARN, we don't need to retrieve the secret value. We probably want to do so in the in the config editor though, so we can show/store the cluster identifier so the user doesn't need to type it.

In any case, my point is that the handling of the secret will be different depending on the data source so we cannot have a logic that fits all in the sdk.

sunker commented 3 years ago

I'm talking here about the query API. For Redshift, if we know the database name, the cluster identifier and the secret ARN, we don't need to retrieve the secret value. We probably want to do so in the in the config editor though, so we can show/store the cluster identifier so the user doesn't need to type it.

I think this would be nice. Auto filling these fields could potentially be one of the reasons that this is a stopper for AWS.

In any case, my point is that the handling of the secret will be different depending on the data source so we cannot have a logic that fits all in the sdk.

My idea was that the SDK would only handle the generic parts, such as listing the secrets. Using for example render props, it could be up to each plugin to parse the secret string and decide what parts of it needs to be stored in the json model. But I might be missing something.,