Design/Managing External Grafana Instance

[elamaran11]

Hi @NissesSenap Per our discussion, have created a design document for a proposal for Amazon Managed Grafana Integration . Appreciate if you can take a look and let me know what you think. We can jump in to a call if needed.

[NissesSenap]

So I think we misunderstood each other, to me this is a long list of issues instead of a design proposal.

We need to have a basic discussion on how to design the CRD:s to support external grafana instances. Assuming that the secrets needed to connect to your external grafana instance is available inside the cluster how would we use these secrets to talk to the external grafana instance?

How would the grafana crd look like?
How would the datasource crd look like?

If you don't feel comfortable with go that is fine, we will figure out the controller part but as an end user how would you like the operator work with external resources?

[NissesSenap]

To my knowledge all of the mentioned datasources should be able to run inside the operator today.
Adding custom config to your datasources was added in: grafana/grafana-operator#803

So there are now two fields: customJsonData and customSecureJsonData which will solve this use case.

[pb82]

That's correct, we don't make any assumptions about the type of data source. You might have to install a plugin for a certain data source, but other than that, the datasource CR is compatible with all of them.

[elamaran11]

Understood @pb82 @NissesSenap Thankyou for clarifying this. We may need to add couple of samples on using other AWS data sources in the examples folder.

[NissesSenap]

We are happy to accept a PR around it

[elamaran11]

Sure @NissesSenap Once we have external Grafana Instance working, i can submit a PR.

[pb82]

I think most pieces are already there to support externally managed Grafana instances. We just need a way to tell the Operator than a given Grafana CR refers to an external, already existing instance and where the credentials are located. We could do this by adding new configuration options to the Grafana CR spec, e.g.

spec:
  external:
    url: <external grafana url, type string>
    admin_username: <type SecretKeySelector>
    admin_password: <type SecretKeySelector>

all other fields in the spec would be ignored for external instances. The url gets reflected into the status, so the dashboad and datasource reconcilers know how to talk to those instances. We'll have to add some logic to the Grafana client wrapper to obtain the credentials from the secret key selectors (instead of the generated secret).

[elamaran11]

I think most pieces are already there to support externally managed Grafana instances. We just need a way to tell the Operator than a given Grafana CR refers to an external, already existing instance and where the credentials are located. We could do this by adding new configuration options to the Grafana CR spec, e.g.

spec:
  external:
    url: <external grafana url, type string>
    admin_username: <type SecretKeySelector>
    admin_password: <type SecretKeySelector>

all other fields in the spec would be ignored for external instances. The url gets reflected into the status, so the dashboad and datasource reconcilers know how to talk to those instances. We'll have to add some logic to the Grafana client wrapper to obtain the credentials from the secret key selectors (instead of the generated secret).

@pb82 The approach looks reasonable and good but only caveat is each and every dashboard will have dupllication of snippets for URL and API reference for external grafana. Im working on a different approach for this. I will submit an updated Pr.

[pb82]

Thanks @elamaran11 , looking forward to your proposal!

[elamaran11]

Hi @pb82 @NissesSenap Please check this updated design proposal. Please let me know what you think. Happy to jump in to a call.

[NissesSenap]

I understand that my end @pb82 comments was a bit unclear but we think that we can use the existing Grafana kind so no neeed to create a new CRD/controller.

So you can remove that part.
About alerting please write a separate proposal around it since it's a new CRD and there we will need to look around much more at the Alerting API.

Since this proposal will solve all external grafana instances you can remove most of the Amazon specific mentions but I do like the example of pointing to a good use-case.

[elamaran11]

I understand that my end @pb82 comments was a bit unclear but we think that we can use the existing Grafana kind so no neeed to create a new CRD/controller.

So you can remove that part. About alerting please write a separate proposal around it since it's a new CRD and there we will need to look around much more at the Alerting API.

Since this proposal will solve all external grafana instances you can remove most of the Amazon specific mentions but I do like the example of pointing to a good use-case.

I have fixed all comments on the PR. I also agree to the approach of using existing Grafana CRD which simplies the process even more. Only thing i want to iterate is, We want to use url and grafana_api_key as an approach to access remote Grafana Instance such as AMG to use the same while adding data souces and while creating dashboards.

[NissesSenap]

Added some minor comments.

[NissesSenap]

Hmm, why do you think it needs to be a ConfigMapSelector?
Isn't it enough with a string to the specific URL just as @pb82 described?

    url: <external grafana url, type string>

[elamaran11]

@NissesSenap I think ConfigMapSelector is not needed. We can go with using url pointed directly as above. I fixed the PR.

[NissesSenap]

Hmm you have a good point about using grafana_api_key.
First of all I think api_key is a better variable name. I at least think assumed since we are in the grafana instance config.

But the second question is how do we want to login to external grafana instances.
A best practice is decadently to use a API key instead of doing

    admin_username: <type SecretKeySelector>
    admin_password: <type SecretKeySelector>

The question is if we want to support the other possible solution as well?
Or we can just say that any external grafana instance should always be using a API key.
Or we could support both by using username & password.
And if you are using an API key you could set username as empty or something like that, i have seen similar solutions on a few API:s through out the years.

But it's not super straight forward and we would have to do allot of docs.
What do you think @elamaran11 and @pb82 ?

[elamaran11]

HI @NissesSenap IMO we should support both authentication mechanisms. API_KEY and username, password so that the user has a flexbility to choose the same accordingly to their needs. We should support separate paremeters for API_KEY and username, password. If API_KEY is passed that should take precedence over username, password.

[NissesSenap]

Yeah it's probably easier then doing some strange workaround supporting two in one.

Please describe all of the three options in the document and write somewhere that API key should take precedence over username, password.
Then we don't forget this during the implementation :)

[elamaran11]

@NissesSenap Added, Please check. I have covered all 3 variants of options.

[pb82]

Yes, agree to support both. So basically:

spec:
  external:
      url: <type string>
      admin_username: <type SecretKeySelector>
      admin_password: <type SecretKeySelector>
      admin_api_key:  <type SecretKeySelector>

right? And if the API key is provided, the other two fields are ignored (if provided).

[elamaran11]

@pb82 Thats right. If API Key is provided, that takes precedence over U/P. But also remember url should also a mandatory field for external grafana instance.

[pb82]

@elamaran11 right, I forgot to include the url.

[NissesSenap]

LGTM, do you have any other thoughts @pb82 ?

[elamaran11]

@pb82 @NissesSenap COuld you please merge this PR, if there is no other changes?